Generate query jumble automaticly#1827
Open
chenjinbao1989 wants to merge 5 commits into
Open
Conversation
yjhjstz
reviewed
Jun 24, 2026
|
|
||
| if (get_dirent_type(path, de, false, ERROR) != PGFILETYPE_LNK) | ||
| ereport(allow_in_place_tablespaces ? WARNING : PANIC, | ||
| ereport(allow_in_place_tablespaces ? WARNING : WARNING, |
Member
There was a problem hiding this comment.
PANIC is need to abort transcation.
yjhjstz
reviewed
Jun 24, 2026
| * code that executes the command in't prepared for a NULL. | ||
| */ | ||
| if (conninfo == NULL) | ||
| conninfo = pstrdup(""); |
yjhjstz
reviewed
Jun 24, 2026
|
|
||
| my $last_nodetag = 'WindowObjectData'; | ||
| my $last_nodetag_no = 557; | ||
| my $last_nodetag_no = 584; |
leborchuk
approved these changes
Jun 29, 2026
leborchuk
left a comment
leborchuk
left a comment
Contributor
There was a problem hiding this comment.
Yeah, let's clear the mess. We have 2 functions with the same name:
src/backend/nodes/queryjumblefuncs.c:JumbleQuery(Query *query)
src/backend/utils/misc/queryjumble.c:static void JumbleQueryInternal(JumbleState *jstate, Query *query);
Should use only one - those which was moved to the src/backend/nodes
Contributor
|
It will be great to add good PR description there. I asked Copilot for review changes, one could get the base description from Copilot output |
There was a problem hiding this comment.
Pull request overview
This PR refactors Cloudberry’s node infrastructure to generate query jumble and node support code automatically (including nodetag generation and updated node read/write/copy behavior), and updates regression expected files to match the new serialized node output.
Changes:
- Move/modernize query jumble interfaces under
src/include/nodes/and switch callers to the updated API. - Extend
gen_node_support.pl+ build rules to generate/shipnodetags.hand additional generated node support outputs (including query-jumble support). - Update multiple node struct definitions/attributes and refresh regression expected outputs accordingly.
Reviewed changes
Copilot reviewed 47 out of 51 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/singlenode_regress/expected/partition.out | Updates expected partition template serialization output. |
| src/test/regress/expected/xml.out | Adjusts expected XML serialization formatting. |
| src/test/regress/expected/partition_append.out | Updates expected partition template serialization output. |
| src/test/regress/expected/expressions.out | Updates expected output for NOT IN (...) NULL semantics. |
| src/include/utils/queryenvironment.h | Adds node attribute annotation for generated support behavior. |
| src/include/parser/analyze.h | Switches queryjumble include to the new header path. |
| src/include/nodes/tidbitmap.h | Refactors bitmap word array sizing constant. |
| src/include/nodes/queryjumble.h | Moves/updates query jumble public header and API surface. |
| src/include/nodes/primnodes.h | Updates node attributes / renames anonymous structs. |
| src/include/nodes/plannodes.h | Adds node attributes for generated copy/read/write behavior. |
| src/include/nodes/pathnodes.h | Adds/adjusts node attributes for generated support behavior. |
| src/include/nodes/parsenodes.h | Adds/adjusts node attributes and a few struct typedef names. |
| src/include/nodes/nodes.h | Replaces large inline NodeTag enum with generated nodetags.h include. |
| src/include/nodes/altertablenodes.h | Adds node attributes for generated support behavior. |
| src/include/Makefile | Installs generated nodes/nodetags.h; cleans generated node headers. |
| src/include/executor/execdesc.h | Adds node attributes / renames anonymous struct typedef. |
| src/include/commands/explain_gp.h | New header extracting EXPLAIN GP structs. |
| src/include/catalog/heap.h | Adds read/write-ignore defaults via node attributes. |
| src/include/access/tupdesc.h | Renames typedef + adds node attributes. |
| src/backend/utils/misc/queryjumble.c | Removes legacy query jumble implementation. |
| src/backend/utils/misc/Makefile | Stops building the removed queryjumble.o. |
| src/backend/utils/misc/guc_tables.c | Updates includes to new queryjumble header location. |
| src/backend/postmaster/postmaster.c | Updates includes to new queryjumble header location. |
| src/backend/parser/analyze.c | Switches to new JumbleQuery() call signature. |
| src/backend/nodes/read.c | Changes error severity for token parsing failure. |
| src/backend/nodes/nodeFuncs.c | Adjusts mutator behavior for certain node tags. |
| src/backend/nodes/Makefile | Adds generated-header rules and new node support objects. |
| src/backend/nodes/gen_node_support.pl | Major generator extensions (out/read fast + queryjumble generation). |
| src/backend/nodes/copyfuncs.switch.c | Removes generated switch file (now regenerated differently). |
| src/backend/nodes/copyfuncs.c | Adjusts pointer-copy macro + adds custom copy routines. |
| src/backend/nodes/.gitignore | Ignores newly generated node-support artifacts. |
| src/backend/Makefile | Re-enables node header generation step. |
| src/backend/executor/execProcnode.c | Removes references to a no-longer-present planstate tag. |
| src/backend/executor/execAmi.c | Removes references to a no-longer-present planstate tag. |
| src/backend/commands/subscriptioncmds.c | Normalizes conninfo to avoid NULL vs empty-string ambiguity. |
| src/backend/commands/explain.c | Switches to new JumbleQuery() call signature. |
| src/backend/commands/explain_gp.c | Uses new extracted header for EXPLAIN GP stats structs. |
| src/backend/access/transam/xlogrecovery.c | Changes severity handling for unexpected pg_tblspc/ entries. |
| gpcontrib/pg_hint_plan/pg_hint_plan.c | Switches to new JumbleQuery() call signature. |
| gpcontrib/gp_stats_collector/src/stat_statements_parser/pg_stat_statements_parser.c | Updates includes to new queryjumble header location. |
| contrib/pax_storage/src/test/regress/expected/xml.out | Mirrors XML expected-output change for pax_storage tests. |
| contrib/pax_storage/src/test/regress/expected/partition.out | Mirrors partition expected-output change for pax_storage tests. |
| contrib/pax_storage/src/test/regress/expected/partition_optimizer.out | Mirrors partition optimizer expected-output change for pax_storage tests. |
| contrib/pax_storage/src/test/regress/expected/expressions.out | Mirrors expressions expected-output change for pax_storage tests. |
| contrib/pax_storage/src/test/regress/expected/bfv_partition.out | Mirrors partition template serialization changes for BFV output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1454
to
+1461
| elsif($t eq 'bytea*') | ||
| { | ||
| print $ofaf | ||
| "\tWRITE_BYTEA_FIELD($f);\n"; | ||
| print $rfaf | ||
| "\tREAD_BYTEA_FIELD($f);\n" | ||
| unless $no_read; | ||
| } |
Comment on lines
+1518
to
+1524
| if (defined $read_as_field) | ||
| { | ||
| print $rff "\tlocal_node->$f = $read_as_field;\n" unless $no_read; | ||
|
|
||
| # for out and read fast | ||
| print $rff "\tlocal_node->$f = $read_as_field;\n" unless $no_read; | ||
| } |
| if (jumblequery) | ||
| { | ||
| jstate = JumbleQueryDirect(query, query_str); | ||
| jstate = JumbleQuery(query); |
| /* | ||
| * conninfo can be an empty string, but the serialization | ||
| * doesn't distinguish an empty string from NULL. The | ||
| * code that executes the command in't prepared for a NULL. |
| else | ||
| { | ||
| elog(ERROR, "unrecognized token: \"%.*s\"", tok_len, token); | ||
| elog(PANIC, "unrecognized token: \"%.*s\"", tok_len, token); |
|
|
||
| if (get_dirent_type(path, de, false, ERROR) != PGFILETYPE_LNK) | ||
| ereport(allow_in_place_tablespaces ? WARNING : PANIC, | ||
| ereport(allow_in_place_tablespaces ? WARNING : WARNING, |
Comment on lines
+21
to
+22
|
|
||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #ISSUE_Number
What does this PR do?
Type of Change
Breaking Changes
Test Plan
make installcheckmake -C src/test installcheck-cbdb-parallelImpact
Performance:
User-facing changes:
Dependencies:
Checklist
Additional Context
CI Skip Instructions