From 9a3b54bc95c2ec70032307dd1db64ecfb85ec488 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= 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 (10492.1531515255@sss.pgh.pa.us). 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/3856834.1783087886@sss.pgh.pa.us --- 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. + + Ordinarily a single operator 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 OPERATOR() syntax (see + ), giving one operator for + each column of the rows: + +row_constructor OPERATOR(operator , operator ... ) row_constructor + + 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; + PostgreSQL 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. + + The = and <> 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: IS NOT DISTINCT FROM (see ) and NULLIF - (see ). + (see ). A row-constructor comparison + goes a step further and accepts a comma-separated list + of operators inside OPERATOR(), one for each column of + the compared rows (see ). 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