Skip to content

Fix crash when nested sub-functions access NEW after DECLARE uses NEW#1202

Merged
gaoxueyu merged 2 commits intoIvorySQL:masterfrom
NotHimmel:refield_bug_fix
Mar 8, 2026
Merged

Fix crash when nested sub-functions access NEW after DECLARE uses NEW#1202
gaoxueyu merged 2 commits intoIvorySQL:masterfrom
NotHimmel:refield_bug_fix

Conversation

@NotHimmel
Copy link
Copy Markdown
Collaborator

@NotHimmel NotHimmel commented Mar 4, 2026 •
edited by coderabbitai bot
Loading

Summary by CodeRabbit

  • Bug Fixes

    • Fixed incorrect handling of certain record-field datums during subprocedure compilation, preventing misinitialization and wrong OUT assignments.

  • Tests

    • Added a comprehensive trigger-based test scenario that exercises nested subprocedure behavior with sample inserts and informational logs.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 4, 2026 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 76fd0822-fcfc-498e-b34a-9a490d68738b

📥 Commits

Reviewing files that changed from the base of the PR and between 42b974c and 91a6541.

📒 Files selected for processing (2)
  • src/pl/plisql/src/expected/plisql_nested_subproc.out
  • src/pl/plisql/src/sql/plisql_nested_subproc.sql
📝 Walkthrough

Walkthrough

Adds RECFIELD datum skipping to two compilation-time routines in PL/pgSQL subprocedure handling and introduces new trigger tests exercising nested subprocedure/recfield behavior; ensures RECFIELD datums are not treated as independent globals or OUT assignments.

Changes

Cohort / File(s) Summary
Subprocedure datum handling
src/pl/plisql/src/pl_subproc_function.c		 Detect and skip RECFIELD datums in plisql_init_subprocfunc_globalvar and plisql_assign_out_subprocfunc_globalvar; assert parent recfield precedes child recfield.
Tests / SQL for nested subprocs (triggers)
src/pl/plisql/src/sql/plisql_nested_subproc.sql, src/pl/plisql/src/expected/plisql_nested_subproc.out		 Add test_subproc_recfield table, three trigger functions (trig_subproc_recfield1/2/3) and triggers; insert sample rows and cleanup. Note: SQL block is duplicated within the file.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • OreoYang

Poem

🐰 I hopped through code with floppy ear,

Skipping RECFIELDs that shouldn't appear,
Parents first, then children stay light,
Triggers clap on insert tonight,
A tidy hop, a tidy night ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the primary fix: preventing crashes when nested sub-functions access the NEW record after DECLARE has used NEW, which aligns with the code changes addressing RECFIELD datum handling.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@bigplaice
Copy link
Copy Markdown
Collaborator

bigplaice commented Mar 4, 2026

could we add some regression test case for this change?

@NotHimmel
Copy link
Copy Markdown
Collaborator Author

NotHimmel commented Mar 5, 2026

could we add some regression test case for this change?

Sure, I've added regression tests for this change.

c2main
c2main approved these changes Mar 6, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@c2main c2main left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirm it fixed bug on our testing and the added regression tests correctly ensure the case is correctly managed.

@gaoxueyu gaoxueyu merged commit 7fb9fe7 into IvorySQL:master Mar 8, 2026
6 checks passed
@NotHimmel NotHimmel deleted the refield_bug_fix branch March 9, 2026 00:55
@yuanyl630 yuanyl630 mentioned this pull request Mar 19, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@c2main c2main c2main approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@NotHimmel @bigplaice @c2main @gaoxueyu