Replace xmax upsert trick with RETURNING full row

Upsert methods now RETURNING all struct columns and scan the result
back into the pointer receiver, keeping the caller in sync with the
actual DB state (id, created_at, etc. from the existing row on
conflict). Insert detection compares the saved original ID with the
returned ID instead of relying on the PostgreSQL-internal xmax column.

Signed-off-by: Émile Ré <emile@probo.com>

contrib/claude/coredata.md

@@ -145,6 +145,50 @@ if err != nil {
 }
 ```
 
+## Upsert with insert detection and receiver sync
+
+When an upsert needs to report whether a row was inserted or already existed, `RETURNING` all struct columns and scan the full row back into the receiver. Save the original ID before the query; on a fresh insert the returned ID matches, on a conflict/update the existing row's ID is returned. The receiver is a **pointer** so the caller always sees the actual DB state after the upsert.
+
+Do **not** use `RETURNING (xmax = 0) AS inserted` — `xmax` is a PostgreSQL internal system column and is fragile.
+
+```go
+// Good — RETURNING full row, sync receiver, compare original ID
+func (t *Thing) Upsert(ctx context.Context, conn pg.Tx) (inserted bool, err error) {
+    q := `
+INSERT INTO things (id, name, created_at, updated_at)
+VALUES (@id, @name, @created_at, @updated_at)
+ON CONFLICT (name) DO UPDATE
+SET
+    name       = EXCLUDED.name,
+    updated_at = EXCLUDED.updated_at
+RETURNING
+    id,
+    name,
+    created_at,
+    updated_at
+`
+    originalID := t.ID
+    args := pgx.StrictNamedArgs{...}
+
+    rows, err := conn.Query(ctx, q, args)
+    if err != nil {
+        return false, fmt.Errorf("cannot upsert thing: %w", err)
+    }
+    defer rows.Close()
+
+    row, err := pgx.CollectExactlyOneRow(rows, pgx.RowToStructByName[Thing])
+    if err != nil {
+        return false, fmt.Errorf("cannot collect upsert result: %w", err)
+    }
+
+    *t = row
+    return originalID == t.ID, nil
+}
+
+// Bad — xmax trick: relies on PostgreSQL internal column
+RETURNING (xmax = 0) AS inserted
+```
+
 ## Sentinel errors
 
 ```go

pkg/cookiebanner/tracker_mapping_worker.go

@@ -252,12 +252,10 @@ func (h *trackerMappingHandler) matchByDomain(
 		UpdatedAt:          now,
 	}
 
