From 27b7dd3d253fe9959c1edf6df51f8a4f319ad2a2 Mon Sep 17 00:00:00 2001 From: Henson Choi Date: Mon, 28 Sep 2026 15:53:21 +0900 Subject: [PATCH 06/10] Tighten RPRPattern node I/O and reuse existing helpers in RPR code This commit makes the copy, out and read functions of RPRPattern agree with each other, and replaces hand-written code in the RPR paths with existing functions. No query behaves differently. 1. RPRPattern node I/O RPRPattern needs hand-written copy, out and read functions because of its two arrays, and the three had drifted apart from each other and from plannodes.h. None of the disagreements is reachable from a tree the system builds, but each was kept safe only by a fact outside these functions, so settle one contract: - RPRPatternElement has eight fields. _copyRPRPattern() memcpy()s all eight; out and read carry seven and leave "reserved" zeroed. The header now says that reserved is padding the round trip drops, and that a field taking the byte over must join the seven first. - varNames went out through outToken(), which spells NULL as <>, and came back through debackslash(), which does not know that spelling. Read it with nullable_string() instead. Out also accepted numVars > 0 with varNames == NULL and wrote <> for it, read restored that state, and copy would then dereference the NULL array. makeRPRPattern() always fills the array, so copy and out now assert it and read rejects anything else. - read stepped over the list delimiters unchecked and opened the element array with an Assert, so a numVars or numElements that disagreed with the text left the token stream off by one, silently in a build without assertions. Every delimiter the format has is now checked and a mismatch there raises an internal error. The element array has no closing delimiter, so a numElements smaller than the number of elements written is still not caught here. - flags is written with %u but was read with atoi(); read it with atoui() into an unsigned int. 2. Existing functions in place of hand-written code - rpr.c: rprPatternEqual() and rprPatternChildrenEqual() compared the fields equal() compares for RPRPatternNode; use equal(). equal() also looks at trailing_alt, which is always false once parsing is done. - parse_func.c, ruleutils.c: the parser and the deparser each listed the four navigation names PREV, NEXT, FIRST and LAST. They must agree, or a user function deparsed unqualified would re-parse as navigation, so both now call a new is_rpr_navigation_name(). - setrefs.c, costsize.c: pass the whole defineClause list to fix_upper_expr() and cost_qual_eval_node() rather than one TargetEntry at a time. The resulting expressions and cost are the same. - parse_rpr.c: transformExpr() followed by coerce_to_boolean() is transformWhereClause(). - ruleutils.c: the quantifier deparse tested PG_INT32_MAX where the rest of the code says RPR_QUANTITY_INF, which has the same value. - nodeFuncs.c: drop the NULL tests ahead of WALK() on the two RPRNavExpr offset arguments; walkers return early on NULL. 3. Other cleanups - Drop four #include lines added without need: nodes/plannodes.h in copyfuncs.c, outfuncs.c and readfuncs.c, which the generated *funcs.funcs.c files already include ahead of the RPRPattern functions, and optimizer/rpr.h in costsize.c, which uses nothing it declares. Those four files' include lists now match upstream. parse_rpr.c includes parser/parse_clause.h in place of parse_coerce.h and parse_expr.h, and no longer includes optimizer/optimizer.h, which nothing in it uses. - Correct the comment on the duplicate-clause check in optimize_window_clauses(). It called the RPR field comparisons a defensive backstop, since an RPR clause's frame options would differ anyway. That holds only for the built-in support functions; another one may give a non-RPR clause the frame options an RPR clause uses, and then the RPR fields are what keep the two apart. - In createplan.c, make_windowagg() lists runCondition and compiledPattern on one line, as it does for its other parameters. The two static RPRNavExpr_walker() functions, in createplan.c and nodeWindowAgg.c, do unrelated jobs under one name; they are now define_metadata_walker() and nav_offsets_walker(). Error messages, deparse output, plan shape and costs are unchanged. The only difference is that a malformed RPRPattern node string of the kinds above now fails with an internal error instead of being misread. Author: Henson Choi --- src/backend/executor/nodeWindowAgg.c | 8 +-- src/backend/nodes/copyfuncs.c | 9 ++-- src/backend/nodes/nodeFuncs.c | 4 +- src/backend/nodes/outfuncs.c | 31 ++++++----- src/backend/nodes/readfuncs.c | 50 +++++++++++------- src/backend/optimizer/path/costsize.c | 10 ++-- src/backend/optimizer/plan/createplan.c | 9 ++-- src/backend/optimizer/plan/planner.c | 8 +-- src/backend/optimizer/plan/rpr.c | 70 ++----------------------- src/backend/optimizer/plan/setrefs.c | 25 +++------ src/backend/parser/parse_func.c | 25 ++++++--- src/backend/parser/parse_rpr.c | 9 ++-- src/backend/utils/adt/ruleutils.c | 11 ++-- src/include/nodes/plannodes.h | 6 +++ src/include/parser/parse_func.h | 2 + 15 files changed, 117 insertions(+), 160 deletions(-) diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c index 615bff94029..2dd1f7c26af 100644 --- a/src/backend/executor/nodeWindowAgg.c +++ b/src/backend/executor/nodeWindowAgg.c @@ -264,7 +264,7 @@ static bool rpr_prepare_row(WindowObject winobj, int64 pos, RPRVarMatch *varMatc static void build_define_offsets(WindowAggState *winstate, List *defineClause); static void resolve_nav_offsets(WindowAggState *winstate); static void resolve_one_nav(RPRNavOffsets *entry, EvalDefineOffsetsContext *context); -static bool RPRNavExpr_walker(Node *node, WindowAggState *winstate); +static bool nav_offsets_walker(Node *node, WindowAggState *winstate); static void build_nav_offsets(RPRNavExpr *nav, WindowAggState *winstate); /* @@ -4158,14 +4158,14 @@ build_nav_offsets(RPRNavExpr *nav, WindowAggState *winstate) } static bool -RPRNavExpr_walker(Node *node, WindowAggState *winstate) +nav_offsets_walker(Node *node, WindowAggState *winstate) { if (node == NULL) return false; if (IsA(node, RPRNavExpr)) build_nav_offsets(castNode(RPRNavExpr, node), winstate); - return expression_tree_walker(node, RPRNavExpr_walker, winstate); + return expression_tree_walker(node, nav_offsets_walker, winstate); } /* @@ -4191,7 +4191,7 @@ build_define_offsets(WindowAggState *winstate, List *defineClause) foreach_node(TargetEntry, te, defineClause) { - RPRNavExpr_walker((Node *) te->expr, winstate); + nav_offsets_walker((Node *) te->expr, winstate); } /* diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 17d45930d7b..abc6d693f64 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -16,7 +16,6 @@ #include "postgres.h" #include "miscadmin.h" -#include "nodes/plannodes.h" #include "utils/datum.h" @@ -177,12 +176,16 @@ _copyRPRPattern(const RPRPattern *from) COPY_SCALAR_FIELD(numElements); /* Deep copy the varNames array (DEFINE clause is required) */ - Assert(from->numVars > 0); + Assert(from->numVars > 0 && from->varNames != NULL); newnode->varNames = palloc0_array(char *, from->numVars); for (int i = 0; i < from->numVars; i++) newnode->varNames[i] = pstrdup(from->varNames[i]); - /* Deep copy the elements array (always has at least one element + FIN) */ + /* + * Deep copy the elements array (always has at least one element + FIN). + * This carries the whole struct, reserved byte included, where out/read + * carry seven fields and zero that byte -- see RPRPatternElement. + */ Assert(from->numElements >= 2); newnode->elements = palloc_array(RPRPatternElement, from->numElements); memcpy(newnode->elements, from->elements, diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c index aab609f406e..350eef906f7 100644 --- a/src/backend/nodes/nodeFuncs.c +++ b/src/backend/nodes/nodeFuncs.c @@ -2213,9 +2213,9 @@ expression_tree_walker_impl(Node *node, if (WALK(expr->arg)) return true; - if (expr->offset_arg && WALK(expr->offset_arg)) + if (WALK(expr->offset_arg)) return true; - if (expr->compound_offset_arg && WALK(expr->compound_offset_arg)) + if (WALK(expr->compound_offset_arg)) return true; } break; diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index 3756f65185d..0a99e602942 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -23,7 +23,6 @@ #include "nodes/bitmapset.h" #include "nodes/nodes.h" #include "nodes/pg_list.h" -#include "nodes/plannodes.h" #include "utils/datum.h" /* State flag that determines how nodeToStringInternal() should treat location fields */ @@ -728,23 +727,27 @@ _outRPRPattern(StringInfo str, const RPRPattern *node) WRITE_INT_FIELD(maxDepth); WRITE_INT_FIELD(numElements); - /* Write varNames array as list of strings */ + /* + * Write varNames array as list of strings. makeRPRPattern() guarantees + * the array, so the list has exactly one spelling and the read side has + * no second shape to interpret. + */ appendStringInfoString(str, " :varNames"); - if (node->numVars > 0 && node->varNames != NULL) + Assert(node->numVars > 0 && node->varNames != NULL); + appendStringInfoString(str, " ("); + for (int i = 0; i < node->numVars; i++) { - appendStringInfoString(str, " ("); - for (int i = 0; i < node->numVars; i++) - { - if (i > 0) - appendStringInfoChar(str, ' '); - outToken(str, node->varNames[i]); - } - appendStringInfoChar(str, ')'); + if (i > 0) + appendStringInfoChar(str, ' '); + outToken(str, node->varNames[i]); } - else - appendStringInfoString(str, " <>"); + appendStringInfoChar(str, ')'); - /* Write elements array (makeRPRPattern guarantees numElements >= 2) */ + /* + * Write elements array (makeRPRPattern guarantees numElements >= 2). + * Seven fields go out; the reserved byte is padding and stays behind -- + * see RPRPatternElement in plannodes.h. + */ appendStringInfoString(str, " :elements"); Assert(node->numElements > 0 && node->elements != NULL); appendStringInfoChar(str, ' '); diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c index cbf5deab80b..b8f905aaa91 100644 --- a/src/backend/nodes/readfuncs.c +++ b/src/backend/nodes/readfuncs.c @@ -28,7 +28,6 @@ #include "miscadmin.h" #include "nodes/bitmapset.h" -#include "nodes/plannodes.h" #include "nodes/readfuncs.h" @@ -568,40 +567,47 @@ _readRPRPattern(ReadNodeContext *ctx) READ_INT_FIELD(maxDepth); READ_INT_FIELD(numElements); - /* Read varNames array */ + /* + * Read varNames array. _outRPRPattern() always writes the list, so every + * token here has one spelling and any other input is malformed. The + * delimiters are checked rather than counted on, because a numVars that + * disagrees with the list would otherwise leave the token stream off by + * one for everything that follows. + */ token = pg_strtok(ctx, &length); /* skip :varNames */ - token = pg_strtok(ctx, &length); /* get '(' or '<>' */ - if (local_node->numVars > 0 && token[0] == '(') - { - local_node->varNames = palloc_array(char *, local_node->numVars); - for (int i = 0; i < local_node->numVars; i++) - { - token = pg_strtok(ctx, &length); - local_node->varNames[i] = debackslash(token, length); - } - token = pg_strtok(ctx, &length); /* skip ')' */ - } - else + token = pg_strtok(ctx, &length); /* get '(' */ + if (local_node->numVars <= 0 || token == NULL || token[0] != '(') + elog(ERROR, "unexpected varNames in RPRPattern"); + local_node->varNames = palloc_array(char *, local_node->numVars); + for (int i = 0; i < local_node->numVars; i++) { - local_node->varNames = NULL; + token = pg_strtok(ctx, &length); + if (token == NULL) + elog(ERROR, "unexpected end of RPRPattern varNames"); + local_node->varNames[i] = nullable_string(token, length); } + token = pg_strtok(ctx, &length); /* get ')' */ + if (token == NULL || token[0] != ')') + elog(ERROR, "unterminated varNames in RPRPattern"); /* Read elements array */ token = pg_strtok(ctx, &length); /* skip :elements */ token = pg_strtok(ctx, &length); /* get '(' */ /* out always emits the array (makeRPRPattern guarantees numElements >= 2) */ - Assert(local_node->numElements > 0 && token[0] == '('); + if (local_node->numElements <= 0 || token == NULL || token[0] != '(') + elog(ERROR, "unexpected elements in RPRPattern"); + /* palloc0 also zeroes reserved, which the round trip drops */ local_node->elements = palloc0_array(RPRPatternElement, local_node->numElements); for (int i = 0; i < local_node->numElements; i++) { RPRPatternElement *elem = &local_node->elements[i]; int varId, - flags, depth, min, max, next, jump; + unsigned int flags; /* written with %u, unlike the others */ /* Parse "(varId depth flags min max next jump)" */ token = pg_strtok(ctx, &length); @@ -609,7 +615,7 @@ _readRPRPattern(ReadNodeContext *ctx) token = pg_strtok(ctx, &length); depth = atoi(token); token = pg_strtok(ctx, &length); - flags = atoi(token); + flags = atoui(token); token = pg_strtok(ctx, &length); min = atoi(token); token = pg_strtok(ctx, &length); @@ -618,7 +624,9 @@ _readRPRPattern(ReadNodeContext *ctx) next = atoi(token); token = pg_strtok(ctx, &length); jump = atoi(token); - token = pg_strtok(ctx, &length); /* skip ')' */ + token = pg_strtok(ctx, &length); /* get ')' */ + if (token == NULL || token[0] != ')') + elog(ERROR, "unterminated element in RPRPattern"); elem->varId = (RPRVarId) varId; elem->flags = (RPRElemFlags) flags; @@ -630,7 +638,11 @@ _readRPRPattern(ReadNodeContext *ctx) /* Read next element's '(' or end */ if (i < local_node->numElements - 1) + { token = pg_strtok(ctx, &length); /* get '(' */ + if (token == NULL || token[0] != '(') + elog(ERROR, "unexpected end of RPRPattern elements"); + } } READ_BOOL_FIELD(isAbsorbable); diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c index c124415a459..9cc22f43337 100644 --- a/src/backend/optimizer/path/costsize.c +++ b/src/backend/optimizer/path/costsize.c @@ -104,7 +104,6 @@ #include "optimizer/placeholder.h" #include "optimizer/plancat.h" #include "optimizer/restrictinfo.h" -#include "optimizer/rpr.h" #include "parser/parsetree.h" #include "utils/lsyscache.h" #include "utils/selfuncs.h" @@ -3269,12 +3268,9 @@ cost_windowagg(Path *path, PlannerInfo *root, { QualCost defcosts; - foreach_node(TargetEntry, def, winclause->defineClause) - { - cost_qual_eval_node(&defcosts, (Node *) def->expr, root); - startup_cost += defcosts.startup; - total_cost += defcosts.per_tuple * input_tuples; - } + cost_qual_eval_node(&defcosts, (Node *) winclause->defineClause, root); + startup_cost += defcosts.startup; + total_cost += defcosts.per_tuple * input_tuples; } foreach(lc, windowFuncs) diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index 773b86dc14e..dad285eef96 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -2534,7 +2534,7 @@ compute_matchStartDependent(RPRNavExpr *nav, DefineMetadataContext *context) } static bool -RPRNavExpr_walker(Node *node, DefineMetadataContext *ctx) +define_metadata_walker(Node *node, DefineMetadataContext *ctx) { if (node == NULL) return false; @@ -2546,7 +2546,7 @@ RPRNavExpr_walker(Node *node, DefineMetadataContext *ctx) compute_matchStartDependent(nav, ctx); } - return expression_tree_walker(node, RPRNavExpr_walker, ctx); + return expression_tree_walker(node, define_metadata_walker, ctx); } /* @@ -2582,7 +2582,7 @@ compute_define_metadata(List *defineClause, Bitmapset **matchStartDependent) { ctx.curVarIdx = foreach_current_index(te); - RPRNavExpr_walker((Node *) te->expr, &ctx); + define_metadata_walker((Node *) te->expr, &ctx); } *matchStartDependent = ctx.matchStartDependent; @@ -6822,8 +6822,7 @@ static WindowAgg * make_windowagg(List *tlist, WindowClause *wc, int partNumCols, AttrNumber *partColIdx, Oid *partOperators, Oid *partCollations, int ordNumCols, AttrNumber *ordColIdx, Oid *ordOperators, Oid *ordCollations, - List *runCondition, - RPRPattern *compiledPattern, + List *runCondition, RPRPattern *compiledPattern, Bitmapset *defineMatchStartDependent, List *qual, bool topWindow, Plan *lefttree) { diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index f84815bf1a3..336cf109640 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -6281,10 +6281,10 @@ optimize_window_clauses(PlannerInfo *root, WindowFuncLists *wflists) /* * Perform the same duplicate check that is done in - * transformWindowFuncCall. wc is never an RPR clause here - * (those are skipped above), and an RPR existing_wc differs - * in its frame options anyway, so the RPR-related comparisons - * are a defensive backstop for parity. + * transformWindowFuncCall. wc is never an RPR clause here + * (those are skipped above), but a support function is free + * to hand it the frame options an RPR clause uses, so the RPR + * fields still have to be compared. */ if (equal(wc->partitionClause, existing_wc->partitionClause) && equal(wc->orderClause, existing_wc->orderClause) && diff --git a/src/backend/optimizer/plan/rpr.c b/src/backend/optimizer/plan/rpr.c index f88704e47ec..cc0ae0df6ed 100644 --- a/src/backend/optimizer/plan/rpr.c +++ b/src/backend/optimizer/plan/rpr.c @@ -45,8 +45,6 @@ #include "optimizer/rpr.h" /* Forward declarations */ -static bool rprPatternEqual(RPRPatternNode *a, RPRPatternNode *b); -static bool rprPatternChildrenEqual(List *a, List *b); static int64 rprNodeRowCount(RPRPatternNode *node); static int64 rprBodyRowCount(List *children); static bool rprBodyHasUniformLength(List *children); @@ -95,64 +93,6 @@ static void computeAbsorbabilityRecursive(RPRPattern *pattern, bool *hasAbsorbable); static void computeAbsorbability(RPRPattern *pattern); -/* - * rprPatternEqual - * Compare two RPRPatternNode trees for equality. - * - * Returns true if the trees are structurally identical. Neither argument - * may be NULL: a children list never holds one. - */ -static bool -rprPatternEqual(RPRPatternNode *a, RPRPatternNode *b) -{ - /* Must have same node type and quantifiers */ - if (a->nodeType != b->nodeType) - return false; - if (a->min != b->min || a->max != b->max) - return false; - if (a->reluctant != b->reluctant) - return false; - - switch (a->nodeType) - { - case RPR_PATTERN_VAR: - return strcmp(a->varName, b->varName) == 0; - - case RPR_PATTERN_SEQ: - case RPR_PATTERN_ALT: - case RPR_PATTERN_GROUP: - return rprPatternChildrenEqual(a->children, b->children); - } - - pg_unreachable(); - return false; -} - -/* - * rprPatternChildrenEqual - * Compare children lists of two pattern nodes for equality. - * - * Returns true if the children lists are structurally identical. - */ -static bool -rprPatternChildrenEqual(List *a, List *b) -{ - ListCell *lca, - *lcb; - - if (list_length(a) != list_length(b)) - return false; - - forboth(lca, a, lcb, b) - { - if (!rprPatternEqual((RPRPatternNode *) lfirst(lca), - (RPRPatternNode *) lfirst(lcb))) - return false; - } - - return true; -} - /* * rprNodeRowCount * Rows the node always consumes, or -1 if that varies. @@ -261,7 +201,7 @@ rprChildrenMatchAt(List *children, int start, List *content) RPRPatternNode *have; have = list_nth_node(RPRPatternNode, children, start + offset); - if (!rprPatternEqual(have, want)) + if (!equal(have, want)) return false; offset++; } @@ -484,7 +424,7 @@ mergeConsecutiveGroups(List *children) if (other->nodeType != RPR_PATTERN_GROUP || other->reluctant) break; - if (!rprPatternChildrenEqual(node->children, other->children)) + if (!equal(node->children, other->children)) break; /* The body must consume a fixed number of rows; see above */ @@ -555,7 +495,7 @@ mergeConsecutiveAlts(List *children) other = list_nth_node(RPRPatternNode, children, readpos + count); - if (!rprPatternEqual(node, other)) + if (!equal(node, other)) break; count++; @@ -801,8 +741,8 @@ removeDuplicateAlternatives(List *children) */ for (int keptpos = 0; keptpos < writepos; keptpos++) { - if (rprPatternEqual(list_nth_node(RPRPatternNode, children, keptpos), - node)) + if (equal(list_nth_node(RPRPatternNode, children, keptpos), + node)) { isDuplicate = true; break; diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c index 2ee3c8baac2..b1bd7628fe4 100644 --- a/src/backend/optimizer/plan/setrefs.c +++ b/src/backend/optimizer/plan/setrefs.c @@ -2609,26 +2609,15 @@ set_upper_references(PlannerInfo *root, Plan *plan, int rtoffset) */ if (IsA(plan, WindowAgg)) { - List *new_defineClause = NIL; WindowAgg *wplan = (WindowAgg *) plan; - foreach_node(TargetEntry, tle, wplan->defineClause) - { - TargetEntry *newtle; - - newtle = flatCopyTargetEntry(tle); - newtle->expr = (Expr *) - fix_upper_expr(root, - (Node *) tle->expr, - subplan_itlist, - OUTER_VAR, - rtoffset, - NUM_EXEC_QUAL(plan)); - - new_defineClause = lappend(new_defineClause, newtle); - } - - wplan->defineClause = new_defineClause; + wplan->defineClause = (List *) + fix_upper_expr(root, + (Node *) wplan->defineClause, + subplan_itlist, + OUTER_VAR, + rtoffset, + NUM_EXEC_QUAL(plan)); } pfree(subplan_itlist); diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c index 5295271fbce..b95f0ff04be 100644 --- a/src/backend/parser/parse_func.c +++ b/src/backend/parser/parse_func.c @@ -234,12 +234,7 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs, pstate->p_rpr_define && list_length(funcname) == 1) { - const char *name = strVal(linitial(funcname)); - - if (strcmp(name, "prev") == 0 || - strcmp(name, "next") == 0 || - strcmp(name, "first") == 0 || - strcmp(name, "last") == 0) + if (is_rpr_navigation_name(strVal(linitial(funcname)))) could_be_rpr_nav = true; } @@ -2139,6 +2134,24 @@ FuncNameAsType(List *funcname) return result; } +/* + * is_rpr_navigation_name + * Is this unqualified, parser-downcased name a row pattern navigation + * operation inside a DEFINE clause? + * + * ruleutils.c asks the same question to decide when a user function of one + * of these names has to be printed schema-qualified, so both sides share the + * one list. + */ +bool +is_rpr_navigation_name(const char *name) +{ + return strcmp(name, "prev") == 0 || + strcmp(name, "next") == 0 || + strcmp(name, "first") == 0 || + strcmp(name, "last") == 0; +} + /* * ParseRPRNavCall * Recognize a row pattern navigation operation in a DEFINE clause. diff --git a/src/backend/parser/parse_rpr.c b/src/backend/parser/parse_rpr.c index 81cd3736341..63a4acbeb19 100644 --- a/src/backend/parser/parse_rpr.c +++ b/src/backend/parser/parse_rpr.c @@ -26,10 +26,8 @@ #include "miscadmin.h" #include "nodes/makefuncs.h" #include "nodes/nodeFuncs.h" -#include "optimizer/optimizer.h" #include "optimizer/rpr.h" -#include "parser/parse_coerce.h" -#include "parser/parse_expr.h" +#include "parser/parse_clause.h" #include "parser/parse_rpr.h" /* DEFINE clause walker context -- see define_walker for usage. */ @@ -321,9 +319,8 @@ transformDefineClause(ParseState *pstate, WindowDef *windef) * as a whole: it may contain RPRNavExpr nodes (PREV/NEXT/FIRST/LAST) * that only the owning WindowAgg can evaluate. */ - expr = transformExpr(pstate, restarget->val, - EXPR_KIND_RPR_DEFINE); - expr = coerce_to_boolean(pstate, expr, "DEFINE"); + expr = transformWhereClause(pstate, restarget->val, + EXPR_KIND_RPR_DEFINE, "DEFINE"); /* Build the defineClause entry directly from the transformed expr */ teDefine = makeTargetEntry((Expr *) expr, diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 378f10b5e88..697900a231a 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -7240,13 +7240,13 @@ append_pattern_quantifier(StringInfo buf, RPRPatternNode *node) /* {1,1} = no quantifier */ has_quantifier = false; } - else if (node->min == 0 && node->max == PG_INT32_MAX) + else if (node->min == 0 && node->max == RPR_QUANTITY_INF) appendStringInfoChar(buf, '*'); - else if (node->min == 1 && node->max == PG_INT32_MAX) + else if (node->min == 1 && node->max == RPR_QUANTITY_INF) appendStringInfoChar(buf, '+'); else if (node->min == 0 && node->max == 1) appendStringInfoChar(buf, '?'); - else if (node->max == PG_INT32_MAX) + else if (node->max == RPR_QUANTITY_INF) appendStringInfo(buf, "{%d,}", node->min); else if (node->min == node->max) appendStringInfo(buf, "{%d}", node->min); @@ -14242,10 +14242,7 @@ generate_function_name(Oid funcid, int nargs, List *argnames, Oid *argtypes, */ if (inRPRDefine) { - if (strcmp(proname, "prev") == 0 || - strcmp(proname, "next") == 0 || - strcmp(proname, "first") == 0 || - strcmp(proname, "last") == 0) + if (is_rpr_navigation_name(proname)) force_qualify = true; } diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h index db18c9728e6..e2f27eb0034 100644 --- a/src/include/nodes/plannodes.h +++ b/src/include/nodes/plannodes.h @@ -1264,6 +1264,12 @@ typedef int16 RPRElemIdx; /* element array index */ * * Layout optimized for alignment (no padding holes): * varId(1) + depth(1) + flags(1) + reserved(1) + min(4) + max(4) + next(2) + jump(2) + * + * reserved is padding and is not serialized: the round trip drops it. + * _outRPRPattern() writes the other seven fields and _readRPRPattern() zeroes + * this one, so that format string is the whole of what crosses. + * _copyRPRPattern() memcpy()s the struct and therefore carries all eight. A + * field that takes this byte over has to join the seven first. */ typedef struct RPRPatternElement { diff --git a/src/include/parser/parse_func.h b/src/include/parser/parse_func.h index cbff17abee0..ef9fa5cb06e 100644 --- a/src/include/parser/parse_func.h +++ b/src/include/parser/parse_func.h @@ -72,4 +72,6 @@ extern Oid LookupFuncWithArgs(ObjectType objtype, ObjectWithArgs *func, extern void check_srf_call_placement(ParseState *pstate, Node *last_srf, int location); +extern bool is_rpr_navigation_name(const char *name); + #endif /* PARSE_FUNC_H */ -- 2.54.0 (Apple Git-157)