public inbox for [email protected]help / color / mirror / Atom feed
Schema-qualify the equality operator when deparsing NULLIF/IS DISTINCT FROM 4+ messages / 2 participants [nested] [flat]
* Schema-qualify the equality operator when deparsing NULLIF/IS DISTINCT FROM @ 2026-07-03 12:56 [email protected] 0 siblings, 1 reply; 4+ messages in thread From: [email protected] @ 2026-07-03 12:56 UTC (permalink / raw) To: [email protected] Hi hackers, A view that applies NULLIF or IS [NOT] DISTINCT FROM to a value whose "=" operator is not on the search_path (for example an hstore column) cannot be dumped and restored. ruleutils.c deparses those constructs with a bare "=", and pg_dump reloads with an empty search_path, so the reload fails with: ERROR: operator does not exist: public.hstore = public.hstore A plain OpExpr already avoids this by deparsing as OPERATOR(schema.=), but NULLIF and IS DISTINCT FROM have no syntactic slot for a qualified operator name. This is a long-standing report: https://stackoverflow.com/questions/23599926 https://www.postgresql.org/message-id/[email protected] Proposed fix (in the deparser) ------------------------------ When the equality operator would not be found by its bare name under the current search_path -- detected via generate_operator_name(), which already decides when the OPERATOR(...) decoration is required -- emit an equivalent expression that can carry the qualified operator, instead of the normal syntax: NULLIF(a, b) -> CASE WHEN a IS NOT NULL AND b IS NOT NULL AND (a OPERATOR(s.=) b) THEN NULL ELSE a END a IS DISTINCT FROM b -> ((a IS NOT NULL OR b IS NOT NULL) AND (a IS NULL OR b IS NULL OR NOT (a OPERATOR(s.=) b))) Both reproduce the executor's semantics exactly: NULLIF's single evaluation of null inputs (the IS NOT NULL guards make it faithful even for a non-strict "="), and DistinctExpr yielding NULL when "=" yields NULL for two non-null inputs. The DISTINCT form is always parenthesized so an enclosing NOT (IS NOT DISTINCT FROM) binds it correctly; CASE ... END is self-delimiting. When no qualification is needed, the deparsed output is unchanged. The substitute forms evaluate their inputs more than once, whereas NULLIF and DistinctExpr evaluate each input exactly once, so they are used only when the inputs contain no volatile functions; otherwise the original syntax is kept (it still reloads correctly whenever the operator is on the search_path). Details ------- * Target branch: master (the issue affects all live branches). * Tests: compiles cleanly; "make check" is green; new regression coverage added in contrib/hstore. pgindent- and "git diff --check"-clean. * Docs: none needed -- user-visible NULLIF / IS DISTINCT FROM behaviour is unchanged; only the deparsed text differs in the qualified case. * Performance: negligible; one extra check on a rarely-taken deparse path. Known limitation ---------------- A view whose *volatile* input uses an off-search-path operator still cannot be reloaded; the substitute forms are skipped there to avoid double-evaluating volatile inputs. Fully handling that case would require carrying the operator through the constructs' grammar, which this patch does not attempt. Feedback welcome on whether that is worth pursuing. v1 attached. Thanks, Michał Pasternak Attachments: [application/octet-stream] v1-0001-Schema-qualify-the-equality-operator-when-deparsi.patch (16.6K, ../../028117fa-439b-4762-b647-130bc19df89c@Spark/3-v1-0001-Schema-qualify-the-equality-operator-when-deparsi.patch) download | inline diff: From d107fa4d60d61bddb5c249d29641f657274f7fd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= <[email protected]> Date: Mon, 29 Jun 2026 14:00:54 +0200 Subject: [PATCH v1] Schema-qualify the equality operator when deparsing NULLIF/IS DISTINCT FROM The NULLIF and IS DISTINCT FROM constructs use an equality operator that their SQL syntax identifies only by the unqualified name "="; the operator is re-resolved against the prevailing search_path when a dumped rule or view definition is reloaded. Unlike a plain OpExpr, which ruleutils.c deparses using the OPERATOR(schema.op) notation, these constructs have nowhere to write a schema-qualified operator name. As a result a view that applies NULLIF or IS [NOT] DISTINCT FROM to values whose "=" operator is not on the search_path (for example an hstore column) could not be dumped and reloaded: pg_dump reloads with an empty search_path, so the reload failed with "operator does not exist: ... = ...". Fix this in the deparser. When the equality operator would not be found by its bare name under the current search_path (detected via generate_operator_name, which already decides when the OPERATOR(...) decoration is required), emit an equivalent expression that can carry the qualified operator instead of the normal syntax: NULLIF(a, b) -> CASE WHEN a IS NOT NULL AND b IS NOT NULL AND (a OPERATOR(s.=) b) THEN NULL ELSE a END a IS DISTINCT FROM b -> ((a IS NOT NULL OR b IS NOT NULL) AND (a IS NULL OR b IS NULL OR NOT (a OPERATOR(s.=) b))) The DISTINCT substitute is always parenthesized so an enclosing NOT (as in IS NOT DISTINCT FROM) binds correctly; CASE ... END is self-delimiting. Both reproduce the executor's semantics exactly, including NULLIF's single treatment of null inputs and DistinctExpr yielding NULL when "=" yields NULL for two non-null inputs. The substitute forms evaluate their inputs more than once, whereas NULLIF and DistinctExpr evaluate each input exactly once, so they are used only when the inputs contain no volatile functions; otherwise the original syntax is kept. When no qualification is needed the deparsed output is unchanged. A view whose volatile input uses an off-search-path operator still cannot be reloaded; fully handling that would require carrying the operator through the constructs' grammar, which this patch does not attempt. Add regression coverage to contrib/hstore. --- contrib/hstore/expected/hstore.out | 85 ++++++++++++++++++ contrib/hstore/sql/hstore.sql | 40 +++++++++ src/backend/utils/adt/ruleutils.c | 134 +++++++++++++++++++++++++++-- 3 files changed, 250 insertions(+), 9 deletions(-) diff --git a/contrib/hstore/expected/hstore.out b/contrib/hstore/expected/hstore.out index acea880..b433e23 100644 --- a/contrib/hstore/expected/hstore.out +++ b/contrib/hstore/expected/hstore.out @@ -1632,3 +1632,88 @@ WHERE hstore_hash(v)::bit(32) != hstore_hash_extended(v, 0)::bit(32) -------+----------+-----------+----------- (0 rows) +-- Views using NULLIF or IS [NOT] DISTINCT FROM on hstore values must be +-- deparsed with a schema-qualified equality operator when hstore's "=" +-- operator is not reachable through the search_path, so that the dumped +-- definition reloads correctly (e.g. with the empty search_path pg_dump uses). +CREATE SCHEMA hstore_view_test; +CREATE TABLE hstore_view_test.t (id int, c1 hstore, c2 hstore); +CREATE VIEW hstore_view_test.v_nullif AS + SELECT id, NULLIF(c1, c2) AS r FROM hstore_view_test.t; +CREATE VIEW hstore_view_test.v_distinct AS + SELECT id, c1 IS DISTINCT FROM c2 AS r FROM hstore_view_test.t; +CREATE VIEW hstore_view_test.v_notdist AS + SELECT id, c1 IS NOT DISTINCT FROM c2 AS r FROM hstore_view_test.t; +-- With hstore on the search_path the normal, unqualified syntax is used. +SET search_path = hstore_view_test, public; +SELECT pg_get_viewdef('hstore_view_test.v_nullif'::regclass, true); + pg_get_viewdef +------------------------- + SELECT id, + + NULLIF(c1, c2) AS r+ + FROM t; +(1 row) + +SELECT pg_get_viewdef('hstore_view_test.v_distinct'::regclass, true); + pg_get_viewdef +--------------------------------- + SELECT id, + + c1 IS DISTINCT FROM c2 AS r+ + FROM t; +(1 row) + +SELECT pg_get_viewdef('hstore_view_test.v_notdist'::regclass, true); + pg_get_viewdef +------------------------------------- + SELECT id, + + NOT c1 IS DISTINCT FROM c2 AS r+ + FROM t; +(1 row) + +-- With hstore off the search_path the operator must be schema-qualified. +SET search_path = hstore_view_test; +SELECT pg_get_viewdef('hstore_view_test.v_nullif'::regclass, true); + pg_get_viewdef +----------------------------------------------------------------------------------------------------------- + SELECT id, + + CASE WHEN c1 IS NOT NULL AND c2 IS NOT NULL AND (c1 OPERATOR(public.=) c2) THEN NULL ELSE c1 END AS r+ + FROM t; +(1 row) + +SELECT pg_get_viewdef('hstore_view_test.v_distinct'::regclass, true); + pg_get_viewdef +---------------------------------------------------------------------------------------------------------------- + SELECT id, + + ((c1 IS NOT NULL OR c2 IS NOT NULL) AND (c1 IS NULL OR c2 IS NULL OR NOT (c1 OPERATOR(public.=) c2))) AS r+ + FROM t; +(1 row) + +SELECT pg_get_viewdef('hstore_view_test.v_notdist'::regclass, true); + pg_get_viewdef +-------------------------------------------------------------------------------------------------------------------- + SELECT id, + + NOT ((c1 IS NOT NULL OR c2 IS NOT NULL) AND (c1 IS NULL OR c2 IS NULL OR NOT (c1 OPERATOR(public.=) c2))) AS r+ + FROM t; +(1 row) + +-- The deparsed definitions must reparse successfully under that same +-- restricted search_path (this is what failed before). +DO $$ +BEGIN + EXECUTE 'CREATE VIEW hstore_view_test.v_nullif_reload AS ' + || pg_get_viewdef('hstore_view_test.v_nullif'::regclass); + EXECUTE 'CREATE VIEW hstore_view_test.v_distinct_reload AS ' + || pg_get_viewdef('hstore_view_test.v_distinct'::regclass); + EXECUTE 'CREATE VIEW hstore_view_test.v_notdist_reload AS ' + || pg_get_viewdef('hstore_view_test.v_notdist'::regclass); +END $$; +RESET search_path; +DROP SCHEMA hstore_view_test CASCADE; +NOTICE: drop cascades to 7 other objects +DETAIL: drop cascades to table hstore_view_test.t +drop cascades to view hstore_view_test.v_nullif +drop cascades to view hstore_view_test.v_distinct +drop cascades to view hstore_view_test.v_notdist +drop cascades to view hstore_view_test.v_nullif_reload +drop cascades to view hstore_view_test.v_distinct_reload +drop cascades to view hstore_view_test.v_notdist_reload diff --git a/contrib/hstore/sql/hstore.sql b/contrib/hstore/sql/hstore.sql index 8ae95e8..60df633 100644 --- a/contrib/hstore/sql/hstore.sql +++ b/contrib/hstore/sql/hstore.sql @@ -393,3 +393,43 @@ FROM (VALUES (NULL::hstore), (''), ('"a key" =>1'), ('c => null'), ('e => 012345'), ('g => 2.345e+4')) x(v) WHERE hstore_hash(v)::bit(32) != hstore_hash_extended(v, 0)::bit(32) OR hstore_hash(v)::bit(32) = hstore_hash_extended(v, 1)::bit(32); + +-- Views using NULLIF or IS [NOT] DISTINCT FROM on hstore values must be +-- deparsed with a schema-qualified equality operator when hstore's "=" +-- operator is not reachable through the search_path, so that the dumped +-- definition reloads correctly (e.g. with the empty search_path pg_dump uses). +CREATE SCHEMA hstore_view_test; +CREATE TABLE hstore_view_test.t (id int, c1 hstore, c2 hstore); +CREATE VIEW hstore_view_test.v_nullif AS + SELECT id, NULLIF(c1, c2) AS r FROM hstore_view_test.t; +CREATE VIEW hstore_view_test.v_distinct AS + SELECT id, c1 IS DISTINCT FROM c2 AS r FROM hstore_view_test.t; +CREATE VIEW hstore_view_test.v_notdist AS + SELECT id, c1 IS NOT DISTINCT FROM c2 AS r FROM hstore_view_test.t; + +-- With hstore on the search_path the normal, unqualified syntax is used. +SET search_path = hstore_view_test, public; +SELECT pg_get_viewdef('hstore_view_test.v_nullif'::regclass, true); +SELECT pg_get_viewdef('hstore_view_test.v_distinct'::regclass, true); +SELECT pg_get_viewdef('hstore_view_test.v_notdist'::regclass, true); + +-- With hstore off the search_path the operator must be schema-qualified. +SET search_path = hstore_view_test; +SELECT pg_get_viewdef('hstore_view_test.v_nullif'::regclass, true); +SELECT pg_get_viewdef('hstore_view_test.v_distinct'::regclass, true); +SELECT pg_get_viewdef('hstore_view_test.v_notdist'::regclass, true); + +-- The deparsed definitions must reparse successfully under that same +-- restricted search_path (this is what failed before). +DO $$ +BEGIN + EXECUTE 'CREATE VIEW hstore_view_test.v_nullif_reload AS ' + || pg_get_viewdef('hstore_view_test.v_nullif'::regclass); + EXECUTE 'CREATE VIEW hstore_view_test.v_distinct_reload AS ' + || pg_get_viewdef('hstore_view_test.v_distinct'::regclass); + EXECUTE 'CREATE VIEW hstore_view_test.v_notdist_reload AS ' + || pg_get_viewdef('hstore_view_test.v_notdist'::regclass); +END $$; + +RESET search_path; +DROP SCHEMA hstore_view_test CASCADE; diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 88de5c0..0757c40 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -543,6 +543,8 @@ static char *generate_function_name(Oid funcid, int nargs, bool has_variadic, bool *use_variadic_p, bool inGroupBy); static char *generate_operator_name(Oid operid, Oid arg1, Oid arg2); +static bool nullif_distinct_needs_qualified_form(Oid opno, Node *arg1, + Node *arg2, char **opname); static void add_cast_to(StringInfo buf, Oid typid); static char *generate_qualified_type_name(Oid typid); static text *string_to_text(char *str); @@ -9800,6 +9802,52 @@ get_json_expr_options(JsonExpr *jsexpr, deparse_context *context, get_json_behavior(jsexpr->on_error, context, "ERROR"); } +/* + * nullif_distinct_needs_qualified_form + * + * Decide whether a NULLIF or IS [NOT] DISTINCT FROM construct has to be + * deparsed using an explicit, operator-qualified substitute expression + * instead of its normal syntax. + * + * Both constructs depend on an equality operator that is written, in their + * syntax, only as the unqualified name "=" --- the operator is re-resolved + * against the prevailing search_path when the dumped definition is reloaded. + * That fails when the operator would not be found by bare name, for example an + * extension's "=" operator in a view dumped with an empty search_path: the + * NULLIF / IS DISTINCT FROM syntax has nowhere to write a schema-qualified + * operator name (unlike a plain OpExpr, which can use OPERATOR(schema.=)). + * + * generate_operator_name() already schema-qualifies an operator (emitting the + * "OPERATOR(...)" decoration) exactly when the bare name would not resolve to + * the same operator under the current search_path, so we reuse it as the test. + * When qualification is needed, the caller emits an equivalent CASE / boolean + * expression that can carry the decoration; *opname returns the qualified name + * for it to use. Those substitute forms evaluate their input expressions more + * than once, so we only use them when the inputs are free of volatile + * functions; otherwise *opname is set to NULL and the caller falls back to the + * standard syntax (which still reloads correctly whenever the operator is on + * the search_path). + */ +static bool +nullif_distinct_needs_qualified_form(Oid opno, Node *arg1, Node *arg2, + char **opname) +{ + char *name; + + *opname = NULL; + + name = generate_operator_name(opno, exprType(arg1), exprType(arg2)); + if (strncmp(name, "OPERATOR(", 9) != 0) + return false; /* bare "=" resolves correctly; nothing to do */ + + /* the substitute forms evaluate arg1/arg2 more than once */ + if (contain_volatile_functions(arg1) || contain_volatile_functions(arg2)) + return false; + + *opname = name; + return true; +} + /* ---------- * get_rule_expr - Parse back an expression * @@ -9958,24 +10006,92 @@ get_rule_expr(Node *node, deparse_context *context, List *args = expr->args; Node *arg1 = (Node *) linitial(args); Node *arg2 = (Node *) lsecond(args); + char *opname; - if (!PRETTY_PAREN(context)) + if (nullif_distinct_needs_qualified_form(expr->opno, + arg1, arg2, &opname)) + { + /* + * The equality operator is not visible by its bare name + * under the prevailing search_path, but IS DISTINCT FROM + * syntax cannot carry a qualified operator name. Emit + * instead the equivalent boolean expression "(a IS NOT + * NULL OR b IS NOT NULL) AND (a IS NULL OR b IS NULL OR + * NOT (a = b))" using OPERATOR(schema.=). This + * reproduces the executor's DistinctExpr semantics, + * including yielding NULL when "=" yields NULL for two + * non-null inputs. The result is always parenthesized so + * that an enclosing NOT (as in IS NOT DISTINCT FROM) + * binds it correctly. + */ appendStringInfoChar(buf, '('); - get_rule_expr_paren(arg1, context, true, node); - appendStringInfoString(buf, " IS DISTINCT FROM "); - get_rule_expr_paren(arg2, context, true, node); - if (!PRETTY_PAREN(context)) - appendStringInfoChar(buf, ')'); + appendStringInfoChar(buf, '('); + get_rule_expr_paren(arg1, context, true, node); + appendStringInfoString(buf, " IS NOT NULL OR "); + get_rule_expr_paren(arg2, context, true, node); + appendStringInfoString(buf, " IS NOT NULL) AND ("); + get_rule_expr_paren(arg1, context, true, node); + appendStringInfoString(buf, " IS NULL OR "); + get_rule_expr_paren(arg2, context, true, node); + appendStringInfoString(buf, " IS NULL OR NOT ("); + get_rule_expr_paren(arg1, context, true, node); + appendStringInfo(buf, " %s ", opname); + get_rule_expr_paren(arg2, context, true, node); + appendStringInfoString(buf, ")))"); + } + else + { + if (!PRETTY_PAREN(context)) + appendStringInfoChar(buf, '('); + get_rule_expr_paren(arg1, context, true, node); + appendStringInfoString(buf, " IS DISTINCT FROM "); + get_rule_expr_paren(arg2, context, true, node); + if (!PRETTY_PAREN(context)) + appendStringInfoChar(buf, ')'); + } } break; case T_NullIfExpr: { NullIfExpr *nullifexpr = (NullIfExpr *) node; + Node *arg1 = (Node *) linitial(nullifexpr->args); + Node *arg2 = (Node *) lsecond(nullifexpr->args); + char *opname; - appendStringInfoString(buf, "NULLIF("); - get_rule_expr((Node *) nullifexpr->args, context, true); - appendStringInfoChar(buf, ')'); + if (nullif_distinct_needs_qualified_form(nullifexpr->opno, + arg1, arg2, &opname)) + { + /* + * The equality operator is not visible by its bare name + * under the prevailing search_path, but NULLIF syntax + * cannot carry a qualified operator name. Emit instead + * the equivalent expression "CASE WHEN a IS NOT NULL AND + * b IS NOT NULL AND (a = b) THEN NULL ELSE a END" using + * OPERATOR(schema.=). The IS NOT NULL guards keep the + * result faithful to NULLIF even for a non-strict "=" + * operator (NULLIF does not apply "=" when either input + * is null). CASE ... END is self-delimiting, so no outer + * parentheses are needed. + */ + appendStringInfoString(buf, "CASE WHEN "); + get_rule_expr_paren(arg1, context, true, node); + appendStringInfoString(buf, " IS NOT NULL AND "); + get_rule_expr_paren(arg2, context, true, node); + appendStringInfoString(buf, " IS NOT NULL AND ("); + get_rule_expr_paren(arg1, context, true, node); + appendStringInfo(buf, " %s ", opname); + get_rule_expr_paren(arg2, context, true, node); + appendStringInfoString(buf, ") THEN NULL ELSE "); + get_rule_expr_paren(arg1, context, true, node); + appendStringInfoString(buf, " END"); + } + else + { + appendStringInfoString(buf, "NULLIF("); + get_rule_expr((Node *) nullifexpr->args, context, true); + appendStringInfoChar(buf, ')'); + } } break; -- 2.54.0 ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Schema-qualify the equality operator when deparsing NULLIF/IS DISTINCT FROM @ 2026-07-03 14:11 Tom Lane <[email protected]> parent: [email protected] 0 siblings, 1 reply; 4+ messages in thread From: Tom Lane @ 2026-07-03 14:11 UTC (permalink / raw) To: [email protected]; +Cc: [email protected] [email protected] writes: > A view that applies NULLIF or IS [NOT] DISTINCT FROM to a value whose > "=" operator is not on the search_path (for example an hstore column) > cannot be dumped and restored. Yeah, this has been a known issue for a long time. I cataloged a bunch of related cases at https://www.postgresql.org/message-id/10492.1531515255%40sss.pgh.pa.us but I missed JOIN USING, which also fails to mention exactly which operator it resolved the semantics with. It doesn't seem hugely helpful to fix one case without fixing them all, and fixing them all is a lot of work :-( > Proposed fix (in the deparser) > ------------------------------ > When the equality operator would not be found by its bare name under the > current search_path -- detected via generate_operator_name(), which > already decides when the OPERATOR(...) decoration is required -- emit an > equivalent expression that can carry the qualified operator, instead of > the normal syntax: > NULLIF(a, b) > -> CASE WHEN a IS NOT NULL AND b IS NOT NULL AND (a OPERATOR(s.=) b) > THEN NULL ELSE a END Don't like this approach one bit. While the output it produces might be semantically equivalent (for non-volatile expressions anyway), it's not equivalent performance-wise, especially not if the change blocks any optimizations. Also, other cases such as JOIN USING really can't be fixed without new syntax. I'm kind of surprised that we haven't gotten more complaints since 2018, but there really haven't been all that many, so we never got to the point of putting in the work to fix this topic properly. If you feel motivated, though, have at it. regards, tom lane ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Schema-qualify the equality operator when deparsing NULLIF/IS DISTINCT FROM @ 2026-07-07 14:01 [email protected] parent: Tom Lane <[email protected]> 0 siblings, 1 reply; 4+ messages in thread From: [email protected] @ 2026-07-07 14:01 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: [email protected] Tom Lane <[email protected]> writes: > Don't like this approach one bit. While the output it produces might > be semantically equivalent (for non-volatile expressions anyway), > it's not equivalent performance-wise, especially not if the change > blocks any optimizations. Also, other cases such as JOIN USING really > can't be fixed without new syntax. Thanks for the review. You are right that rewriting these constructs into explicit-operator expressions at deparse time was a dead end: the rewritten forms evaluate their inputs more than once and stop being the construct the user wrote. v1 is withdrawn. Attached is v2, which takes the opposite approach: instead of teaching the deparser to avoid the constructs, it gives every construct that resolves an operator by unqualified name a place to write a schema-qualified one. That covers the full list you cataloged back in 2018 ([email protected]) -- IS [NOT] DISTINCT FROM, NULLIF, simple CASE, row comparisons, ANY/ALL subquery comparisons -- plus JOIN USING / NATURAL JOIN, which has the same disease. Why bother? Trawling the lists turned up roughly eleven independent field reports of this failure class since 2014. IS DISTINCT FROM accounts for seven of them, mostly via trigger WHEN clauses comparing extension types (citext, hstore, PostGIS), where the dump either fails to restore or the trigger definition changes meaning. JOIN USING appears twice, including the 2020 case of a citext join inside a materialized view that silently returned wrong results after dump/reload (CAC35HNnNGavaZ=P=rUcwTwYEhfoyXDg32REXCRDgxBmC3No3nA@mail.gmail.com). NULLIF appears twice. And while writing the tests I found one more that nobody had reported: a multi-column (a, b) IN (SELECT ...) whose columns resolve equality operators from different schemas silently swaps one column's operator on reload *today* -- ruleutils prints only the first column's operator name and even has a comment admitting the approximation. Patch 0004 carries a regression test whose setup, run on unpatched master, reproduces the corruption. The design is one principle applied six times. The parser always accepts an OPERATOR() decoration naming the operator(s) explicitly; ruleutils.c emits the decoration only when reparsing the undecorated form would not resolve the very same operator OIDs -- that is, when the operator is not reachable by its bare name under the prevailing search path, or is not named "=" where "=" is what an undecorated reparse would look up. Dumps of ordinary databases are byte-for-byte unchanged. Parse analysis produces exactly the same expression trees as before: v2 changes only what ruleutils emits -- the executor node trees are untouched, so plan shapes, the new PG 19 IS DISTINCT FROM simplifications, and evaluation counts are all unaffected, and no planner optimization is blocked. The syntax, per construct: IS [NOT] DISTINCT FROM a IS DISTINCT OPERATOR(s.=) FROM b NULLIF NULLIF(a, b USING OPERATOR(s.=)) JOIN USING JOIN t USING (a, b) OPERATOR (s.=, s.=) row comparison ROW(a, b) OPERATOR(s.<, s.<) ROW(c, d) ANY/ALL subquery (a, b) OPERATOR(s.=, s.=) ANY (SELECT ...) simple CASE CASE x WHEN y USING OPERATOR(s.=) THEN ... No new keywords; the grammar reuses the existing OPERATOR() production and extends it to carry a comma-separated list where a construct compares multiple columns. bison 3.8.2 reports zero shift/reduce or reduce/reduce conflicts (checked with counterexample analysis at every step, since several superficially nicer spellings do conflict), and the regression tests exercise the forms in combination: mixed bare/qualified lists, arity mismatches, non-boolean operators, and b_expr contexts. The patches, bisectable (the core regression suite is green at every step of the series; check-world at the tip): 0001 IS [NOT] DISTINCT FROM and NULLIF, plus a contrib/hstore round-trip test against a real extension "=" and a trigger WHEN-clause round-trip test. (12 files changed, 658 insertions(+), 10 deletions(-)) 0002 JOIN USING and NATURAL JOIN; JoinExpr gains a usingOperators field, hence a catversion bump. (9 files changed, 372 insertions(+), 29 deletions(-)) 0003 Row comparisons; the single-name OPERATOR() decoration generalizes to a per-column list. Contexts that accept an operator name but not a list (CREATE OPERATOR, DDL options) reject lists with a uniform error message. (12 files changed, 532 insertions(+), 27 deletions(-)) 0004 ANY/ALL and row-op-subquery sublinks; fixes the silent IN operator swap described above. Raw-parse-only change, no catversion bump. (7 files changed, 524 insertions(+), 40 deletions(-)) 0005 Simple CASE, one optional USING OPERATOR() per WHEN arm; CaseWhen gains a field, hence a catversion bump. (9 files changed, 304 insertions(+), 8 deletions(-)) The catversion bumps in 0002 and 0005 are included so that testers get a clean initdb; whoever commits this will of course re-bump. Some spellings are open to bikeshedding, and I am happy to rework them. In particular NULLIF could take the operator as a third argument, NULLIF(a, b, OPERATOR(s.=)), and JOIN USING could attach operators per column, USING (a OPERATOR(s.=), b), instead of the trailing list; I validated both alternatives as conflict-free before settling on the attached forms, which keep the column list readable and match the row-comparison list style. Likewise, a row comparison whose columns all use the same qualified operator currently prints the full repeated list rather than a single-element decoration; that is deliberate (explicit is better than clever) but easily changed. Full disclosure: this series was developed with substantial AI assistance (Claude). Every grammar candidate was validated with bison before adoption, every patch went through adversarial review, and the full check-world plus the pg_upgrade dump/restore round-trip of the regression database (which keeps all the new views, and a trigger whose WHEN clause exercises the same deparse path, precisely so that 002_pg_upgrade re-parses them) are green locally on macOS. If the direction looks right I will register this in the next commitfest (PG 20-1). Feedback on the syntax choices is especially welcome for the simple CASE arm clause and the row-comparison operator lists, where the SQL committee gives us the least precedent to lean on. Best regards, Michał Pasternak -- Z uszanowaniem, Michał Pasternak 📱+48793668733 📧 [email protected] https://bpp.iplweb.pl/ 🗓️ https://calendly.com/mpasternak/ On 3 lip 2026 o 16:11 +0200, Tom Lane <[email protected]>, wrote: > [email protected] writes: > > A view that applies NULLIF or IS [NOT] DISTINCT FROM to a value whose > > "=" operator is not on the search_path (for example an hstore column) > > cannot be dumped and restored. > > Yeah, this has been a known issue for a long time. I cataloged a > bunch of related cases at > > https://www.postgresql.org/message-id/10492.1531515255%40sss.pgh.pa.us > > but I missed JOIN USING, which also fails to mention exactly which > operator it resolved the semantics with. It doesn't seem hugely > helpful to fix one case without fixing them all, and fixing them all > is a lot of work :-( > > > > Proposed fix (in the deparser) > > ------------------------------ > > When the equality operator would not be found by its bare name under the > > current search_path -- detected via generate_operator_name(), which > > already decides when the OPERATOR(...) decoration is required -- emit an > > equivalent expression that can carry the qualified operator, instead of > > the normal syntax: > > > NULLIF(a, b) > > -> CASE WHEN a IS NOT NULL AND b IS NOT NULL AND (a OPERATOR(s.=) b) > > THEN NULL ELSE a END > > Don't like this approach one bit. While the output it produces might > be semantically equivalent (for non-volatile expressions anyway), > it's not equivalent performance-wise, especially not if the change > blocks any optimizations. Also, other cases such as JOIN USING really > can't be fixed without new syntax. > > I'm kind of surprised that we haven't gotten more complaints since > 2018, but there really haven't been all that many, so we never got > to the point of putting in the work to fix this topic properly. > If you feel motivated, though, have at it. > > regards, tom lane Attachments: [application/octet-stream] v2-0001-Allow-schema-qualifying-the-operator-in-IS-DISTIN.patch (39.8K, ../../66fa3fe6-8c99-4120-935c-f32f3b61fc30@Spark/3-v2-0001-Allow-schema-qualifying-the-operator-in-IS-DISTIN.patch) download | inline diff: From 2355f4ecbc8cab46a32bb5e997870f00c47aebea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= <[email protected]> Date: Mon, 6 Jul 2026 22:14:00 +0200 Subject: [PATCH v2 1/5] Allow schema-qualifying the operator in IS DISTINCT FROM/NULLIF The IS [NOT] DISTINCT FROM and NULLIF constructs look up their equality operator by the unqualified name "=" at parse time, and their syntax has no place to write a schema-qualified operator name. Consequently a view or rule using them with an operator that is not reachable through the prevailing search_path -- for example an extension type's "=" while restoring a dump with the restricted search_path pg_dump uses -- either fails to reload or, worse, silently resolves a different operator. Provide the missing syntax: a IS [NOT] DISTINCT OPERATOR(schema.=) FROM b and NULLIF(a, b USING OPERATOR(schema.=)). The decoration is optional; ruleutils.c emits it only when reparsing the undecorated construct would not resolve the same operator -- i.e. when the operator is not reachable by the bare name "=" under the prevailing search path, or is not named "=" at all. Parse analysis is unchanged: A_Expr.name already carries a possibly-qualified name list. Per discussion, this is the first of a series covering all constructs cataloged in [email protected] plus JOIN USING. Document the optional decoration: the IS [NOT] DISTINCT FROM predicates and the NULLIF section gain it in their synopses, each noting that the "=" operator is resolved via the search path and that pg_dump emits the decoration when the bare name would not resolve at restore time or the operator is not named "=" at all; the OPERATOR() discussion in the syntax chapter cross-references both. Add a round-trip test to contrib/hstore, a real extension whose "=" lives outside pg_catalog: views over NULLIF / IS [NOT] DISTINCT FROM on hstore columns deparse with OPERATOR(public.=) once hstore's schema is off the search_path, and reparse successfully under that restricted path. Discussion: https://postgr.es/m/[email protected] --- contrib/hstore/expected/hstore.out | 88 ++++++ contrib/hstore/sql/hstore.sql | 43 +++ doc/src/sgml/func/func-comparison.sgml | 23 +- doc/src/sgml/func/func-conditional.sgml | 17 +- doc/src/sgml/syntax.sgml | 11 + src/backend/parser/gram.y | 20 ++ src/backend/parser/parse_expr.c | 7 + src/backend/utils/adt/ruleutils.c | 50 +++- src/include/nodes/parsenodes.h | 9 +- .../regress/expected/operator_qualify.out | 250 ++++++++++++++++++ src/test/regress/parallel_schedule | 2 +- src/test/regress/sql/operator_qualify.sql | 148 +++++++++++ 12 files changed, 658 insertions(+), 10 deletions(-) create mode 100644 src/test/regress/expected/operator_qualify.out create mode 100644 src/test/regress/sql/operator_qualify.sql diff --git a/contrib/hstore/expected/hstore.out b/contrib/hstore/expected/hstore.out index acea8806ba..159b409934 100644 --- a/contrib/hstore/expected/hstore.out +++ b/contrib/hstore/expected/hstore.out @@ -1632,3 +1632,91 @@ WHERE hstore_hash(v)::bit(32) != hstore_hash_extended(v, 0)::bit(32) -------+----------+-----------+----------- (0 rows) +-- Views using NULLIF or IS [NOT] DISTINCT FROM on hstore values must be +-- deparsed with a schema-qualified equality operator when hstore's "=" +-- operator is not reachable through the search_path, so that the dumped +-- definition reloads correctly (e.g. with the restricted search_path pg_dump +-- uses). +CREATE SCHEMA hstore_view_test; +CREATE TABLE hstore_view_test.t (id int, c1 hstore, c2 hstore); +CREATE VIEW hstore_view_test.v_nullif AS + SELECT id, NULLIF(c1, c2) AS r FROM hstore_view_test.t; +CREATE VIEW hstore_view_test.v_distinct AS + SELECT id, c1 IS DISTINCT FROM c2 AS r FROM hstore_view_test.t; +CREATE VIEW hstore_view_test.v_notdist AS + SELECT id, c1 IS NOT DISTINCT FROM c2 AS r FROM hstore_view_test.t; +-- With hstore's schema (public) on the search_path the normal, unqualified +-- syntax is used. +SET search_path = hstore_view_test, public; +SELECT pg_get_viewdef('hstore_view_test.v_nullif'::regclass, true); + pg_get_viewdef +------------------------- + SELECT id, + + NULLIF(c1, c2) AS r+ + FROM t; +(1 row) + +SELECT pg_get_viewdef('hstore_view_test.v_distinct'::regclass, true); + pg_get_viewdef +--------------------------------- + SELECT id, + + c1 IS DISTINCT FROM c2 AS r+ + FROM t; +(1 row) + +SELECT pg_get_viewdef('hstore_view_test.v_notdist'::regclass, true); + pg_get_viewdef +------------------------------------- + SELECT id, + + NOT c1 IS DISTINCT FROM c2 AS r+ + FROM t; +(1 row) + +-- With public off the search_path the operator must be schema-qualified, +-- using the dedicated OPERATOR() positions of these constructs. +SET search_path = hstore_view_test; +SELECT pg_get_viewdef('hstore_view_test.v_nullif'::regclass, true); + pg_get_viewdef +-------------------------------------------------- + SELECT id, + + NULLIF(c1, c2 USING OPERATOR(public.=)) AS r+ + FROM t; +(1 row) + +SELECT pg_get_viewdef('hstore_view_test.v_distinct'::regclass, true); + pg_get_viewdef +---------------------------------------------------- + SELECT id, + + c1 IS DISTINCT OPERATOR(public.=) FROM c2 AS r+ + FROM t; +(1 row) + +SELECT pg_get_viewdef('hstore_view_test.v_notdist'::regclass, true); + pg_get_viewdef +-------------------------------------------------------- + SELECT id, + + NOT c1 IS DISTINCT OPERATOR(public.=) FROM c2 AS r+ + FROM t; +(1 row) + +-- The deparsed definitions must reparse successfully under that same +-- restricted search_path (this is what failed before). +DO $$ +BEGIN + EXECUTE 'CREATE VIEW hstore_view_test.v_nullif_reload AS ' + || pg_get_viewdef('hstore_view_test.v_nullif'::regclass); + EXECUTE 'CREATE VIEW hstore_view_test.v_distinct_reload AS ' + || pg_get_viewdef('hstore_view_test.v_distinct'::regclass); + EXECUTE 'CREATE VIEW hstore_view_test.v_notdist_reload AS ' + || pg_get_viewdef('hstore_view_test.v_notdist'::regclass); +END $$; +RESET search_path; +DROP SCHEMA hstore_view_test CASCADE; +NOTICE: drop cascades to 7 other objects +DETAIL: drop cascades to table hstore_view_test.t +drop cascades to view hstore_view_test.v_nullif +drop cascades to view hstore_view_test.v_distinct +drop cascades to view hstore_view_test.v_notdist +drop cascades to view hstore_view_test.v_nullif_reload +drop cascades to view hstore_view_test.v_distinct_reload +drop cascades to view hstore_view_test.v_notdist_reload diff --git a/contrib/hstore/sql/hstore.sql b/contrib/hstore/sql/hstore.sql index 8ae95e8a51..41654d0374 100644 --- a/contrib/hstore/sql/hstore.sql +++ b/contrib/hstore/sql/hstore.sql @@ -393,3 +393,46 @@ FROM (VALUES (NULL::hstore), (''), ('"a key" =>1'), ('c => null'), ('e => 012345'), ('g => 2.345e+4')) x(v) WHERE hstore_hash(v)::bit(32) != hstore_hash_extended(v, 0)::bit(32) OR hstore_hash(v)::bit(32) = hstore_hash_extended(v, 1)::bit(32); + +-- Views using NULLIF or IS [NOT] DISTINCT FROM on hstore values must be +-- deparsed with a schema-qualified equality operator when hstore's "=" +-- operator is not reachable through the search_path, so that the dumped +-- definition reloads correctly (e.g. with the restricted search_path pg_dump +-- uses). +CREATE SCHEMA hstore_view_test; +CREATE TABLE hstore_view_test.t (id int, c1 hstore, c2 hstore); +CREATE VIEW hstore_view_test.v_nullif AS + SELECT id, NULLIF(c1, c2) AS r FROM hstore_view_test.t; +CREATE VIEW hstore_view_test.v_distinct AS + SELECT id, c1 IS DISTINCT FROM c2 AS r FROM hstore_view_test.t; +CREATE VIEW hstore_view_test.v_notdist AS + SELECT id, c1 IS NOT DISTINCT FROM c2 AS r FROM hstore_view_test.t; + +-- With hstore's schema (public) on the search_path the normal, unqualified +-- syntax is used. +SET search_path = hstore_view_test, public; +SELECT pg_get_viewdef('hstore_view_test.v_nullif'::regclass, true); +SELECT pg_get_viewdef('hstore_view_test.v_distinct'::regclass, true); +SELECT pg_get_viewdef('hstore_view_test.v_notdist'::regclass, true); + +-- With public off the search_path the operator must be schema-qualified, +-- using the dedicated OPERATOR() positions of these constructs. +SET search_path = hstore_view_test; +SELECT pg_get_viewdef('hstore_view_test.v_nullif'::regclass, true); +SELECT pg_get_viewdef('hstore_view_test.v_distinct'::regclass, true); +SELECT pg_get_viewdef('hstore_view_test.v_notdist'::regclass, true); + +-- The deparsed definitions must reparse successfully under that same +-- restricted search_path (this is what failed before). +DO $$ +BEGIN + EXECUTE 'CREATE VIEW hstore_view_test.v_nullif_reload AS ' + || pg_get_viewdef('hstore_view_test.v_nullif'::regclass); + EXECUTE 'CREATE VIEW hstore_view_test.v_distinct_reload AS ' + || pg_get_viewdef('hstore_view_test.v_distinct'::regclass); + EXECUTE 'CREATE VIEW hstore_view_test.v_notdist_reload AS ' + || pg_get_viewdef('hstore_view_test.v_notdist'::regclass); +END $$; + +RESET search_path; +DROP SCHEMA hstore_view_test CASCADE; diff --git a/doc/src/sgml/func/func-comparison.sgml b/doc/src/sgml/func/func-comparison.sgml index ecb1d89463..649d672c62 100644 --- a/doc/src/sgml/func/func-comparison.sgml +++ b/doc/src/sgml/func/func-comparison.sgml @@ -202,7 +202,7 @@ <row> <entry role="func_table_entry"><para role="func_signature"> - <replaceable>datatype</replaceable> <literal>IS DISTINCT FROM</literal> <replaceable>datatype</replaceable> + <replaceable>datatype</replaceable> <literal>IS DISTINCT</literal> <optional> <literal>OPERATOR(</literal><replaceable>operator</replaceable><literal>)</literal> </optional> <literal>FROM</literal> <replaceable>datatype</replaceable> <returnvalue>boolean</returnvalue> </para> <para> @@ -220,7 +220,7 @@ <row> <entry role="func_table_entry"><para role="func_signature"> - <replaceable>datatype</replaceable> <literal>IS NOT DISTINCT FROM</literal> <replaceable>datatype</replaceable> + <replaceable>datatype</replaceable> <literal>IS NOT DISTINCT</literal> <optional> <literal>OPERATOR(</literal><replaceable>operator</replaceable><literal>)</literal> </optional> <literal>FROM</literal> <replaceable>datatype</replaceable> <returnvalue>boolean</returnvalue> </para> <para> @@ -450,8 +450,8 @@ this behavior is not suitable, use the <literal>IS <optional> NOT </optional> DISTINCT FROM</literal> predicates: <synopsis> -<replaceable>a</replaceable> IS DISTINCT FROM <replaceable>b</replaceable> -<replaceable>a</replaceable> IS NOT DISTINCT FROM <replaceable>b</replaceable> +<replaceable>a</replaceable> IS DISTINCT <optional> OPERATOR(<replaceable>operator</replaceable>) </optional> FROM <replaceable>b</replaceable> +<replaceable>a</replaceable> IS NOT DISTINCT <optional> OPERATOR(<replaceable>operator</replaceable>) </optional> FROM <replaceable>b</replaceable> </synopsis> For non-null inputs, <literal>IS DISTINCT FROM</literal> is the same as the <literal><></literal> operator. However, if both @@ -463,6 +463,21 @@ were a normal data value, rather than <quote>unknown</quote>. </para> + <para> + The equality test underlying these predicates uses the operator + named <literal>=</literal>, resolved by name through the current search + path. To pin a specific operator instead — for example one belonging + to an extension whose schema is not on the search path — write its + name with the optional <literal>OPERATOR()</literal> decoration + (see <xref linkend="sql-expressions-operator-calls"/>). When a view or + rule that uses these predicates is dumped, + <application>pg_dump</application> emits this decoration automatically + whenever the bare name <literal>=</literal> would not resolve to the same + operator under the search path used at restore time, or the intended + operator is not named <literal>=</literal> at all, so that the definition + reloads unambiguously. + </para> + <para> <indexterm> <primary>IS NULL</primary> diff --git a/doc/src/sgml/func/func-conditional.sgml b/doc/src/sgml/func/func-conditional.sgml index 7ca53dbf1a..137d5c7e43 100644 --- a/doc/src/sgml/func/func-conditional.sgml +++ b/doc/src/sgml/func/func-conditional.sgml @@ -210,7 +210,7 @@ SELECT COALESCE(description, short_description, '(none)') ... </indexterm> <synopsis> -<function>NULLIF</function>(<replaceable>value1</replaceable>, <replaceable>value2</replaceable>) +<function>NULLIF</function>(<replaceable>value1</replaceable>, <replaceable>value2</replaceable> <optional> USING OPERATOR(<replaceable>operator</replaceable>) </optional>) </synopsis> <para> @@ -235,6 +235,21 @@ SELECT NULLIF(value, '(none)') ... suitable <literal>=</literal> operator available. </para> + <para> + That <literal>=</literal> operator is resolved by name through the current + search path. To pin a specific operator instead — for example one + belonging to an extension whose schema is not on the search path — + give its name with the optional <literal>USING OPERATOR()</literal> clause + (see <xref linkend="sql-expressions-operator-calls"/> for + the <literal>OPERATOR()</literal> notation). When a view or rule that + uses <function>NULLIF</function> is dumped, + <application>pg_dump</application> emits this clause automatically whenever + the bare name <literal>=</literal> would not resolve to the same operator + under the search path used at restore time, or the intended operator is not + named <literal>=</literal> at all, so that the definition reloads + unambiguously. + </para> + <para> The result has the same type as the first argument — but there is a subtlety. What is actually returned is the first argument of the diff --git a/doc/src/sgml/syntax.sgml b/doc/src/sgml/syntax.sgml index 6748299686..b5aad8b4c7 100644 --- a/doc/src/sgml/syntax.sgml +++ b/doc/src/sgml/syntax.sgml @@ -1128,6 +1128,17 @@ SELECT 3 OPERATOR(pg_catalog.+) 4; which specific operator appears inside <literal>OPERATOR()</literal>. </para> + <para> + A few constructs that resolve an equality operator only by its bare + name accept the <literal>OPERATOR()</literal> decoration in a dedicated + syntactic position of their own, so that a schema-qualified operator, or + one whose name is not <literal>=</literal>, can be pinned there as + well: <literal>IS <optional> NOT </optional> DISTINCT + FROM</literal> (see <xref linkend="functions-comparison-pred-table"/>) + and <function>NULLIF</function> + (see <xref linkend="functions-nullif"/>). + </para> + <note> <para> <productname>PostgreSQL</productname> versions before 9.5 used slightly different diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index ff4e1388c5..54511313dc 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -16134,6 +16134,14 @@ a_expr: c_expr { $$ = $1; } { $$ = (Node *) makeSimpleA_Expr(AEXPR_NOT_DISTINCT, "=", $1, $6, @2); } + | a_expr IS DISTINCT OPERATOR '(' any_operator ')' FROM a_expr %prec IS + { + $$ = (Node *) makeA_Expr(AEXPR_DISTINCT, $6, $1, $9, @2); + } + | a_expr IS NOT DISTINCT OPERATOR '(' any_operator ')' FROM a_expr %prec IS + { + $$ = (Node *) makeA_Expr(AEXPR_NOT_DISTINCT, $7, $1, $10, @2); + } | a_expr BETWEEN opt_asymmetric b_expr AND a_expr %prec BETWEEN { $$ = (Node *) makeSimpleA_Expr(AEXPR_BETWEEN, @@ -16397,6 +16405,14 @@ b_expr: c_expr { $$ = (Node *) makeSimpleA_Expr(AEXPR_NOT_DISTINCT, "=", $1, $6, @2); } + | b_expr IS DISTINCT OPERATOR '(' any_operator ')' FROM b_expr %prec IS + { + $$ = (Node *) makeA_Expr(AEXPR_DISTINCT, $6, $1, $9, @2); + } + | b_expr IS NOT DISTINCT OPERATOR '(' any_operator ')' FROM b_expr %prec IS + { + $$ = (Node *) makeA_Expr(AEXPR_NOT_DISTINCT, $7, $1, $10, @2); + } | b_expr IS DOCUMENT_P %prec IS { $$ = makeXmlExpr(IS_DOCUMENT, NULL, NIL, @@ -16912,6 +16928,10 @@ func_expr_common_subexpr: { $$ = (Node *) makeSimpleA_Expr(AEXPR_NULLIF, "=", $3, $5, @1); } + | NULLIF '(' a_expr ',' a_expr USING OPERATOR '(' any_operator ')' ')' + { + $$ = (Node *) makeA_Expr(AEXPR_NULLIF, $9, $3, $5, @1); + } | COALESCE '(' expr_list ')' { CoalesceExpr *c = makeNode(CoalesceExpr); diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c index 9adc9d4c0f..0c5e1a3487 100644 --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -1054,6 +1054,13 @@ transformAExprDistinct(ParseState *pstate, A_Expr *a) * If either input is an undecorated NULL literal, transform to a NullTest * on the other input. That's simpler to process than a full DistinctExpr, * and it avoids needing to require that the datatype have an = operator. + * + * Note that any operator given explicitly (a->name, e.g. via the + * OPERATOR() decoration) is deliberately neither consulted nor validated + * in this path: "x IS DISTINCT FROM NULL" is equivalent to "x IS NOT + * NULL" regardless of the operator, so the result does not depend on it + * and we do not require that any such operator even exist (see + * make_nulltest_from_distinct). */ if (exprIsNullConstant(rexpr)) return make_nulltest_from_distinct(pstate, a, lexpr); diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 88de5c0481..8a9260fcd7 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -9958,11 +9958,37 @@ get_rule_expr(Node *node, deparse_context *context, List *args = expr->args; Node *arg1 = (Node *) linitial(args); Node *arg2 = (Node *) lsecond(args); + char *opname; + bool decorate; + /* + * IS DISTINCT FROM resolves its operator by the unqualified + * name "=" when it is parsed back. We must therefore show + * the operator explicitly -- a IS DISTINCT OPERATOR(s.=) FROM + * b -- whenever reparsing the bare form would not recover + * this same operator. There are two such cases: the operator + * is not reachable by an unqualified lookup of its own name + * (then generate_operator_name() already returns the + * OPERATOR(...) form), or its name is something other than + * "=" (then a bare construct would wrongly resolve "="; wrap + * the bare name in OPERATOR(), which any_operator syntax + * accepts). + */ + opname = generate_operator_name(expr->opno, + exprType(arg1), + exprType(arg2)); + decorate = (strncmp(opname, "OPERATOR(", 9) == 0) || + (strcmp(opname, "=") != 0); if (!PRETTY_PAREN(context)) appendStringInfoChar(buf, '('); get_rule_expr_paren(arg1, context, true, node); - appendStringInfoString(buf, " IS DISTINCT FROM "); + if (!decorate) + appendStringInfoString(buf, " IS DISTINCT FROM "); + else if (strncmp(opname, "OPERATOR(", 9) == 0) + appendStringInfo(buf, " IS DISTINCT %s FROM ", opname); + else + appendStringInfo(buf, " IS DISTINCT OPERATOR(%s) FROM ", + opname); get_rule_expr_paren(arg2, context, true, node); if (!PRETTY_PAREN(context)) appendStringInfoChar(buf, ')'); @@ -9972,9 +9998,31 @@ get_rule_expr(Node *node, deparse_context *context, case T_NullIfExpr: { NullIfExpr *nullifexpr = (NullIfExpr *) node; + Node *arg1 = (Node *) linitial(nullifexpr->args); + Node *arg2 = (Node *) lsecond(nullifexpr->args); + char *opname; + bool decorate; + /* + * As above, but NULLIF(a, b USING OPERATOR(s.=)): decorate + * whenever the bare form would reparse to a different + * operator, i.e. the operator is not reachable by an + * unqualified lookup of its own name, or its name is not "=". + */ + opname = generate_operator_name(nullifexpr->opno, + exprType(arg1), + exprType(arg2)); + decorate = (strncmp(opname, "OPERATOR(", 9) == 0) || + (strcmp(opname, "=") != 0); appendStringInfoString(buf, "NULLIF("); get_rule_expr((Node *) nullifexpr->args, context, true); + if (decorate) + { + if (strncmp(opname, "OPERATOR(", 9) == 0) + appendStringInfo(buf, " USING %s", opname); + else + appendStringInfo(buf, " USING OPERATOR(%s)", opname); + } appendStringInfoChar(buf, ')'); } break; diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index e03556399a..4fb4001475 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -335,9 +335,12 @@ typedef enum A_Expr_Kind AEXPR_OP, /* normal operator */ AEXPR_OP_ANY, /* scalar op ANY (array) */ AEXPR_OP_ALL, /* scalar op ALL (array) */ - AEXPR_DISTINCT, /* IS DISTINCT FROM - name must be "=" */ - AEXPR_NOT_DISTINCT, /* IS NOT DISTINCT FROM - name must be "=" */ - AEXPR_NULLIF, /* NULLIF - name must be "=" */ + AEXPR_DISTINCT, /* IS DISTINCT FROM - name is "=" or an + * explicitly given equality operator */ + AEXPR_NOT_DISTINCT, /* IS NOT DISTINCT FROM - name is "=" or an + * explicitly given equality operator */ + AEXPR_NULLIF, /* NULLIF - name is "=" or an explicitly given + * equality operator */ AEXPR_IN, /* [NOT] IN - name must be "=" or "<>" */ AEXPR_LIKE, /* [NOT] LIKE - name must be "~~" or "!~~" */ AEXPR_ILIKE, /* [NOT] ILIKE - name must be "~~*" or "!~~*" */ diff --git a/src/test/regress/expected/operator_qualify.out b/src/test/regress/expected/operator_qualify.out new file mode 100644 index 0000000000..a812953b11 --- /dev/null +++ b/src/test/regress/expected/operator_qualify.out @@ -0,0 +1,250 @@ +-- +-- Verify that constructs which resolve an operator by unqualified name +-- (IS [NOT] DISTINCT FROM, NULLIF, ...) deparse with a schema-qualified +-- OPERATOR() decoration when the operator is not reachable via search_path, +-- and that the decorated syntax parses back to the same semantics. +-- +CREATE SCHEMA alt_ops; +-- A shadow "=" over int4: same semantics as pg_catalog's (int4eq), but a +-- distinct OID, so resolution differences are observable without contrib. +CREATE OPERATOR alt_ops.= ( + leftarg = int4, rightarg = int4, procedure = int4eq, + commutator = operator(alt_ops.=), restrict = eqsel, join = eqjoinsel, + hashes, merges +); +-- alt_ops.= declares "hashes", so back the declaration up: give the operator +-- the hash opfamily membership the planner will look for whenever it builds +-- a hash table with it (e.g. for a hashed subplan). +CREATE OPERATOR CLASS alt_ops.int4_alt_hash_ops FOR TYPE int4 USING hash AS + OPERATOR 1 alt_ops.=, + FUNCTION 1 hashint4(int4); +CREATE TABLE oq_t (id int, a int, b int); +-- Views created while alt_ops.= shadows pg_catalog.= +SET search_path = alt_ops, pg_catalog; +CREATE VIEW public.oq_v_distinct AS + SELECT id, a IS DISTINCT FROM b AS r FROM public.oq_t; +CREATE VIEW public.oq_v_notdistinct AS + SELECT id, a IS NOT DISTINCT FROM b AS r FROM public.oq_t; +CREATE VIEW public.oq_v_nullif AS + SELECT id, NULLIF(a, b) AS r FROM public.oq_t; +RESET search_path; +-- With alt_ops off the search_path, the operator must be qualified. +SELECT pg_get_viewdef('oq_v_distinct'::regclass, true); + pg_get_viewdef +--------------------------------------------------- + SELECT id, + + a IS DISTINCT OPERATOR(alt_ops.=) FROM b AS r+ + FROM oq_t; +(1 row) + +SELECT pg_get_viewdef('oq_v_notdistinct'::regclass, true); + pg_get_viewdef +------------------------------------------------------- + SELECT id, + + NOT a IS DISTINCT OPERATOR(alt_ops.=) FROM b AS r+ + FROM oq_t; +(1 row) + +SELECT pg_get_viewdef('oq_v_nullif'::regclass, true); + pg_get_viewdef +------------------------------------------------- + SELECT id, + + NULLIF(a, b USING OPERATOR(alt_ops.=)) AS r+ + FROM oq_t; +(1 row) + +-- With alt_ops reachable again, the plain syntax comes back. +SET search_path = alt_ops, pg_catalog, public; +SELECT pg_get_viewdef('oq_v_distinct'::regclass, true); + pg_get_viewdef +------------------------------- + SELECT id, + + a IS DISTINCT FROM b AS r+ + FROM oq_t; +(1 row) + +SELECT pg_get_viewdef('oq_v_nullif'::regclass, true); + pg_get_viewdef +----------------------- + SELECT id, + + NULLIF(a, b) AS r+ + FROM oq_t; +(1 row) + +RESET search_path; +-- A bare-resolvable operator whose name is not "=" must still be decorated: +-- reparsing the undecorated construct would resolve "=", not this operator. +-- pg_catalog.< is on the default search_path, so its name prints unqualified, +-- but it is wrapped in OPERATOR() so the reparsed semantics stay identical. +CREATE VIEW public.oq_v_distinct_lt AS SELECT 1 IS DISTINCT OPERATOR(<) FROM 2 AS r; +CREATE VIEW public.oq_v_nullif_lt AS SELECT NULLIF(1, 2 USING OPERATOR(<)) AS r; +SELECT pg_get_viewdef('oq_v_distinct_lt'::regclass, true); + pg_get_viewdef +------------------------------------------------ + SELECT 1 IS DISTINCT OPERATOR(<) FROM 2 AS r; +(1 row) + +SELECT pg_get_viewdef('oq_v_nullif_lt'::regclass, true); + pg_get_viewdef +---------------------------------------------- + SELECT NULLIF(1, 2 USING OPERATOR(<)) AS r; +(1 row) + +-- The decorated syntax must be directly acceptable (a_expr and b_expr). +SELECT 1 IS DISTINCT OPERATOR(alt_ops.=) FROM 2 AS t; + t +--- + t +(1 row) + +SELECT 1 IS NOT DISTINCT OPERATOR(alt_ops.=) FROM 2 AS f; + f +--- + f +(1 row) + +SELECT NULLIF(1, 2 USING OPERATOR(alt_ops.=)) AS one; + one +----- + 1 +(1 row) + +SELECT NULLIF(2, 2 USING OPERATOR(alt_ops.=)) AS n; + n +--- + +(1 row) + +-- b_expr context (typmod-less contexts use b_expr) +CREATE TABLE oq_bexpr_check (x bool DEFAULT 1 IS DISTINCT OPERATOR(pg_catalog.=) FROM 2); +DROP TABLE oq_bexpr_check; +-- unqualified name inside the decoration is fine too +SELECT 1 IS DISTINCT OPERATOR(=) FROM 2 AS t; + t +--- + t +(1 row) + +-- Semantics identical to the plain construct, including NULL handling. +SELECT x IS DISTINCT OPERATOR(alt_ops.=) FROM y AS dist, + NULLIF(x, y USING OPERATOR(alt_ops.=)) AS nif +FROM (VALUES (1, 1), (1, 2), (NULL::int, 1), (NULL::int, NULL::int)) v(x, y); + dist | nif +------+----- + f | + t | 1 + t | + f | +(4 rows) + +-- A non-boolean operator is rejected, same as parse analysis rejects today. +CREATE OPERATOR alt_ops.+ (leftarg = int4, rightarg = int4, procedure = int4pl); +SELECT NULLIF(1, 2 USING OPERATOR(alt_ops.+)); +ERROR: NULLIF requires = operator to yield boolean +LINE 1: SELECT NULLIF(1, 2 USING OPERATOR(alt_ops.+)); + ^ +SELECT 1 IS DISTINCT OPERATOR(alt_ops.+) FROM 2; +ERROR: IS DISTINCT FROM requires = operator to yield boolean +LINE 1: SELECT 1 IS DISTINCT OPERATOR(alt_ops.+) FROM 2; + ^ +-- When one side is the NULL literal, the construct degenerates to a +-- NullTest and the given operator is (deliberately) not consulted. +SELECT 1 IS DISTINCT OPERATOR(alt_ops.=) FROM NULL AS t; + t +--- + t +(1 row) + +-- The deparsed definitions must reparse under a restricted search_path +-- (this is what pg_dump does; it failed before this patch). +BEGIN; +SET LOCAL search_path = pg_catalog; +DO $$ +BEGIN + EXECUTE 'CREATE VIEW public.oq_v_distinct_reload AS ' + || pg_get_viewdef('public.oq_v_distinct'::regclass); + EXECUTE 'CREATE VIEW public.oq_v_notdistinct_reload AS ' + || pg_get_viewdef('public.oq_v_notdistinct'::regclass); + EXECUTE 'CREATE VIEW public.oq_v_nullif_reload AS ' + || pg_get_viewdef('public.oq_v_nullif'::regclass); + EXECUTE 'CREATE VIEW public.oq_v_distinct_lt_reload AS ' + || pg_get_viewdef('public.oq_v_distinct_lt'::regclass); + EXECUTE 'CREATE VIEW public.oq_v_nullif_lt_reload AS ' + || pg_get_viewdef('public.oq_v_nullif_lt'::regclass); +END $$; +COMMIT; +-- Reloaded views resolve the same operator OIDs as the originals. +SELECT ev_class::regclass FROM pg_rewrite +WHERE ev_action LIKE '%:opno ' || 'alt_ops.=(int4,int4)'::regoperator::oid || ' %' + AND ev_class::regclass::text LIKE 'oq_v%reload' +ORDER BY 1; + ev_class +------------------------- + oq_v_distinct_reload + oq_v_notdistinct_reload + oq_v_nullif_reload +(3 rows) + +-- A bare-but-non-"=" operator (here pg_catalog.<) survives the round-trip too: +-- the OPERATOR(<) decoration pins int4lt instead of letting "=" resolve. +SELECT ev_class::regclass FROM pg_rewrite +WHERE ev_action LIKE '%:opno ' || 'pg_catalog.<(int4,int4)'::regoperator::oid || ' %' + AND ev_class::regclass::text LIKE 'oq_v%lt_reload%' +ORDER BY 1; + ev_class +------------------------- + oq_v_distinct_lt_reload + oq_v_nullif_lt_reload +(2 rows) + +DROP VIEW oq_v_distinct_reload, oq_v_notdistinct_reload, oq_v_nullif_reload, + oq_v_distinct_lt_reload, oq_v_nullif_lt_reload; +-- +-- Trigger WHEN clauses deparse through the same code path +-- (pg_get_triggerdef). This is the dominant real-world shape of the +-- problem: a BEFORE UPDATE trigger whose WHEN test uses IS DISTINCT FROM on +-- a column whose "=" is off the search_path used at restore time. +-- +CREATE FUNCTION oq_trgfn() RETURNS trigger LANGUAGE plpgsql + AS $$ BEGIN RETURN NEW; END $$; +SET search_path = alt_ops, pg_catalog; +CREATE TRIGGER oq_trg BEFORE UPDATE ON public.oq_t FOR EACH ROW + WHEN (OLD.a IS DISTINCT FROM NEW.a) EXECUTE FUNCTION public.oq_trgfn(); +RESET search_path; +-- With alt_ops off the search_path the WHEN operator must be qualified. +SELECT pg_get_triggerdef(oid, true) FROM pg_trigger WHERE tgname = 'oq_trg'; + pg_get_triggerdef +---------------------------------------------------------------------------------------------------------------------------------------------- + CREATE TRIGGER oq_trg BEFORE UPDATE ON oq_t FOR EACH ROW WHEN (old.a IS DISTINCT OPERATOR(alt_ops.=) FROM new.a) EXECUTE FUNCTION oq_trgfn() +(1 row) + +-- The deparsed definition must recreate the trigger under a restricted +-- search_path (as pg_dump does at restore time). alt_ops is deliberately +-- absent below, so the OPERATOR(alt_ops.=) decoration in the WHEN clause is +-- what pins the correct operator; public is present only so the trigger's +-- table and function resolve. Capture the definition before dropping. +BEGIN; +SET LOCAL search_path = public, pg_catalog; +DO $$ +DECLARE def text; +BEGIN + SELECT pg_get_triggerdef(oid) INTO def FROM pg_trigger + WHERE tgname = 'oq_trg' AND tgrelid = 'public.oq_t'::regclass; + EXECUTE 'DROP TRIGGER oq_trg ON public.oq_t'; + EXECUTE def; +END $$; +COMMIT; +-- The reloaded trigger's WHEN condition resolves the shadow alt_ops.= (its +-- OID appears in tgqual), not pg_catalog.=. +SELECT tgname FROM pg_trigger +WHERE tgqual LIKE '%:opno ' || 'alt_ops.=(int4,int4)'::regoperator::oid || ' %' + AND tgname = 'oq_trg'; + tgname +-------- + oq_trg +(1 row) + +-- NOTE: oq_t, the oq_v_* views, and the oq_trg trigger (backed by oq_trgfn) +-- are intentionally kept (not dropped): the pg_upgrade test suite +-- dump/restores the regression database and thereby exercises the qualified +-- deparse end-to-end. diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule index 8fa0a6c47f..fe25b6b2cc 100644 --- a/src/test/regress/parallel_schedule +++ b/src/test/regress/parallel_schedule @@ -81,7 +81,7 @@ test: create_table_like alter_generic alter_operator misc async dbsize merge mis # collate.linux.utf8 and collate.icu.utf8 tests cannot be run in parallel with each other # psql depends on create_am # amutils depends on geometry, create_index_spgist, hash_index, brin -test: rules psql psql_crosstab psql_pipeline amutils stats_ext collate.linux.utf8 collate.windows.win1252 +test: rules psql psql_crosstab psql_pipeline amutils stats_ext collate.linux.utf8 collate.windows.win1252 operator_qualify # ---------- # Run these alone so they don't run out of parallel workers diff --git a/src/test/regress/sql/operator_qualify.sql b/src/test/regress/sql/operator_qualify.sql new file mode 100644 index 0000000000..41144ba867 --- /dev/null +++ b/src/test/regress/sql/operator_qualify.sql @@ -0,0 +1,148 @@ +-- +-- Verify that constructs which resolve an operator by unqualified name +-- (IS [NOT] DISTINCT FROM, NULLIF, ...) deparse with a schema-qualified +-- OPERATOR() decoration when the operator is not reachable via search_path, +-- and that the decorated syntax parses back to the same semantics. +-- +CREATE SCHEMA alt_ops; +-- A shadow "=" over int4: same semantics as pg_catalog's (int4eq), but a +-- distinct OID, so resolution differences are observable without contrib. +CREATE OPERATOR alt_ops.= ( + leftarg = int4, rightarg = int4, procedure = int4eq, + commutator = operator(alt_ops.=), restrict = eqsel, join = eqjoinsel, + hashes, merges +); +-- alt_ops.= declares "hashes", so back the declaration up: give the operator +-- the hash opfamily membership the planner will look for whenever it builds +-- a hash table with it (e.g. for a hashed subplan). +CREATE OPERATOR CLASS alt_ops.int4_alt_hash_ops FOR TYPE int4 USING hash AS + OPERATOR 1 alt_ops.=, + FUNCTION 1 hashint4(int4); +CREATE TABLE oq_t (id int, a int, b int); + +-- Views created while alt_ops.= shadows pg_catalog.= +SET search_path = alt_ops, pg_catalog; +CREATE VIEW public.oq_v_distinct AS + SELECT id, a IS DISTINCT FROM b AS r FROM public.oq_t; +CREATE VIEW public.oq_v_notdistinct AS + SELECT id, a IS NOT DISTINCT FROM b AS r FROM public.oq_t; +CREATE VIEW public.oq_v_nullif AS + SELECT id, NULLIF(a, b) AS r FROM public.oq_t; +RESET search_path; + +-- With alt_ops off the search_path, the operator must be qualified. +SELECT pg_get_viewdef('oq_v_distinct'::regclass, true); +SELECT pg_get_viewdef('oq_v_notdistinct'::regclass, true); +SELECT pg_get_viewdef('oq_v_nullif'::regclass, true); + +-- With alt_ops reachable again, the plain syntax comes back. +SET search_path = alt_ops, pg_catalog, public; +SELECT pg_get_viewdef('oq_v_distinct'::regclass, true); +SELECT pg_get_viewdef('oq_v_nullif'::regclass, true); +RESET search_path; + +-- A bare-resolvable operator whose name is not "=" must still be decorated: +-- reparsing the undecorated construct would resolve "=", not this operator. +-- pg_catalog.< is on the default search_path, so its name prints unqualified, +-- but it is wrapped in OPERATOR() so the reparsed semantics stay identical. +CREATE VIEW public.oq_v_distinct_lt AS SELECT 1 IS DISTINCT OPERATOR(<) FROM 2 AS r; +CREATE VIEW public.oq_v_nullif_lt AS SELECT NULLIF(1, 2 USING OPERATOR(<)) AS r; +SELECT pg_get_viewdef('oq_v_distinct_lt'::regclass, true); +SELECT pg_get_viewdef('oq_v_nullif_lt'::regclass, true); + +-- The decorated syntax must be directly acceptable (a_expr and b_expr). +SELECT 1 IS DISTINCT OPERATOR(alt_ops.=) FROM 2 AS t; +SELECT 1 IS NOT DISTINCT OPERATOR(alt_ops.=) FROM 2 AS f; +SELECT NULLIF(1, 2 USING OPERATOR(alt_ops.=)) AS one; +SELECT NULLIF(2, 2 USING OPERATOR(alt_ops.=)) AS n; +-- b_expr context (typmod-less contexts use b_expr) +CREATE TABLE oq_bexpr_check (x bool DEFAULT 1 IS DISTINCT OPERATOR(pg_catalog.=) FROM 2); +DROP TABLE oq_bexpr_check; +-- unqualified name inside the decoration is fine too +SELECT 1 IS DISTINCT OPERATOR(=) FROM 2 AS t; + +-- Semantics identical to the plain construct, including NULL handling. +SELECT x IS DISTINCT OPERATOR(alt_ops.=) FROM y AS dist, + NULLIF(x, y USING OPERATOR(alt_ops.=)) AS nif +FROM (VALUES (1, 1), (1, 2), (NULL::int, 1), (NULL::int, NULL::int)) v(x, y); + +-- A non-boolean operator is rejected, same as parse analysis rejects today. +CREATE OPERATOR alt_ops.+ (leftarg = int4, rightarg = int4, procedure = int4pl); +SELECT NULLIF(1, 2 USING OPERATOR(alt_ops.+)); +SELECT 1 IS DISTINCT OPERATOR(alt_ops.+) FROM 2; + +-- When one side is the NULL literal, the construct degenerates to a +-- NullTest and the given operator is (deliberately) not consulted. +SELECT 1 IS DISTINCT OPERATOR(alt_ops.=) FROM NULL AS t; + +-- The deparsed definitions must reparse under a restricted search_path +-- (this is what pg_dump does; it failed before this patch). +BEGIN; +SET LOCAL search_path = pg_catalog; +DO $$ +BEGIN + EXECUTE 'CREATE VIEW public.oq_v_distinct_reload AS ' + || pg_get_viewdef('public.oq_v_distinct'::regclass); + EXECUTE 'CREATE VIEW public.oq_v_notdistinct_reload AS ' + || pg_get_viewdef('public.oq_v_notdistinct'::regclass); + EXECUTE 'CREATE VIEW public.oq_v_nullif_reload AS ' + || pg_get_viewdef('public.oq_v_nullif'::regclass); + EXECUTE 'CREATE VIEW public.oq_v_distinct_lt_reload AS ' + || pg_get_viewdef('public.oq_v_distinct_lt'::regclass); + EXECUTE 'CREATE VIEW public.oq_v_nullif_lt_reload AS ' + || pg_get_viewdef('public.oq_v_nullif_lt'::regclass); +END $$; +COMMIT; +-- Reloaded views resolve the same operator OIDs as the originals. +SELECT ev_class::regclass FROM pg_rewrite +WHERE ev_action LIKE '%:opno ' || 'alt_ops.=(int4,int4)'::regoperator::oid || ' %' + AND ev_class::regclass::text LIKE 'oq_v%reload' +ORDER BY 1; +-- A bare-but-non-"=" operator (here pg_catalog.<) survives the round-trip too: +-- the OPERATOR(<) decoration pins int4lt instead of letting "=" resolve. +SELECT ev_class::regclass FROM pg_rewrite +WHERE ev_action LIKE '%:opno ' || 'pg_catalog.<(int4,int4)'::regoperator::oid || ' %' + AND ev_class::regclass::text LIKE 'oq_v%lt_reload%' +ORDER BY 1; +DROP VIEW oq_v_distinct_reload, oq_v_notdistinct_reload, oq_v_nullif_reload, + oq_v_distinct_lt_reload, oq_v_nullif_lt_reload; + +-- +-- Trigger WHEN clauses deparse through the same code path +-- (pg_get_triggerdef). This is the dominant real-world shape of the +-- problem: a BEFORE UPDATE trigger whose WHEN test uses IS DISTINCT FROM on +-- a column whose "=" is off the search_path used at restore time. +-- +CREATE FUNCTION oq_trgfn() RETURNS trigger LANGUAGE plpgsql + AS $$ BEGIN RETURN NEW; END $$; +SET search_path = alt_ops, pg_catalog; +CREATE TRIGGER oq_trg BEFORE UPDATE ON public.oq_t FOR EACH ROW + WHEN (OLD.a IS DISTINCT FROM NEW.a) EXECUTE FUNCTION public.oq_trgfn(); +RESET search_path; +-- With alt_ops off the search_path the WHEN operator must be qualified. +SELECT pg_get_triggerdef(oid, true) FROM pg_trigger WHERE tgname = 'oq_trg'; +-- The deparsed definition must recreate the trigger under a restricted +-- search_path (as pg_dump does at restore time). alt_ops is deliberately +-- absent below, so the OPERATOR(alt_ops.=) decoration in the WHEN clause is +-- what pins the correct operator; public is present only so the trigger's +-- table and function resolve. Capture the definition before dropping. +BEGIN; +SET LOCAL search_path = public, pg_catalog; +DO $$ +DECLARE def text; +BEGIN + SELECT pg_get_triggerdef(oid) INTO def FROM pg_trigger + WHERE tgname = 'oq_trg' AND tgrelid = 'public.oq_t'::regclass; + EXECUTE 'DROP TRIGGER oq_trg ON public.oq_t'; + EXECUTE def; +END $$; +COMMIT; +-- The reloaded trigger's WHEN condition resolves the shadow alt_ops.= (its +-- OID appears in tgqual), not pg_catalog.=. +SELECT tgname FROM pg_trigger +WHERE tgqual LIKE '%:opno ' || 'alt_ops.=(int4,int4)'::regoperator::oid || ' %' + AND tgname = 'oq_trg'; +-- NOTE: oq_t, the oq_v_* views, and the oq_trg trigger (backed by oq_trgfn) +-- are intentionally kept (not dropped): the pg_upgrade test suite +-- dump/restores the regression database and thereby exercises the qualified +-- deparse end-to-end. -- 2.54.0 [application/octet-stream] v2-0002-Allow-schema-qualifying-the-operators-of-JOIN-USI.patch (30.3K, ../../66fa3fe6-8c99-4120-935c-f32f3b61fc30@Spark/4-v2-0002-Allow-schema-qualifying-the-operators-of-JOIN-USI.patch) download | inline diff: From e50e151b1be1c89085c1474a1d90361474140a35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= <[email protected]> Date: Tue, 7 Jul 2026 00:07:10 +0200 Subject: [PATCH v2 2/5] Allow schema-qualifying the operators of JOIN USING JOIN ... USING resolves each column's equality operator by the unqualified name "=" at parse time, and its syntax has no place to write a schema-qualified operator name. Consequently a view or rule whose join operator is not reachable through the prevailing search_path -- for example an extension type's "=" while restoring a dump with the restricted search_path pg_dump uses -- either fails to reload or, worse, silently resolves a different operator. In one real-world incident, a materialized view joining on a domain over citext silently resolved texteq after dump/restore and returned wrong results: https://postgr.es/m/CAC35HNnNGavaZ%3DP%3DrUcwTwYEhfoyXDg32REXCRDgxBmC3No3nA%40mail.gmail.com Provide the missing syntax: an optional per-column operator list JOIN tab USING (a, b) OPERATOR (schema.=, =) [AS alias] whose members may be schema-qualified; the n'th operator is used to compare the n'th USING column. NATURAL JOIN has no syntactic slot of its own, but since parse analysis converts it to the equivalent USING form, such a join deparses as USING (...) OPERATOR (...) whenever qualification is needed. The decoration is optional; ruleutils.c emits the list only when reparsing a bare USING clause would not recover the same operators -- i.e. when some column's operator is not reachable by an unqualified lookup of its name, or is not named "=" at all (a bare clause always reparses as "="). This mirrors the rule that governs plain OpExpr deparsing. JoinExpr gains a usingOperators field carrying the per-column operator names, so catversion is bumped. Discussion: https://postgr.es/m/[email protected] --- doc/src/sgml/queries.sgml | 12 ++ doc/src/sgml/ref/select.sgml | 18 ++- src/backend/parser/gram.y | 44 +++++-- src/backend/parser/parse_clause.c | 46 ++++++- src/backend/utils/adt/ruleutils.c | 124 +++++++++++++++++- src/include/catalog/catversion.h | 2 +- src/include/nodes/primnodes.h | 9 ++ .../regress/expected/operator_qualify.out | 97 ++++++++++++++ src/test/regress/sql/operator_qualify.sql | 49 +++++++ 9 files changed, 372 insertions(+), 29 deletions(-) diff --git a/doc/src/sgml/queries.sgml b/doc/src/sgml/queries.sgml index d8d4c3c53e..4529317183 100644 --- a/doc/src/sgml/queries.sgml +++ b/doc/src/sgml/queries.sgml @@ -369,6 +369,18 @@ FROM <replaceable>table_reference</replaceable> <optional>, <replaceable>table_r = <replaceable>T2</replaceable>.b</literal>. </para> + <para> + By default each comparison uses the <literal>=</literal> operator + found in the search path. An optional operator list, written as + <literal>USING (a, b) OPERATOR (<replaceable>operator</replaceable>, + <replaceable>operator</replaceable>)</literal>, pins the equality + operator used for each join column: one operator per column, and + each name can be schema-qualified. <application>pg_dump</application> + emits this clause when the join would not otherwise use the same + operators, either because an operator is not reachable by an + unqualified name or because it is not named <literal>=</literal>. + </para> + <para> Furthermore, the output of <literal>JOIN USING</literal> suppresses redundant columns: there is no need to print both of the matched diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml index 09b6ce809b..e2a01535b4 100644 --- a/doc/src/sgml/ref/select.sgml +++ b/doc/src/sgml/ref/select.sgml @@ -60,7 +60,7 @@ SELECT [ ALL | DISTINCT [ ON ( <replaceable class="parameter">expression</replac [ LATERAL ] ROWS FROM( <replaceable class="parameter">function_name</replaceable> ( [ <replaceable class="parameter">argument</replaceable> [, ...] ] ) [ AS ( <replaceable class="parameter">column_definition</replaceable> [, ...] ) ] [, ...] ) [ WITH ORDINALITY ] [ [ AS ] <replaceable class="parameter">alias</replaceable> [ ( <replaceable class="parameter">column_alias</replaceable> [, ...] ) ] ] GRAPH_TABLE ( <replaceable class="parameter">graph_name</replaceable> MATCH <replaceable class="parameter">graph_pattern</replaceable> COLUMNS ( { <replaceable class="parameter">expression</replaceable> [ AS <replaceable class="parameter">name</replaceable> ] } [, ...] ) ) [ [ AS ] <replaceable class="parameter">alias</replaceable> [ ( <replaceable class="parameter">column_alias</replaceable> [, ...] ) ] ] - <replaceable class="parameter">from_item</replaceable> <replaceable class="parameter">join_type</replaceable> <replaceable class="parameter">from_item</replaceable> { ON <replaceable class="parameter">join_condition</replaceable> | USING ( <replaceable class="parameter">join_column</replaceable> [, ...] ) [ AS <replaceable class="parameter">join_using_alias</replaceable> ] } + <replaceable class="parameter">from_item</replaceable> <replaceable class="parameter">join_type</replaceable> <replaceable class="parameter">from_item</replaceable> { ON <replaceable class="parameter">join_condition</replaceable> | USING ( <replaceable class="parameter">join_column</replaceable> [, ...] ) [ OPERATOR ( <replaceable class="parameter">operator</replaceable> [, ...] ) ] [ AS <replaceable class="parameter">join_using_alias</replaceable> ] } <replaceable class="parameter">from_item</replaceable> NATURAL <replaceable class="parameter">join_type</replaceable> <replaceable class="parameter">from_item</replaceable> <replaceable class="parameter">from_item</replaceable> CROSS JOIN <replaceable class="parameter">from_item</replaceable> @@ -712,7 +712,7 @@ TABLE [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ * ] </varlistentry> <varlistentry> - <term><literal>USING ( <replaceable class="parameter">join_column</replaceable> [, ...] ) [ AS <replaceable class="parameter">join_using_alias</replaceable> ]</literal></term> + <term><literal>USING ( <replaceable class="parameter">join_column</replaceable> [, ...] ) [ OPERATOR ( <replaceable class="parameter">operator</replaceable> [, ...] ) ] [ AS <replaceable class="parameter">join_using_alias</replaceable> ]</literal></term> <listitem> <para> A clause of the form <literal>USING ( a, b, ... )</literal> is @@ -723,6 +723,20 @@ TABLE [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ * ] both. </para> + <para> + Normally each column's comparison uses the <literal>=</literal> + operator found in the search path. The optional + <literal>OPERATOR</literal> clause pins the equality operator used + for each join column instead: it must list exactly one + <replaceable class="parameter">operator</replaceable> per + <replaceable class="parameter">join_column</replaceable>, and each + operator name can be schema-qualified. + <application>pg_dump</application> emits this clause when the join + would not otherwise use the same operators, either because an operator + is not reachable by an unqualified name or because it is not + named <literal>=</literal>. + </para> + <para> If a <replaceable class="parameter">join_using_alias</replaceable> name is specified, it provides a table alias for the join columns. diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 54511313dc..2df4192e2a 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -492,6 +492,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); %type <node> join_qual %type <jtype> join_type +%type <list> opt_join_using_operators any_operator_list %type <list> extract_list overlay_list position_list %type <list> substr_list trim_list @@ -14581,7 +14582,8 @@ joined_table: { /* USING clause */ n->usingClause = linitial_node(List, castNode(List, $5)); - n->join_using_alias = lsecond_node(Alias, castNode(List, $5)); + n->usingOperators = lsecond_node(List, castNode(List, $5)); + n->join_using_alias = lthird_node(Alias, castNode(List, $5)); } else { @@ -14603,7 +14605,8 @@ joined_table: { /* USING clause */ n->usingClause = linitial_node(List, castNode(List, $4)); - n->join_using_alias = lsecond_node(Alias, castNode(List, $4)); + n->usingOperators = lsecond_node(List, castNode(List, $4)); + n->join_using_alias = lthird_node(Alias, castNode(List, $4)); } else { @@ -14671,10 +14674,13 @@ opt_alias_clause: alias_clause { $$ = $1; } ; /* - * The alias clause after JOIN ... USING only accepts the AS ColId spelling, - * per SQL standard. (The grammar could parse the other variants, but they - * don't seem to be useful, and it might lead to parser problems in the - * future.) + * A JOIN ... USING clause is spelled + * USING ( column list ) [ OPERATOR ( operator list ) ] [ AS alias ] + * The optional OPERATOR clause (opt_join_using_operators) sits between the + * column list and the alias. The alias clause itself only accepts the AS + * ColId spelling, per SQL standard. (The grammar could parse the other alias + * variants, but they don't seem to be useful, and it might lead to parser + * problems in the future.) */ opt_alias_clause_for_join_using: AS ColId @@ -14732,19 +14738,23 @@ opt_outer: OUTER_P /* JOIN qualification clauses * Possibilities are: - * USING ( column list ) [ AS alias ] + * USING ( column list ) [ OPERATOR ( operator list ) ] [ AS alias ] * allows only unqualified column names, - * which must match between tables. + * which must match between tables. The optional + * OPERATOR clause supplies a per-column, possibly + * schema-qualified equality operator name in place + * of the implicit "=". * ON expr allows more general qualifications. * - * We return USING as a two-element List (the first item being a sub-List - * of the common column names, and the second either an Alias item or NULL). + * We return USING as a three-element List (the first item being a sub-List + * of the common column names, the second the sub-List of per-column operator + * names, or NIL, and the third either an Alias item or NULL). * An ON-expr will not be a List, so it can be told apart that way. */ -join_qual: USING '(' name_list ')' opt_alias_clause_for_join_using +join_qual: USING '(' name_list ')' opt_join_using_operators opt_alias_clause_for_join_using { - $$ = (Node *) list_make2($3, $5); + $$ = (Node *) list_make3($3, $5, $6); } | ON a_expr { @@ -14752,6 +14762,16 @@ join_qual: USING '(' name_list ')' opt_alias_clause_for_join_using } ; +opt_join_using_operators: + OPERATOR '(' any_operator_list ')' { $$ = $3; } + | /*EMPTY*/ { $$ = NIL; } + ; + +any_operator_list: + any_operator { $$ = list_make1($1); } + | any_operator_list ',' any_operator { $$ = lappend($1, $3); } + ; + relation_expr: qualified_name diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c index 881fba2e7b..1642492098 100644 --- a/src/backend/parser/parse_clause.c +++ b/src/backend/parser/parse_clause.c @@ -57,7 +57,8 @@ static int extractRemainingColumns(ParseState *pstate, List **res_colnames, List **res_colvars, ParseNamespaceColumn *res_nscolumns); static Node *transformJoinUsingClause(ParseState *pstate, - List *leftVars, List *rightVars); + List *leftVars, List *rightVars, + List *usingOperators); static Node *transformJoinOnClause(ParseState *pstate, JoinExpr *j, List *namespace); static ParseNamespaceItem *transformTableEntry(ParseState *pstate, RangeVar *r); @@ -309,12 +310,14 @@ extractRemainingColumns(ParseState *pstate, */ static Node * transformJoinUsingClause(ParseState *pstate, - List *leftVars, List *rightVars) + List *leftVars, List *rightVars, + List *usingOperators) { Node *result; List *andargs = NIL; ListCell *lvars, *rvars; + int i; /* * We cheat a little bit here by building an untransformed operator tree @@ -324,23 +327,37 @@ transformJoinUsingClause(ParseState *pstate, * have to mark the columns as requiring SELECT privilege for ourselves; * transformExpr() won't do it. */ + i = 0; forboth(lvars, leftVars, rvars, rightVars) { Var *lvar = (Var *) lfirst(lvars); Var *rvar = (Var *) lfirst(rvars); A_Expr *e; + List *opname; /* Require read access to the join variables */ markVarForSelectPriv(pstate, lvar); markVarForSelectPriv(pstate, rvar); - /* Now create the lvar = rvar join condition */ - e = makeSimpleA_Expr(AEXPR_OP, "=", - (Node *) copyObject(lvar), (Node *) copyObject(rvar), - -1); + /* + * Choose the operator name: an explicit USING ... OPERATOR (...) + * entry for this column if one was given, else the implicit "=". The + * caller has already checked that usingOperators, when present, has + * the same length as the column list. + */ + if (usingOperators != NIL) + opname = (List *) list_nth(usingOperators, i); + else + opname = list_make1(makeString("=")); + + /* Now create the lvar <op> rvar join condition */ + e = makeA_Expr(AEXPR_OP, opname, + (Node *) copyObject(lvar), (Node *) copyObject(rvar), + -1); /* Prepare to combine into an AND clause, if multiple join columns */ andargs = lappend(andargs, e); + i++; } /* Only need an AND if there's more than one join column */ @@ -1542,10 +1559,25 @@ transformFromClauseItem(ParseState *pstate, Node *n, res_colnames = lappend(res_colnames, lfirst(ucol)); } + /* + * If an explicit USING ... OPERATOR (...) list was supplied, it + * must provide exactly one operator per join column. (NATURAL + * joins can never reach here with a non-NIL usingOperators, since + * the grammar does not allow the OPERATOR clause there.) + */ + if (j->usingOperators != NIL && + list_length(j->usingOperators) != list_length(j->usingClause)) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("number of JOIN/USING operators (%d) does not match number of columns (%d)", + list_length(j->usingOperators), + list_length(j->usingClause)))); + /* Construct the generated JOIN ON clause */ j->quals = transformJoinUsingClause(pstate, l_usingvars, - r_usingvars); + r_usingvars, + j->usingOperators); } else if (j->quals) { diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 8a9260fcd7..db154a1f63 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -520,6 +520,7 @@ static void get_from_clause(Query *query, const char *prefix, deparse_context *context); static void get_from_clause_item(Node *jtnode, Query *query, deparse_context *context); +static List *get_join_using_opexprs(JoinExpr *j, int nusing); static void get_rte_alias(RangeTblEntry *rte, int varno, bool use_as, deparse_context *context); static void get_column_alias_list(deparse_columns *colinfo, @@ -543,6 +544,8 @@ static char *generate_function_name(Oid funcid, int nargs, bool has_variadic, bool *use_variadic_p, bool inGroupBy); static char *generate_operator_name(Oid operid, Oid arg1, Oid arg2); +static char *generate_operator_name_extended(Oid operid, Oid arg1, Oid arg2, + bool *needs_qual); static void add_cast_to(StringInfo buf, Oid typid); static char *generate_qualified_type_name(Oid typid); static text *string_to_text(char *str); @@ -13014,6 +13017,43 @@ get_from_clause(Query *query, const char *prefix, deparse_context *context) } } +/* + * get_join_using_opexprs + * Extract the per-column equality comparisons of a JOIN/USING clause + * from its stored quals, as a list of OpExpr nodes. + * + * Returns NIL if the quals do not have the expected shape (e.g. a column's + * comparison acquired a coercion we don't recognize); callers must then + * fall back to undecorated output. + */ +static List * +get_join_using_opexprs(JoinExpr *j, int nusing) +{ + Node *quals = j->quals; + List *result = NIL; + List *arms; + ListCell *lc; + + if (quals == NULL) + return NIL; + if (IsA(quals, BoolExpr) && ((BoolExpr *) quals)->boolop == AND_EXPR) + arms = ((BoolExpr *) quals)->args; + else + arms = list_make1(quals); + if (list_length(arms) != nusing) + return NIL; + foreach(lc, arms) + { + Node *arm = strip_implicit_coercions((Node *) lfirst(lc)); + + if (!IsA(arm, OpExpr) || + list_length(((OpExpr *) arm)->args) != 2) + return NIL; + result = lappend(result, arm); + } + return result; +} + static void get_from_clause_item(Node *jtnode, Query *query, deparse_context *context) { @@ -13265,6 +13305,9 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context) { ListCell *lc; bool first = true; + List *opexprs; + List *opnames = NIL; + bool need_op_list = false; appendStringInfoString(buf, " USING ("); /* Use the assigned names, not what's in usingClause */ @@ -13280,6 +13323,49 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context) } appendStringInfoChar(buf, ')'); + /* + * A bare USING clause reparses each column's operator by the + * unqualified name "=", so we must decorate the clause with an + * explicit per-column operator list whenever that would not + * recover the same operators. This is the case if any column's + * operator is not reachable by an unqualified lookup of its own + * name (then it appears schema-qualified in the list), or is + * named something other than "=" (then a bare clause would + * wrongly resolve "="; it appears by its bare name in the list). + * If we cannot recover the operators from the quals, print the + * traditional undecorated form, as before. + */ + opexprs = get_join_using_opexprs(j, + list_length(colinfo->usingNames)); + foreach(lc, opexprs) + { + OpExpr *opexpr = (OpExpr *) lfirst(lc); + bool needs_qual; + char *opname; + + opname = generate_operator_name_extended(opexpr->opno, + exprType(linitial(opexpr->args)), + exprType(lsecond(opexpr->args)), + &needs_qual); + opnames = lappend(opnames, opname); + need_op_list |= (needs_qual || strcmp(opname, "=") != 0); + } + + if (need_op_list) + { + first = true; + appendStringInfoString(buf, " OPERATOR ("); + foreach(lc, opnames) + { + if (first) + first = false; + else + appendStringInfoString(buf, ", "); + appendStringInfoString(buf, (char *) lfirst(lc)); + } + appendStringInfoChar(buf, ')'); + } + if (j->join_using_alias) appendStringInfo(buf, " AS %s", quote_identifier(j->join_using_alias->aliasname)); @@ -14064,12 +14150,37 @@ generate_function_name(Oid funcid, int nargs, List *argnames, Oid *argtypes, */ static char * generate_operator_name(Oid operid, Oid arg1, Oid arg2) +{ + bool needs_qual; + char *oprname; + + oprname = generate_operator_name_extended(operid, arg1, arg2, + &needs_qual); + if (needs_qual) + oprname = psprintf("OPERATOR(%s)", oprname); + + return oprname; +} + +/* + * generate_operator_name_extended + * Guts of generate_operator_name: compute the name to display for an + * operator specified by OID, given the specified actual arg types. + * + * The result includes all necessary quoting and schema-prefixing, but not + * the OPERATOR() decoration that generate_operator_name would add; this + * form is suitable for constructs that take a bare (but possibly + * qualified) operator name, such as JOIN/USING's operator list. Whether + * schema-prefixing was required is additionally reported in *needs_qual. + */ +static char * +generate_operator_name_extended(Oid operid, Oid arg1, Oid arg2, + bool *needs_qual) { StringInfoData buf; HeapTuple opertup; Form_pg_operator operform; char *oprname; - char *nspname; Operator p_result; initStringInfo(&buf); @@ -14102,18 +14213,17 @@ generate_operator_name(Oid operid, Oid arg1, Oid arg2) } if (p_result != NULL && oprid(p_result) == operid) - nspname = NULL; + *needs_qual = false; else { - nspname = get_namespace_name_or_temp(operform->oprnamespace); - appendStringInfo(&buf, "OPERATOR(%s.", quote_identifier(nspname)); + char *nspname = get_namespace_name_or_temp(operform->oprnamespace); + + *needs_qual = true; + appendStringInfo(&buf, "%s.", quote_identifier(nspname)); } appendStringInfoString(&buf, oprname); - if (nspname) - appendStringInfoChar(&buf, ')'); - if (p_result != NULL) ReleaseSysCache(p_result); diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index 73ed6a0fa4..7571c5cd08 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -57,6 +57,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 202607022 +#define CATALOG_VERSION_NO 202607061 #endif diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h index bb05aeebee..690d2976cd 100644 --- a/src/include/nodes/primnodes.h +++ b/src/include/nodes/primnodes.h @@ -2352,6 +2352,15 @@ typedef struct JoinExpr Node *rarg; /* right subtree */ /* USING clause, if any (list of String) */ List *usingClause pg_node_attr(query_jumble_ignore); + + /* + * USING ... OPERATOR (...): per-column possibly-qualified operator names + * (list of String lists), or NIL for the implicit "=". This is consulted + * only during parse analysis to build the join quals; ruleutils.c + * re-derives the operators from quals, so these raw names may go stale + * harmlessly (e.g. after ALTER OPERATOR SET SCHEMA). + */ + List *usingOperators pg_node_attr(query_jumble_ignore); /* alias attached to USING clause, if any */ Alias *join_using_alias pg_node_attr(query_jumble_ignore); /* qualifiers on join, if any */ diff --git a/src/test/regress/expected/operator_qualify.out b/src/test/regress/expected/operator_qualify.out index a812953b11..4fa8df62c4 100644 --- a/src/test/regress/expected/operator_qualify.out +++ b/src/test/regress/expected/operator_qualify.out @@ -248,3 +248,100 @@ WHERE tgqual LIKE '%:opno ' || 'alt_ops.=(int4,int4)'::regoperator::oid || ' %' -- are intentionally kept (not dropped): the pg_upgrade test suite -- dump/restores the regression database and thereby exercises the qualified -- deparse end-to-end. +-- JOIN USING with an off-path equality operator +CREATE TABLE oq_t2 (id int, a int); +SET search_path = alt_ops, pg_catalog; +CREATE VIEW public.oq_v_using AS + SELECT t.id FROM public.oq_t t JOIN public.oq_t2 u USING (id, a); +CREATE VIEW public.oq_v_natural AS + SELECT t.id FROM public.oq_t t NATURAL JOIN public.oq_t2 u; +-- Mixed list: pin id's operator to pg_catalog.= and a's to the shadow +-- alt_ops.=. Under the default path the former resolves bare while the +-- latter must be qualified, so the emitted list is "OPERATOR (=, alt_ops.=)". +CREATE VIEW public.oq_v_using_mixed AS + SELECT t.id FROM public.oq_t t JOIN public.oq_t2 u + USING (id, a) OPERATOR (pg_catalog.=, alt_ops.=); +RESET search_path; +-- A join operator whose name is not "=" (here pg_catalog.<, on the default +-- path) must still force the list: a bare USING clause would reparse as "=". +CREATE VIEW public.oq_v_using_lt AS + SELECT t.id FROM public.oq_t t JOIN public.oq_t2 u USING (id) OPERATOR (<); +SELECT pg_get_viewdef('oq_v_using'::regclass, true); + pg_get_viewdef +------------------------------------------------------------------ + SELECT t.id + + FROM oq_t t + + JOIN oq_t2 u USING (id, a) OPERATOR (alt_ops.=, alt_ops.=); +(1 row) + +SELECT pg_get_viewdef('oq_v_natural'::regclass, true); + pg_get_viewdef +------------------------------------------------------------------ + SELECT t.id + + FROM oq_t t + + JOIN oq_t2 u USING (id, a) OPERATOR (alt_ops.=, alt_ops.=); +(1 row) + +SELECT pg_get_viewdef('oq_v_using_mixed'::regclass, true); + pg_get_viewdef +---------------------------------------------------------- + SELECT t.id + + FROM oq_t t + + JOIN oq_t2 u USING (id, a) OPERATOR (=, alt_ops.=); +(1 row) + +SELECT pg_get_viewdef('oq_v_using_lt'::regclass, true); + pg_get_viewdef +-------------------------------------------- + SELECT t.id + + FROM oq_t t + + JOIN oq_t2 u USING (id) OPERATOR (<); +(1 row) + +-- explicit syntax parses; mixed bare/qualified names allowed +SELECT t.id FROM oq_t t JOIN oq_t2 u USING (id, a) OPERATOR (pg_catalog.=, alt_ops.=) WHERE false; + id +---- +(0 rows) + +SELECT t.id FROM oq_t t JOIN oq_t2 u USING (id, a) OPERATOR (=, alt_ops.=) AS j WHERE false; + id +---- +(0 rows) + +-- arity mismatch is an error +SELECT t.id FROM oq_t t JOIN oq_t2 u USING (id, a) OPERATOR (alt_ops.=) WHERE false; +ERROR: number of JOIN/USING operators (1) does not match number of columns (2) +-- reload round-trip under restricted search_path +BEGIN; SET LOCAL search_path = pg_catalog; +DO $$ BEGIN + EXECUTE 'CREATE VIEW public.oq_v_using_reload AS ' || pg_get_viewdef('public.oq_v_using'::regclass); + EXECUTE 'CREATE VIEW public.oq_v_natural_reload AS ' || pg_get_viewdef('public.oq_v_natural'::regclass); + EXECUTE 'CREATE VIEW public.oq_v_using_mixed_reload AS ' || pg_get_viewdef('public.oq_v_using_mixed'::regclass); + EXECUTE 'CREATE VIEW public.oq_v_using_lt_reload AS ' || pg_get_viewdef('public.oq_v_using_lt'::regclass); +END $$; COMMIT; +-- Reloaded views resolve the same operator OIDs as the originals: alt_ops.= +-- for oq_v_using/natural (both columns) and the mixed view (column a only). +SELECT ev_class::regclass FROM pg_rewrite +WHERE ev_action LIKE '%:opno ' || 'alt_ops.=(int4,int4)'::regoperator::oid || ' %' + AND ev_class::regclass::text LIKE 'oq_v%reload' +ORDER BY 1; + ev_class +------------------------- + oq_v_using_reload + oq_v_natural_reload + oq_v_using_mixed_reload +(3 rows) + +-- The non-"=" operator OPERATOR(<) pins int4lt across the reload. +SELECT ev_class::regclass FROM pg_rewrite +WHERE ev_action LIKE '%:opno ' || 'pg_catalog.<(int4,int4)'::regoperator::oid || ' %' + AND ev_class::regclass::text LIKE 'oq_v%reload' +ORDER BY 1; + ev_class +---------------------- + oq_v_using_lt_reload +(1 row) + +DROP VIEW oq_v_using_reload, oq_v_natural_reload, oq_v_using_mixed_reload, + oq_v_using_lt_reload; diff --git a/src/test/regress/sql/operator_qualify.sql b/src/test/regress/sql/operator_qualify.sql index 41144ba867..c8faddddde 100644 --- a/src/test/regress/sql/operator_qualify.sql +++ b/src/test/regress/sql/operator_qualify.sql @@ -146,3 +146,52 @@ WHERE tgqual LIKE '%:opno ' || 'alt_ops.=(int4,int4)'::regoperator::oid || ' %' -- are intentionally kept (not dropped): the pg_upgrade test suite -- dump/restores the regression database and thereby exercises the qualified -- deparse end-to-end. + +-- JOIN USING with an off-path equality operator +CREATE TABLE oq_t2 (id int, a int); +SET search_path = alt_ops, pg_catalog; +CREATE VIEW public.oq_v_using AS + SELECT t.id FROM public.oq_t t JOIN public.oq_t2 u USING (id, a); +CREATE VIEW public.oq_v_natural AS + SELECT t.id FROM public.oq_t t NATURAL JOIN public.oq_t2 u; +-- Mixed list: pin id's operator to pg_catalog.= and a's to the shadow +-- alt_ops.=. Under the default path the former resolves bare while the +-- latter must be qualified, so the emitted list is "OPERATOR (=, alt_ops.=)". +CREATE VIEW public.oq_v_using_mixed AS + SELECT t.id FROM public.oq_t t JOIN public.oq_t2 u + USING (id, a) OPERATOR (pg_catalog.=, alt_ops.=); +RESET search_path; +-- A join operator whose name is not "=" (here pg_catalog.<, on the default +-- path) must still force the list: a bare USING clause would reparse as "=". +CREATE VIEW public.oq_v_using_lt AS + SELECT t.id FROM public.oq_t t JOIN public.oq_t2 u USING (id) OPERATOR (<); +SELECT pg_get_viewdef('oq_v_using'::regclass, true); +SELECT pg_get_viewdef('oq_v_natural'::regclass, true); +SELECT pg_get_viewdef('oq_v_using_mixed'::regclass, true); +SELECT pg_get_viewdef('oq_v_using_lt'::regclass, true); +-- explicit syntax parses; mixed bare/qualified names allowed +SELECT t.id FROM oq_t t JOIN oq_t2 u USING (id, a) OPERATOR (pg_catalog.=, alt_ops.=) WHERE false; +SELECT t.id FROM oq_t t JOIN oq_t2 u USING (id, a) OPERATOR (=, alt_ops.=) AS j WHERE false; +-- arity mismatch is an error +SELECT t.id FROM oq_t t JOIN oq_t2 u USING (id, a) OPERATOR (alt_ops.=) WHERE false; +-- reload round-trip under restricted search_path +BEGIN; SET LOCAL search_path = pg_catalog; +DO $$ BEGIN + EXECUTE 'CREATE VIEW public.oq_v_using_reload AS ' || pg_get_viewdef('public.oq_v_using'::regclass); + EXECUTE 'CREATE VIEW public.oq_v_natural_reload AS ' || pg_get_viewdef('public.oq_v_natural'::regclass); + EXECUTE 'CREATE VIEW public.oq_v_using_mixed_reload AS ' || pg_get_viewdef('public.oq_v_using_mixed'::regclass); + EXECUTE 'CREATE VIEW public.oq_v_using_lt_reload AS ' || pg_get_viewdef('public.oq_v_using_lt'::regclass); +END $$; COMMIT; +-- Reloaded views resolve the same operator OIDs as the originals: alt_ops.= +-- for oq_v_using/natural (both columns) and the mixed view (column a only). +SELECT ev_class::regclass FROM pg_rewrite +WHERE ev_action LIKE '%:opno ' || 'alt_ops.=(int4,int4)'::regoperator::oid || ' %' + AND ev_class::regclass::text LIKE 'oq_v%reload' +ORDER BY 1; +-- The non-"=" operator OPERATOR(<) pins int4lt across the reload. +SELECT ev_class::regclass FROM pg_rewrite +WHERE ev_action LIKE '%:opno ' || 'pg_catalog.<(int4,int4)'::regoperator::oid || ' %' + AND ev_class::regclass::text LIKE 'oq_v%reload' +ORDER BY 1; +DROP VIEW oq_v_using_reload, oq_v_natural_reload, oq_v_using_mixed_reload, + oq_v_using_lt_reload; -- 2.54.0 [application/octet-stream] v2-0003-Allow-per-column-operator-lists-in-row-comparison.patch (38.5K, ../../66fa3fe6-8c99-4120-935c-f32f3b61fc30@Spark/5-v2-0003-Allow-per-column-operator-lists-in-row-comparison.patch) download | inline diff: From 9a3b54bc95c2ec70032307dd1db64ecfb85ec488 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= <[email protected]> Date: Tue, 7 Jul 2026 07:54:28 +0200 Subject: [PATCH v2 3/5] Allow per-column operator lists in row comparisons A row-constructor comparison such as ROW(a, b) < ROW(c, d) resolves the written operator name independently for each pair of columns, so the specific operator chosen for one column may live in a different schema from the operator chosen for another. ruleutils.c could not reproduce that: it printed the first column's operator name for the whole row and admitted as much -- We assume that the name of the first-column operator will do for all the rest too. This is definitely open to failure, eg if some but not all operators were renamed since the construct was parsed, but there seems no way to be perfect. When the columns' operators disagree -- or the first-column name would resolve to a different operator on the reader's search_path -- that approximation silently deparses to a row comparison with different semantics, so a dumped-and-reloaded view could compare rows using the wrong operators. Give the row-comparison syntax an optional per-column operator list, ROW(...) OPERATOR(op1, op2, ...) ROW(...) one operator per column, each independently schema-qualifiable. This is Tom Lane's own suggestion for the problem ([email protected]). make_row_comparison_op resolves the n'th name against the n'th column; ruleutils emits the list whenever a single written name could not recover every column's operator (some column needs schema-qualification, or the columns use differently-named operators), and otherwise prints the traditional single-operator form byte-for-byte unchanged. The operator list is only meaningful for row (and, with the next patch, subquery) comparisons. It is rejected in every other context that consumes an operator name -- scalar infix/prefix operators, ANY/ALL over an array, ORDER BY ... USING, and DDL definition items such as a CREATE OPERATOR commutator -- through a single shared helper, reject_operator_name_list(). Subquery comparisons reject it too for now; that rejection is temporary and lifted by the follow-on patch that teaches ANY/ALL and row subqueries to honor the list. Only the raw-parse representation changes -- the A_Expr, SubLink and SortBy operator-name fields already round-trip a nested list -- and stored rules keep the same post-analysis RowCompareExpr, so no catversion bump is required. Discussion: https://postgr.es/m/[email protected] --- doc/src/sgml/func/func-comparisons.sgml | 19 +++ doc/src/sgml/syntax.sgml | 5 +- src/backend/commands/define.c | 5 + src/backend/parser/gram.y | 29 +++- src/backend/parser/parse_clause.c | 9 + src/backend/parser/parse_expr.c | 115 ++++++++++++- src/backend/parser/parse_oper.c | 22 +++ src/backend/utils/adt/ruleutils.c | 66 +++++++- src/include/parser/parse_oper.h | 20 +++ .../regress/expected/operator_qualify.out | 156 ++++++++++++++++++ src/test/regress/parallel_schedule | 7 +- src/test/regress/sql/operator_qualify.sql | 106 ++++++++++++ 12 files changed, 532 insertions(+), 27 deletions(-) diff --git a/doc/src/sgml/func/func-comparisons.sgml b/doc/src/sgml/func/func-comparisons.sgml index 6a6e0bd401..5245f012bb 100644 --- a/doc/src/sgml/func/func-comparisons.sgml +++ b/doc/src/sgml/func/func-comparisons.sgml @@ -229,6 +229,25 @@ AND or has semantics similar to one of these. </para> + <para> + Ordinarily a single <replaceable>operator</replaceable> is written and its + bare name is resolved independently for each pair of columns. You can + instead pin the operator used for each column by writing an explicit + per-column operator list with the <literal>OPERATOR()</literal> syntax (see + <xref linkend="sql-expressions-operator-calls"/>), giving one operator for + each column of the rows: +<synopsis> +<replaceable>row_constructor</replaceable> OPERATOR(<replaceable>operator</replaceable> <optional>, <replaceable>operator</replaceable> ... </optional>) <replaceable>row_constructor</replaceable> +</synopsis> + The list must contain exactly one operator per column, and each operator may + be schema-qualified independently. This form is useful when a single + unqualified name would not resolve to the intended operator for every column; + <productname>PostgreSQL</productname> itself falls back to it when deparsing a + stored row comparison (for example in a view definition) whose per-column + operators cannot all be recovered from one written name. The selected + operators must still meet the B-tree requirements described above. + </para> + <para> The <literal>=</literal> and <literal><></literal> cases work slightly differently from the others. Two rows are considered diff --git a/doc/src/sgml/syntax.sgml b/doc/src/sgml/syntax.sgml index b5aad8b4c7..094c2c2ffb 100644 --- a/doc/src/sgml/syntax.sgml +++ b/doc/src/sgml/syntax.sgml @@ -1136,7 +1136,10 @@ SELECT 3 OPERATOR(pg_catalog.+) 4; well: <literal>IS <optional> NOT </optional> DISTINCT FROM</literal> (see <xref linkend="functions-comparison-pred-table"/>) and <function>NULLIF</function> - (see <xref linkend="functions-nullif"/>). + (see <xref linkend="functions-nullif"/>). A row-constructor comparison + goes a step further and accepts a comma-separated <emphasis>list</emphasis> + of operators inside <literal>OPERATOR()</literal>, one for each column of + the compared rows (see <xref linkend="row-wise-comparison"/>). </para> <note> diff --git a/src/backend/commands/define.c b/src/backend/commands/define.c index 4172cc9bac..b05bb95719 100644 --- a/src/backend/commands/define.c +++ b/src/backend/commands/define.c @@ -24,6 +24,7 @@ #include "catalog/namespace.h" #include "commands/defrem.h" #include "nodes/makefuncs.h" +#include "parser/parse_oper.h" #include "parser/parse_type.h" #include "utils/fmgrprotos.h" @@ -51,6 +52,8 @@ defGetString(DefElem *def) case T_TypeName: return TypeNameToString((TypeName *) def->arg); case T_List: + /* must be an operator name */ + reject_operator_name_list(NULL, (List *) def->arg, -1); return NameListToString((List *) def->arg); case T_A_Star: return pstrdup("*"); @@ -247,6 +250,8 @@ defGetQualifiedName(DefElem *def) case T_TypeName: return ((TypeName *) def->arg)->names; case T_List: + /* must be an operator name */ + reject_operator_name_list(NULL, (List *) def->arg, -1); return (List *) def->arg; case T_String: /* Allow quoted name for backwards compatibility */ diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 2df4192e2a..30bd69c01f 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -17624,24 +17624,41 @@ MathOp: '+' { $$ = "+"; } | '|' { $$ = "|"; } ; +/* + * The OPERATOR() decoration accepts a comma-separated list of operator + * names. A one-element list is collapsed to the traditional single + * possibly-qualified name (a List of Strings); multiple names are kept + * as a List of such sub-lists, which parse analysis accepts only where a + * per-column operator list is meaningful (row and subquery comparisons) + * and rejects everywhere else. See OperatorNameIsList in parse_oper.h. + */ qual_Op: Op { $$ = list_make1(makeString($1)); } - | OPERATOR '(' any_operator ')' - { $$ = $3; } + | OPERATOR '(' any_operator_list ')' + { + $$ = (list_length($3) == 1) ? + (List *) linitial($3) : $3; + } ; qual_all_Op: all_Op { $$ = list_make1(makeString($1)); } - | OPERATOR '(' any_operator ')' - { $$ = $3; } + | OPERATOR '(' any_operator_list ')' + { + $$ = (list_length($3) == 1) ? + (List *) linitial($3) : $3; + } ; subquery_Op: all_Op { $$ = list_make1(makeString($1)); } - | OPERATOR '(' any_operator ')' - { $$ = $3; } + | OPERATOR '(' any_operator_list ')' + { + $$ = (list_length($3) == 1) ? + (List *) linitial($3) : $3; + } | LIKE { $$ = list_make1(makeString("~~")); } | NOT_LA LIKE diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c index 1642492098..9bde5c3b30 100644 --- a/src/backend/parser/parse_clause.c +++ b/src/backend/parser/parse_clause.c @@ -3695,6 +3695,15 @@ addTargetToSortList(ParseState *pstate, TargetEntry *tle, break; case SORTBY_USING: Assert(sortby->useOp != NIL); + + /* + * USING takes a single operator; a per-column operator list is + * only meaningful in row and subquery comparisons. We pass + * location -1 because the errposition callback set up above + * already points the error at the operator. + */ + reject_operator_name_list(pstate, sortby->useOp, -1); + sortop = compatible_oper_opid(sortby->useOp, restype, restype, diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c index 0c5e1a3487..bd16e73a88 100644 --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -1001,6 +1001,13 @@ transformAExprOp(ParseState *pstate, A_Expr *a) /* Ordinary scalar operator */ Node *last_srf = pstate->p_last_srf; + /* + * A per-column operator list only makes sense for the row-comparison + * cases above (this also covers prefix operators, which carry no row + * comparison semantics at all). + */ + reject_operator_name_list(pstate, a->name, a->location); + lexpr = transformExprRecurse(pstate, lexpr); rexpr = transformExprRecurse(pstate, rexpr); @@ -1018,8 +1025,17 @@ transformAExprOp(ParseState *pstate, A_Expr *a) static Node * transformAExprOpAny(ParseState *pstate, A_Expr *a) { - Node *lexpr = transformExprRecurse(pstate, a->lexpr); - Node *rexpr = transformExprRecurse(pstate, a->rexpr); + Node *lexpr; + Node *rexpr; + + /* + * The grammar's subquery_Op admits an operator list, but ANY/ALL over an + * array compares with a single operator. + */ + reject_operator_name_list(pstate, a->name, a->location); + + lexpr = transformExprRecurse(pstate, a->lexpr); + rexpr = transformExprRecurse(pstate, a->rexpr); return (Node *) make_scalar_array_op(pstate, a->name, @@ -1032,8 +1048,14 @@ transformAExprOpAny(ParseState *pstate, A_Expr *a) static Node * transformAExprOpAll(ParseState *pstate, A_Expr *a) { - Node *lexpr = transformExprRecurse(pstate, a->lexpr); - Node *rexpr = transformExprRecurse(pstate, a->rexpr); + Node *lexpr; + Node *rexpr; + + /* As in transformAExprOpAny */ + reject_operator_name_list(pstate, a->name, a->location); + + lexpr = transformExprRecurse(pstate, a->lexpr); + rexpr = transformExprRecurse(pstate, a->rexpr); return (Node *) make_scalar_array_op(pstate, a->name, @@ -1050,6 +1072,13 @@ transformAExprDistinct(ParseState *pstate, A_Expr *a) Node *rexpr = a->rexpr; Node *result; + /* + * IS [NOT] DISTINCT FROM takes a single operator even in the row case; + * its OPERATOR() decoration is grammatically restricted to one name, so + * this is merely defensive. + */ + reject_operator_name_list(pstate, a->name, a->location); + /* * If either input is an undecorated NULL literal, transform to a NullTest * on the other input. That's simpler to process than a full DistinctExpr, @@ -1104,10 +1133,19 @@ transformAExprDistinct(ParseState *pstate, A_Expr *a) static Node * transformAExprNullIf(ParseState *pstate, A_Expr *a) { - Node *lexpr = transformExprRecurse(pstate, a->lexpr); - Node *rexpr = transformExprRecurse(pstate, a->rexpr); + Node *lexpr; + Node *rexpr; OpExpr *result; + /* + * NULLIF takes a single operator; its OPERATOR() decoration is + * grammatically restricted to one name, so this is merely defensive. + */ + reject_operator_name_list(pstate, a->name, a->location); + + lexpr = transformExprRecurse(pstate, a->lexpr); + rexpr = transformExprRecurse(pstate, a->rexpr); + result = (OpExpr *) make_op(pstate, a->name, lexpr, @@ -1156,6 +1194,14 @@ transformAExprIn(ParseState *pstate, A_Expr *a) ListCell *l; bool has_rvars = false; + /* + * IN always compares with the implicit "=" (or "<>" for NOT IN); there is + * no OPERATOR() decoration for it in the grammar, so an operator list + * cannot appear here (not even for row-valued IN lists). This is merely + * defensive, and also protects the strVal() just below. + */ + reject_operator_name_list(pstate, a->name, a->location); + /* * If the operator is <>, combine with AND not OR. */ @@ -1974,6 +2020,17 @@ transformSubLink(ParseState *pstate, SubLink *sublink) List *right_list; ListCell *l; + /* + * TEMPORARY: reject a per-column operator list here. A later patch + * teaches this path (and ruleutils.c) to apply the n'th name to the + * n'th column, at which point this check goes away. Note this also + * catches lists arriving via transformAExprOp's conversion of "row op + * (subselect)" to a ROWCOMPARE sublink, which passes the A_Expr's + * name through operName. + */ + reject_operator_name_list(pstate, sublink->operName, + sublink->location); + /* * If the source was "x IN (select)", convert to "x = ANY (select)". */ @@ -2858,6 +2915,10 @@ transformCollateClause(ParseState *pstate, CollateClause *c) * As with coerce_type, pstate may be NULL if no special unknown-Param * processing is wanted. * + * opname is either a single possibly-qualified operator name, applied to + * every column pair, or a list of such names, one per column pair (see + * OperatorNameIsList). + * * The output may be a single OpExpr, an AND or OR combination of OpExprs, * or a RowCompareExpr. In all cases it is guaranteed to return boolean. * The AND, OR, and RowCompareExpr cases further imply things about the @@ -2876,8 +2937,10 @@ make_row_comparison_op(ParseState *pstate, List *opname, *r; List **opinfo_lists; Bitmapset *cmptypes; + bool multi = OperatorNameIsList(opname); int nopers; int i; + int badpos; nopers = list_length(largs); if (nopers != list_length(rargs)) @@ -2896,18 +2959,34 @@ make_row_comparison_op(ParseState *pstate, List *opname, errmsg("cannot compare rows of zero length"), parser_errposition(pstate, location))); + /* + * A per-column operator list must supply exactly one operator per column. + */ + if (multi && list_length(opname) != nopers) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("number of operators (%d) does not match number of columns (%d)", + list_length(opname), nopers), + parser_errposition(pstate, location))); + /* * Identify all the pairwise operators, using make_op so that behavior is * the same as in the simple scalar case. */ opexprs = NIL; + i = 0; forboth(l, largs, r, rargs) { Node *larg = (Node *) lfirst(l); Node *rarg = (Node *) lfirst(r); + List *this_opname; OpExpr *cmp; - cmp = castNode(OpExpr, make_op(pstate, opname, larg, rarg, + /* With an operator list, the n'th name compares the n'th column */ + this_opname = multi ? (List *) list_nth(opname, i) : opname; + i++; + + cmp = castNode(OpExpr, make_op(pstate, this_opname, larg, rarg, pstate->p_last_srf, location)); /* @@ -2947,6 +3026,7 @@ make_row_comparison_op(ParseState *pstate, List *opname, opinfo_lists = palloc_array(List *, nopers); cmptypes = NULL; i = 0; + badpos = -1; foreach(l, opexprs) { Oid opno = ((OpExpr *) lfirst(l))->opno; @@ -2970,6 +3050,14 @@ make_row_comparison_op(ParseState *pstate, List *opname, cmptypes = this_cmptypes; else cmptypes = bms_int_members(cmptypes, this_cmptypes); + + /* + * Remember the first column at which the running intersection went + * empty; if we fail below, that column's operator is the one to + * complain about. + */ + if (badpos < 0 && bms_is_empty(cmptypes)) + badpos = i; i++; } @@ -2981,11 +3069,15 @@ make_row_comparison_op(ParseState *pstate, List *opname, i = bms_next_member(cmptypes, -1); if (i < 0) { + List *bad_opname; + /* No common interpretation, so fail */ + Assert(!multi || badpos >= 0); + bad_opname = multi ? (List *) list_nth(opname, badpos) : opname; ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("could not determine interpretation of row comparison operator %s", - strVal(llast(opname))), + strVal(llast(bad_opname))), errhint("Row comparison operators must be associated with btree operator families."), parser_errposition(pstate, location))); } @@ -3023,12 +3115,17 @@ make_row_comparison_op(ParseState *pstate, List *opname, if (OidIsValid(opfamily)) opfamilies = lappend_oid(opfamilies, opfamily); else /* should not happen */ + { + List *this_opname; + + this_opname = multi ? (List *) list_nth(opname, i) : opname; ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("could not determine interpretation of row comparison operator %s", - strVal(llast(opname))), + strVal(llast(this_opname))), errdetail("There are multiple equally-plausible candidates."), parser_errposition(pstate, location))); + } } /* diff --git a/src/backend/parser/parse_oper.c b/src/backend/parser/parse_oper.c index a7e5c68636..a626a1beb7 100644 --- a/src/backend/parser/parse_oper.c +++ b/src/backend/parser/parse_oper.c @@ -84,6 +84,28 @@ static void InvalidateOprCacheCallBack(Datum arg, SysCacheIdentifier cacheid, uint32 hashvalue); +/* + * reject_operator_name_list + * Reject a per-column operator name list where only a single name is + * meaningful. + * + * The grammar's OPERATOR() decoration accepts a comma-separated list of + * operator names, but a per-column list (see OperatorNameIsList) is only + * meaningful for row and subquery comparisons. Every other consumer of an + * operator name expects the traditional single (possibly-qualified) name and + * must call this before resolving it. pstate and location are used only to + * report the error position; pass NULL/-1 if not available. + */ +void +reject_operator_name_list(ParseState *pstate, List *opname, int location) +{ + if (OperatorNameIsList(opname)) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("a list of operators is only allowed in row and subquery comparisons"), + parser_errposition(pstate, location))); +} + /* * LookupOperName * Given a possibly-qualified operator name and exact input datatypes, diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index db154a1f63..7cca15bbb5 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -10576,6 +10576,13 @@ get_rule_expr(Node *node, deparse_context *context, case T_RowCompareExpr: { RowCompareExpr *rcexpr = (RowCompareExpr *) node; + ListCell *opno_cell; + ListCell *larg_cell; + ListCell *rarg_cell; + List *opnames = NIL; + bool need_op_list = false; + bool all_same = true; + char *firstname = NULL; /* * SQL99 allows "ROW" to be omitted when there is more than @@ -10585,18 +10592,59 @@ get_rule_expr(Node *node, deparse_context *context, */ appendStringInfoString(buf, "(ROW("); get_rule_list_toplevel(rcexpr->largs, context, true); + appendStringInfoString(buf, ") "); /* - * We assume that the name of the first-column operator will - * do for all the rest too. This is definitely open to - * failure, eg if some but not all operators were renamed - * since the construct was parsed, but there seems no way to - * be perfect. + * The undecorated syntax ROW(...) op ROW(...) reparses by + * resolving one written operator name for every column pair, + * so it is honest only when a single bare name recovers each + * column's operator: no column needs schema-qualification and + * every column shares the same operator name. When that + * holds we print that shared name, exactly as before. + * Otherwise we emit the explicit per-column operator list + * ROW(...) OPERATOR(op1, op2, ...) ROW(...), which pins each + * column's operator independently rather than silently + * letting one name (mis)resolve the rest. */ - appendStringInfo(buf, ") %s ROW(", - generate_operator_name(linitial_oid(rcexpr->opnos), - exprType(linitial(rcexpr->largs)), - exprType(linitial(rcexpr->rargs)))); + forthree(opno_cell, rcexpr->opnos, + larg_cell, rcexpr->largs, + rarg_cell, rcexpr->rargs) + { + bool needs_qual; + char *opname; + + opname = generate_operator_name_extended(lfirst_oid(opno_cell), + exprType((Node *) lfirst(larg_cell)), + exprType((Node *) lfirst(rarg_cell)), + &needs_qual); + opnames = lappend(opnames, opname); + if (needs_qual) + need_op_list = true; + if (firstname == NULL) + firstname = opname; + else if (strcmp(firstname, opname) != 0) + all_same = false; + } + + if (need_op_list || !all_same) + { + ListCell *lc; + bool first = true; + + appendStringInfoString(buf, "OPERATOR("); + foreach(lc, opnames) + { + if (first) + first = false; + else + appendStringInfoString(buf, ", "); + appendStringInfoString(buf, (char *) lfirst(lc)); + } + appendStringInfoString(buf, ") ROW("); + } + else + appendStringInfo(buf, "%s ROW(", firstname); + get_rule_list_toplevel(rcexpr->rargs, context, true); appendStringInfoString(buf, "))"); } diff --git a/src/include/parser/parse_oper.h b/src/include/parser/parse_oper.h index 97f6aa139c..53b6cccb50 100644 --- a/src/include/parser/parse_oper.h +++ b/src/include/parser/parse_oper.h @@ -21,6 +21,26 @@ typedef HeapTuple Operator; +/* + * An operator name List as produced by the grammar's OPERATOR() decoration + * is EITHER a single possibly-qualified operator name (a List of Strings, + * the traditional shape) OR a list of such names, one per column of a row + * or subquery comparison (a List of such sub-lists). + * + * Does this operator name List carry multiple per-column names? + */ +#define OperatorNameIsList(opname) \ + ((opname) != NIL && IsA(linitial((List *) (opname)), List)) + +/* + * Reject a per-column operator name list (see OperatorNameIsList) where only a + * single operator name is meaningful. Pass a ParseState and location where + * available so the error can point at the offending decoration; callers with + * neither (e.g. DDL definition items) pass NULL and -1. + */ +extern void reject_operator_name_list(ParseState *pstate, List *opname, + int location); + /* Routines to look up an operator given name and exact input type(s) */ extern Oid LookupOperName(ParseState *pstate, List *opername, Oid oprleft, Oid oprright, diff --git a/src/test/regress/expected/operator_qualify.out b/src/test/regress/expected/operator_qualify.out index 4fa8df62c4..f6d70e886e 100644 --- a/src/test/regress/expected/operator_qualify.out +++ b/src/test/regress/expected/operator_qualify.out @@ -345,3 +345,159 @@ ORDER BY 1; DROP VIEW oq_v_using_reload, oq_v_natural_reload, oq_v_using_mixed_reload, oq_v_using_lt_reload; +-- +-- Row-comparison operator lists: the OPERATOR() decoration may carry a +-- comma-separated LIST of operators, one per column of the compared rows. +-- +-- make_row_comparison_op intersects the per-column operators' btree opfamily +-- interpretations, so a bare "<" with no operator class fails outright with +-- "could not determine interpretation of row comparison operator". A btree +-- operator class over int4 is therefore required. alt_ops.= already exists +-- above; add the rest of the strategy set and wrap them in a class. +CREATE OPERATOR alt_ops.< (leftarg = int4, rightarg = int4, procedure = int4lt); +CREATE OPERATOR alt_ops.<= (leftarg = int4, rightarg = int4, procedure = int4le); +CREATE OPERATOR alt_ops.> (leftarg = int4, rightarg = int4, procedure = int4gt); +CREATE OPERATOR alt_ops.>= (leftarg = int4, rightarg = int4, procedure = int4ge); +CREATE OPERATOR CLASS alt_ops.int4_alt_ops FOR TYPE int4 USING btree AS + OPERATOR 1 alt_ops.<, + OPERATOR 2 alt_ops.<=, + OPERATOR 3 alt_ops.=, + OPERATOR 4 alt_ops.>=, + OPERATOR 5 alt_ops.>, + FUNCTION 1 btint4cmp(int4, int4); +-- View built while alt_ops shadows pg_catalog: the row comparison resolves to +-- alt_ops.< for both columns. +SET search_path = alt_ops, pg_catalog; +CREATE VIEW public.oq_v_rowcmp AS + SELECT ROW(a, b) < ROW(b, a) AS r FROM public.oq_t; +RESET search_path; +-- Off the path, both column operators must be qualified: the decoration is a +-- LIST, one operator per column (ROW(...) OPERATOR(alt_ops.<, alt_ops.<) ROW(...)). +SELECT pg_get_viewdef('oq_v_rowcmp'::regclass, true); + pg_get_viewdef +------------------------------------------------------------------- + SELECT (ROW(a, b) OPERATOR(alt_ops.<, alt_ops.<) ROW(b, a)) AS r+ + FROM oq_t; +(1 row) + +-- With alt_ops reachable again, the plain single-operator syntax comes back. +SET search_path = alt_ops, pg_catalog, public; +SELECT pg_get_viewdef('oq_v_rowcmp'::regclass, true); + pg_get_viewdef +-------------------------------------- + SELECT (ROW(a, b) < ROW(b, a)) AS r+ + FROM oq_t; +(1 row) + +RESET search_path; +-- Mixed-schema deparse: build (under the default search_path) a view whose row +-- comparison pins column 1 to alt_ops.< (off the path -> must be qualified) and +-- column 2 to pg_catalog.< (reachable bare). A single written name cannot +-- recover two differently-spelled operators, so the deparse emits the per-column +-- list OPERATOR(alt_ops.<, <): only column 1 is qualified, but the list form is +-- forced because the two column names differ. +CREATE VIEW public.oq_v_rowcmp_mixed AS + SELECT ROW(a, b) OPERATOR(alt_ops.<, pg_catalog.<) ROW(b, a) AS r + FROM public.oq_t; +SELECT pg_get_viewdef('oq_v_rowcmp_mixed'::regclass, true); + pg_get_viewdef +----------------------------------------------------------- + SELECT (ROW(a, b) OPERATOR(alt_ops.<, <) ROW(b, a)) AS r+ + FROM oq_t; +(1 row) + +-- The list syntax is directly acceptable and N operators compare N columns. +SELECT ROW(1,2) OPERATOR(pg_catalog.<, pg_catalog.<) ROW(2,1) AS t; + t +--- + t +(1 row) + +-- Mixed-schema list: column 1 pinned to alt_ops.<, column 2 to pg_catalog.<. +SELECT ROW(1,2) OPERATOR(alt_ops.<, pg_catalog.<) ROW(2,3) AS t; + t +--- + t +(1 row) + +-- A one-element list is exactly today's single-operator syntax (invariance +-- guard: this line is already accepted before the list grammar lands). +SELECT ROW(1,2) OPERATOR(pg_catalog.<) ROW(1,3) AS t; + t +--- + t +(1 row) + +-- A mixed-schema equality list is accepted; both columns compare equal (t). +SELECT ROW(1,2) OPERATOR(pg_catalog.=, alt_ops.=) ROW(1,2) AS t; + t +--- + t +(1 row) + +-- Errors below run under terse verbosity: one line per error, the message +-- text plus "at character N" pinning the OPERATOR() decoration's position. +\set VERBOSITY terse +-- Too many operators for the row width. +SELECT ROW(1,2) OPERATOR(pg_catalog.<, pg_catalog.<, pg_catalog.<) ROW(2,1); +ERROR: number of operators (3) does not match number of columns (2) at character 17 +-- Too few operators for the row width (two operators, three columns). +SELECT ROW(1,2,3) OPERATOR(pg_catalog.<, pg_catalog.<) ROW(1,2,4); +ERROR: number of operators (2) does not match number of columns (3) at character 19 +-- A list is only meaningful for row (and, later, subquery) comparisons. +-- Scalar infix rejects it. +SELECT 1 OPERATOR(pg_catalog.=, pg_catalog.=) 2; +ERROR: a list of operators is only allowed in row and subquery comparisons at character 10 +-- Scalar prefix rejects it too. +SELECT OPERATOR(pg_catalog.-, pg_catalog.-) 1; +ERROR: a list of operators is only allowed in row and subquery comparisons at character 8 +-- The array form of ANY/ALL resolves a single operator; a list is rejected. +SELECT 1 OPERATOR(pg_catalog.=, pg_catalog.=) ANY (ARRAY[1,2]); +ERROR: a list of operators is only allowed in row and subquery comparisons at character 10 +-- ORDER BY ... USING rejects it. +SELECT 1 FROM public.oq_t ORDER BY a USING OPERATOR(pg_catalog.<, pg_catalog.<); +ERROR: a list of operators is only allowed in row and subquery comparisons at character 44 +-- Sublinks reject operator lists in this patch (a later patch enables them). +SELECT (1, 2) OPERATOR(pg_catalog.=, pg_catalog.=) ANY (SELECT 1, 2); +ERROR: a list of operators is only allowed in row and subquery comparisons at character 15 +-- DDL definition items (here a CREATE OPERATOR commutator) take a single +-- operator name. define.c has no ParseState, so this error carries no position. +CREATE OPERATOR ### (leftarg = int4, rightarg = int4, procedure = int4eq, + commutator = OPERATOR(pg_catalog.=, alt_ops.=)); +ERROR: a list of operators is only allowed in row and subquery comparisons +\set VERBOSITY default +-- The deparsed row-comparison list must reparse under a restricted search_path +-- (the pg_dump scenario), pinning the same operator OIDs as the original. +BEGIN; SET LOCAL search_path = pg_catalog; +DO $$ BEGIN + EXECUTE 'CREATE VIEW public.oq_v_rowcmp_reload AS ' || pg_get_viewdef('public.oq_v_rowcmp'::regclass); + EXECUTE 'CREATE VIEW public.oq_v_rowcmp_mixed_reload AS ' || pg_get_viewdef('public.oq_v_rowcmp_mixed'::regclass); +END $$; COMMIT; +-- The reloaded view resolves alt_ops.< for both columns, same as the original. +SELECT ev_class::regclass FROM pg_rewrite +WHERE ev_action LIKE '%(o ' || 'alt_ops.<(int4,int4)'::regoperator::oid || ' %' + AND ev_class::regclass::text = 'oq_v_rowcmp_reload' +ORDER BY 1; + ev_class +-------------------- + oq_v_rowcmp_reload +(1 row) + +-- The reloaded mixed view pins the per-column operators in order: alt_ops.< for +-- column 1 immediately followed by pg_catalog's int4lt for column 2, exactly as +-- the deparsed OPERATOR(alt_ops.<, <) list encodes them. +SELECT ev_class::regclass FROM pg_rewrite +WHERE ev_action LIKE '%(o ' || 'alt_ops.<(int4,int4)'::regoperator::oid + || ' ' || 'pg_catalog.<(int4,int4)'::regoperator::oid || '%' + AND ev_class::regclass::text = 'oq_v_rowcmp_mixed_reload' +ORDER BY 1; + ev_class +-------------------------- + oq_v_rowcmp_mixed_reload +(1 row) + +DROP VIEW oq_v_rowcmp_reload; +DROP VIEW oq_v_rowcmp_mixed_reload; +-- NOTE: oq_v_rowcmp and oq_v_rowcmp_mixed are intentionally kept (like the other +-- oq_v_* views) so the pg_upgrade suite dump/restores them and exercises the +-- qualified deparse. diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule index fe25b6b2cc..8e05742f28 100644 --- a/src/test/regress/parallel_schedule +++ b/src/test/regress/parallel_schedule @@ -81,7 +81,7 @@ test: create_table_like alter_generic alter_operator misc async dbsize merge mis # collate.linux.utf8 and collate.icu.utf8 tests cannot be run in parallel with each other # psql depends on create_am # amutils depends on geometry, create_index_spgist, hash_index, brin -test: rules psql psql_crosstab psql_pipeline amutils stats_ext collate.linux.utf8 collate.windows.win1252 operator_qualify +test: rules psql psql_crosstab psql_pipeline amutils stats_ext collate.linux.utf8 collate.windows.win1252 # ---------- # Run these alone so they don't run out of parallel workers @@ -101,8 +101,11 @@ test: publication subscription # ---------- # Another group of parallel tests # select_views depends on create_view +# operator_qualify creates a (non-default) btree operator class over int4, +# which would show up in psql's "\dAf btree int4"; it must run after the psql +# group, and no test in this group enumerates operators or operator classes. # ---------- -test: select_views portals_p2 foreign_key dependency guc bitmapops combocid tsearch tsdicts foreign_data window xmlmap functional_deps advisory_lock indirect_toast equivclass stats_rewrite graph_table +test: select_views portals_p2 foreign_key dependency guc bitmapops combocid tsearch tsdicts foreign_data window xmlmap functional_deps advisory_lock indirect_toast equivclass stats_rewrite graph_table operator_qualify # ---------- # Another group of parallel tests (JSON related) diff --git a/src/test/regress/sql/operator_qualify.sql b/src/test/regress/sql/operator_qualify.sql index c8faddddde..74a9428d43 100644 --- a/src/test/regress/sql/operator_qualify.sql +++ b/src/test/regress/sql/operator_qualify.sql @@ -195,3 +195,109 @@ WHERE ev_action LIKE '%:opno ' || 'pg_catalog.<(int4,int4)'::regoperator::oid || ORDER BY 1; DROP VIEW oq_v_using_reload, oq_v_natural_reload, oq_v_using_mixed_reload, oq_v_using_lt_reload; + +-- +-- Row-comparison operator lists: the OPERATOR() decoration may carry a +-- comma-separated LIST of operators, one per column of the compared rows. +-- +-- make_row_comparison_op intersects the per-column operators' btree opfamily +-- interpretations, so a bare "<" with no operator class fails outright with +-- "could not determine interpretation of row comparison operator". A btree +-- operator class over int4 is therefore required. alt_ops.= already exists +-- above; add the rest of the strategy set and wrap them in a class. +CREATE OPERATOR alt_ops.< (leftarg = int4, rightarg = int4, procedure = int4lt); +CREATE OPERATOR alt_ops.<= (leftarg = int4, rightarg = int4, procedure = int4le); +CREATE OPERATOR alt_ops.> (leftarg = int4, rightarg = int4, procedure = int4gt); +CREATE OPERATOR alt_ops.>= (leftarg = int4, rightarg = int4, procedure = int4ge); +CREATE OPERATOR CLASS alt_ops.int4_alt_ops FOR TYPE int4 USING btree AS + OPERATOR 1 alt_ops.<, + OPERATOR 2 alt_ops.<=, + OPERATOR 3 alt_ops.=, + OPERATOR 4 alt_ops.>=, + OPERATOR 5 alt_ops.>, + FUNCTION 1 btint4cmp(int4, int4); + +-- View built while alt_ops shadows pg_catalog: the row comparison resolves to +-- alt_ops.< for both columns. +SET search_path = alt_ops, pg_catalog; +CREATE VIEW public.oq_v_rowcmp AS + SELECT ROW(a, b) < ROW(b, a) AS r FROM public.oq_t; +RESET search_path; +-- Off the path, both column operators must be qualified: the decoration is a +-- LIST, one operator per column (ROW(...) OPERATOR(alt_ops.<, alt_ops.<) ROW(...)). +SELECT pg_get_viewdef('oq_v_rowcmp'::regclass, true); +-- With alt_ops reachable again, the plain single-operator syntax comes back. +SET search_path = alt_ops, pg_catalog, public; +SELECT pg_get_viewdef('oq_v_rowcmp'::regclass, true); +RESET search_path; + +-- Mixed-schema deparse: build (under the default search_path) a view whose row +-- comparison pins column 1 to alt_ops.< (off the path -> must be qualified) and +-- column 2 to pg_catalog.< (reachable bare). A single written name cannot +-- recover two differently-spelled operators, so the deparse emits the per-column +-- list OPERATOR(alt_ops.<, <): only column 1 is qualified, but the list form is +-- forced because the two column names differ. +CREATE VIEW public.oq_v_rowcmp_mixed AS + SELECT ROW(a, b) OPERATOR(alt_ops.<, pg_catalog.<) ROW(b, a) AS r + FROM public.oq_t; +SELECT pg_get_viewdef('oq_v_rowcmp_mixed'::regclass, true); + +-- The list syntax is directly acceptable and N operators compare N columns. +SELECT ROW(1,2) OPERATOR(pg_catalog.<, pg_catalog.<) ROW(2,1) AS t; +-- Mixed-schema list: column 1 pinned to alt_ops.<, column 2 to pg_catalog.<. +SELECT ROW(1,2) OPERATOR(alt_ops.<, pg_catalog.<) ROW(2,3) AS t; +-- A one-element list is exactly today's single-operator syntax (invariance +-- guard: this line is already accepted before the list grammar lands). +SELECT ROW(1,2) OPERATOR(pg_catalog.<) ROW(1,3) AS t; +-- A mixed-schema equality list is accepted; both columns compare equal (t). +SELECT ROW(1,2) OPERATOR(pg_catalog.=, alt_ops.=) ROW(1,2) AS t; + +-- Errors below run under terse verbosity: one line per error, the message +-- text plus "at character N" pinning the OPERATOR() decoration's position. +\set VERBOSITY terse +-- Too many operators for the row width. +SELECT ROW(1,2) OPERATOR(pg_catalog.<, pg_catalog.<, pg_catalog.<) ROW(2,1); +-- Too few operators for the row width (two operators, three columns). +SELECT ROW(1,2,3) OPERATOR(pg_catalog.<, pg_catalog.<) ROW(1,2,4); +-- A list is only meaningful for row (and, later, subquery) comparisons. +-- Scalar infix rejects it. +SELECT 1 OPERATOR(pg_catalog.=, pg_catalog.=) 2; +-- Scalar prefix rejects it too. +SELECT OPERATOR(pg_catalog.-, pg_catalog.-) 1; +-- The array form of ANY/ALL resolves a single operator; a list is rejected. +SELECT 1 OPERATOR(pg_catalog.=, pg_catalog.=) ANY (ARRAY[1,2]); +-- ORDER BY ... USING rejects it. +SELECT 1 FROM public.oq_t ORDER BY a USING OPERATOR(pg_catalog.<, pg_catalog.<); +-- Sublinks reject operator lists in this patch (a later patch enables them). +SELECT (1, 2) OPERATOR(pg_catalog.=, pg_catalog.=) ANY (SELECT 1, 2); +-- DDL definition items (here a CREATE OPERATOR commutator) take a single +-- operator name. define.c has no ParseState, so this error carries no position. +CREATE OPERATOR ### (leftarg = int4, rightarg = int4, procedure = int4eq, + commutator = OPERATOR(pg_catalog.=, alt_ops.=)); +\set VERBOSITY default + +-- The deparsed row-comparison list must reparse under a restricted search_path +-- (the pg_dump scenario), pinning the same operator OIDs as the original. +BEGIN; SET LOCAL search_path = pg_catalog; +DO $$ BEGIN + EXECUTE 'CREATE VIEW public.oq_v_rowcmp_reload AS ' || pg_get_viewdef('public.oq_v_rowcmp'::regclass); + EXECUTE 'CREATE VIEW public.oq_v_rowcmp_mixed_reload AS ' || pg_get_viewdef('public.oq_v_rowcmp_mixed'::regclass); +END $$; COMMIT; +-- The reloaded view resolves alt_ops.< for both columns, same as the original. +SELECT ev_class::regclass FROM pg_rewrite +WHERE ev_action LIKE '%(o ' || 'alt_ops.<(int4,int4)'::regoperator::oid || ' %' + AND ev_class::regclass::text = 'oq_v_rowcmp_reload' +ORDER BY 1; +-- The reloaded mixed view pins the per-column operators in order: alt_ops.< for +-- column 1 immediately followed by pg_catalog's int4lt for column 2, exactly as +-- the deparsed OPERATOR(alt_ops.<, <) list encodes them. +SELECT ev_class::regclass FROM pg_rewrite +WHERE ev_action LIKE '%(o ' || 'alt_ops.<(int4,int4)'::regoperator::oid + || ' ' || 'pg_catalog.<(int4,int4)'::regoperator::oid || '%' + AND ev_class::regclass::text = 'oq_v_rowcmp_mixed_reload' +ORDER BY 1; +DROP VIEW oq_v_rowcmp_reload; +DROP VIEW oq_v_rowcmp_mixed_reload; +-- NOTE: oq_v_rowcmp and oq_v_rowcmp_mixed are intentionally kept (like the other +-- oq_v_* views) so the pg_upgrade suite dump/restores them and exercises the +-- qualified deparse. -- 2.54.0 [application/octet-stream] v2-0004-Allow-per-column-operator-lists-in-ANY-ALL-subque.patch (37.2K, ../../66fa3fe6-8c99-4120-935c-f32f3b61fc30@Spark/6-v2-0004-Allow-per-column-operator-lists-in-ANY-ALL-subque.patch) download | inline diff: From c48d226675d9cbcf569e5df4bccee18836b7963a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= <[email protected]> Date: Tue, 7 Jul 2026 09:51:55 +0200 Subject: [PATCH v2 4/5] Allow per-column operator lists in ANY/ALL subquery comparisons A multi-column subquery comparison such as (a, b) = ANY (SELECT ...) or ROW(a, b) < (SELECT ...) resolves the written operator name independently for each column pair, so the operator chosen for one column may live in a different schema from the operator chosen for another. ruleutils.c could not reproduce that: it printed the first column's operator name for the whole comparison and admitted as much -- Note that we print the name of only the first operator, when there are multiple combining operators. This is an approximation that could go wrong in various scenarios (operators in different schemas, renamed operators, etc) but there is not a whole lot we can do about it, since the syntax allows only one operator to be shown. Worse, ANY_SUBLINK was displayed as IN whenever the first operator's name resolved bare to "=", even if another column's equality operator was not reachable that way; a dumped-and-reloaded view then silently reparsed that column with a different operator -- changed semantics, no error. Extend the previous commit's per-column operator list to subquery comparisons, (a, b) OPERATOR(op1, op2) ANY (SELECT ...) one operator per column of the test row, each independently schema-qualifiable, accepted through both the subquery_Op grammar path and transformAExprOp's conversion of "row op (subselect)" to a ROWCOMPARE sublink. transformSubLink now passes the list through to make_row_comparison_op, which already resolves the n'th name against the n'th column, so its temporary rejection goes away. When deparsing, get_sublink_expr now collects every column's combining operator from the testexpr (a single OpExpr, an AND/OR chain of OpExprs, or a RowCompareExpr) and prints a single bare name only when that name alone recovers every column's operator; otherwise it emits the explicit OPERATOR(...) list. IN is printed only when a plain IN -- which compares with the bare name "=" -- would reparse to the very same operators. Deparse output is unchanged whenever a single written name already represented every column's operator exactly; in particular no expected output outside the new tests changes. Only the raw-parse representation changes shape -- SubLink.operName already round-trips a nested list, stored rules keep the same post-analysis testexpr, and nothing reads operName from stored trees -- so no catversion bump is required. Discussion: https://postgr.es/m/[email protected] --- doc/src/sgml/func/func-subquery.sgml | 40 +++ doc/src/sgml/syntax.sgml | 4 +- src/backend/parser/parse_expr.c | 11 - src/backend/utils/adt/ruleutils.c | 111 ++++++-- src/include/nodes/primnodes.h | 10 +- .../regress/expected/operator_qualify.out | 259 +++++++++++++++++- src/test/regress/sql/operator_qualify.sql | 129 ++++++++- 7 files changed, 524 insertions(+), 40 deletions(-) diff --git a/doc/src/sgml/func/func-subquery.sgml b/doc/src/sgml/func/func-subquery.sgml index f954f3bf13..eef0e8382f 100644 --- a/doc/src/sgml/func/func-subquery.sgml +++ b/doc/src/sgml/func/func-subquery.sgml @@ -263,6 +263,28 @@ WHERE EXISTS (SELECT 1 FROM tab2 WHERE col2 = tab1.col2); and at least one comparison returns NULL. </para> + <para> + As in a direct row-constructor comparison, the written + <replaceable>operator</replaceable>'s bare name is resolved independently + for each pair of columns. You can instead pin the operator used for each + column by writing an explicit per-column operator list with the + <literal>OPERATOR()</literal> syntax (see + <xref linkend="sql-expressions-operator-calls"/>), giving one operator for + each column of the row: +<synopsis> +<replaceable>row_constructor</replaceable> OPERATOR(<replaceable>operator</replaceable> <optional>, <replaceable>operator</replaceable> ... </optional>) ANY (<replaceable>subquery</replaceable>) +</synopsis> + The list must contain exactly one operator per column, and each operator + may be schema-qualified independently. + <productname>PostgreSQL</productname> itself falls back to this form when + deparsing a stored subquery comparison (for example in a view definition) + whose per-column operators cannot all be recovered from one written name. + In particular, a row-wise <token>IN</token> is displayed as + <literal>OPERATOR(...) ANY</literal> whenever a plain <token>IN</token> + would not resolve every column's equality operator to the operator + originally selected. + </para> + <para> See <xref linkend="row-wise-comparison"/> for details about the meaning of a row constructor comparison. @@ -319,6 +341,15 @@ WHERE EXISTS (SELECT 1 FROM tab2 WHERE col2 = tab1.col2); and at least one comparison returns NULL. </para> + <para> + The <replaceable>operator</replaceable> can also be written as an explicit + per-column operator list, exactly as described for <token>ANY</token> + (see <xref linkend="functions-subquery-any-some"/>): +<synopsis> +<replaceable>row_constructor</replaceable> OPERATOR(<replaceable>operator</replaceable> <optional>, <replaceable>operator</replaceable> ... </optional>) ALL (<replaceable>subquery</replaceable>) +</synopsis> + </para> + <para> See <xref linkend="row-wise-comparison"/> for details about the meaning of a row constructor comparison. @@ -347,6 +378,15 @@ WHERE EXISTS (SELECT 1 FROM tab2 WHERE col2 = tab1.col2); compared row-wise to the single subquery result row. </para> + <para> + The <replaceable>operator</replaceable> can also be written as an explicit + per-column operator list, exactly as described for <token>ANY</token> + (see <xref linkend="functions-subquery-any-some"/>): +<synopsis> +<replaceable>row_constructor</replaceable> OPERATOR(<replaceable>operator</replaceable> <optional>, <replaceable>operator</replaceable> ... </optional>) (<replaceable>subquery</replaceable>) +</synopsis> + </para> + <para> See <xref linkend="row-wise-comparison"/> for details about the meaning of a row constructor comparison. diff --git a/doc/src/sgml/syntax.sgml b/doc/src/sgml/syntax.sgml index 094c2c2ffb..5165533813 100644 --- a/doc/src/sgml/syntax.sgml +++ b/doc/src/sgml/syntax.sgml @@ -1139,7 +1139,9 @@ SELECT 3 OPERATOR(pg_catalog.+) 4; (see <xref linkend="functions-nullif"/>). A row-constructor comparison goes a step further and accepts a comma-separated <emphasis>list</emphasis> of operators inside <literal>OPERATOR()</literal>, one for each column of - the compared rows (see <xref linkend="row-wise-comparison"/>). + the compared rows (see <xref linkend="row-wise-comparison"/>); this + includes row-constructor comparisons against a subquery + (see <xref linkend="functions-subquery"/>). </para> <note> diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c index bd16e73a88..a8630e6c34 100644 --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -2020,17 +2020,6 @@ transformSubLink(ParseState *pstate, SubLink *sublink) List *right_list; ListCell *l; - /* - * TEMPORARY: reject a per-column operator list here. A later patch - * teaches this path (and ruleutils.c) to apply the n'th name to the - * n'th column, at which point this check goes away. Note this also - * catches lists arriving via transformAExprOp's conversion of "row op - * (subselect)" to a ROWCOMPARE sublink, which passes the A_Expr's - * name through operName. - */ - reject_operator_name_list(pstate, sublink->operName, - sublink->location); - /* * If the source was "x IN (select)", convert to "x = ANY (select)". */ diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 7cca15bbb5..487ee4b6e0 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -12526,7 +12526,11 @@ get_sublink_expr(SubLink *sublink, deparse_context *context) { StringInfo buf = context->buf; Query *query = (Query *) (sublink->subselect); + List *opnos = NIL; + List *ltypes = NIL; + List *rtypes = NIL; char *opname = NULL; + bool all_bare_eq = true; bool need_paren; if (sublink->subLinkType == ARRAY_SUBLINK) @@ -12535,11 +12539,11 @@ get_sublink_expr(SubLink *sublink, deparse_context *context) appendStringInfoChar(buf, '('); /* - * Note that we print the name of only the first operator, when there are - * multiple combining operators. This is an approximation that could go - * wrong in various scenarios (operators in different schemas, renamed - * operators, etc) but there is not a whole lot we can do about it, since - * the syntax allows only one operator to be shown. + * Dissect the testexpr into the per-column combining operators, printing + * the lefthand expression(s) as we go. Historically we printed only the + * first operator's name for the whole comparison; now that the syntax + * accepts an explicit per-column operator list we can represent the + * combining operators exactly (see the emission rule below). */ if (sublink->testexpr) { @@ -12549,9 +12553,9 @@ get_sublink_expr(SubLink *sublink, deparse_context *context) OpExpr *opexpr = (OpExpr *) sublink->testexpr; get_rule_expr(linitial(opexpr->args), context, true); - opname = generate_operator_name(opexpr->opno, - exprType(linitial(opexpr->args)), - exprType(lsecond(opexpr->args))); + opnos = lappend_oid(opnos, opexpr->opno); + ltypes = lappend_oid(ltypes, exprType(linitial(opexpr->args))); + rtypes = lappend_oid(rtypes, exprType(lsecond(opexpr->args))); } else if (IsA(sublink->testexpr, BoolExpr)) { @@ -12563,14 +12567,14 @@ get_sublink_expr(SubLink *sublink, deparse_context *context) sep = ""; foreach(l, ((BoolExpr *) sublink->testexpr)->args) { - OpExpr *opexpr = lfirst_node(OpExpr, l); + Node *arm = strip_implicit_coercions(lfirst(l)); + OpExpr *opexpr = castNode(OpExpr, arm); appendStringInfoString(buf, sep); get_rule_expr(linitial(opexpr->args), context, true); - if (!opname) - opname = generate_operator_name(opexpr->opno, - exprType(linitial(opexpr->args)), - exprType(lsecond(opexpr->args))); + opnos = lappend_oid(opnos, opexpr->opno); + ltypes = lappend_oid(ltypes, exprType(linitial(opexpr->args))); + rtypes = lappend_oid(rtypes, exprType(lsecond(opexpr->args))); sep = ", "; } appendStringInfoChar(buf, ')'); @@ -12579,12 +12583,20 @@ get_sublink_expr(SubLink *sublink, deparse_context *context) { /* multiple combining operators, < <= > >= cases */ RowCompareExpr *rcexpr = (RowCompareExpr *) sublink->testexpr; + ListCell *opno_cell; + ListCell *larg_cell; + ListCell *rarg_cell; appendStringInfoChar(buf, '('); get_rule_expr((Node *) rcexpr->largs, context, true); - opname = generate_operator_name(linitial_oid(rcexpr->opnos), - exprType(linitial(rcexpr->largs)), - exprType(linitial(rcexpr->rargs))); + forthree(opno_cell, rcexpr->opnos, + larg_cell, rcexpr->largs, + rarg_cell, rcexpr->rargs) + { + opnos = lappend_oid(opnos, lfirst_oid(opno_cell)); + ltypes = lappend_oid(ltypes, exprType((Node *) lfirst(larg_cell))); + rtypes = lappend_oid(rtypes, exprType((Node *) lfirst(rarg_cell))); + } appendStringInfoChar(buf, ')'); } else @@ -12592,6 +12604,71 @@ get_sublink_expr(SubLink *sublink, deparse_context *context) (int) nodeTag(sublink->testexpr)); } + /* + * Resolve the per-column operator names and decide how to spell the + * combining operator(s). The undecorated syntax writes one operator name + * that reparses by resolving it independently for every column, so it is + * honest only when a single bare name recovers each column's operator: no + * column needs schema-qualification and every column shares the same + * operator name. In that case we print that shared name, as before. + * Otherwise we emit an explicit per-column operator list, OPERATOR(op1, + * op2, ...), which pins each column's operator independently. (For one + * column the list form degenerates to the familiar OPERATOR(op) + * decoration.) Likewise, ANY_SUBLINK is displayed as IN only if a plain + * IN -- which compares with the bare name "=" -- would reparse to the + * very same operators. + */ + if (opnos != NIL) + { + List *opnames = NIL; + bool need_op_list = false; + char *firstname = NULL; + ListCell *lc1; + ListCell *lc2; + ListCell *lc3; + + forthree(lc1, opnos, lc2, ltypes, lc3, rtypes) + { + bool needs_qual; + char *thisname; + + thisname = generate_operator_name_extended(lfirst_oid(lc1), + lfirst_oid(lc2), + lfirst_oid(lc3), + &needs_qual); + opnames = lappend(opnames, thisname); + if (needs_qual || strcmp(thisname, "=") != 0) + all_bare_eq = false; + if (firstname == NULL) + firstname = thisname; + else if (strcmp(firstname, thisname) != 0) + need_op_list = true; + if (needs_qual) + need_op_list = true; + } + + if (need_op_list) + { + StringInfoData opbuf; + bool first = true; + + initStringInfo(&opbuf); + appendStringInfoString(&opbuf, "OPERATOR("); + foreach(lc1, opnames) + { + if (first) + first = false; + else + appendStringInfoString(&opbuf, ", "); + appendStringInfoString(&opbuf, (char *) lfirst(lc1)); + } + appendStringInfoChar(&opbuf, ')'); + opname = opbuf.data; + } + else + opname = firstname; + } + need_paren = true; switch (sublink->subLinkType) @@ -12601,7 +12678,7 @@ get_sublink_expr(SubLink *sublink, deparse_context *context) break; case ANY_SUBLINK: - if (strcmp(opname, "=") == 0) /* Represent = ANY as IN */ + if (all_bare_eq) /* Represent = ANY as IN */ appendStringInfoString(buf, " IN "); else appendStringInfo(buf, " %s ANY ", opname); diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h index 690d2976cd..bf343cbec0 100644 --- a/src/include/nodes/primnodes.h +++ b/src/include/nodes/primnodes.h @@ -987,10 +987,12 @@ typedef struct BoolExpr * planning. * * NOTE: in the raw output of gram.y, testexpr contains just the raw form - * of the lefthand expression (if any), and operName is the String name of - * the combining operator. Also, subselect is a raw parsetree. During parse - * analysis, the parser transforms testexpr into a complete boolean expression - * that compares the lefthand value(s) to PARAM_SUBLINK nodes representing the + * of the lefthand expression (if any), and operName is the possibly-qualified + * name of the combining operator (a List of String), or a List of such names, + * one per column, when the comparison was written with an explicit per-column + * operator list. Also, subselect is a raw parsetree. During parse analysis, + * the parser transforms testexpr into a complete boolean expression that + * compares the lefthand value(s) to PARAM_SUBLINK nodes representing the * output columns of the subselect. And subselect is transformed to a Query. * This is the representation seen in saved rules and in the rewriter. * diff --git a/src/test/regress/expected/operator_qualify.out b/src/test/regress/expected/operator_qualify.out index f6d70e886e..c0887e6311 100644 --- a/src/test/regress/expected/operator_qualify.out +++ b/src/test/regress/expected/operator_qualify.out @@ -444,7 +444,7 @@ ERROR: number of operators (3) does not match number of columns (2) at characte -- Too few operators for the row width (two operators, three columns). SELECT ROW(1,2,3) OPERATOR(pg_catalog.<, pg_catalog.<) ROW(1,2,4); ERROR: number of operators (2) does not match number of columns (3) at character 19 --- A list is only meaningful for row (and, later, subquery) comparisons. +-- A list is only meaningful for row and subquery comparisons. -- Scalar infix rejects it. SELECT 1 OPERATOR(pg_catalog.=, pg_catalog.=) 2; ERROR: a list of operators is only allowed in row and subquery comparisons at character 10 @@ -457,9 +457,6 @@ ERROR: a list of operators is only allowed in row and subquery comparisons at c -- ORDER BY ... USING rejects it. SELECT 1 FROM public.oq_t ORDER BY a USING OPERATOR(pg_catalog.<, pg_catalog.<); ERROR: a list of operators is only allowed in row and subquery comparisons at character 44 --- Sublinks reject operator lists in this patch (a later patch enables them). -SELECT (1, 2) OPERATOR(pg_catalog.=, pg_catalog.=) ANY (SELECT 1, 2); -ERROR: a list of operators is only allowed in row and subquery comparisons at character 15 -- DDL definition items (here a CREATE OPERATOR commutator) take a single -- operator name. define.c has no ParseState, so this error carries no position. CREATE OPERATOR ### (leftarg = int4, rightarg = int4, procedure = int4eq, @@ -501,3 +498,257 @@ DROP VIEW oq_v_rowcmp_mixed_reload; -- NOTE: oq_v_rowcmp and oq_v_rowcmp_mixed are intentionally kept (like the other -- oq_v_* views) so the pg_upgrade suite dump/restores them and exercises the -- qualified deparse. +-- +-- Subquery comparisons: ANY/ALL and row-op-subquery sublinks accept the +-- per-column operator list too, and their deparse represents the combining +-- operators exactly (in particular, IN is printed only when a plain IN +-- would reparse to the very same per-column operators). +-- +-- A multi-column comparison must find a btree interpretation for every +-- combining operator. A "<>" is interpreted through its negator's btree +-- equality membership, so link alt_ops.<> to alt_ops.= (strategy 3 of the +-- operator class above) instead of extending the class. +CREATE OPERATOR alt_ops.<> (leftarg = int4, rightarg = int4, procedure = int4ne, + negator = OPERATOR(alt_ops.=)); +-- The list syntax is directly acceptable in all three sublink forms, with +-- mixed bare/qualified names. +SELECT (1, 2) OPERATOR(pg_catalog.=, pg_catalog.=) ANY (SELECT 1, 2) AS t; + t +--- + t +(1 row) + +SELECT (1, 2) OPERATOR(pg_catalog.=, alt_ops.=) ANY (SELECT 1, 2) AS t; + t +--- + t +(1 row) + +SELECT (1, 2) OPERATOR(alt_ops.<>, pg_catalog.<>) ALL (SELECT 2, 1) AS t; + t +--- + t +(1 row) + +-- "row op (subquery)" reaches the sublink through transformAExprOp's +-- ROWCOMPARE conversion; the list flows through that path as well. +SELECT (1, 2) OPERATOR(alt_ops.<, alt_ops.<) (SELECT 1, 3) AS t; + t +--- + t +(1 row) + +-- A one-element list is the traditional single-operator form (invariance +-- guard: this line is accepted before the list grammar lands). +SELECT (1, 2) OPERATOR(pg_catalog.=) ANY (SELECT 1, 2) AS t; + t +--- + t +(1 row) + +-- The operator count must match the row width. +\set VERBOSITY terse +SELECT (1, 2) OPERATOR(pg_catalog.=, pg_catalog.=, pg_catalog.=) ANY (SELECT 1, 2); +ERROR: number of operators (3) does not match number of columns (2) at character 15 +\set VERBOSITY default +-- Views built while alt_ops shadows pg_catalog: every combining operator +-- resolves into alt_ops. +SET search_path = alt_ops, pg_catalog; +CREATE VIEW public.oq_v_any AS + SELECT (a, b) = ANY (SELECT a, b FROM public.oq_t) AS r FROM public.oq_t; +CREATE VIEW public.oq_v_in AS + SELECT (a, b) IN (SELECT a, b FROM public.oq_t) AS r FROM public.oq_t; +CREATE VIEW public.oq_v_ne_all AS + SELECT (a, b) <> ALL (SELECT a, b FROM public.oq_t) AS r FROM public.oq_t; +CREATE VIEW public.oq_v_rowcmp_sub AS + SELECT (a, b) < (SELECT a, b FROM public.oq_t LIMIT 1) AS r FROM public.oq_t; +-- Single-column comparison: one off-path operator, no list needed. +CREATE VIEW public.oq_v_any_scalar AS + SELECT a = ANY (SELECT b FROM public.oq_t) AS r FROM public.oq_t; +-- An IN whose per-column "=" lookups land in *different* schemas: column 1 +-- compares int8 (alt_ops has no int8 operator, pg_catalog.= applies) while +-- column 2 finds alt_ops.=. Before this patch the view deparsed back to a +-- plain IN, so a dump/reload under the default search_path silently replaced +-- column 2's operator with pg_catalog.= -- changed semantics, no error. +CREATE VIEW public.oq_v_in_mixed AS + SELECT (a::int8, b) IN (SELECT a::int8, b FROM public.oq_t) AS r FROM public.oq_t; +RESET search_path; +-- A mixed explicit list, written under the default path. +CREATE VIEW public.oq_v_any_mixed AS + SELECT (a, b) OPERATOR(pg_catalog.=, alt_ops.=) ANY (SELECT a, b FROM public.oq_t) AS r + FROM public.oq_t; +-- Guards: IN over pg_catalog operators only must keep printing IN. +CREATE VIEW public.oq_v_in_cat AS + SELECT (a, b) IN (SELECT a, b FROM public.oq_t) AS r FROM public.oq_t; +CREATE VIEW public.oq_v_in_scalar_cat AS + SELECT a IN (SELECT b FROM public.oq_t) AS r FROM public.oq_t; +-- Off the path, IN may not be printed: a plain IN would reparse every column +-- with pg_catalog.=. The honest form is the per-column operator list. +SELECT pg_get_viewdef('oq_v_any'::regclass, true); + pg_get_viewdef +----------------------------------------------------------------------- + SELECT ((a, b) OPERATOR(alt_ops.=, alt_ops.=) ANY ( SELECT oq_t_1.a,+ + oq_t_1.b + + FROM oq_t oq_t_1)) AS r + + FROM oq_t; +(1 row) + +SELECT pg_get_viewdef('oq_v_in'::regclass, true); + pg_get_viewdef +----------------------------------------------------------------------- + SELECT ((a, b) OPERATOR(alt_ops.=, alt_ops.=) ANY ( SELECT oq_t_1.a,+ + oq_t_1.b + + FROM oq_t oq_t_1)) AS r + + FROM oq_t; +(1 row) + +SELECT pg_get_viewdef('oq_v_ne_all'::regclass, true); + pg_get_viewdef +------------------------------------------------------------------------- + SELECT ((a, b) OPERATOR(alt_ops.<>, alt_ops.<>) ALL ( SELECT oq_t_1.a,+ + oq_t_1.b + + FROM oq_t oq_t_1)) AS r + + FROM oq_t; +(1 row) + +SELECT pg_get_viewdef('oq_v_rowcmp_sub'::regclass, true); + pg_get_viewdef +------------------------------------------------------------------- + SELECT ((a, b) OPERATOR(alt_ops.<, alt_ops.<) ( SELECT oq_t_1.a,+ + oq_t_1.b + + FROM oq_t oq_t_1 + + LIMIT 1)) AS r + + FROM oq_t; +(1 row) + +-- The single-column form needs no list, just the qualified operator. +SELECT pg_get_viewdef('oq_v_any_scalar'::regclass, true); + pg_get_viewdef +------------------------------------------------------ + SELECT (a OPERATOR(alt_ops.=) ANY ( SELECT oq_t_1.b+ + FROM oq_t oq_t_1)) AS r + + FROM oq_t; +(1 row) + +-- The mixed views print a list of one bare and one qualified name. +SELECT pg_get_viewdef('oq_v_in_mixed'::regclass, true); + pg_get_viewdef +------------------------------------------------------------------------------------ + SELECT ((a::bigint, b) OPERATOR(=, alt_ops.=) ANY ( SELECT oq_t_1.a::bigint AS a,+ + oq_t_1.b + + FROM oq_t oq_t_1)) AS r + + FROM oq_t; +(1 row) + +SELECT pg_get_viewdef('oq_v_any_mixed'::regclass, true); + pg_get_viewdef +--------------------------------------------------------------- + SELECT ((a, b) OPERATOR(=, alt_ops.=) ANY ( SELECT oq_t_1.a,+ + oq_t_1.b + + FROM oq_t oq_t_1)) AS r + + FROM oq_t; +(1 row) + +-- The pg_catalog-only views still print IN. +SELECT pg_get_viewdef('oq_v_in_cat'::regclass, true); + pg_get_viewdef +--------------------------------------- + SELECT ((a, b) IN ( SELECT oq_t_1.a,+ + oq_t_1.b + + FROM oq_t oq_t_1)) AS r + + FROM oq_t; +(1 row) + +SELECT pg_get_viewdef('oq_v_in_scalar_cat'::regclass, true); + pg_get_viewdef +------------------------------------ + SELECT (a IN ( SELECT oq_t_1.b + + FROM oq_t oq_t_1)) AS r+ + FROM oq_t; +(1 row) + +-- With alt_ops reachable again, every column's operator resolves bare to "=" +-- and the readable IN comes back. +SET search_path = alt_ops, pg_catalog, public; +SELECT pg_get_viewdef('oq_v_in'::regclass, true); + pg_get_viewdef +--------------------------------------- + SELECT ((a, b) IN ( SELECT oq_t_1.a,+ + oq_t_1.b + + FROM oq_t oq_t_1)) AS r + + FROM oq_t; +(1 row) + +RESET search_path; +-- The deparsed sublinks must reparse under a restricted search_path (the +-- pg_dump scenario), pinning the same operator OIDs as the originals. +BEGIN; SET LOCAL search_path = pg_catalog; +DO $$ BEGIN + EXECUTE 'CREATE VIEW public.oq_v_any_reload AS ' || pg_get_viewdef('public.oq_v_any'::regclass); + EXECUTE 'CREATE VIEW public.oq_v_in_reload AS ' || pg_get_viewdef('public.oq_v_in'::regclass); + EXECUTE 'CREATE VIEW public.oq_v_ne_all_reload AS ' || pg_get_viewdef('public.oq_v_ne_all'::regclass); + EXECUTE 'CREATE VIEW public.oq_v_rowcmp_sub_reload AS ' || pg_get_viewdef('public.oq_v_rowcmp_sub'::regclass); + EXECUTE 'CREATE VIEW public.oq_v_any_scalar_reload AS ' || pg_get_viewdef('public.oq_v_any_scalar'::regclass); + EXECUTE 'CREATE VIEW public.oq_v_in_mixed_reload AS ' || pg_get_viewdef('public.oq_v_in_mixed'::regclass); + EXECUTE 'CREATE VIEW public.oq_v_any_mixed_reload AS ' || pg_get_viewdef('public.oq_v_any_mixed'::regclass); +END $$; COMMIT; +-- Every reloaded view that pinned alt_ops.= still pins it (the = and <> +-- forms decompose into per-column OpExprs, serialized with ":opno"). +SELECT ev_class::regclass FROM pg_rewrite +WHERE ev_action LIKE '%:opno ' || 'alt_ops.=(int4,int4)'::regoperator::oid || ' %' + AND ev_class::regclass::text LIKE 'oq_v%reload' +ORDER BY 1; + ev_class +------------------------ + oq_v_any_reload + oq_v_in_reload + oq_v_any_scalar_reload + oq_v_in_mixed_reload + oq_v_any_mixed_reload +(5 rows) + +-- The mixed views pin pg_catalog's "=" for column 1 *followed by* alt_ops.= +-- for column 2: the combining OpExprs are serialized in column order. +SELECT ev_class::regclass FROM pg_rewrite +WHERE ev_action LIKE '%:opno ' || 'pg_catalog.=(int8,int8)'::regoperator::oid + || ' %:opno ' || 'alt_ops.=(int4,int4)'::regoperator::oid || ' %' + AND ev_class::regclass::text = 'oq_v_in_mixed_reload'; + ev_class +---------------------- + oq_v_in_mixed_reload +(1 row) + +SELECT ev_class::regclass FROM pg_rewrite +WHERE ev_action LIKE '%:opno ' || 'pg_catalog.=(int4,int4)'::regoperator::oid + || ' %:opno ' || 'alt_ops.=(int4,int4)'::regoperator::oid || ' %' + AND ev_class::regclass::text = 'oq_v_any_mixed_reload'; + ev_class +----------------------- + oq_v_any_mixed_reload +(1 row) + +-- <> ALL keeps alt_ops.<> ... +SELECT ev_class::regclass FROM pg_rewrite +WHERE ev_action LIKE '%:opno ' || 'alt_ops.<>(int4,int4)'::regoperator::oid || ' %' + AND ev_class::regclass::text = 'oq_v_ne_all_reload'; + ev_class +-------------------- + oq_v_ne_all_reload +(1 row) + +-- ... and the row comparison keeps alt_ops.< for both columns (a sublink's +-- RowCompareExpr serializes its operators as one oid list "(o ...)"). +SELECT ev_class::regclass FROM pg_rewrite +WHERE ev_action LIKE '%(o ' || 'alt_ops.<(int4,int4)'::regoperator::oid + || ' ' || 'alt_ops.<(int4,int4)'::regoperator::oid || '%' + AND ev_class::regclass::text = 'oq_v_rowcmp_sub_reload'; + ev_class +------------------------ + oq_v_rowcmp_sub_reload +(1 row) + +DROP VIEW oq_v_any_reload, oq_v_in_reload, oq_v_ne_all_reload, + oq_v_rowcmp_sub_reload, oq_v_any_scalar_reload, + oq_v_in_mixed_reload, oq_v_any_mixed_reload; +-- NOTE: the new oq_v_* base views are intentionally kept (like the others) +-- so the pg_upgrade suite dump/restores them and exercises the deparse. diff --git a/src/test/regress/sql/operator_qualify.sql b/src/test/regress/sql/operator_qualify.sql index 74a9428d43..f029d0eb73 100644 --- a/src/test/regress/sql/operator_qualify.sql +++ b/src/test/regress/sql/operator_qualify.sql @@ -259,7 +259,7 @@ SELECT ROW(1,2) OPERATOR(pg_catalog.=, alt_ops.=) ROW(1,2) AS t; SELECT ROW(1,2) OPERATOR(pg_catalog.<, pg_catalog.<, pg_catalog.<) ROW(2,1); -- Too few operators for the row width (two operators, three columns). SELECT ROW(1,2,3) OPERATOR(pg_catalog.<, pg_catalog.<) ROW(1,2,4); --- A list is only meaningful for row (and, later, subquery) comparisons. +-- A list is only meaningful for row and subquery comparisons. -- Scalar infix rejects it. SELECT 1 OPERATOR(pg_catalog.=, pg_catalog.=) 2; -- Scalar prefix rejects it too. @@ -268,8 +268,6 @@ SELECT OPERATOR(pg_catalog.-, pg_catalog.-) 1; SELECT 1 OPERATOR(pg_catalog.=, pg_catalog.=) ANY (ARRAY[1,2]); -- ORDER BY ... USING rejects it. SELECT 1 FROM public.oq_t ORDER BY a USING OPERATOR(pg_catalog.<, pg_catalog.<); --- Sublinks reject operator lists in this patch (a later patch enables them). -SELECT (1, 2) OPERATOR(pg_catalog.=, pg_catalog.=) ANY (SELECT 1, 2); -- DDL definition items (here a CREATE OPERATOR commutator) take a single -- operator name. define.c has no ParseState, so this error carries no position. CREATE OPERATOR ### (leftarg = int4, rightarg = int4, procedure = int4eq, @@ -301,3 +299,128 @@ DROP VIEW oq_v_rowcmp_mixed_reload; -- NOTE: oq_v_rowcmp and oq_v_rowcmp_mixed are intentionally kept (like the other -- oq_v_* views) so the pg_upgrade suite dump/restores them and exercises the -- qualified deparse. + +-- +-- Subquery comparisons: ANY/ALL and row-op-subquery sublinks accept the +-- per-column operator list too, and their deparse represents the combining +-- operators exactly (in particular, IN is printed only when a plain IN +-- would reparse to the very same per-column operators). +-- +-- A multi-column comparison must find a btree interpretation for every +-- combining operator. A "<>" is interpreted through its negator's btree +-- equality membership, so link alt_ops.<> to alt_ops.= (strategy 3 of the +-- operator class above) instead of extending the class. +CREATE OPERATOR alt_ops.<> (leftarg = int4, rightarg = int4, procedure = int4ne, + negator = OPERATOR(alt_ops.=)); + +-- The list syntax is directly acceptable in all three sublink forms, with +-- mixed bare/qualified names. +SELECT (1, 2) OPERATOR(pg_catalog.=, pg_catalog.=) ANY (SELECT 1, 2) AS t; +SELECT (1, 2) OPERATOR(pg_catalog.=, alt_ops.=) ANY (SELECT 1, 2) AS t; +SELECT (1, 2) OPERATOR(alt_ops.<>, pg_catalog.<>) ALL (SELECT 2, 1) AS t; +-- "row op (subquery)" reaches the sublink through transformAExprOp's +-- ROWCOMPARE conversion; the list flows through that path as well. +SELECT (1, 2) OPERATOR(alt_ops.<, alt_ops.<) (SELECT 1, 3) AS t; +-- A one-element list is the traditional single-operator form (invariance +-- guard: this line is accepted before the list grammar lands). +SELECT (1, 2) OPERATOR(pg_catalog.=) ANY (SELECT 1, 2) AS t; +-- The operator count must match the row width. +\set VERBOSITY terse +SELECT (1, 2) OPERATOR(pg_catalog.=, pg_catalog.=, pg_catalog.=) ANY (SELECT 1, 2); +\set VERBOSITY default + +-- Views built while alt_ops shadows pg_catalog: every combining operator +-- resolves into alt_ops. +SET search_path = alt_ops, pg_catalog; +CREATE VIEW public.oq_v_any AS + SELECT (a, b) = ANY (SELECT a, b FROM public.oq_t) AS r FROM public.oq_t; +CREATE VIEW public.oq_v_in AS + SELECT (a, b) IN (SELECT a, b FROM public.oq_t) AS r FROM public.oq_t; +CREATE VIEW public.oq_v_ne_all AS + SELECT (a, b) <> ALL (SELECT a, b FROM public.oq_t) AS r FROM public.oq_t; +CREATE VIEW public.oq_v_rowcmp_sub AS + SELECT (a, b) < (SELECT a, b FROM public.oq_t LIMIT 1) AS r FROM public.oq_t; +-- Single-column comparison: one off-path operator, no list needed. +CREATE VIEW public.oq_v_any_scalar AS + SELECT a = ANY (SELECT b FROM public.oq_t) AS r FROM public.oq_t; +-- An IN whose per-column "=" lookups land in *different* schemas: column 1 +-- compares int8 (alt_ops has no int8 operator, pg_catalog.= applies) while +-- column 2 finds alt_ops.=. Before this patch the view deparsed back to a +-- plain IN, so a dump/reload under the default search_path silently replaced +-- column 2's operator with pg_catalog.= -- changed semantics, no error. +CREATE VIEW public.oq_v_in_mixed AS + SELECT (a::int8, b) IN (SELECT a::int8, b FROM public.oq_t) AS r FROM public.oq_t; +RESET search_path; +-- A mixed explicit list, written under the default path. +CREATE VIEW public.oq_v_any_mixed AS + SELECT (a, b) OPERATOR(pg_catalog.=, alt_ops.=) ANY (SELECT a, b FROM public.oq_t) AS r + FROM public.oq_t; +-- Guards: IN over pg_catalog operators only must keep printing IN. +CREATE VIEW public.oq_v_in_cat AS + SELECT (a, b) IN (SELECT a, b FROM public.oq_t) AS r FROM public.oq_t; +CREATE VIEW public.oq_v_in_scalar_cat AS + SELECT a IN (SELECT b FROM public.oq_t) AS r FROM public.oq_t; + +-- Off the path, IN may not be printed: a plain IN would reparse every column +-- with pg_catalog.=. The honest form is the per-column operator list. +SELECT pg_get_viewdef('oq_v_any'::regclass, true); +SELECT pg_get_viewdef('oq_v_in'::regclass, true); +SELECT pg_get_viewdef('oq_v_ne_all'::regclass, true); +SELECT pg_get_viewdef('oq_v_rowcmp_sub'::regclass, true); +-- The single-column form needs no list, just the qualified operator. +SELECT pg_get_viewdef('oq_v_any_scalar'::regclass, true); +-- The mixed views print a list of one bare and one qualified name. +SELECT pg_get_viewdef('oq_v_in_mixed'::regclass, true); +SELECT pg_get_viewdef('oq_v_any_mixed'::regclass, true); +-- The pg_catalog-only views still print IN. +SELECT pg_get_viewdef('oq_v_in_cat'::regclass, true); +SELECT pg_get_viewdef('oq_v_in_scalar_cat'::regclass, true); +-- With alt_ops reachable again, every column's operator resolves bare to "=" +-- and the readable IN comes back. +SET search_path = alt_ops, pg_catalog, public; +SELECT pg_get_viewdef('oq_v_in'::regclass, true); +RESET search_path; + +-- The deparsed sublinks must reparse under a restricted search_path (the +-- pg_dump scenario), pinning the same operator OIDs as the originals. +BEGIN; SET LOCAL search_path = pg_catalog; +DO $$ BEGIN + EXECUTE 'CREATE VIEW public.oq_v_any_reload AS ' || pg_get_viewdef('public.oq_v_any'::regclass); + EXECUTE 'CREATE VIEW public.oq_v_in_reload AS ' || pg_get_viewdef('public.oq_v_in'::regclass); + EXECUTE 'CREATE VIEW public.oq_v_ne_all_reload AS ' || pg_get_viewdef('public.oq_v_ne_all'::regclass); + EXECUTE 'CREATE VIEW public.oq_v_rowcmp_sub_reload AS ' || pg_get_viewdef('public.oq_v_rowcmp_sub'::regclass); + EXECUTE 'CREATE VIEW public.oq_v_any_scalar_reload AS ' || pg_get_viewdef('public.oq_v_any_scalar'::regclass); + EXECUTE 'CREATE VIEW public.oq_v_in_mixed_reload AS ' || pg_get_viewdef('public.oq_v_in_mixed'::regclass); + EXECUTE 'CREATE VIEW public.oq_v_any_mixed_reload AS ' || pg_get_viewdef('public.oq_v_any_mixed'::regclass); +END $$; COMMIT; +-- Every reloaded view that pinned alt_ops.= still pins it (the = and <> +-- forms decompose into per-column OpExprs, serialized with ":opno"). +SELECT ev_class::regclass FROM pg_rewrite +WHERE ev_action LIKE '%:opno ' || 'alt_ops.=(int4,int4)'::regoperator::oid || ' %' + AND ev_class::regclass::text LIKE 'oq_v%reload' +ORDER BY 1; +-- The mixed views pin pg_catalog's "=" for column 1 *followed by* alt_ops.= +-- for column 2: the combining OpExprs are serialized in column order. +SELECT ev_class::regclass FROM pg_rewrite +WHERE ev_action LIKE '%:opno ' || 'pg_catalog.=(int8,int8)'::regoperator::oid + || ' %:opno ' || 'alt_ops.=(int4,int4)'::regoperator::oid || ' %' + AND ev_class::regclass::text = 'oq_v_in_mixed_reload'; +SELECT ev_class::regclass FROM pg_rewrite +WHERE ev_action LIKE '%:opno ' || 'pg_catalog.=(int4,int4)'::regoperator::oid + || ' %:opno ' || 'alt_ops.=(int4,int4)'::regoperator::oid || ' %' + AND ev_class::regclass::text = 'oq_v_any_mixed_reload'; +-- <> ALL keeps alt_ops.<> ... +SELECT ev_class::regclass FROM pg_rewrite +WHERE ev_action LIKE '%:opno ' || 'alt_ops.<>(int4,int4)'::regoperator::oid || ' %' + AND ev_class::regclass::text = 'oq_v_ne_all_reload'; +-- ... and the row comparison keeps alt_ops.< for both columns (a sublink's +-- RowCompareExpr serializes its operators as one oid list "(o ...)"). +SELECT ev_class::regclass FROM pg_rewrite +WHERE ev_action LIKE '%(o ' || 'alt_ops.<(int4,int4)'::regoperator::oid + || ' ' || 'alt_ops.<(int4,int4)'::regoperator::oid || '%' + AND ev_class::regclass::text = 'oq_v_rowcmp_sub_reload'; +DROP VIEW oq_v_any_reload, oq_v_in_reload, oq_v_ne_all_reload, + oq_v_rowcmp_sub_reload, oq_v_any_scalar_reload, + oq_v_in_mixed_reload, oq_v_any_mixed_reload; +-- NOTE: the new oq_v_* base views are intentionally kept (like the others) +-- so the pg_upgrade suite dump/restores them and exercises the deparse. -- 2.54.0 [application/octet-stream] v2-0005-Allow-schema-qualifying-the-comparison-operator-o.patch (21.3K, ../../66fa3fe6-8c99-4120-935c-f32f3b61fc30@Spark/7-v2-0005-Allow-schema-qualifying-the-comparison-operator-o.patch) download | inline diff: From b0cdce84f7825651a6bda1a371bcc2458311545f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= <[email protected]> Date: Tue, 7 Jul 2026 10:30:40 +0200 Subject: [PATCH v2 5/5] Allow schema-qualifying the comparison operator of a simple CASE A simple CASE, CASE x WHEN y THEN ..., is defined to compare the test expression against each WHEN value exactly as if "x = y" had been written, and each of those "=" operators is resolved independently by unqualified name. Two arms of one CASE can therefore resolve to different operators: in CASE x WHEN 4 THEN ... WHEN 5.0 THEN ... END the first arm resolves integer equality while the second resolves numeric equality, and either could belong to a schema that is not on the search path. ruleutils.c could not reproduce that at all: when it recognized the implicit "CaseTestExpr = RHS" form it printed just the RHS and silently dropped the operator, so a dumped-and-reloaded view reparsed every arm with whatever "=" the restore-time search path happened to resolve -- changed semantics, no error. Give the simple CASE an optional per-arm operator syntax mirroring the one added for NULLIF and IS [NOT] DISTINCT FROM, CASE x WHEN y USING OPERATOR(schema.=) THEN ... which pins the comparison operator for that arm (and only that arm). A new raw-only List field CaseWhen.opname carries the written name from the grammar to parse analysis, where transformCaseExpr expands the arm into the chosen operator instead of a bare "="; the field is reset to NIL in analyzed trees, and using the clause on a searched CASE (which has no test expression) is rejected. When deparsing, each recognized arm now emits USING OPERATOR() honestly: whenever a bare "WHEN value" would not reparse to the same operator -- the operator is not reachable by an unqualified lookup of its own name, or is not named "=" -- the clause is printed, schema-qualified as needed, per arm. Output is unchanged whenever the bare form already recovered the operator. CaseWhen now serializes an additional field, so bump the catalog version. Discussion: https://postgr.es/m/[email protected] --- doc/src/sgml/func/func-conditional.sgml | 22 ++- doc/src/sgml/syntax.sgml | 6 +- src/backend/parser/gram.y | 10 ++ src/backend/parser/parse_expr.c | 23 ++- src/backend/utils/adt/ruleutils.c | 29 ++++ src/include/catalog/catversion.h | 2 +- src/include/nodes/primnodes.h | 6 + .../regress/expected/operator_qualify.out | 139 ++++++++++++++++++ src/test/regress/sql/operator_qualify.sql | 75 ++++++++++ 9 files changed, 304 insertions(+), 8 deletions(-) diff --git a/doc/src/sgml/func/func-conditional.sgml b/doc/src/sgml/func/func-conditional.sgml index 137d5c7e43..6729788e3a 100644 --- a/doc/src/sgml/func/func-conditional.sgml +++ b/doc/src/sgml/func/func-conditional.sgml @@ -99,7 +99,7 @@ SELECT a, <synopsis> CASE <replaceable>expression</replaceable> - WHEN <replaceable>value</replaceable> THEN <replaceable>result</replaceable> + WHEN <replaceable>value</replaceable> <optional> USING OPERATOR(<replaceable>operator</replaceable>) </optional> THEN <replaceable>result</replaceable> <optional>WHEN ...</optional> <optional>ELSE <replaceable>result</replaceable></optional> END @@ -114,6 +114,26 @@ END to the <function>switch</function> statement in C. </para> + <para> + Each comparison is performed exactly as if you had + written <literal><replaceable>expression</replaceable> + = <replaceable>value</replaceable></literal>, so a + suitable <literal>=</literal> operator must be available, and it is + resolved by name through the current search path independently for each + <token>WHEN</token> clause. To pin a specific operator for a + given clause instead — for example one belonging to an extension whose + schema is not on the search path — give its name with the + optional <literal>USING OPERATOR()</literal> clause (see + <xref linkend="sql-expressions-operator-calls"/> for + the <literal>OPERATOR()</literal> notation). When a view or rule that uses + the simple <token>CASE</token> form is dumped, + <application>pg_dump</application> emits this clause automatically for any + clause whose bare name <literal>=</literal> would not resolve to the same + operator under the search path used at restore time, or whose intended + operator is not named <literal>=</literal> at all, so that the definition + reloads unambiguously. + </para> + <para> The example above can be written using the simple <token>CASE</token> syntax: diff --git a/doc/src/sgml/syntax.sgml b/doc/src/sgml/syntax.sgml index 5165533813..f06b98e728 100644 --- a/doc/src/sgml/syntax.sgml +++ b/doc/src/sgml/syntax.sgml @@ -1141,7 +1141,11 @@ SELECT 3 OPERATOR(pg_catalog.+) 4; of operators inside <literal>OPERATOR()</literal>, one for each column of the compared rows (see <xref linkend="row-wise-comparison"/>); this includes row-constructor comparisons against a subquery - (see <xref linkend="functions-subquery"/>). + (see <xref linkend="functions-subquery"/>). The simple + <token>CASE</token> form accepts the decoration on each + <token>WHEN</token> clause with <literal>USING OPERATOR()</literal>, + pinning the comparison operator for that arm + (see <xref linkend="functions-case"/>). </para> <note> diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 30bd69c01f..f28adc2b55 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -17900,6 +17900,16 @@ when_clause: w->location = @1; $$ = (Node *) w; } + | WHEN a_expr USING OPERATOR '(' any_operator ')' THEN a_expr + { + CaseWhen *w = makeNode(CaseWhen); + + w->opname = $6; + w->expr = (Expr *) $2; + w->result = (Expr *) $9; + w->location = @1; + $$ = (Node *) w; + } ; case_default: diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c index a8630e6c34..09d91c4378 100644 --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -1766,12 +1766,25 @@ transformCaseExpr(ParseState *pstate, CaseExpr *c) warg = (Node *) w->expr; if (placeholder) { - /* shorthand form was specified, so expand... */ - warg = (Node *) makeSimpleA_Expr(AEXPR_OP, "=", - (Node *) placeholder, - warg, - w->location); + /* + * Shorthand form was specified, so expand it into an equality + * comparison between the CASE test expression and the WHEN value. + * The comparison operator defaults to "=", but the user may pin a + * specific (possibly schema-qualified) operator with the optional + * USING OPERATOR() clause. + */ + warg = (Node *) makeA_Expr(AEXPR_OP, + w->opname ? w->opname : + list_make1(makeString("=")), + (Node *) placeholder, warg, + w->location); } + else if (w->opname != NIL) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("CASE/WHEN ... USING OPERATOR requires a CASE test expression"), + parser_errposition(pstate, w->location))); + neww->opname = NIL; /* raw-only; keep analyzed trees clean */ neww->expr = (Expr *) transformExprRecurse(pstate, warg); neww->expr = (Expr *) coerce_to_boolean(pstate, diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 487ee4b6e0..aea9628106 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -10433,6 +10433,7 @@ get_rule_expr(Node *node, deparse_context *context, { CaseWhen *when = (CaseWhen *) lfirst(temp); Node *w = (Node *) when->expr; + OpExpr *usingop = NULL; if (caseexpr->arg) { @@ -10455,7 +10456,10 @@ get_rule_expr(Node *node, deparse_context *context, if (list_length(args) == 2 && IsA(strip_implicit_coercions(linitial(args)), CaseTestExpr)) + { + usingop = (OpExpr *) w; w = (Node *) lsecond(args); + } } } @@ -10464,6 +10468,31 @@ get_rule_expr(Node *node, deparse_context *context, appendContextKeyword(context, "WHEN ", 0, 0, 0); get_rule_expr(w, context, false); + + /* + * If we recognized the implicit-equality WHEN form, the + * comparison operator was resolved from the bare name "=" + * against the CASE test expression and the WHEN value. A + * bare "WHEN value" reparses by resolving "=" again, so + * we must emit an explicit USING OPERATOR() clause + * whenever that would not recover the same operator: i.e. + * the operator is not reachable by an unqualified lookup + * of its own name (then it appears schema-qualified), or + * it is named something other than "=". + */ + if (usingop) + { + bool needs_qual; + char *opname; + + opname = generate_operator_name_extended(usingop->opno, + exprType(linitial(usingop->args)), + exprType(lsecond(usingop->args)), + &needs_qual); + if (needs_qual || strcmp(opname, "=") != 0) + appendStringInfo(buf, " USING OPERATOR(%s)", opname); + } + appendStringInfoString(buf, " THEN "); get_rule_expr((Node *) when->result, context, true); } diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index 7571c5cd08..79291aaaf5 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -57,6 +57,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 202607061 +#define CATALOG_VERSION_NO 202607071 #endif diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h index bf343cbec0..00f37c2295 100644 --- a/src/include/nodes/primnodes.h +++ b/src/include/nodes/primnodes.h @@ -1340,6 +1340,12 @@ typedef struct CaseExpr typedef struct CaseWhen { Expr xpr; + + /* + * possibly-qualified operator name given with USING OPERATOR(), or NIL; + * consumed by parse analysis, always NIL in analyzed trees + */ + List *opname pg_node_attr(query_jumble_ignore); Expr *expr; /* condition expression */ Expr *result; /* substitution result */ ParseLoc location; /* token location, or -1 if unknown */ diff --git a/src/test/regress/expected/operator_qualify.out b/src/test/regress/expected/operator_qualify.out index c0887e6311..f13496ef78 100644 --- a/src/test/regress/expected/operator_qualify.out +++ b/src/test/regress/expected/operator_qualify.out @@ -752,3 +752,142 @@ DROP VIEW oq_v_any_reload, oq_v_in_reload, oq_v_ne_all_reload, oq_v_in_mixed_reload, oq_v_any_mixed_reload; -- NOTE: the new oq_v_* base views are intentionally kept (like the others) -- so the pg_upgrade suite dump/restores them and exercises the deparse. +-- +-- Simple CASE: each "CASE x WHEN y" arm resolves its own "=" operator by +-- unqualified name, independently per WHEN clause. The comparison operator +-- can be pinned per arm with USING OPERATOR(); the deparse decorates only the +-- arms whose bare "=" would not recover the same operator at restore time. +-- +-- Built while alt_ops shadows pg_catalog: the implicit "=" resolves alt_ops.=. +SET search_path = alt_ops, pg_catalog; +CREATE VIEW public.oq_v_case AS + SELECT CASE a WHEN b THEN 'eq' ELSE 'ne' END AS r FROM public.oq_t; +RESET search_path; +-- Off the path, the arm's operator must be qualified: the bare form would +-- reparse "=" (pg_catalog.=), a different operator. +SELECT pg_get_viewdef('oq_v_case'::regclass, true); + pg_get_viewdef +-------------------------------------------------------------- + SELECT + + CASE a + + WHEN b USING OPERATOR(alt_ops.=) THEN 'eq'::text+ + ELSE 'ne'::text + + END AS r + + FROM oq_t; +(1 row) + +-- With alt_ops reachable again, the plain simple-CASE syntax comes back. +SET search_path = alt_ops, pg_catalog, public; +SELECT pg_get_viewdef('oq_v_case'::regclass, true); + pg_get_viewdef +------------------------------------ + SELECT + + CASE a + + WHEN b THEN 'eq'::text+ + ELSE 'ne'::text + + END AS r + + FROM oq_t; +(1 row) + +RESET search_path; +-- A bare-resolvable operator whose name is not "=" must still be decorated: +-- reparsing "WHEN b" would resolve "=", not this operator. pg_catalog.< is on +-- the default search_path, so its name prints unqualified, but it is wrapped in +-- OPERATOR() so the reparsed semantics stay identical. +CREATE VIEW public.oq_v_case_lt AS + SELECT CASE a WHEN b USING OPERATOR(<) THEN 'lt' ELSE 'ge' END AS r + FROM public.oq_t; +SELECT pg_get_viewdef('oq_v_case_lt'::regclass, true); + pg_get_viewdef +------------------------------------------------------ + SELECT + + CASE a + + WHEN b USING OPERATOR(<) THEN 'lt'::text+ + ELSE 'ge'::text + + END AS r + + FROM oq_t; +(1 row) + +-- Per-arm operators may differ within one CASE. Built under the alt_ops path: +-- the first arm's implicit "=" resolves alt_ops.= (off the default path -> must +-- be qualified), while the second arm pins pg_catalog.= explicitly (reachable +-- bare -> stays undecorated). The deparse decorates only the first arm. +SET search_path = alt_ops, pg_catalog; +CREATE VIEW public.oq_v_case_mixed AS + SELECT CASE a WHEN b THEN 'x' + WHEN id USING OPERATOR(pg_catalog.=) THEN 'y' + ELSE 'z' END AS r + FROM public.oq_t; +RESET search_path; +SELECT pg_get_viewdef('oq_v_case_mixed'::regclass, true); + pg_get_viewdef +------------------------------------------------------------- + SELECT + + CASE a + + WHEN b USING OPERATOR(alt_ops.=) THEN 'x'::text+ + WHEN id THEN 'y'::text + + ELSE 'z'::text + + END AS r + + FROM oq_t; +(1 row) + +-- The decorated syntax is directly acceptable, and it pins the given operator. +SELECT CASE 1 WHEN 2 USING OPERATOR(alt_ops.=) THEN 'x' ELSE 'y' END AS y; + y +--- + y +(1 row) + +SELECT CASE 1 WHEN 1 USING OPERATOR(pg_catalog.=) THEN 'x' ELSE 'y' END AS x; + x +--- + x +(1 row) + +-- an unqualified name inside the decoration is fine too +SELECT CASE 1 WHEN 1 USING OPERATOR(=) THEN 'x' ELSE 'y' END AS x; + x +--- + x +(1 row) + +-- USING OPERATOR() only makes sense for the simple form; the searched CASE +-- (no CASE test expression) rejects it, pointing at the offending WHEN. +SELECT CASE WHEN 1=1 USING OPERATOR(=) THEN 1 END; +ERROR: CASE/WHEN ... USING OPERATOR requires a CASE test expression +LINE 1: SELECT CASE WHEN 1=1 USING OPERATOR(=) THEN 1 END; + ^ +-- The deparsed simple-CASE definitions must reparse under a restricted +-- search_path (the pg_dump scenario), pinning the same operator OIDs. +BEGIN; SET LOCAL search_path = pg_catalog; +DO $$ BEGIN + EXECUTE 'CREATE VIEW public.oq_v_case_reload AS ' || pg_get_viewdef('public.oq_v_case'::regclass); + EXECUTE 'CREATE VIEW public.oq_v_case_lt_reload AS ' || pg_get_viewdef('public.oq_v_case_lt'::regclass); + EXECUTE 'CREATE VIEW public.oq_v_case_mixed_reload AS ' || pg_get_viewdef('public.oq_v_case_mixed'::regclass); +END $$; COMMIT; +-- The reloaded views pin alt_ops.= for the arms that were decorated: the whole +-- CASE view (arm 1) and the mixed view (arm 1). The simple CASE expands each +-- arm into an OpExpr, serialized with ":opno". +SELECT ev_class::regclass FROM pg_rewrite +WHERE ev_action LIKE '%:opno ' || 'alt_ops.=(int4,int4)'::regoperator::oid || ' %' + AND ev_class::regclass::text LIKE 'oq_v_case%reload' +ORDER BY 1; + ev_class +------------------------ + oq_v_case_reload + oq_v_case_mixed_reload +(2 rows) + +-- The non-"=" operator OPERATOR(<) pins int4lt across the reload. +SELECT ev_class::regclass FROM pg_rewrite +WHERE ev_action LIKE '%:opno ' || 'pg_catalog.<(int4,int4)'::regoperator::oid || ' %' + AND ev_class::regclass::text LIKE 'oq_v_case%lt_reload' +ORDER BY 1; + ev_class +--------------------- + oq_v_case_lt_reload +(1 row) + +DROP VIEW oq_v_case_reload, oq_v_case_lt_reload, oq_v_case_mixed_reload; +-- NOTE: the oq_v_case* base views are intentionally kept (like the others) so +-- the pg_upgrade suite dump/restores them and exercises the deparse. diff --git a/src/test/regress/sql/operator_qualify.sql b/src/test/regress/sql/operator_qualify.sql index f029d0eb73..a2b35e3b0b 100644 --- a/src/test/regress/sql/operator_qualify.sql +++ b/src/test/regress/sql/operator_qualify.sql @@ -424,3 +424,78 @@ DROP VIEW oq_v_any_reload, oq_v_in_reload, oq_v_ne_all_reload, oq_v_in_mixed_reload, oq_v_any_mixed_reload; -- NOTE: the new oq_v_* base views are intentionally kept (like the others) -- so the pg_upgrade suite dump/restores them and exercises the deparse. + +-- +-- Simple CASE: each "CASE x WHEN y" arm resolves its own "=" operator by +-- unqualified name, independently per WHEN clause. The comparison operator +-- can be pinned per arm with USING OPERATOR(); the deparse decorates only the +-- arms whose bare "=" would not recover the same operator at restore time. +-- +-- Built while alt_ops shadows pg_catalog: the implicit "=" resolves alt_ops.=. +SET search_path = alt_ops, pg_catalog; +CREATE VIEW public.oq_v_case AS + SELECT CASE a WHEN b THEN 'eq' ELSE 'ne' END AS r FROM public.oq_t; +RESET search_path; +-- Off the path, the arm's operator must be qualified: the bare form would +-- reparse "=" (pg_catalog.=), a different operator. +SELECT pg_get_viewdef('oq_v_case'::regclass, true); +-- With alt_ops reachable again, the plain simple-CASE syntax comes back. +SET search_path = alt_ops, pg_catalog, public; +SELECT pg_get_viewdef('oq_v_case'::regclass, true); +RESET search_path; + +-- A bare-resolvable operator whose name is not "=" must still be decorated: +-- reparsing "WHEN b" would resolve "=", not this operator. pg_catalog.< is on +-- the default search_path, so its name prints unqualified, but it is wrapped in +-- OPERATOR() so the reparsed semantics stay identical. +CREATE VIEW public.oq_v_case_lt AS + SELECT CASE a WHEN b USING OPERATOR(<) THEN 'lt' ELSE 'ge' END AS r + FROM public.oq_t; +SELECT pg_get_viewdef('oq_v_case_lt'::regclass, true); + +-- Per-arm operators may differ within one CASE. Built under the alt_ops path: +-- the first arm's implicit "=" resolves alt_ops.= (off the default path -> must +-- be qualified), while the second arm pins pg_catalog.= explicitly (reachable +-- bare -> stays undecorated). The deparse decorates only the first arm. +SET search_path = alt_ops, pg_catalog; +CREATE VIEW public.oq_v_case_mixed AS + SELECT CASE a WHEN b THEN 'x' + WHEN id USING OPERATOR(pg_catalog.=) THEN 'y' + ELSE 'z' END AS r + FROM public.oq_t; +RESET search_path; +SELECT pg_get_viewdef('oq_v_case_mixed'::regclass, true); + +-- The decorated syntax is directly acceptable, and it pins the given operator. +SELECT CASE 1 WHEN 2 USING OPERATOR(alt_ops.=) THEN 'x' ELSE 'y' END AS y; +SELECT CASE 1 WHEN 1 USING OPERATOR(pg_catalog.=) THEN 'x' ELSE 'y' END AS x; +-- an unqualified name inside the decoration is fine too +SELECT CASE 1 WHEN 1 USING OPERATOR(=) THEN 'x' ELSE 'y' END AS x; + +-- USING OPERATOR() only makes sense for the simple form; the searched CASE +-- (no CASE test expression) rejects it, pointing at the offending WHEN. +SELECT CASE WHEN 1=1 USING OPERATOR(=) THEN 1 END; + +-- The deparsed simple-CASE definitions must reparse under a restricted +-- search_path (the pg_dump scenario), pinning the same operator OIDs. +BEGIN; SET LOCAL search_path = pg_catalog; +DO $$ BEGIN + EXECUTE 'CREATE VIEW public.oq_v_case_reload AS ' || pg_get_viewdef('public.oq_v_case'::regclass); + EXECUTE 'CREATE VIEW public.oq_v_case_lt_reload AS ' || pg_get_viewdef('public.oq_v_case_lt'::regclass); + EXECUTE 'CREATE VIEW public.oq_v_case_mixed_reload AS ' || pg_get_viewdef('public.oq_v_case_mixed'::regclass); +END $$; COMMIT; +-- The reloaded views pin alt_ops.= for the arms that were decorated: the whole +-- CASE view (arm 1) and the mixed view (arm 1). The simple CASE expands each +-- arm into an OpExpr, serialized with ":opno". +SELECT ev_class::regclass FROM pg_rewrite +WHERE ev_action LIKE '%:opno ' || 'alt_ops.=(int4,int4)'::regoperator::oid || ' %' + AND ev_class::regclass::text LIKE 'oq_v_case%reload' +ORDER BY 1; +-- The non-"=" operator OPERATOR(<) pins int4lt across the reload. +SELECT ev_class::regclass FROM pg_rewrite +WHERE ev_action LIKE '%:opno ' || 'pg_catalog.<(int4,int4)'::regoperator::oid || ' %' + AND ev_class::regclass::text LIKE 'oq_v_case%lt_reload' +ORDER BY 1; +DROP VIEW oq_v_case_reload, oq_v_case_lt_reload, oq_v_case_mixed_reload; +-- NOTE: the oq_v_case* base views are intentionally kept (like the others) so +-- the pg_upgrade suite dump/restores them and exercises the deparse. -- 2.54.0 [application/octet-stream] v2-0000-cover-letter.patch (3.9K, ../../66fa3fe6-8c99-4120-935c-f32f3b61fc30@Spark/8-v2-0000-cover-letter.patch) download | inline diff: From b0cdce84f7825651a6bda1a371bcc2458311545f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= <[email protected]> Date: Tue, 7 Jul 2026 15:09:36 +0200 Subject: [PATCH v2 0/5] Schema-qualifiable operator syntax for implicit-operator constructs (was: Schema-qualify the equality operator when deparsing NULLIF/IS DISTINCT FROM) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Several SQL constructs resolve an operator by unqualified name at parse time and provide no place in their syntax to write a schema-qualified operator name: IS [NOT] DISTINCT FROM, NULLIF, simple CASE, row comparisons, and ANY/ALL subquery comparisons -- the classes Tom cataloged in [email protected] -- plus JOIN USING / NATURAL JOIN. When such a construct ends up bound to an operator that is not reachable through the search_path in effect at reload time (typically pg_dump's restricted search_path), the dumped definition either fails to restore or, worse, silently reparses with a different operator. This series gives each construct an optional OPERATOR() decoration, one construct per patch. The parser always accepts the decoration; ruleutils.c emits it only when reparsing the undecorated form would not resolve the very same operator OID(s). Post-analysis expression trees and executor nodes are untouched. 0001: a IS [NOT] DISTINCT OPERATOR(s.=) FROM b and NULLIF(a, b USING OPERATOR(s.=)) 0002: JOIN ... USING (a, b) OPERATOR (s.=, s.=), one operator per USING column; NATURAL JOIN deparses via the same path 0003: ROW(a, b) OPERATOR(s.<, s.<) ROW(c, d), generalizing the existing single-name decoration to a per-column list 0004: (a, b) OPERATOR(op1, op2) ANY (SELECT ...), likewise for ANY/ALL and row-op-subquery sublinks; fixes today's silent operator swap when a multi-column IN is dumped and reloaded 0005: CASE x WHEN y USING OPERATOR(s.=) THEN ..., per WHEN arm The whole series weighs in at 22 files changed, 2366 insertions(+), 90 deletions(-), of which about 1530 lines are regression tests and expected output and about 165 lines are documentation. Michał Pasternak (5): Allow schema-qualifying the operator in IS DISTINCT FROM/NULLIF Allow schema-qualifying the operators of JOIN USING Allow per-column operator lists in row comparisons Allow per-column operator lists in ANY/ALL subquery comparisons Allow schema-qualifying the comparison operator of a simple CASE contrib/hstore/expected/hstore.out | 88 ++ contrib/hstore/sql/hstore.sql | 43 + doc/src/sgml/func/func-comparison.sgml | 23 +- doc/src/sgml/func/func-comparisons.sgml | 19 + doc/src/sgml/func/func-conditional.sgml | 39 +- doc/src/sgml/func/func-subquery.sgml | 40 + doc/src/sgml/queries.sgml | 12 + doc/src/sgml/ref/select.sgml | 18 +- doc/src/sgml/syntax.sgml | 20 + src/backend/commands/define.c | 5 + src/backend/parser/gram.y | 103 +- src/backend/parser/parse_clause.c | 55 +- src/backend/parser/parse_expr.c | 134 ++- src/backend/parser/parse_oper.c | 22 + src/backend/utils/adt/ruleutils.c | 380 +++++++- src/include/catalog/catversion.h | 2 +- src/include/nodes/parsenodes.h | 9 +- src/include/nodes/primnodes.h | 25 +- src/include/parser/parse_oper.h | 20 + .../regress/expected/operator_qualify.out | 893 ++++++++++++++++++ src/test/regress/parallel_schedule | 5 +- src/test/regress/sql/operator_qualify.sql | 501 ++++++++++ 22 files changed, 2366 insertions(+), 90 deletions(-) create mode 100644 src/test/regress/expected/operator_qualify.out create mode 100644 src/test/regress/sql/operator_qualify.sql -- 2.54.0 ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Schema-qualify the equality operator when deparsing NULLIF/IS DISTINCT FROM @ 2026-07-07 14:27 Tom Lane <[email protected]> parent: [email protected] 0 siblings, 0 replies; 4+ messages in thread From: Tom Lane @ 2026-07-07 14:27 UTC (permalink / raw) To: [email protected]; +Cc: [email protected] [email protected] writes: > Attached is v2, which takes the opposite approach: instead of teaching > the deparser to avoid the constructs, it gives every construct that > resolves an operator by unqualified name a place to write a > schema-qualified one. That covers the full list you cataloged back in > 2018 ([email protected]) -- IS [NOT] DISTINCT FROM, > NULLIF, simple CASE, row comparisons, ANY/ALL subquery comparisons -- > plus JOIN USING / NATURAL JOIN, which has the same disease. I didn't read the patch yet, but this sounds promising. > If the direction looks right I will register this in the next > commitfest (PG 20-1). Next one is already 20-2, but please do register it. regards, tom lane ^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2026-07-07 14:27 UTC | newest] Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2026-07-03 12:56 Schema-qualify the equality operator when deparsing NULLIF/IS DISTINCT FROM [email protected] 2026-07-03 14:11 ` Tom Lane <[email protected]> 2026-07-07 14:01 ` [email protected] 2026-07-07 14:27 ` Tom Lane <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox