Return connection errors from GetSelectedColumns and other pgx queries missing them

[ilidemi]

Error (sanitized)

Postgres snapshot partitions fail during qrep pull with a claim that the source table has no columns:

[qrep] failed to pull records: table "public"."ferly_inoculum" doesn't have queriable columns

The line emits errorClass=OTHER / errorCode=UNKNOWN with source other. It has appeared on several pipes across multiple regions over the past few weeks, always in a short burst that clears within minutes.

Investigation

This a bug in PeerDB. GetSelectedColumns iterates pg_attribute rows but never checks rows.Err(), so when the source connection dies after the query is sent, Next stops immediately and the function returns an empty column list with a nil error. corePullQRepRecords reads that empty list as "this table has no columns" and reports it, hiding the connection failure that actually happened.

• pgx Conn.Query — documents that Query returns before reading any rows, so a failure after the query is sent surfaces only through Rows.Err().
• PeerDB #4583 — added this pull path and its empty-column check; the message cannot predate it, and the first occurrences follow it.
• Postgres error codes — 57P01 and 57P03 cover the backend terminations that the logs show at the same second as each burst.

Every occurrence in the logs lands in the same second as the source connection dropping: a source restart refusing new connections, an administrator terminating the backend, or an SSH tunnel closing. Partitions for the same table succeed before and after each burst, which rules out a genuine all-columns-excluded configuration. Confidence is high: the pgx contract explains why the empty list appears, and the logs confirm the trigger in each case. Nobody needs to act on the message itself — the fix removes it, and the real connection error takes its place. Two things worth a reviewer's attention: a permanently misconfigured exclusion list would still produce the same message legitimately, and that case belongs in validation rather than here; and SSH Tunnel Closed: EOF is matched by the io.EOF branch before it reaches the SSH-tunnel branch, which predates this change.

Change

• GetSelectedColumns now checks rows.Err() after the iteration loop and wraps it as error getting selected columns for table %s, matching the check its sibling function in the same file already performs.
• All three callers — the qrep pull, table-schema fetch, and validation — previously read a truncated or empty list as fact; they now receive the underlying error.
• The connection failures that caused these bursts already classify on their own: 57P03 and a closed tunnel as NOTIFY_CONNECTIVITY, 57P01 as NOTIFY_TERMINATE, a bare unexpected EOF as IGNORE_EOF. No classifier change is needed.

Verification

• TestGetSelectedColumnsReportsMidStreamConnectionLoss breaks a live connection after the query is sent and asserts that GetSelectedColumns returns an error rather than an empty column list. It fails against the unfixed code with a nil error and zero columns, reproducing the production symptom exactly.
• The full classifier suite is green, as is the Postgres connector package.
• An internal check rebuilt each observed production failure with the post-fix wrap chain and confirmed all of them classify into an existing class; none fall through to OTHER.

Resolves DBI-979

[claude]

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

[dtunikov]

smth like this would be more handy:

	columns, err := pgx.CollectRows[string](rows, pgx.RowTo)
	if err != nil {
		return nil, fmt.Errorf("error scanning columns while getting selected columns for table %s: %w", sourceTable, err)
	}

	return columns, nil

we already use CollectRows in postgres_source.go file

[dtunikov]

can also leave instructions for agents in future to use CollectRows instead of raw Query + rows.Next + rows.Err != nil

[ilidemi]

Good idea, full refactor sounded like too much for this but found a few other places not handling errors and turned them into CollectRows

[github-actions]

🔄 Flaky Test Detected

Analysis: A 20-minute package timeout panic — triggered by an environment-wide mirror-setup stall (79 tests hit UNEXPECTED STATUS TIMEOUT STATUS_SETUP) that let two MySQL server_id tests each consume their full 8-minute timeout budget — mass-failed ~100 tests with no correctness assertions failing, while the other two matrix legs passed on the same commit.
Confidence: 0.85

✅ Automatically retrying the workflow

View workflow run