Skip to content

fix(goctl): preserve PostgreSQL partial unique index WHERE clause in model generation#5559

Open
hoshi200 wants to merge 2 commits into
zeromicro:masterfrom
hoshi200:fix/goctl-pg-partial-unique-index
Open

fix(goctl): preserve PostgreSQL partial unique index WHERE clause in model generation#5559
hoshi200 wants to merge 2 commits into
zeromicro:masterfrom
hoshi200:fix/goctl-pg-partial-unique-index

Conversation

@hoshi200

Copy link
Copy Markdown
Contributor

Problem

When running goctl model pg datasource against a PostgreSQL database with partial unique indexes, the generated FindOneByXxx methods lose the index WHERE clause.

CREATE UNIQUE INDEX idx_product_sku ON product (sku) WHERE status = 1 AND deleted_at IS NULL;

Before this fix, goctl generates:

SELECT * FROM product WHERE sku = $1 LIMIT 1
-- Missing: AND status = 1 AND deleted_at IS NULL

Fixes #3841

Root Cause

FindIndex() in postgresqlmodel.go joins pg_index but does not select indpred. The entire code generation pipeline has no concept of partial index predicates.

Fix

6 files, 7 layers:

Layer File Change
PG catalog postgresqlmodel.go pg_get_expr(C.INDPRED, C.INDRELID) AS predicate in FindIndex() SQL
Struct postgresqlmodel.go PostgreIndex.Predicate field
Index model infoschemamodel.go DbIndex.Predicate + Table.UniqueIndexPredicate map + population in Convert()
Propagation postgresqlmodel.go getIndex() copies predicate to DbIndex
Parser parser.go parser.Table.UniqueIndexPredicate + propagation in ConvertDataType()
Cache keys keys.go Key.Predicate field + index-name CamelCase suffix for disambiguation
Code gen findonebyfield.go Appends AND <predicate> to generated SQL WHERE clause

Before / After

Before:

query := fmt.Sprintf("select %s from %s where sku = $1 limit 1", ...)

After:

query := fmt.Sprintf("select %s from %s where sku = $1 and ((status = 1) AND (deleted_at IS NULL)) limit 1", ...)

Cache Key Disambiguation

Multiple partial indexes on the same column with different predicates now produce distinct cache keys:

cachePublicProductSkuPrefixIdxProductSku         = "cache:..:sku:IdxProductSku:"
cachePublicProductSkuPrefixIdxProductSkuArchived = "cache:..:sku:IdxProductSkuArchived:"

Verification

  • All existing tests pass
  • 2 new test cases in TestGenCacheKeys
  • End-to-end tested against live PostgreSQL

…model generation

Fixes zeromicro#3841

pg_get_expr(C.INDPRED, C.INDRELID) now retrieves the partial index
predicate from pg_index, propagated through all layers (PostgreIndex →
DbIndex → Column → model.Table → parser.Table → Key) and appended to
generated FindOneByXxx SQL WHERE clauses.

Cache keys are disambiguated using the index name as suffix for partial
unique indexes sharing the same column set but different predicates.
@codecov

codecov Bot commented Apr 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@kevwan kevwan added kind/bug Something isn't working area/goctl Categorizes issue or PR as related to goctl. labels Apr 30, 2026
@kevwan

kevwan commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

Review

This PR fixes a real gap in goctl model pg datasource — partial unique indexes are a PostgreSQL-specific feature that MySQL doesn't support, and the current code path had no concept of index predicates.

What the fix does:

  • Queries pg_get_expr(C.INDPRED, C.INDRELID) AS predicate from the PostgreSQL catalog to retrieve the partial index predicate
  • Propagates the predicate through the model pipeline: PostgreIndexDbIndexparser.Table → code generation
  • Appends AND <predicate> to the generated SQL WHERE clause in FindOneByXxx methods
  • Uses the index name (CamelCase suffix) to disambiguate cache keys when multiple partial indexes exist on the same column

Questions / concerns:

  1. SQL injection in predicates — The predicate is fetched via pg_get_expr() which returns a human-readable expression like ((status = 1) AND (deleted_at IS NULL)). This is injected directly into the generated SQL string in findonebyfield.go. Since this predicate comes from the DB schema (not user input), there is no runtime injection risk. However, it may contain unusual characters if a DB admin wrote exotic predicates — please confirm this is sanitized or add a comment noting the trust assumption.

  2. Generated SQL formatting — The after example shows:

    where sku = $1 and ((status = 1) AND (deleted_at IS NULL)) limit 1

    The mixed case (and vs AND) is a minor style inconsistency. Not blocking but worth noting for consistency.

  3. Cache key disambiguation — Using the index name CamelCase suffix is a good approach. The naming looks clean (IdxProductSku, IdxProductSkuArchived).

  4. Non-partial indexes — When indpred is NULL (non-partial index), pg_get_expr() returns an empty string or NULL. Is this handled correctly? The predicate field should be empty and no AND clause appended — please confirm this path is tested.

  5. Tests — 2 new test cases in TestGenCacheKeys are mentioned. Would be helpful to also have an integration test for the generated SQL with a partial predicate.

Overall: This is a well-scoped fix for a long-standing gap (fixes #3841). The pipeline approach (propagating predicate through all layers) is correct. Please clarify the NULL predicate handling before merging.

@hoshi200

Copy link
Copy Markdown
Contributor Author

@kevwan thanks for the thorough review — addressed each point below.

1. NULL predicate (non-partial indexes)

This path is safe end-to-end. When indpred is NULL, pg_get_expr returns SQL NULL → Go unmarshals to sql.NullString{Valid: false, String: ""}. Each downstream gate holds:

  • getIndex() copies e.Predicate.String""
  • Convert() skips via one.Index.Predicate != "" → not added to UniqueIndexPredicate
  • genCacheKeys() sets key.Predicate = ""
  • genFindOneByField() checks len(key.Predicate) > 0false → no AND appended

The new TestGenFindOneByFieldWithPartialIndex includes a sub-case that asserts a regular unique index produces where email = $1 with no extra clause.

2. Trust assumption

Added a comment in findonebyfield.go:31-33 explicitly noting the predicate originates from pg_get_expr() on the PostgreSQL system catalog — schema metadata, not user input — so there is no runtime injection surface.

3. and / AND casing

The template uses lowercase for all SQL keywords (select, from, where, limit), and convertJoin() joins column conditions with lowercase and via argJoin.With(" and "). Our appended " and " follows the same convention. The uppercase AND inside the predicate is pg_get_expr's own normalization — PostgreSQL treats both identically.

4. Cache key disambiguation

The CamelCase index-name suffix approach looks good, no concerns here.

5. Integration test

Added TestGenFindOneByFieldWithPartialIndex. It constructs a parser.Table with UniqueIndexPredicate set, calls genFindOneByField(postgreSql=true), and asserts the rendered template output contains the predicate in the WHERE clause. The second sub-case verifies that a regular (non-partial) unique index produces no extra AND suffix. No live PG connection required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/goctl Categorizes issue or PR as related to goctl. kind/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

goctl generated code issue with partial index on PostgreSQL model

2 participants