public inbox for [email protected]
help / color / mirror / Atom feedFrom: Henson Choi <[email protected]>
To: Tatsuo Ishii <[email protected]>
To: jian he <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: Row pattern recognition
Date: Wed, 27 May 2026 07:31:02 +0900
Message-ID: <CAAAe_zD7vCLCb+vxpO3P-NsDUZ=JcN8EGCV0dz0BNgBKsbOGcQ@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<CAAAe_zBdwAUDNs_WFdLkFF=ewhkDkv-AqizVEVzhsfremGFb4w@mail.gmail.com>
<[email protected]>
<[email protected]>
Hi Tatsuo, jian,
While looking into v47, I noticed that
> raw_expression_tree_walker_impl() lacks tracking RPCommonSyntax and
> its children nodes. Probably this does nothing wrong with RPR
> functionalities but just for completeness I created a patch on top of
> v47.
Thanks for catching this. I applied the patch and the RPR regress passes
cleanly. I also re-ran it with debug_raw_expression_coverage_test turned
on (on an assert-enabled build), and the full regress is green as well.
One observation while testing: the GUC catches missing case handlers
once the walker actually reaches a node, but it cannot flag a missing
WALK on its own -- if no caller drives the walker into a subtree, the
omission stays silent. So your inspection was the part that found the
gap; the GUC just confirms the patch closes it. With the patch in, RPR
raw subtrees are on the safety net for any future node-type additions.
I'll include the patch in v48 as nocfbot-0015. My suggestion would
be to defer the fold until the jian-response patches (numbered from
0016 onward, which I'll be sending shortly) have also gone through a
review round, so the whole bundle can land together in one pass.
Patch attached below for convenience.
jian -- thanks for the thorough review. It covers a lot of ground,
and I'm still working through it. Current expectation is that most
items will be accepted; a few smaller ones may end up with a
different conclusion than your suggestion, and those are still under
analysis on my side.
The plan is to turn the responses into a patch series and send them
out for another round of review. More to follow once the batch is in
shape.
Thanks,
Henson
From dbefdd47ab63c099b8cb9e7a7af3907ed2212dcd Mon Sep 17 00:00:00 2001
From: Henson Choi <[email protected]>
Date: Wed, 27 May 2026 07:02:38 +0900
Subject: [PATCH] Add raw_expression_tree_walker coverage for RPR raw nodes
WindowDef.rpCommonSyntax was not walked, and there were no case
arms for T_RPCommonSyntax or T_RPRPatternNode. RPR core was
unaffected -- contain_rpr_walker() in parse_cte.c intercepts
WindowDef before delegating -- but debug_raw_expression_coverage_test
silently skipped these subtrees, leaving any future raw-node
omission on the RPR side undetected.
---
src/backend/nodes/nodeFuncs.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index 734bb0554fe..101c03b6ae8 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -4641,6 +4641,8 @@ raw_expression_tree_walker_impl(Node *node,
return true;
if (WALK(wd->endOffset))
return true;
+ if (WALK(wd->rpCommonSyntax))
+ return true;
}
break;
case T_RangeSubselect:
@@ -4896,6 +4898,24 @@ raw_expression_tree_walker_impl(Node *node,
return true;
}
break;
+ case T_RPCommonSyntax:
+ {
+ RPCommonSyntax *rc = (RPCommonSyntax *) node;
+
+ if (WALK(rc->rpPattern))
+ return true;
+ if (WALK(rc->rpDefs))
+ return true;
+ }
+ break;
+ case T_RPRPatternNode:
+ {
+ RPRPatternNode *rp = (RPRPatternNode *) node;
+
+ if (WALK(rp->children))
+ return true;
+ }
+ break;
default:
elog(ERROR, "unrecognized node type: %d",
(int) nodeTag(node));
--
2.50.1 (Apple Git-155)
Attachments:
[text/plain] nocfbot-0015-RPR-raw-walker-coverage.txt (1.6K, 3-nocfbot-0015-RPR-raw-walker-coverage.txt)
download | inline diff:
From dbefdd47ab63c099b8cb9e7a7af3907ed2212dcd Mon Sep 17 00:00:00 2001
From: Henson Choi <[email protected]>
Date: Wed, 27 May 2026 07:02:38 +0900
Subject: [PATCH] Add raw_expression_tree_walker coverage for RPR raw nodes
WindowDef.rpCommonSyntax was not walked, and there were no case
arms for T_RPCommonSyntax or T_RPRPatternNode. RPR core was
unaffected -- contain_rpr_walker() in parse_cte.c intercepts
WindowDef before delegating -- but debug_raw_expression_coverage_test
silently skipped these subtrees, leaving any future raw-node
omission on the RPR side undetected.
---
src/backend/nodes/nodeFuncs.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index 734bb0554fe..101c03b6ae8 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -4641,6 +4641,8 @@ raw_expression_tree_walker_impl(Node *node,
return true;
if (WALK(wd->endOffset))
return true;
+ if (WALK(wd->rpCommonSyntax))
+ return true;
}
break;
case T_RangeSubselect:
@@ -4896,6 +4898,24 @@ raw_expression_tree_walker_impl(Node *node,
return true;
}
break;
+ case T_RPCommonSyntax:
+ {
+ RPCommonSyntax *rc = (RPCommonSyntax *) node;
+
+ if (WALK(rc->rpPattern))
+ return true;
+ if (WALK(rc->rpDefs))
+ return true;
+ }
+ break;
+ case T_RPRPatternNode:
+ {
+ RPRPatternNode *rp = (RPRPatternNode *) node;
+
+ if (WALK(rp->children))
+ return true;
+ }
+ break;
default:
elog(ERROR, "unrecognized node type: %d",
(int) nodeTag(node));
--
2.50.1 (Apple Git-155)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Row pattern recognition
In-Reply-To: <CAAAe_zD7vCLCb+vxpO3P-NsDUZ=JcN8EGCV0dz0BNgBKsbOGcQ@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox