[ajitpratap0]

Summary

#620 schema/postgres: loader misreports index columns and key flags (column order, INCLUDE, expression and partitioned indexes, read-only roles)

This fixes #620 in the PostgreSQL schema loader. It now reads index entries from pg_index.indkey in key order. Only the first indnkeyatts entries are key columns, and the new additive Index.IncludeColumns field holds the INCLUDE columns. Expression keys are reported as pg_get_indexdef text, and indexes on partitioned parents (relkind 'p') are now listed. PK/UNIQUE column flags now come from pg_constraint, aggregated per column. Each column appears once, IsUnique is set only for a single-column UNIQUE, and a SELECT-only role sees the same schema as the owner. Foreign keys now carry the referenced schema in the new additive ForeignKey.RefSchema field. To verify, the new tests were run against a live PostgreSQL 17.11: they reproduce every symptom in the issue on the parent loader and pass on HEAD. The full race gate, go vet and golangci-lint are clean. Deferred as out of scope, with sound reasons: RefSchema for the MySQL and SQLite loaders, IsUnique from non-constraint UNIQUE indexes (behaviour unchanged), invalid and partial index predicates, and expression/INCLUDE handling for MySQL and SQLite. The new TestMariaDBLoader variant has not been run locally (no Docker) and will first run in CI.

Commits (1), one implementation commit per issue plus test-only follow-ups:

• a708511 fix(schema/postgres): report index key order, INCLUDE and expression keys, partitioned-parent indexes, FK referenced schema and catalog-based key flags (schema/postgres: loader misreports index columns and key flags (column order, INCLUDE, expression and partitioned indexes, read-only roles) #620)

4 files changed, 317 insertions(+), 60 deletions(-)

Test Plan

• Each issue's reproduction cases were run against main before the fix (Haiku scout), and a failing regression test was written before the code change.
• Independent review (Opus, or Sonnet on small chains) with a bounded fix loop.
• Independent Opus validator ran the repository race gate (CGO_ENABLED=1 go test -race -timeout 300s $(go list ./... | grep -v /cbinding)), go vet, golangci-lint, re-ran the reproduction cases on HEAD, and confirmed the new tests fail on the parent commit.
• Perf items: benchstat -count=6 before/after on the same machine.

#620 evidence (validator)

Not a perf item, so there is no benchstat. Repro evidence from the live PostgreSQL 17.11 server, running HEAD's test file against the parent a708511^ loader (parent loader.go with HEAD's additive schema.go so the file compiles):
- Fails on the parent: TestPostgresLoader_IndexAndKeyMetadata (index_order_include_expression, composite_unique_order, partitioned_parent, column_flags, cross_schema_fk, select_only_role), TestPostgresLoader_QuotedIndexColumns and all 7 TestPostgresLoader_ForeignKeyJoinKeys subtests.
- Parent output matches the issue: idx_ba [a b]; idx_incl [a c]; idx_expr missing; idx_mixed [b]; u_k_id_key [id k]; p and events have no indexes; table u has 3 columns (k twice, and id flagged IsUnique); r_uid_fkey has an empty RefSchema; the SELECT-only role sees every PK flag false ("users.id not primary for SELECT-only role").
- On HEAD every postgres test passes.
Extra edge probe on HEAD:
- `CREATE INDEX e_opc ON e (lower("Sp Ace") text_pattern_ops DESC, b DESC NULLS LAST)` gives Columns ["lower(\"Sp Ace\")" "b"], with no opclass or DESC text.
- `UNIQUE INDEX (b) INCLUDE (d, c)` gives Columns [b] and IncludeColumns [d c] in declared order.
- A non-PK index on a partitioned parent (pp_v) is reported.
- A partial index reports its key column only.
- IncludeColumns is nil whenever there is no INCLUDE.

#620 race gate: PASS. Ran CGO_ENABLED=1 go test -race -timeout 300s $(go list ./... | grep -v /cbinding) on HEAD a708511 and it exited 0. All 44 packages are ok and none failed, including pkg/schema, pkg/schema/mysql, pkg/schema/postgres and pkg/schema/sqlite. Docker is not available, so the testcontainers tests skip in this gate. To cover them, I copied HEAD into the scratchpad, pointed the test helper at a throwaway local PostgreSQL 17.11 server, and ran go test -race on pkg/schema/postgres there. It passed, including all 6 subtests of TestPostgresLoader_IndexAndKeyMetadata.

