fix: guard against NULL subplan in full-path MERGE sub-case 1 (#2536) - #2546
Open
waterWang wants to merge 1 commit into
Open
fix: guard against NULL subplan in full-path MERGE sub-case 1 (#2536)#2546waterWang wants to merge 1 commit into
waterWang wants to merge 1 commit into
Conversation
…#2536) When MERGE is the first clause followed by WITH (Case 3, sub-case 1), the code assumes the child execution node is always a SubqueryScanState and dereferences sss->subplan to obtain the tuple descriptor for the remade scan slot. When the MERGE path pattern produces no base-relation scan (e.g. empty graph), the planner emits a plain Result plan instead of a SubqueryScan, the cast-to-SubqueryScanState reads through the wrong struct layout, sss->subplan is NULL, and ExecGetResultType(NULL) segfaults. Fix: check IsA(lefttree, SubqueryScanState) before using SubqueryScanState internals. When the child is a SubqueryScan the original code path (remake child scan slot via sss->ss + ExecGetResultType(sss->subplan)) is preserved without regression. When the child is a Result plan, use the generic PlanState API (ExecGetResultType(lefttree)) and store the remade slot in the MERGE node's own ScanState. Fixes apache#2536.
There was a problem hiding this comment.
Pull request overview
Fixes a PostgreSQL backend crash in the Cypher MERGE executor by avoiding an invalid SubqueryScanState cast and guarding against a NULL subplan/result-type lookup in the “full-path MERGE + WITH” sub-case.
Changes:
- Add a
SubqueryScanStatetype check before accessingsss->subplan, and use genericPlanStateAPIs when the child is a plainResultplan. - Add a regression test reproducing Issue #2536’s query shape.
- Update expected regression output for the new test.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/backend/executor/cypher_merge.c | Adds a safe execution-path split for SubqueryScan vs Result child plan states to prevent NULL/struct-layout crashes. |
| regress/sql/expr.sql | Adds a regression SQL test for Issue #2536 (but currently introduces duplicated/inconsistent end-of-tests markers). |
| regress/expected/expr.out | Adds expected output for the new test (but currently contains a premature end-of-tests marker and inconsistent final marker formatting). |
Suppressed comments (1)
regress/expected/expr.out:10748
- The final end-of-tests marker in this expected file is a single line ("-- End of tests"), but other regression expected files (and the earlier marker in this same file) use the standard three-line block. Update the final marker to the standard form for consistency.
-- End of tests
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
4203
to
+4220
| -- | ||
| -- End of tests | ||
| -- | ||
|
|
||
| -- Issue #2536: Full-path MERGE with WITH followed by WHERE | ||
| -- on a non-existing pattern (empty graph). The child plan is a | ||
| -- Result node, not a SubqueryScan, and the code must not assume | ||
| -- SubqueryScanState internals when remaking the scan tuple slot. | ||
| SELECT * FROM create_graph('issue_2536'); | ||
| SELECT * FROM cypher('issue_2536', $$ | ||
| MERGE p = (:a)-[:r]->(n) | ||
| WITH 1 AS y | ||
| WHERE ('a' STARTS WITH 'a') | ||
| RETURN y | ||
| $$) AS (y agtype); | ||
| SELECT * FROM drop_graph('issue_2536', true); | ||
|
|
||
| -- End of tests |
Comment on lines
10711
to
10713
| -- | ||
| -- End of tests | ||
| -- |
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 #2536.
Root cause: When a full-path MERGE is the first clause followed by a
WITH (Case 3, sub-case 1, path-not-found → create path), the code assumes
node->ss.ps.lefttreeis always aSubqueryScanStateand readsExecGetResultType(sss->subplan). When the MERGE path pattern producesno base-relation scan (e.g. empty graph), the planner emits a plain
Resultplan, theSubqueryScanStatecast reads through the wrong structlayout,
sss->subplanisNULL, andExecGetResultType(NULL)segfaults.Fix: check
IsA(lefttree, SubqueryScanState)before usingSubqueryScanStateinternals. For aSubqueryScanchild the originalcode path (remake child scan slot via
sss->ss+ExecGetResultType(sss->subplan))is preserved. For a
Resultchild, use the genericPlanStateAPI(
ExecGetResultType(lefttree)) and store the remade slot in the MERGEnode's own
ScanState.Tests: 41/43 regression tests pass (age_load and age_upgrade are
pre-existing environment failures). New regression test added to
regress/sql/expr.sqlreproduces the exact query shape from #2536.