Skip to content

Commit 0f1690a

Browse files
authored
fix: handle PostgreSQL identifier quoting in partial index predicate comparison (#5788)
## Description Closes #5787 ## Problem After #5780, partial indexes with object literal `where` clauses are still recreated on every migration when predicates contain multiple conditions. PostgreSQL's `pg_get_expr()` returns unquoted identifiers for lowercase column names (`postcode`), while Prisma generates quoted identifiers (`"postcode"`). The AST comparison treats these as different, causing needless drop+recreate cycles. ```sql -- Prisma generates: ("deletedAt" IS NULL AND "postcode" IS NOT NULL) -- pg_get_expr() returns: ((("deletedAt" IS NULL) AND (postcode IS NOT NULL))) ``` ## Solution Extend `exprs_semantically_eq` to treat quoted and unquoted identifiers as equivalent when the unquoted form is a valid PostgreSQL identifier. Uses `sqlparser` with `PostgreSqlDialect` to verify that the identifier value can be safely used without quotes (i.e., it's not a reserved keyword and follows identifier rules). ## Changes - **PG `schema_differ.rs`** — Added identifier quoting comparison in `exprs_semantically_eq`; checks if `"foo"` and `foo` refer to the same column by re-parsing with `sqlparser` - **`partial.rs`** — Regression test for object literal predicates with `null` and `not: null` conditions <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved PostgreSQL schema comparison to more robustly treat identifiers as equivalent across quote styles and within expressions. * **Tests** * Added an idempotency test for Postgres partial indexes using object-literal filters with null and not-null conditions. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent ab73dcf commit 0f1690a

2 files changed

Lines changed: 44 additions & 6 deletions

File tree

  • schema-engine

schema-engine/connectors/sql-schema-connector/src/flavour/postgres/schema_differ.rs

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@ use sql_schema_describer::{
1717
postgres::PostgresSchemaExt,
1818
walkers::{IndexWalker, TableColumnWalker},
1919
};
20-
use sqlparser::ast::Expr;
20+
use sqlparser::{
21+
ast::{Expr, Ident},
22+
dialect::PostgreSqlDialect,
23+
parser::Parser,
24+
};
2125

2226
/// These can be tables or views, depending on the PostGIS version. In both cases, they should be ignored.
2327
static POSTGIS_TABLES_OR_VIEWS: LazyLock<RegexSet> = LazyLock::new(|| {
@@ -749,11 +753,7 @@ fn push_alter_enum_previous_usages_as_default(db: &DifferDatabase<'_>, alter_enu
749753

750754
// Accounts for PG expression normalization (operator aliases, parens, implicit literal casts).
751755
fn pg_predicates_semantically_equal(a: &str, b: &str) -> bool {
752-
use sqlparser::{
753-
ast::{Value, VisitMut, VisitorMut},
754-
dialect::PostgreSqlDialect,
755-
parser::Parser,
756-
};
756+
use sqlparser::ast::{Value, VisitMut, VisitorMut};
757757
use std::ops::ControlFlow;
758758

759759
struct StripPgNormalization;
@@ -792,6 +792,20 @@ fn pg_predicates_semantically_equal(a: &str, b: &str) -> bool {
792792
exprs_semantically_eq(&ast_a, &ast_b)
793793
}
794794

795+
// Compares two identifiers semantically.
796+
fn idents_semantically_eq(a: &Ident, b: &Ident) -> bool {
797+
if a == b {
798+
return true;
799+
}
800+
801+
matches!((a.quote_style, b.quote_style), (Some('"'), None) | (None, Some('"')))
802+
&& a.value == b.value
803+
&& Parser::new(&PostgreSqlDialect {})
804+
.try_with_sql(&a.value)
805+
.and_then(|mut parser| parser.parse_expr())
806+
.is_ok_and(|expr| matches!(expr, Expr::Identifier(_)))
807+
}
808+
795809
// Compares two expressions that have already been normalized by `StripPgNormalization`.
796810
fn exprs_semantically_eq(a: &Expr, b: &Expr) -> bool {
797811
use sqlparser::ast::CastKind;
@@ -801,6 +815,12 @@ fn exprs_semantically_eq(a: &Expr, b: &Expr) -> bool {
801815
}
802816

803817
match (a, b) {
818+
(Expr::Identifier(ia), Expr::Identifier(ib)) => idents_semantically_eq(ia, ib),
819+
820+
(Expr::CompoundIdentifier(ia), Expr::CompoundIdentifier(ib)) if ia.len() == ib.len() => {
821+
ia.iter().zip(ib.iter()).all(|(a, b)| idents_semantically_eq(a, b))
822+
}
823+
804824
// Value(x) matches Cast(::, Value(x), _): PG's single-level annotation on a bare literal.
805825
(
806826
Expr::Value(va),

schema-engine/sql-migration-tests/tests/migrations/indexes/partial.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1082,3 +1082,21 @@ fn partial_index_not_with_and_or_roundtrip_postgres(api: TestApi) {
10821082
api.schema_push(&dm).send().assert_green();
10831083
api.schema_push(&dm).send().assert_no_steps();
10841084
}
1085+
1086+
#[test_connector(tags(Postgres), exclude(CockroachDb), preview_features("partialIndexes"))]
1087+
fn partial_index_object_literal_null_and_not_null_is_idempotent_postgres(api: TestApi) {
1088+
let dm = api.datamodel_with_provider_and_features(
1089+
r#"model Shop {
1090+
id BigInt @id
1091+
postcode String?
1092+
deletedAt DateTime?
1093+
1094+
@@index([postcode], where: { deletedAt: null, postcode: { not: null } })
1095+
}"#,
1096+
&[],
1097+
PREVIEW_FEATURES,
1098+
);
1099+
1100+
api.schema_push(&dm).send().assert_green();
1101+
api.schema_push(&dm).send().assert_no_steps();
1102+
}

0 commit comments

Comments
 (0)