-	actualID, _, err := commonPattern.Upsert(ctx, tx)
-	if err != nil {
+	if _, err := commonPattern.Upsert(ctx, tx); err != nil {
 		return nil, nil, fmt.Errorf("cannot upsert common tracker pattern from domain match: %w", err)
 	}
 
-	commonPattern.ID = actualID
 	thirdPartyID, err := h.resolveThirdParty(ctx, tx, tp, &commonPattern)
 	if err != nil {
 		return nil, nil, fmt.Errorf("cannot resolve third party from domain match: %w", err)
@@ -346,12 +344,10 @@ func (h *trackerMappingHandler) identifyWithAgent(
 		UpdatedAt:          now,
 	}
 
-	actualID, _, err := commonPattern.Upsert(ctx, tx)
-	if err != nil {
+	if _, err := commonPattern.Upsert(ctx, tx); err != nil {
 		return nil, nil, fmt.Errorf("cannot upsert common tracker pattern from agent: %w", err)
 	}
 
-	commonPattern.ID = actualID
 	thirdPartyID, err := h.resolveThirdParty(ctx, tx, tp, &commonPattern)
 	if err != nil {
 		return nil, nil, fmt.Errorf("cannot resolve third party from agent match: %w", err)
@@ -447,12 +443,11 @@ func (h *trackerMappingHandler) createUnmatchedPattern(
 		UpdatedAt:     now,
 	}
 
-	actualID, _, err := commonPattern.Upsert(ctx, tx)
-	if err != nil {
+	if _, err := commonPattern.Upsert(ctx, tx); err != nil {
 		return nil, fmt.Errorf("cannot upsert unmatched common tracker pattern: %w", err)
 	}
 
-	return &actualID, nil
+	return &commonPattern.ID, nil
 }
 
 func (h *trackerMappingHandler) resolveThirdParty(

pkg/coredata/common_third_party.go

@@ -310,7 +310,7 @@ INSERT INTO common_third_parties (
 // Upsert inserts a row, or on slug conflict updates every column except
 // id and created_at. Returns true if a new row was inserted, false if an
 // existing row was updated.
-func (t CommonThirdParty) Upsert(
+func (t *CommonThirdParty) Upsert(
 	ctx context.Context,
 	conn pg.Tx,
 ) (inserted bool, err error) {
@@ -379,9 +379,32 @@ SET
     security_page_url                = EXCLUDED.security_page_url,
     trust_page_url                   = EXCLUDED.trust_page_url,
     updated_at                       = EXCLUDED.updated_at
-RETURNING (xmax = 0) AS inserted
+RETURNING
+    id,
+    name,
+    slug,
+    category,
+    headquarter_address,
+    legal_name,
+    website_url,
+    privacy_policy_url,
+    service_level_agreement_url,
+    service_software_agreement_url,
+    data_processing_agreement_url,
+    business_associate_agreement_url,
+    subprocessors_list_url,
+    certifications,
+    status_page_url,
+    terms_of_service_url,
+    security_page_url,
+    trust_page_url,
+    logo_file_id,
+    created_at,
+    updated_at
 `
 
+	originalID := t.ID
+
 	args := pgx.StrictNamedArgs{
 		"id":                               t.ID,
 		"name":                             t.Name,
@@ -412,12 +435,14 @@ RETURNING (xmax = 0) AS inserted
 	}
 	defer rows.Close()
 
-	inserted, err = pgx.CollectExactlyOneRow(rows, pgx.RowTo[bool])
+	row, err := pgx.CollectExactlyOneRow(rows, pgx.RowToStructByName[CommonThirdParty])
 	if err != nil {
 		return false, fmt.Errorf("cannot collect upsert result: %w", err)
 	}
 
-	return inserted, nil
+	*t = row
+
+	return originalID == t.ID, nil
 }
 
 func (t CommonThirdParty) Delete(

pkg/coredata/common_third_party_domain.go

@@ -118,7 +118,7 @@ INSERT INTO common_third_party_domains (
 	return nil
 }
 
-func (d CommonThirdPartyDomain) Upsert(
+func (d *CommonThirdPartyDomain) Upsert(
 	ctx context.Context,
 	conn pg.Tx,
 ) (inserted bool, err error) {
@@ -139,9 +139,16 @@ INSERT INTO common_third_party_domains (
 ON CONFLICT (common_third_party_id, domain) DO UPDATE
 SET
     updated_at = EXCLUDED.updated_at
-RETURNING (xmax = 0) AS inserted
+RETURNING
+    id,
+    common_third_party_id,
+    domain,
+    created_at,
+    updated_at
 `
 
+	originalID := d.ID
+
 	args := pgx.StrictNamedArgs{
 		"id":                    d.ID,
 		"common_third_party_id": d.CommonThirdPartyID,
@@ -156,12 +163,14 @@ RETURNING (xmax = 0) AS inserted
 	}
 	defer rows.Close()
 
-	inserted, err = pgx.CollectExactlyOneRow(rows, pgx.RowTo[bool])
+	row, err := pgx.CollectExactlyOneRow(rows, pgx.RowToStructByName[CommonThirdPartyDomain])
 	if err != nil {
 		return false, fmt.Errorf("cannot collect upsert result: %w", err)
 	}
 
-	return inserted, nil
+	*d = row
+
+	return originalID == d.ID, nil
 }
 
 func (d CommonThirdPartyDomain) Delete(

pkg/coredata/common_tracker_pattern.go

@@ -189,10 +189,10 @@ INSERT INTO common_tracker_patterns (
 	return nil
 }
 
-func (p CommonTrackerPattern) Upsert(
+func (p *CommonTrackerPattern) Upsert(
 	ctx context.Context,
 	conn pg.Tx,
-) (actualID gid.GID, inserted bool, err error) {
+) (inserted bool, err error) {
 	q := `
 INSERT INTO common_tracker_patterns (
     id,
@@ -224,9 +224,21 @@ SET
     description           = EXCLUDED.description,
     confidence            = EXCLUDED.confidence,
     updated_at            = EXCLUDED.updated_at
-RETURNING id, (xmax = 0) AS inserted
+RETURNING
+    id,
+    common_third_party_id,
+    tracker_type,
+    pattern,
+    match_type,
+    description,
+    max_age_seconds,
+    confidence,
+    created_at,
+    updated_at
 `
 
+	originalID := p.ID
+
 	args := pgx.StrictNamedArgs{
 		"id":                    p.ID,
 		"common_third_party_id": p.CommonThirdPartyID,
@@ -242,27 +254,18 @@ RETURNING id, (xmax = 0) AS inserted
 
 	rows, err := conn.Query(ctx, q, args)
 	if err != nil {
-		return gid.GID{}, false, fmt.Errorf("cannot upsert common tracker pattern: %w", err)
+		return false, fmt.Errorf("cannot upsert common tracker pattern: %w", err)
 	}
 	defer rows.Close()
 
-	type upsertResult struct {
-		ID       gid.GID
-		Inserted bool
-	}
-
-	res, err := pgx.CollectExactlyOneRow(
-		rows,
-		func(row pgx.CollectableRow) (upsertResult, error) {
-			var r upsertResult
-			return r, row.Scan(&r.ID, &r.Inserted)
-		},
-	)
+	row, err := pgx.CollectExactlyOneRow(rows, pgx.RowToStructByName[CommonTrackerPattern])
 	if err != nil {
-		return gid.GID{}, false, fmt.Errorf("cannot collect upsert result: %w", err)
+		return false, fmt.Errorf("cannot collect upsert result: %w", err)
 	}
 
-	return res.ID, res.Inserted, nil
+	*p = row
+
+	return originalID == p.ID, nil
 }
 
 func (p CommonTrackerPattern) Delete(

pkg/coredata/tracker_resource.go

@@ -308,9 +308,24 @@ INSERT INTO tracker_resources (
 ON CONFLICT (cookie_banner_id, resource_type, origin, path) DO UPDATE SET
 	last_detected_at = GREATEST(tracker_resources.last_detected_at, EXCLUDED.last_detected_at),
 	updated_at = EXCLUDED.updated_at
-RETURNING (xmax = 0) AS inserted
+RETURNING
+	id,
+	organization_id,
+	cookie_banner_id,
+	cookie_category_id,
+	resource_type,
+	origin,
+	path,
+	display_name,
+	description,
+	excluded,
+	last_detected_at,
+	created_at,
+	updated_at
 `
 
+	originalID := tr.ID
+
 	args := pgx.StrictNamedArgs{
 		"id":                 tr.ID,
 		"tenant_id":          scope.GetTenantID(),
@@ -328,12 +343,20 @@ RETURNING (xmax = 0) AS inserted
 		"updated_at":         tr.UpdatedAt,
 	}
 
-	var inserted bool
-	if err := tx.QueryRow(ctx, q, args).Scan(&inserted); err != nil {
+	rows, err := tx.Query(ctx, q, args)
+	if err != nil {
 		return false, fmt.Errorf("cannot upsert tracker resource: %w", err)
 	}
+	defer rows.Close()
 
-	return inserted, nil
+	row, err := pgx.CollectExactlyOneRow(rows, pgx.RowToStructByName[TrackerResource])
+	if err != nil {
+		return false, fmt.Errorf("cannot collect upsert result: %w", err)
+	}
+
+	*tr = row
+
+	return originalID == tr.ID, nil
 }
 
 func (tr *TrackerResource) Update(

pkg/proboctl/seed/common-tracker-patterns/common_tracker_patterns.go

@@ -145,7 +145,7 @@ func NewCmdCommonTrackerPatterns(f *cmdutil.Factory) *cobra.Command {
 							UpdatedAt:          now,
 						}
 
-						_, wasInserted, err := pattern.Upsert(ctx, tx)
+						wasInserted, err := pattern.Upsert(ctx, tx)
 						if err != nil {
 							return fmt.Errorf("cannot upsert common tracker pattern %q: %w", p.Pattern, err)
 						}