Conversation
1) nested function support out parameter` 2) package support out parameters 3) psql /? support \dk 4) supoort select * from func which include out paramster 5) change check variable assignable in pl_gram.y
WalkthroughThis PR introduces comprehensive OUT-parameter support for PL/iSQL procedures and functions, including new infrastructure for handling subprocedure metadata, branching logic to distinguish PostgreSQL procedures from internal subprocs, return-type adjustment mechanisms for Oracle compatibility, and extensive test coverage demonstrating OUT-parameter usage across nested procedures, packages, and function calls. Changes
Sequence Diagram(s)sequenceDiagram
participant Executor
participant ExecInitFunc as ExecInitFuncWithOutParams
participant PGProc as PG Proc Path
participant SubprocPath as Subproc Path
participant Helper as Subproc Helpers
Executor->>ExecInitFunc: Initialize function with OUT params
alt Is PG Proc?
ExecInitFunc->>PGProc: Get func_tuple from catalog
PGProc->>PGProc: Validate prokind & prolang
PGProc->>PGProc: Get arg info via get_func_arg_info
PGProc->>PGProc: Get return type via get_func_real_rettype
else Is Subproc
ExecInitFunc->>SubprocPath: Skip PG catalog lookup
SubprocPath->>Helper: Call get_subproc_arg_info
Helper-->>SubprocPath: Arg types, names, modes
SubprocPath->>Helper: Call subproc_should_change_return_type
Helper-->>SubprocPath: Adjusted return type if needed
end
ExecInitFunc->>Executor: Proceed with OUT-param handling
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (11)
src/oracle_test/regress/sql/ora_plisql.sql (2)
908-929: Nested IN OUT test looks coherent; consider adding an execution assertionThe example correctly exercises IN OUT mutation via a nested subproc and sets the outer IN OUT parameter. To harden the test, add a simple invocation and expected output check (e.g., via RAISE or a SELECT) so CI asserts behavior, not just definition. Also consider using RAISE NOTICE for consistency with the rest of this file.
- raise info '%',original; + RAISE NOTICE '%', original;If adding an assertion is acceptable, I can propose a minimal SELECT block that validates both the return value and the mutated IN OUT argument.
913-915: PL/iSQL subproc signature style is Oracle-idiomatic; keep consistentInner subproc uses “return integer” (Oracle style) while the outer function uses “returns integer” (PostgreSQL style). That’s valid here, but if you prefer uniformity, switch the inner declaration to match the Oracle style consistently across this section.
src/backend/executor/execSRF.c (1)
147-147: Minor: Initial funcrettype value is unused.Line 147 assigns
funcrettype = exprType(...), but this value is immediately overwritten in the conditional blocks below (lines 152, 162) when the type adjustment conditions are met. If the initial value isn't needed for comparison or fallback, this assignment could be moved inside the else block (line 168) to make the logic clearer.src/backend/executor/execExpr.c (2)
5125-5269: Consider refactoring for better maintainability.The cache management logic with multiple conditional release points (lines 5135, 5188, 5268-5269) is correct but complex and error-prone for future maintenance. Consider using a single cleanup path at the end with a goto or extracting the PG_PROC-specific logic into a helper function that manages its own cache entry lifecycle.
5267-5267: Minor: Remove trailing whitespace.Line 5267 appears to contain only whitespace. Please remove it for cleaner code formatting.
src/pl/plisql/src/pl_package.c (1)
3496-3555: Align arg info allocations with get_subprocfunc_arg_info_from_arguments.Use consistent lengths; p_argmodes doesn’t need a sentinel. Also consider palloc for exact nargs for symmetry.
Apply this diff:
- *p_argmodes = (char *) palloc0((nargs + 1) * sizeof(char)); + *p_argmodes = (char *) palloc(nargs * sizeof(char));src/pl/plisql/src/pl_subproc_function.c (3)
405-462: Don’t expose internal “RETVAL” in the user namespace.Publishing a reserved identifier can collide with user code and leaks internals. Keep only the positional “$n” alias.
Apply this diff:
- add_parameter_name(argitemtype, argvariable->dno, buf); - add_parameter_name(argitemtype, argvariable->dno, "_RETVAL_"); + add_parameter_name(argitemtype, argvariable->dno, buf);
486-525: Result type promotion with OUT params looks correct. Add a fast path comment.Minor: clarify why polymorphic returns are rejected when OUTs exist.
2603-2662: Column naming for OUT row depends on a GUC — ensure stable defaults.If the GUC is off, OUT column names mirror arg names; when on, names become _column_N. Confirm default value and document in user-facing help.
src/pl/plisql/src/sql/plisql_nested_subproc2.sql (2)
758-783: Nested OUT handling now returns integer instead of record — validate column shape.Given OUT+RETURN paths, ensure this block exercises both:
- single OUT with void return (scalar result),
- OUT with non-void (RECORD with RETVAL).
Add an extra SELECT * call to assert column names/order when out_parameter_column_position toggles.
1662-1709: NO COPY with OUT params test is useful; assert both OUT and RETURN.Consider asserting both j (OUT) and function return in a single query to pin behavior.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
src/oracle_test/regress/expected/ora_package.outis excluded by!**/*.outsrc/oracle_test/regress/expected/ora_plisql.outis excluded by!**/*.outsrc/pl/plisql/src/expected/plisql_nested_subproc.outis excluded by!**/*.outsrc/pl/plisql/src/expected/plisql_nested_subproc2.outis excluded by!**/*.outsrc/pl/plisql/src/expected/plisql_out_parameter.outis excluded by!**/*.out
📒 Files selected for processing (19)
src/backend/commands/tablecmds.c(1 hunks)src/backend/executor/execExpr.c(2 hunks)src/backend/executor/execSRF.c(3 hunks)src/backend/executor/execTuples.c(1 hunks)src/backend/parser/parse_clause.c(1 hunks)src/backend/utils/fmgr/funcapi.c(2 hunks)src/bin/psql/help.c(1 hunks)src/bin/psql/po/zh_CN.po(1 hunks)src/include/funcapi.h(2 hunks)src/oracle_test/regress/sql/ora_package.sql(9 hunks)src/oracle_test/regress/sql/ora_plisql.sql(1 hunks)src/pl/plisql/src/pl_exec.c(0 hunks)src/pl/plisql/src/pl_gram.y(2 hunks)src/pl/plisql/src/pl_package.c(1 hunks)src/pl/plisql/src/pl_package.h(1 hunks)src/pl/plisql/src/pl_subproc_function.c(16 hunks)src/pl/plisql/src/sql/plisql_nested_subproc.sql(14 hunks)src/pl/plisql/src/sql/plisql_nested_subproc2.sql(13 hunks)src/pl/plisql/src/sql/plisql_out_parameter.sql(0 hunks)
💤 Files with no reviewable changes (2)
- src/pl/plisql/src/pl_exec.c
- src/pl/plisql/src/sql/plisql_out_parameter.sql
🧰 Additional context used
🧬 Code graph analysis (9)
src/pl/plisql/src/pl_package.h (1)
src/pl/plisql/src/pl_package.c (3)
plisql_get_subproc_arg_info(3495-3556)plisql_get_subproc_prokind(3562-3581)plisql_subproc_should_change_return_type(3587-3653)
src/backend/executor/execSRF.c (3)
src/backend/executor/spi.c (1)
SPI_get_connected(3444-3448)src/backend/nodes/nodeFuncs.c (1)
exprType(44-316)src/backend/utils/fmgr/funcapi.c (2)
func_should_change_return_type(2635-2766)subproc_should_change_return_type(1966-1978)
src/pl/plisql/src/pl_package.c (2)
src/include/nodes/pg_list.h (1)
list_length(151-155)src/backend/utils/mmgr/mcxt.c (3)
palloc(1320-1348)palloc0(1350-1370)pstrdup(1702-1706)
src/backend/parser/parse_clause.c (6)
src/backend/utils/cache/syscache.c (2)
SearchSysCache1(220-229)ReleaseSysCache(268-272)src/include/postgres.h (1)
ObjectIdGetDatum(256-260)src/include/access/htup_details.h (1)
GETSTRUCT(751-755)src/backend/utils/mmgr/mcxt.c (1)
pstrdup(1702-1706)src/backend/utils/fmgr/funcapi.c (3)
get_func_arg_info(1405-1495)get_subproc_arg_info(1949-1961)get_internal_function_name(1809-1831)src/backend/parser/parse_param.c (1)
ParseParamVariable(436-469)
src/include/funcapi.h (1)
src/backend/utils/fmgr/funcapi.c (3)
get_subproc_kind(1938-1944)get_subproc_arg_info(1949-1961)subproc_should_change_return_type(1966-1978)
src/backend/executor/execTuples.c (1)
src/backend/utils/fmgr/funcapi.c (1)
subproc_should_change_return_type(1966-1978)
src/backend/executor/execExpr.c (5)
src/backend/utils/cache/syscache.c (2)
SearchSysCache1(220-229)ReleaseSysCache(268-272)src/include/postgres.h (1)
ObjectIdGetDatum(256-260)src/include/access/htup_details.h (1)
GETSTRUCT(751-755)src/backend/utils/fmgr/funcapi.c (4)
get_func_arg_info(1405-1495)get_subproc_kind(1938-1944)get_subproc_arg_info(1949-1961)get_internal_function_name(1809-1831)src/backend/utils/cache/lsyscache.c (1)
get_func_real_rettype(1822-1875)
src/pl/plisql/src/pl_subproc_function.c (4)
src/backend/utils/mmgr/mcxt.c (2)
palloc(1320-1348)pstrdup(1702-1706)src/backend/utils/fmgr/fmgr.c (1)
get_fn_expr_rettype(1956-1971)src/pl/plisql/src/pl_comp.c (3)
plisql_build_datatype(2926-2942)plisql_build_variable(2707-2766)add_parameter_name(1518-1538)src/pl/plisql/src/pl_package.c (3)
plisql_get_subproc_arg_info(3495-3556)plisql_get_subproc_prokind(3562-3581)plisql_subproc_should_change_return_type(3587-3653)
src/backend/utils/fmgr/funcapi.c (1)
src/backend/utils/cache/packagecache.c (1)
plisql_internel_funcs_init(393-404)
⏰ 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). (6)
- GitHub Check: contrib_regression
- GitHub Check: build (ubuntu-latest)
- GitHub Check: meson_build (ubuntu-latest)
- GitHub Check: oracle_pg_regression (ubuntu-latest)
- GitHub Check: pg_regression (ubuntu-latest)
- GitHub Check: oracle_regression (ubuntu-latest)
🔇 Additional comments (24)
src/oracle_test/regress/sql/ora_package.sql (7)
13-13: Global flag placement for OUT parameter testingThe
SET ivorysql.allow_out_parameter_const = true;at line 13 enables OUT parameter support for the test file. This setting is correctly placed before the first use and properly reset at line 6098.
238-247: Function with OUT parameters in nested declare blockCorrectly defines and invokes a nested function with mixed parameter modes:
ids OUT mds%rowtype: output parameterids1 mds%rowtype: input parameter (default IN)name1 OUT varchar: output parameter- Returns
mds%rowtypeThe implementation properly assigns values to OUT parameters and invokes the function with both IN and OUT arguments.
2006-2031: Package-level OUT parameters for functions and proceduresComprehensive package definitions demonstrating:
- Package
pkg(lines 2006-2031) with OUT parameters in function signature- Package
pg_myfirst(lines 2782-2821) with mixed OUT function/procedure definitionsAll function and procedure bodies correctly implement OUT semantics with proper value assignments. The test invocations validate functionality across multiple call patterns.
Also applies to: 2782-2821
5629-5720: Inline procedure OUT parameter testsLines 5629-5720 provide comprehensive inline procedure testing covering:
- Single OUT parameter procedures (line 5633)
- OUT + IN parameter combinations (line 5645)
- Multiple mixed-mode parameters (line 5657)
- IN OUT and OUT rowtype parameters (lines 5671, 5688)
- Nested procedures with OUT parameters (lines 5706-5710)
All test cases properly assign values to OUT parameters and verify results with
raise infostatements. Scope handling in nested contexts is correctly demonstrated.
5746-5869: Inline function OUT parameter testsLines 5746-5869 provide extensive function OUT parameter testing:
- Functions returning scalar types with OUT parameter (line 5749)
- Functions returning varchar with OUT parameter (line 5766)
- Multiple OUT parameters in function signature (line 5780, 5810)
- IN OUT parameter handling (line 5832)
- Deeply nested functions with OUT parameters (lines 5853-5857)
All implementations correctly show assignment to OUT parameters before return statements. Nested function scope resolution is properly demonstrated.
5898-6095: Comprehensive package and function OUT parameter tests with SELECT * FROM supportLines 5898-6095 demonstrate the full feature set:
Function/Procedure Definitions (lines 5898-5961):
- Mixed OUT parameters with nested functions/procedures
- Proper parameter forwarding between levels
Package Definitions (lines 5986-6035):
- Package specification with multiple OUT-parameter functions/procedures
- Matching implementations with correct OUT handling
- Package initialization block
Test Invocations (lines 6037-6093):
- Line 6040:
SELECT * FROM test_pkg.test_f(NULL)demonstrates "SELECT * FROM func" with OUT parameters- Line 6041:
SELECT * FROM test_pkg.test_f1(NULL, NULL)with multiple OUT parameters- Lines 6046-6083: DECLARE blocks verify OUT parameter assignment and retrieval
This section provides end-to-end validation of the OUT parameter feature, including the PR objective of supporting
SELECT * FROM funcwhen functions include OUT parameters.
6098-6098: Proper cleanup of test flagLine 6098 correctly resets the
ivorysql.allow_out_parameter_constflag after all OUT parameter tests complete, ensuring subsequent tests run with the default setting.src/backend/commands/tablecmds.c (1)
6960-6961: No issues found—implementation is complete and correct.Verification confirms:
AT_ForceViewCompileis properly defined inAlterTableTypeenum (parsenodes.h:2509)- All necessary switch statements in tablecmds.c handle this case:
- Line 4962: Lock mode assignment (
AccessExclusiveLock)- Line 5454: Permission check (
ATSimplePermissions) and pass assignment- Line 5884: Main execution logic calling
compile_force_view()with error handling- Line 6960: String representation returns
NULL(shown in provided code)- Callers of
alter_table_type_to_string()properly check for NULL before use (line 7018)The
NULLreturn at lines 6960-6961 is appropriate and safe.src/pl/plisql/src/pl_gram.y (2)
539-563: ****The review comment requests verification of
infofield initialization for package variables, but thePLiSQL_pkg_datumstructure does not contain aninfofield. The structure only contains: dtype, dno, pkgoid, refname, lineno, item, and pkgvar. Theinfofield exists in other PLiSQL structures (e.g.,PLiSQL_var,PLiSQL_rec) but is not applicable to package datums.Likely an incorrect or invalid review comment.
467-491: Theinfofield is properly initialized. No changes needed.Verification confirms the implementation is safe:
All PLiSQL_var and PLiSQL_rec structures are allocated using
palloc0, which initializes theinfofield to 0 for non-parameter variables.Function parameters explicitly set
infoto the parameter mode:PROARGMODE_IN('i'),PROARGMODE_OUT('o'), orPROARGMODE_INOUT('b').The check
if (info == PROARGMODE_IN)correctly distinguishes IN parameters (info='i') from non-parameters (info=0), preventing false positives.src/bin/psql/po/zh_CN.po (1)
3008-3012: Translation and option flags verified; msgfmt validation deferred to manual checkThe Chinese translation "列出包" correctly translates "list packages," and the option flags
[S+]are consistent between the English msgid and Chinese msgstr. The PO file entry is syntactically sound with proper format markers. However, msgfmt is unavailable in the sandbox environment. Please manually runmsgfmt -c -o /dev/null src/bin/psql/po/zh_CN.poto validate the catalog compiles without errors.src/backend/executor/execTuples.c (1)
2208-2216: Verified: subproc_should_change_return_type() properly declared and inexpensiveThe function is correctly exported in
src/include/funcapi.h:286. The underlyingplisql_internel_funcs_init()uses a guard flag (if (!plisql_internal_funcs.isload)) that makes it cheap after the first call—just a flag check and return. The function is called per-targetlist entry at query planning time, not per-row, mirroring the PG-proc path above it. Cost is acceptable.src/backend/executor/execSRF.c (1)
138-141: Verify the SPI connection check logic.The condition
SPI_get_connected() < 0means SPI is not connected. This seems counterintuitive—why would return type adjustment only apply when SPI is not connected? Typically, SPI connection status indicates whether we're executing from within a PL function context.Please verify this is the intended behavior and consider adding a comment explaining why this check is necessary.
src/backend/executor/execExpr.c (1)
5142-5143: Verify memory management for function argument metadata.The
get_func_arg_info()andget_subproc_arg_info()functions allocate memory forargtypes,argnames, andargmodes(via palloc internally), but there are no correspondingpfree()calls in this function.Please verify that these allocations happen in the appropriate memory context that will be automatically freed (e.g., per-query context) and won't accumulate across multiple expression initializations. If these should be explicitly freed, please add the necessary cleanup.
Also applies to: 5152-5153
src/pl/plisql/src/pl_package.c (1)
3561-3580: LGTM: prokind lookup is straightforward.src/pl/plisql/src/pl_subproc_function.c (5)
170-195: Safe transfer of newly compiled subprocs into function->subprocfuncs.Resize and append logic is sound; assertions help guard boundaries.
3057-3063: Guard against self-referential arg scanning.The additional check avoids misclassifying top-level args; good defensive change.
897-910: Wiring new public hooks is complete.The registry includes all three new helpers; good integration point.
46-46: GUC symbolout_parameter_column_positionis properly declared and defined with no link-time or ODR concerns.The extern declaration in
src/include/utils/guc.h:356withPGDLLIMPORTcorrectly exports the symbol; the single definition exists insrc/backend/utils/misc/ivy_guc.c:47and is registered in the GUC system. The include of"utils/guc.h"inpl_subproc_function.cis correct and necessary.
344-370: Arg mode enforcement verified — no issues found.Assignment checks are already properly enforced. The
infofield is checked in pl_gram.y (lines 4472, 4485, 4544, 4557) where attempts to assign to IN-mode parameters raise errors: "cannot be used as an assignment target." The enforcement covers bothPLiSQL_varandPLiSQL_rectypes at the grammar level, which is the appropriate validation point. Additionally, initialization logic in pl_exec.c correctly distinguishes IN/INOUT (initialized from call args) from OUT (skipped).src/pl/plisql/src/sql/plisql_nested_subproc2.sql (1)
23-29: OUT param mutation test is appropriate.Switching to “in out” verifies pass-by-reference semantics; keep as-is.
Please add an assertion on both the returned value and the mutated actual to prevent regressions (e.g., SELECT r, a FROM (SELECT test_subproc_func(23) AS r, 23 AS a) s).
src/pl/plisql/src/sql/plisql_nested_subproc.sql (3)
30-31: OUT parameter signature changes look correct.The transition from
function square(original integer)tofunction square(original in out integer)aligns with the PR goal of adding OUT parameter support. The function bodies (lines 36-38, 200-206) correctly modify the OUT parameter before returning, which matches Oracle OUT semantics.Also applies to: 197-198
801-825: Nested function OUT parameter propagation is sound.The test at lines 801-825 properly demonstrates nested functions with OUT parameters. The inner function at line 805 modifies both
id(IN OUT) andsalary(OUT) parameters, and the outer function correctly calls it (line 816) and propagates the results. The return type shift fromrecordtointeger(line 801) is consistent with OUT parameter semantics.
2189-2210: Polymorphic OUT parameters with complex types—verify semantics.Lines 2189-2210 extend OUT parameter support to polymorphic types (
anyelement,anyarray,anycompatiblerange, etc.). The test invokesf1(...)multiple times with different type combinations and captures OUT results invar1,var2,var3. Spot-check: Line 2198 callsf1(11, array[1, 2], 42, 34.5, var1, var2)and expectsvar1andvar2to be populated withanyarrayandanycompatiblearraytypes respectively. This appears logically consistent with the function signature at line 2187–2189, but confirm that the polymorphic type inference and OUT parameter binding work correctly in practice.
| funcrettype = resulttype; | ||
| funcexpr->funcresulttype = resulttype; | ||
| funcexpr->funccollid = chcollationoid; |
There was a problem hiding this comment.
Potential issue: Modifying FuncExpr during execution may cause problems with plan reuse.
The code directly modifies funcexpr->funcresulttype and funcexpr->funccollid during execution. FuncExpr nodes are typically part of the plan tree and should be treated as read-only during execution to support plan caching and reuse. If the same plan is executed multiple times or in different sessions, these mutations could cause incorrect results or corruption.
Consider storing the adjusted type information in the execution state (e.g., in SetExprState) rather than modifying the plan tree node.
Also applies to: 162-164
| { | ||
| FuncExpr *func = (FuncExpr *) lfirst(lc); | ||
| int i; | ||
| ListCell *lc1; | ||
| Oid *argtypes = NULL; | ||
| char **argnames = NULL; | ||
| char *argmodes= NULL; | ||
| HeapTuple procTup = NULL; | ||
| char *proname = NULL; | ||
|
|
||
| procTup = SearchSysCache1(PROCOID, | ||
| ObjectIdGetDatum(func->funcid)); | ||
| if (!IsA(func, FuncExpr)) | ||
| return; | ||
|
|
||
| if (!HeapTupleIsValid(procTup)) | ||
| ereport(ERROR, | ||
| (errcode(ERRCODE_DATA_EXCEPTION), | ||
| errmsg("cache lookup failed for function %u", func->funcid))); | ||
| if (FUNC_EXPR_FROM_PG_PROC(func->function_from)) | ||
| { | ||
| Form_pg_proc procStruct; | ||
|
|
There was a problem hiding this comment.
Do not exit the loop when the item isn’t a FuncExpr.
If a ROWS FROM entry contains several expressions, the first non-FuncExpr element causes this function to return, so the remaining function expressions—possibly with OUT/INOUT args needing validation—are skipped entirely. That regresses the guard that enforces “OUT args must be variables”. Instead, just skip non-FuncExpr entries and keep checking the rest.
- FuncExpr *func = (FuncExpr *) lfirst(lc);
+ Node *item = lfirst(lc);
+ FuncExpr *func;
+
+ if (!IsA(item, FuncExpr))
+ continue;
+
+ func = castNode(FuncExpr, item);
-
- if (!IsA(func, FuncExpr))
- return;🤖 Prompt for AI Agents
In src/backend/parser/parse_clause.c around lines 3964-3980 the loop currently
does "return" when an item is not a FuncExpr which aborts processing remaining
ROWS FROM elements and skips validation for other function expressions; change
the control flow to skip the non-FuncExpr item and continue the loop (i.e.,
replace the early return with logic that simply moves to the next list element)
so subsequent FuncExpr nodes are still examined and their OUT/INOUT arguments
validated.
| HELP0(" \\dg[Sx+] [PATTERN] list roles\n"); | ||
| HELP0(" \\di[Sx+] [PATTERN] list indexes\n"); | ||
| HELP0(" \\dl[x+] list large objects, same as \\lo_list\n"); | ||
| HELP0(" \\dk[S+] [PATTERN] list packages\n"); |
There was a problem hiding this comment.
🧩 Analysis chain
Add x-option or confirm it’s intentionally unsupported
Most “Informational” list commands advertise [Sx+] when they support expanded mode. If \dk does not support -x, fine; otherwise update to [Sx+]. Also ensure ref/psql-ref.sgml reflects this new command, per the header note in this file.
Run to spot-check related places:
🏁 Script executed:
#!/bin/bash
# Verify help entry, parser, describe support, and docs.
rg -nP "\\\\dk\\[.*\\]" src/bin/psql
rg -nP "case.*K" --type=c src/bin/psql # look for backslash command handling
rg -nP "describe.*package|packages" src/bin/psql
rg -nP "\\\\dk\\[Sx\\+\\]|\\\\dk\\[S\\+\\]" src/bin/psql/po
rg -nP "\\\\dk" doc -g '!**/html/**' || rg -nP "\\\\dk" src/bin/psql/doc -SLength of output: 2111
I need to investigate the actual \dk command implementation to determine if it supports expanded mode. Let me search for the command handler and documentation.
🏁 Script executed:
#!/bin/bash
# Find \dk command implementation and check if it supports expanded/verbose mode
rg -nP "describePackages|packageQuery" src/bin/psql --type=c -A 5
rg -nP "case.*'k'" src/bin/psql --type=c -B 2 -A 2
rg -nP "\\\\dk" src/bin/psql/doc src/bin/psql/poLength of output: 218
🌐 Web query:
PostgreSQL psql \dk command expanded mode support
💡 Result:
Yes — recent psql adds an "x" suffix to list-style backslash commands (the \d* family) to force expanded output for that command only. So you can use \dkx or \dk+x to show operators in expanded mode. [1][2]
Sources:
- psql change/commit adding per-command expanded-mode "x". [1]
- psql documentation (psql list commands / \d* / expanded-mode). [2]
I need to check what other list commands show in the help text to understand the pattern, and verify if the \dk function actually implements expanded mode support.
🏁 Script executed:
#!/bin/bash
# See what other \d* commands show in help text to establish the pattern
rg -nP "HELP0.*\\\\d[a-z].*\[S" src/bin/psql/help.c -A 0
# Find the describePackages function to check if it accepts expanded/verbose parameters
rg -nP "describePackages|static.*packages" src/bin/psql/describe.c -A 10 -B 1
# Check if S+ or Sx+ appears elsewhere in help
rg -nP "\\[S\+" src/bin/psql/help.c | head -20Length of output: 1422
Now let me verify the command parser handles the 'x' flag for \dk, and check the documentation:
🏁 Script executed:
#!/bin/bash
# Find where \k command is dispatched and check if x flag is handled
rg -nP "case 'k':|\\\\\\\\k" src/bin/psql/command.c -B 5 -A 10
# Search for psql-ref.sgml
find . -name "psql-ref.sgml" -o -name "*psql*ref*" 2>/dev/null | grep -v ".git"
# Check if \dk is documented in sgml
rg -nP "\\\\dk" . -g "*.sgml" 2>/dev/null | head -10Length of output: 577
Let me check the documentation and verify the flag parsing:
🏁 Script executed:
#!/bin/bash
# Check psql-ref.sgml for \dk documentation
rg -nP "\\\\dk" ./doc/src/sgml/ref/psql-ref.sgml -B 2 -A 5
# Find where backslash command flags are parsed (look for where 'x' maps to show_verbose)
rg -nP "show_verbose.*=|strchr.*'x'" src/bin/psql/command.c -B 3 -A 3 | head -40Length of output: 1264
Update help text to [Sx+] and add \dk documentation to psql-ref.sgml
The \dk command implementation supports expanded mode (code at command.c:1070 checks for 'x' flag), but the help text at help.c:251 shows only [S+] instead of [Sx+]. This is inconsistent with all other list commands (\da[Sx], \dc[Sx+], \dg[Sx+], etc.).
Additionally, \dk is not documented in doc/src/sgml/ref/psql-ref.sgml—add it to match the reference documentation pattern for other \d* commands.
Changes needed:
- help.c:251: Change
\dk[S+]to\dk[Sx+] - doc/src/sgml/ref/psql-ref.sgml: Add
\dkdocumentation entry
🤖 Prompt for AI Agents
In src/bin/psql/help.c around line 251, update the help text token from
"\dk[S+]" to "\dk[Sx+]" to reflect support for expanded mode (the 'x' flag); and
in doc/src/sgml/ref/psql-ref.sgml add a new documentation entry for the \dk
command following the same pattern and placement used for other \d* list
commands (use the existing \da/\dc/\dg entries as templates for tag names,
synopsis, description, and examples) so the reference includes \dk and documents
the S and x flags.
| plisql_subproc_should_change_return_type(FuncExpr *fexpr, | ||
| Oid *rettype, | ||
| int32 *typmod, | ||
| Oid *collationoid) | ||
| { | ||
| PLiSQL_function *function = NULL; | ||
| PLiSQL_subproc_function *subprocfunc = NULL; | ||
| Oid real_retype; | ||
| int result = false; | ||
|
|
||
| if (FUNC_EXPR_FROM_PG_PROC(fexpr->function_from)) | ||
| elog(ERROR, "FuncExpr is not an internal function"); | ||
|
|
||
| if (fexpr->parent_func == NULL) | ||
| elog(ERROR, "parent_func has not been set"); | ||
|
|
||
| function = (PLiSQL_function *) fexpr->parent_func; | ||
|
|
||
| if (fexpr->funcid < 0 || fexpr->funcid >= function->nsubprocfuncs) | ||
| elog(ERROR, "invalid fno %d", fexpr->funcid); | ||
|
|
||
| subprocfunc = function->subprocfuncs[fexpr->funcid]; | ||
| real_retype = (subprocfunc->rettype == NULL ? VOIDOID : subprocfunc->rettype->typoid); | ||
|
|
||
| if (real_retype == RECORDOID) | ||
| return false; | ||
|
|
||
| /* | ||
| * If there are more than one out-parameters, or only one out-parameter | ||
| * and return type does exist, change the rettype to be record. | ||
| */ | ||
| if (subprocfunc->noutargs >= 2 || | ||
| (subprocfunc->noutargs == 1 && | ||
| real_retype != VOIDOID)) | ||
| { | ||
| result = true; | ||
| *rettype = RECORDOID; | ||
| *typmod = -1; | ||
| *collationoid = 0; | ||
| } | ||
| else if (subprocfunc->noutargs == 1 && | ||
| real_retype == VOIDOID) | ||
| { | ||
| /* only one out-parameter, change the rettype to be out-parameter type */ | ||
| ListCell *lc; | ||
| bool found = false; | ||
|
|
||
| foreach (lc, subprocfunc->arg) | ||
| { | ||
| PLiSQL_function_argitem *argitem = (PLiSQL_function_argitem *) lfirst(lc); | ||
|
|
||
| if (argitem->argmode == ARGMODE_OUT || | ||
| argitem->argmode == ARGMODE_INOUT) | ||
| { | ||
| found = true; | ||
| result = true; | ||
| *rettype = argitem->type->typoid; | ||
| *typmod = argitem->type->atttypmod; | ||
| *collationoid = argitem->type->collation; | ||
| break; | ||
| } | ||
| } | ||
| Assert(found); | ||
| } | ||
| return result; | ||
| } |
There was a problem hiding this comment.
Return a proper bool (not int) from plisql_subproc_should_change_return_type.
Use a bool local for clarity and type safety.
Apply this diff:
- Oid real_retype;
- int result = false;
+ Oid real_retype;
+ bool result = false;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/pl/plisql/src/pl_package.c around lines 3588 to 3653, the function
plisql_subproc_should_change_return_type currently returns an int and declares
"int result = false"; change the function return type from int to bool and
change the local variable to "bool result = false" so the function uses a proper
bool for type safety; keep the existing true/false returns and final "return
result;" as-is (no other logic changes required).
Summary by CodeRabbit
Release Notes
New Features
\dkcommand to psql for listing packages.Improvements