Fix type coercion failure with mixed positional/named parameters in packages and subprocedures

[rophy]

Fixes #1006

Problem

Calling package procedures or subprocedures with mixed positional and named parameters using variables as named argument values fails with:

ERROR: failed to find conversion function from unknown to varchar2

The root cause is that after plisql_expand_and_reorder_functionargs() reorders fargs to declared parameter order, the type arrays (actual_arg_types and declared_arg_types) remain in call order, causing type coercion to fail.

Solution

Rebuild type arrays in declared order after argument reordering:

• For packages: rebuild actual_arg_types in parse_func.c
• For subprocedures: rebuild declared_arg_types in pl_subproc_function.c

Both fixes are applied after overload resolution completes, preserving correct function matching.

Summary by CodeRabbit

• Bug Fixes

  • Corrected handling of mixed positional and named parameters in function and subprocedure calls to properly align arguments with declared order and type coercion.

• Tests

  • Added comprehensive test suite for mixed positional and named parameter calling patterns.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

The changes fix a type resolution bug in PL/SQL where mixed positional and named parameters with variable values failed to resolve correctly. Two core modules were updated to reconstruct type arrays (actual_arg_types and true_typeids) after argument reordering to ensure they match the declared parameter order. Comprehensive tests validate the fix across various call patterns and parameter combinations.

Changes

Cohort / File(s)	Summary
Argument Type Array Reconstruction src/backend/parser/parse_func.c	After resolving package functions or subprocedures with reordered/expanded arguments due to defaults, reconstructs the actual_arg_types array from reordered fargs and updates nargs to reflect the new argument order, ensuring proper type coercion and matching.
Subproc True Type IDs Rebuild src/pl/plisql/src/pl_subproc_function.c	When completing subproc function detail resolution with named/default arguments involved, rebuilds true_typeids to match the declared argument order by constructing a declared_order_types array from the subproc's argument types.
Mixed Parameters Test Suite src/pl/plisql/src/sql/plisql_call.sql	Adds comprehensive test package test_mixed_params_pkg with proc_with_defaults procedure and multiple test blocks covering positional, named, mixed, and overloaded call patterns to validate argument type coercion and parameter binding.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• parse_func.c: Verify actual_arg_types reconstruction logic handles all argument reordering scenarios correctly, especially with mixed positional/named parameters and defaults
• pl_subproc_function.c: Ensure declared_order_types array construction properly maps original types to declared order and correctly assigns to *true_typeids when argnumbers is non-NULL
• plisql_call.sql: Review test coverage comprehensiveness for edge cases in mixed parameter resolution and variable type inference

Possibly related PRs

• package phase II #945: Also modifies PL/SQL subprocedure argument handling—reconstructing/reordering true/actual argument type arrays after expanding/reordering named/default args.
• plisql code for function and procedure #921: Touches PL/SQL subprocedure/function argument handling and modifies pl_subproc_function APIs including the true_typeids parameter and plisql_get_subprocfunc_detail.
• package phase II #944: Modifies subprocedure/package argument-type handling with additions that expose subproc argument info and rebuild declared-order type arrays during subproc resolution.

Suggested reviewers

• OreoYang
• jiaoshuntian
• NotHimmel

Poem

🐰 A hop through arguments, mixed and refined,
Types reordered to the declared design,
Variables whisper their names with care,
No more "unknown" in the PL/SQL air! ✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and clearly identifies the primary bug fix: resolving type coercion failure when using mixed positional/named parameters in packages and subprocedures.
Linked Issues check	✅ Passed	The code changes comprehensively address all requirements from issue #1006: rebuilding type arrays for packages in parse_func.c, subprocedures in pl_subproc_function.c, and adding extensive test coverage in plisql_call.sql.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the type coercion issue: argument reordering logic in two C files and comprehensive test cases for mixed parameter calls; no unrelated modifications present.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3dfb676 and 21bdd5f.

⛔ Files ignored due to path filters (1)

• src/pl/plisql/src/expected/plisql_call.out is excluded by !**/*.out

📒 Files selected for processing (3)

• src/backend/parser/parse_func.c (1 hunks)
• src/pl/plisql/src/pl_subproc_function.c (1 hunks)
• src/pl/plisql/src/sql/plisql_call.sql (1 hunks)

🧰 Additional context used

🧠 Learnings (1)

📓 Common learnings

Learnt from: rophy
Repo: IvorySQL/IvorySQL PR: 998
File: contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c:82-146
Timestamp: 2025-12-03T06:50:27.755Z
Learning: In contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c, the behavior where ENABLE() clears the existing buffer (rather than preserving it like Oracle does) is an intentional known difference tracked in issue #26 at https://github.com/rophy/IvorySQL/issues/26 (medium impact, open).

🧬 Code graph analysis (1)

src/backend/parser/parse_func.c (1)

src/backend/nodes/nodeFuncs.c (1)

• exprType (44-316)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)

• GitHub Check: pg_regression (ubuntu-latest)
• GitHub Check: build (ubuntu-latest)
• GitHub Check: oracle_regression (ubuntu-latest)
• GitHub Check: contrib_regression
• GitHub Check: oracle_pg_regression (ubuntu-latest)

🔇 Additional comments (3)

src/pl/plisql/src/pl_subproc_function.c (1)

1519-1545: Fix correctly rebuilds true_typeids in declared order after argument reordering.

The logic is sound: when argnumbers is non-NULL (indicating named/default arguments were involved and reordering occurred), the code rebuilds true_typeids by iterating through the function's arguments in declared order and extracting their type OIDs. This ensures true_typeids aligns with the reordered fargs, allowing type coercion to succeed.

The palloc allocation at line 1536 assumes the current memory context is appropriate for this short-lived array. Since this is within plisql_get_subprocfunc_detail during parsing, the allocation should be in a suitable parse-time context and will be available for the subsequent make_fn_arguments call.

src/backend/parser/parse_func.c (1)

366-392: Correct placement and logic for rebuilding actual_arg_types after argument reordering.

The fix is placed at the right location—after function resolution completes but before type coercion begins. By rebuilding actual_arg_types from the reordered fargs using exprType, the code ensures that both actual_arg_types and declared_arg_types are in the same order (declared parameter order). This alignment is essential for make_fn_arguments (line 836) to correctly match and coerce argument types.

The condition at lines 377-379 appropriately restricts this logic to package functions and subprocedures where argument reordering can occur.

src/pl/plisql/src/sql/plisql_call.sql (1)

612-793: Comprehensive test coverage for the mixed parameter bug fix.

The test cases thoroughly validate the fix for issue #1006 across multiple scenarios:

1. Package procedures (lines 616-681): Tests all combinations—positional, named, mixed with literals, and mixed with variables.
2. Subprocedures (lines 683-720): Similar coverage including procedure and function variants.
3. Overloading validation (lines 722-762): Ensures the fix doesn't break overload resolution.
4. Minimal reproduction (lines 764-793): Documents the exact failure scenario with clear comments.

The test at line 667 directly exercises the bug case (mixed positional + named with variable) that previously failed with "failed to find conversion function from unknown to varchar2." This comprehensive coverage gives confidence that the fix works correctly and doesn't introduce regressions.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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