Breaking Changes

• schema/postgres: loader misreports index columns and key flags (column order, INCLUDE, expression and partitioned indexes, read-only roles) #620: Additive only. pkg/schema/db.Index gains IncludeColumns []string (PostgreSQL INCLUDE columns, nil otherwise). pkg/schema/db.ForeignKey gains RefSchema string, set by the PostgreSQL loader and empty for MySQL and SQLite. Observable PostgreSQL loader changes:
• Index.Columns is now in index key order and excludes INCLUDE columns.
• Expression keys appear as expression text, and expression-only indexes are no longer dropped.
• Partitioned parent tables now report their indexes.
• Each column appears once.
• IsUnique is true only for a single-column UNIQUE constraint.
• PK and UNIQUE flags are correct for SELECT-only roles.
  No signatures change.

Notes

• CHANGELOG.md is intentionally not touched; the consolidated CHANGELOG PR docs: consolidate CHANGELOG for the perf audit and batch-2/batch-3 fixes; refresh performance baselines (#566) #598 gets the entries for this batch before it merges.
• Part of the batch-3 merge train (follow-ups cli: gosqlx format silently corrupts many expressions; switch the CLI to pkg/formatter #599-schema/postgres: loader misreports index columns and key flags (column order, INCLUDE, expression and partitioned indexes, read-only roles) #620); later PRs in the train are rebased onto this one after it merges.
• Base: main.

Fixes #620
Follow-up to #566

🤖 Generated with Claude Code

https://claude.ai/code/session_01UZSk1292jPc8ipDboNxeve

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
gosqlx-website	Ready	Preview	Oct 14, 2026 6:20am UTC

[github-actions]

Claude finished @ajitpratap0's task in 23s —— View job

---

Review of PR #622 (fix for #620, PostgreSQL index and key metadata)

I read the diff of schema.go and postgres/loader.go and did not run the tests. I skimmed the test changes only. The PR description says the postgres tests passed against a live PostgreSQL 17.11.

Verdict: The change is correct and well scoped. I have no blocking concerns.

What looks good

• Index key order: unnest(ix.indkey) WITH ORDINALITY gives the real key order. Filtering on k.ord <= ix.indnkeyatts cleanly separates key columns from INCLUDE columns. The old attnum = ANY(indkey) join ordered by attnum and dropped expression entries (attnum 0).
• Expression keys: pg_get_indexdef(indexrelid, ord, true) returns only that key's expression. The LEFT JOIN ... AND k.attnum <> 0 keeps expression-only indexes in the result.
• Partitioned parents: adding relkind 'p' is right.
• Key flags: the pg_constraint aggregate fixes the duplicate-column rows and the SELECT-only-role gap. information_schema hides constraints from roles that don't own the table. Using cardinality(conkey) = 1 for IsUnique matches MySQL's COLUMN_KEY = 'UNI'.
• Compatibility: the new IncludeColumns and RefSchema fields are additive, and the doc comments say which loaders set them.
• JSON transport: keeping JSON for the column lists (instead of array literals) preserves the existing quoted-identifier handling. Treating a NULL include_columns as nil is handled explicitly.

Minor points (non-blocking)

1. Ambiguous expression text: an expression key is returned as a bare string in Columns. A column literally named lower(email), if quoted, would be indistinguishable from the expression. This is rare, but a doc note or a parallel IsExpression marker would help consumers. Since the schema types are public, it may be worth deciding now.
2. Partition index clones: relkind 'r' partitions still list their cloned child indexes alongside the parent's. That is reasonable, but consumers will see N+1 indexes for a partitioned table. A one-line comment would help.
3. Invalid and partial indexes: indisvalid and indpred are not surfaced, so a partial or invalid unique index looks the same as a full one. The PR already lists this as deferred.
4. Version note: indnkeyatts needs PostgreSQL 11+, which the package doc already states. Older servers will fail at query time with a column error, and a clearer error message could be added.
5. MariaDB test: the new TestMariaDBLoader variant has not run locally, as the PR says. Please confirm it is green in CI before merging.
6. Test gap: the tests cover a lot, but none cover a quoted column name that contains a comma inside an INCLUDE list. TestPostgresLoader_QuotedIndexColumns covers key columns only.

Performance and security

• The queries remain single catalog round trips and take parameters ($1, $2). The added LATERAL unnest is cheap, and there are no injection concerns.

[github-actions]

Claude finished @ajitpratap0's task in 21s —— View job

---

Review of PR #622 (PostgreSQL index and key metadata, fixes #620)

I read the diff of pkg/schema/db/schema.go and pkg/schema/postgres/loader.go. I did not run the tests. I did not read the test files in detail, and I did not run the CI workflows.

Verdict: The change is correct and well scoped. I found no blocking issues.

Correctness

• Key order and INCLUDE split: unnest(ix.indkey) WITH ORDINALITY plus FILTER (WHERE k.ord <= ix.indnkeyatts) gives key columns in index order. INCLUDE columns go to a separate aggregate. json_agg(... ORDER BY k.ord) keeps the declared order.
• Expression keys: k.attnum = 0 maps to pg_get_indexdef(indexrelid, ord, true), and the LEFT JOIN ... AND k.attnum <> 0 keeps expression-only indexes in the result.
• NULL handling: The columns aggregate can't be empty, because every index has at least one key column, so json.Unmarshal on it is safe. include_columns is NULL when there are none, and the code checks inclJSON != nil before decoding it.
• Key flags from pg_constraint: pg_constraint is readable by any role. Grouping by (relname, attname) removes the duplicate-column rows. cardinality(conkey) = 1 keeps IsUnique consistent with MySQL's COLUMN_KEY = 'UNI'.
• Foreign key RefSchema: The added JOIN pg_namespace rn is an inner join. That is safe, since a referenced relation always has a namespace.
• Compatibility: The two new struct fields are additive. Both doc comments say which loaders set them.

Minor points (non-blocking)

1. Expression text vs. column name: An expression key and a quoted column such as "lower(email)" both appear as plain strings in Columns. A parallel marker, for example an ExprColumns set or an IsExpression flag, would let consumers tell them apart. It is cheap to decide now, since the schema types are public.
2. Partition clones: Child partitions (relkind r) still list their cloned indexes alongside the parent's. A one-line comment in loadIndexes would tell consumers that.
3. Key flags and the $1 schema filter: The pg_constraint subquery filters on the table's own schema, the same as before, so behavior there is unchanged. It joins to information_schema.columns by bare (table_name, column_name), which is fine because both sides are already restricted to $1.
4. Invalid and partial indexes: indisvalid and indpred are not surfaced. The PR lists this as deferred.
5. Pre-PG11 error: Older servers fail with a raw "column indnkeyatts does not exist" error. The package doc already states the PostgreSQL 11 requirement.
6. Test gaps: I could not find a test for a quoted INCLUDE column whose name contains a comma, quote or brace. TestPostgresLoader_QuotedIndexColumns covers key columns only. The JSON path should handle it, but a test would lock that in.
7. MariaDB test: The PR says the new TestMariaDBLoader variant has not run locally. Please confirm it is green in CI before merging.

Performance and security

• Each query is still a single catalog round trip. The added LATERAL unnest is cheap.
• All inputs are bound parameters ($1, $2), so I see no injection concerns.

No files changed.

[ajitpratap0]

Merge train (batch 3): rebased onto main (0aeaa50). The adversarial + security pass (Opus) found no issues. It confirmed the catalog queries remain parameterized ($1/$2) with no identifier concatenation. CI ran the testcontainers suites for the first time: TestMariaDBLoader and TestPostgresLoader_* pass on ubuntu (postgres:16 / mariadb:11.4). The Vercel failure was a transient Google Fonts fetch in the website build; a redeploy is green. The bot review's points are non-blocking: an expression-key marker, partition clone indexes, and indisvalid/indpred surfacing are noted for follow-up.