Reviewing files that changed from the base of the PR and between c51d958 and c58c5ec.

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

• src/pl/plisql/src/Makefile (4 hunks)
• src/pl/plisql/src/pl_autonomous.c (1 hunks)
• src/pl/plisql/src/pl_autonomous.h (1 hunks)
• src/pl/plisql/src/pl_exec.c (4 hunks)
• src/pl/plisql/src/pl_gram.y (3 hunks)
• src/pl/plisql/src/pl_handler.c (6 hunks)
• src/pl/plisql/src/pl_unreserved_kwlist.h (2 hunks)
• src/pl/plisql/src/plisql.h (2 hunks)
• src/pl/plisql/src/sql/plisql_autonomous.sql (1 hunks)

src/pl/plisql/src/pl_unreserved_kwlist.h (1)

40-40: New unreserved keywords look correctly wired here

autonomous_transaction and pragma are added in ASCII order and use distinct token codes (K_AUTONOMOUS_TRANSACTION, K_PRAGMA), which is consistent with the surrounding list. Any follow‑up alignment between this list and unreserved_keyword in pl_gram.y is handled in that file’s review.

Also applies to: 105-105

src/pl/plisql/src/plisql.h (1)

1065-1076: Autonomous-transaction flags are cleanly integrated into function and global state

Adding fn_is_autonomous to PLiSQL_function alongside the other per-function flags, and exposing plisql_inside_autonomous_transaction as a global, is a straightforward way to surface autonomous-transaction state to both the compiler and executor paths. Struct layout remains coherent, and the new fields are clearly documented by their naming and comment.

Assuming the corresponding definitions, GUC wiring, and initialization are implemented in the C modules, this header change looks good.

Also applies to: 1278-1279

src/pl/plisql/src/pl_exec.c (3)

45-45: pl_autonomous.h include placement looks correct

Including pl_autonomous.h alongside other PL/iSQL headers is appropriate and keeps the dependency localized to the executor. No issues here.

---

490-513: Function-level comment matches current plisql_exec_function behavior

The expanded comment accurately describes the responsibilities and parameters of plisql_exec_function, including autonomous-transaction support and the role of procedure_resowner/atomic. This improves readability without affecting behavior.

---

9884-9931: No functional change in plisql_anonymous_return_out_parameter

Only the closing brace line is touched here; the implementation of plisql_anonymous_return_out_parameter itself is unchanged and remains consistent with how anonymous blocks with OUT parameters are handled elsewhere.

src/pl/plisql/src/pl_handler.c (3)

62-63: LGTM - Global flag for autonomous transaction detection.

The global boolean plisql_inside_autonomous_transaction is correctly declared and will be controlled by the GUC defined later in _PG_init. This enables detection of recursive autonomous transaction attempts.

---

202-215: LGTM - Well-designed hidden GUC for recursion prevention.

The GUC design is sound:

• PGC_USERSET is required since the dblink session must be able to SET this flag
• Hidden flags (GUC_NOT_IN_SAMPLE | GUC_NO_SHOW_ALL | GUC_NO_RESET_ALL) appropriately prevent user discovery while allowing the autonomous session to modify it
• The comment clearly documents the design rationale

---

248-250: LGTM - Initialization hook for autonomous transaction support.

The call to plisql_autonomous_init() correctly registers the syscache invalidation callback for dblink OID caching, placed appropriately near the end of _PG_init after other module initialization.

src/pl/plisql/src/pl_autonomous.h (1)

1-45: LGTM - Clean public header for autonomous transaction support.

The header is well-structured with:

• Proper include guard
• Minimal dependency on plisql.h for required types
• Clear documentation comments for each API function
• Declarations correctly match the implementations in pl_autonomous.c

src/pl/plisql/src/Makefile (2)

55-57: LGTM - Correct EXTRA_INSTALL for dblink dependency.

EXTRA_INSTALL = contrib/dblink ensures the dblink extension is built and installed in the test instance before running the autonomous transaction tests.

---

109-109: LGTM - Build dependencies correctly updated.

The dependency line properly includes pl_autonomous.o in the list and adds pl_autonomous.h as a prerequisite, ensuring correct rebuild ordering when the autonomous transaction module changes.

src/pl/plisql/src/pl_autonomous.c (4)

251-275: Consider handling BYTEA and other binary types explicitly.

The switch statement handles common scalar types but falls through to quote_literal_cstr for everything else. For BYTEA (binary data), OidOutputFunctionCall returns a hex-encoded string, which quote_literal_cstr will quote correctly. However, depending on the bytea_output setting, this could produce \x... or escape format, both of which should work but may warrant a note.

The current fallback to quote_literal_cstr is safe and handles SQL injection correctly. This is acceptable for initial implementation.

---

395-404: Double-free of query on error path.

In the PG_CATCH block, pfree(query) is called at line 399, then PG_RE_THROW() is called. However, if an error occurs, execution never reaches line 404. But wait - PG_RE_THROW() doesn't return, so line 404 is only reached on success. Let me re-analyze...

Actually, on closer inspection: PG_RE_THROW() does not return, so line 404 is only reached on the success path. The cleanup is correct. However, for clarity and consistency with other cleanup patterns in this file, consider restructuring:

The error handling is correct - PG_RE_THROW() never returns, so line 404 is only executed on success.

---

1-33: LGTM - File structure and includes are appropriate.

The includes cover all necessary headers for the functionality:

• Catalog access for procedure/type lookups
• SPI for query execution
• libpq-be for MyProcPort access
• Syscache invalidation infrastructure

---

423-426: I'm unable to access the IvorySQL repository directly due to cloning issues, and web search yields no specific documentation about the internal GUC plisql_inside_autonomous_transaction or the recursion prevention mechanism described in your review comment.

Manual verification required.

The review comment makes specific claims about:

1. The GUC being set in SQL at lines 286 and 296
2. Missing recursion checks at the function entry (lines 423-426)
3. A potential workaround via pl_exec.c

Since I cannot access the source code to verify these details, and there is no public documentation available for this internal mechanism, you will need to manually verify:

• Whether plisql_inside_autonomous_transaction is actually checked before plisql_exec_autonomous_function executes
• Whether recursion guards exist upstream in the calling code (likely in pl_exec.c)
• Whether the dblink-based approach with the GUC setting actually provides adequate recursion prevention

If such checks are confirmed to exist elsewhere in the codebase, the review comment should be updated to reference their location or be resolved. If no checks exist, the concern is valid and warrants correction.

src/pl/plisql/src/sql/plisql_autonomous.sql (3)

301-317: Test 15 relies on psql error behavior - verify expected output file handles error.

Test 15 intentionally triggers an error by inserting into nonexistent_table. The test has commented-out \set ON_ERROR_STOP directives. Ensure the expected output file (plisql_autonomous.out) correctly captures this error, or the test may cause the entire regression suite to fail depending on psql settings.

---

1-454: LGTM - Comprehensive test coverage for autonomous transactions.

The test suite provides good coverage including:

• Basic procedure/function execution
• Transaction isolation (autonomous commits survive outer rollbacks)
• Multiple parameter types and NULL handling
• OID invalidation after extension recreation
• Pass-by-reference return types (TEXT, NUMERIC)
• Error handling paths
• Appropriate cleanup at the end

The commented tests for explicit COMMIT/ROLLBACK clearly document the known limitation (Issue #985).

---

343-367: I'll help you verify the nested autonomous function behavior. Let me search the codebase for the recursion prevention logic and how it handles nested autonomous calls.
<function_calls>


#!/bin/bash

Search for the recursion prevention logic and nested autonomous handling

rg -n "plisql_inside_autonomous" --type c -B 2 -A 5


</function_calls>

This will help me understand how the recursion check works. Let me also search for where this variable is defined and used:
<function_calls>


#!/bin/bash

Find definition and all usages of the recursion check

rg -n "inside_autonomous_transaction" --type c -B 1 -A 2 | head -100


</function_calls>

Let me also look at the actual test file to understand the full context around Test 17:
<function_calls>


#!/bin/bash

Read the test file to see the full Test 17 and surrounding context

cat -n src/pl/plisql/src/sql/plisql_autonomous.sql | sed -n '330,380p'


</function_calls>

Let me also search for dblink connection handling and how autonomous transactions interact with dblink:
<function_calls>


#!/bin/bash

Search for dblink and autonomous transaction interaction

rg -n "dblink|autonomous" src/pl/plisql/src/ --type c -B 1 -A 3 | head -150


</function_calls>

Reviewing files that changed from the base of the PR and between 1cab728 and 160e8bb.

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

• src/pl/plisql/src/Makefile (4 hunks)
• src/pl/plisql/src/pl_autonomous.c (1 hunks)
• src/pl/plisql/src/pl_autonomous.h (1 hunks)
• src/pl/plisql/src/pl_exec.c (3 hunks)
• src/pl/plisql/src/pl_gram.y (3 hunks)
• src/pl/plisql/src/pl_handler.c (6 hunks)
• src/pl/plisql/src/pl_unreserved_kwlist.h (1 hunks)
• src/pl/plisql/src/plisql.h (2 hunks)
• src/pl/plisql/src/sql/plisql_autonomous.sql (1 hunks)

• src/pl/plisql/src/pl_gram.y
• src/pl/plisql/src/pl_autonomous.h
• src/pl/plisql/src/pl_unreserved_kwlist.h

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

src/pl/plisql/src/sql/plisql_autonomous.sql (9)

1-15: Setup is clear and complete.

Test table and dblink extension correctly provisioned. The schema (id, msg, tx_state) is well-suited for verifying autonomous transaction semantics (commit/rollback isolation).

---

20-96: Test 1–4: Basic functionality and parameter handling look solid.

Each test follows a consistent pattern: define, commit, call, verify with SELECT. Tests progressively exercise no parameters → scalar parameters → multiple types (INT, TEXT, BOOLEAN). The isolation test (lines 53–71) is particularly good: it confirms autonomous commits survive outer ROLLBACK, which is the core semantic.

---

100-130: Tests 5–6: NULL handling and sequential calls well covered.

Test 5 (lines 100–111) validates that NULL parameters are properly passed and handled with COALESCE. Test 6 (lines 116–130) confirms multiple back-to-back autonomous calls don't interfere with each other. Both are essential for real-world use.

---

143-166: Test 8: Persistence semantics verified correctly.

The mid-suite TRUNCATE at line 143 is intentional for test isolation, not a red flag. The test cleanly demonstrates that autonomous inserts persist even when the outer transaction rolls back. The inline SELECT COUNT(*) at line 162 within the transaction is a nice touch to show visibility.

---

171-192: Test 9: OID invalidation testing is thorough but operationally risky.

Dropping and recreating the dblink extension (lines 185–186) tests the syscache invalidation callback. This is essential for confirming cached OID correctness. However, if this test fails mid-execution, subsequent tests depend on dblink being present. In a production test harness, this test should be isolated or ordered last. For a single-file test, this is acceptable since cleanup happens afterward.

Confirm whether this test file runs as a single atomic unit or if individual tests can be reordered/skipped. If it's meant to be run with psql -f, consider whether dropping dblink mid-suite could cause issues if an earlier test fails.

---

197-429: Tests 10–20: Comprehensive type coverage and nested/error cases.

Excellent progression through return types (INT, TEXT, NULL, NUMERIC, DATE, BOOLEAN), nested autonomous calls (Test 17, lines 343–367), and pass-by-reference handling. Test 18–20 specifically exercise NUMERIC (variable-length), DATE, and BOOLEAN to stress datumCopy() code paths. Test 15 (error handling) inserts into a nonexistent table to test exception cleanup (SPI_finish, pfree).

---

240-296: Commented COMMIT/ROLLBACK tests appropriately documented.

Four tests (Tests 12–14) exercising explicit COMMIT/ROLLBACK and conditional transaction control are commented with Issue #985 references, making it clear these are known limitations rather than oversights. This is good practice for documenting intent.

---

437-454: Cleanup is comprehensive and matches all defined procedures/functions.

All 17 procedures/functions plus the test table are dropped. The cleanup will succeed even if earlier tests failed (assuming CASCADE is handled gracefully). No dangling objects.

---

301-318: Verify test execution and error-handling behavior for intentional errors in test_function_error().

Lines 315–317 appear to have \set ON_ERROR_STOP off/on directives commented out around an expected error test. Confirm: (1) whether the test harness or runner catches errors automatically, (2) the intended execution mode (manual interactive testing vs. automated suite), and (3) whether execution should halt or continue past this intentional error. Update the code or documentation accordingly.

src/pl/plisql/src/plisql.h (2)

1067-1067: LGTM!

The fn_is_autonomous field is well-placed alongside other function metadata flags like has_exception_block. This correctly extends the function struct to track autonomous transaction capability at compile time.

---

1280-1281: LGTM!

The extern declaration for plisql_inside_autonomous_transaction follows the established pattern for other PL/iSQL global flags (e.g., plisql_check_asserts, plisql_print_strict_params). The blank line after the declaration maintains consistent formatting.

src/pl/plisql/src/pl_handler.c (5)

27-27: LGTM!

Header include is correctly placed with other module-specific headers.

---

62-63: LGTM!

Clear initialization of the recursion-prevention flag with a descriptive comment.

---

154-165: LGTM!

Good documentation update describing the initialization flow, including the new autonomous-transaction support. The warning about not making the function static is helpful for maintainability.

---

202-215: Verify security implications of PGC_USERSET for internal flag.

The comment correctly explains why PGC_USERSET is needed (dblink sessions must SET it). However, this means any user can manually set plisql.inside_autonomous_transaction = true to bypass autonomous transaction execution (causing an error), or set it to false to potentially bypass the recursion guard.

The hiding flags (GUC_NOT_IN_SAMPLE | GUC_NO_SHOW_ALL | GUC_NO_RESET_ALL) provide obscurity but not security. Consider documenting this trade-off or adding a check in the autonomous execution path that the flag is only being set from a trusted context (though this may be impractical with dblink).

---

248-250: LGTM!

Initialization call is correctly placed after other module setup and before marking inited = true. The comment accurately describes the purpose.

src/pl/plisql/src/Makefile (6)

42-42: LGTM!

New object file correctly added to the build in alphabetical order.

---

56-58: LGTM!

Good addition of EXTRA_INSTALL = contrib/dblink with a clear comment explaining the dependency. This ensures the dblink extension is built and installed for the regression tests.

---

66-66: LGTM!

Test file added to the regression suite.

---

95-95: LGTM!

Header installation follows the existing pattern for other public headers (plisql.h, pl_subproc_function.h, pl_package.h).

---

104-104: LGTM!

Uninstall rule correctly mirrors the install rule.

---

110-110: LGTM!

Dependency rule correctly updated to include both pl_autonomous.o and pl_autonomous.h, ensuring proper rebuild when the autonomous transaction module changes.

src/pl/plisql/src/pl_exec.c (3)

46-46: Include of pl_autonomous.h is appropriate and scoped correctly

The new include cleanly introduces the autonomous-transaction API (plisql_exec_autonomous_function, plisql_inside_autonomous_transaction) into pl_exec.c without impacting existing dependencies. No issues here.

---

487-515: Updated plisql_exec_function documentation and prototype look consistent

The expanded comment accurately reflects the current behavior and signature (simple-eval estate, both resource owners, and atomic flag), including autonomous transactions and set-returning results. The added forward declaration for plisql_anonymous_return_out_parameter matches its definition below and is correctly typed.

---

517-538: Verify all call sites of plisql_exec_function populate procedure_resowner and atomic correctly

The concern about caller consistency is valid. Ensure that every invocation of plisql_exec_function passes appropriate values:

• Expression/SELECT paths: procedure_resowner = NULL, atomic = true
• CALL/DO paths: appropriate long-lived procedure_resowner, atomic = false

Inconsistent parameter passing across call sites could lead to subtle lifecycle or atomicity bugs.

src/pl/plisql/src/pl_autonomous.c (7)

34-66: LGTM!

The syscache invalidation callback pattern for caching dblink function OIDs is correctly implemented. The callback properly resets both cached OIDs when pg_proc changes, ensuring stale OIDs are never used.

---

78-103: LGTM!

Defensive checks for MyProcPort and database_name are appropriate. The function correctly handles edge cases like background workers and standalone mode with clear error messages.

---

157-179: LGTM!

The validation logic is correct, and deferring the dblink availability check to execution time (rather than procedure creation time) is a sensible design choice that avoids dependency issues during DDL.

---

186-190: LGTM!

Clean implementation using get_extension_oid with missing_ok=true.

---

280-300: LGTM on SQL construction with proper quoting.

The use of quote_literal_cstr for non-numeric types and direct formatting for validated numeric types provides adequate SQL injection protection. The session variable settings (ivorysql.compatible_mode, plisql.inside_autonomous_transaction) are correctly embedded.

---

386-391: Good memory context handling for pass-by-reference return values.

Correctly switches to fn_mcxt before copying the result to ensure it survives after SPI_finish() releases SPI memory.

---

503-515: Consistent error handling with cleanup across both function and procedure paths.

Both paths correctly use PG_TRY/PG_CATCH to ensure memory cleanup (connstr_buf.data, sql, dbname) before re-throwing. Good defensive programming.

Also applies to: 530-544

Reviewing files that changed from the base of the PR and between 160e8bb and f85ece9.

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

• src/pl/plisql/src/Makefile (4 hunks)
• src/pl/plisql/src/pl_autonomous.c (1 hunks)
• src/pl/plisql/src/pl_autonomous.h (1 hunks)
• src/pl/plisql/src/pl_exec.c (3 hunks)
• src/pl/plisql/src/pl_gram.y (3 hunks)
• src/pl/plisql/src/pl_handler.c (6 hunks)
• src/pl/plisql/src/pl_unreserved_kwlist.h (1 hunks)
• src/pl/plisql/src/plisql.h (2 hunks)
• src/pl/plisql/src/sql/plisql_autonomous.sql (1 hunks)

• src/pl/plisql/src/pl_handler.c
• src/pl/plisql/src/pl_autonomous.h
• src/pl/plisql/src/pl_unreserved_kwlist.h

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

src/pl/plisql/src/plisql.h (1)

1067-1068: Autonomous flags are well-integrated into core PL/iSQL structures

Adding fn_is_autonomous to PLiSQL_function alongside other per-function flags, and exposing plisql_inside_autonomous_transaction for the GUC-backed execution-context flag, is consistent with the existing design and should default correctly with zeroed allocations. Note that plugins that depend on the exact PLiSQL_function layout will need a rebuild against this updated header, which is expected when introducing a feature of this scope.

If you ship out-of-tree instrumentation plugins, please recompile them against this new header to avoid layout mismatches.

Also applies to: 1280-1281

src/pl/plisql/src/Makefile (1)

42-42: Build/test wiring for autonomous support is consistent; mind the new dblink dependency

• Adding pl_autonomous.o to OBJS and to the explicit dependency line, and exporting pl_autonomous.h via install/uninstall, correctly integrates the new module into the core PL/iSQL shared library and its public headers.
• Extending REGRESS with plisql_autonomous ensures the autonomous-transaction test suite runs under oracle-(install)check.
• EXTRA_INSTALL = contrib/dblink makes dblink a required installed extension for this PL’s tests (and possibly for installs, depending on your top-level rules), which matches the implementation choice but tightens the build-time dependency on contrib. This is fine if dblink is always built in your supported configurations; otherwise, consider documenting this requirement or guarding it with the same conditions under which contrib/dblink is available.

Please verify in your CI/build matrix that environments which skip building contrib still either (a) build/install contrib/dblink or (b) are adjusted to skip autonomous PL/iSQL tests accordingly.

Also applies to: 56-59, 66-67, 95-96, 104-105, 110-111

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

46-46: Autonomous header inclusion is appropriate

Including pl_autonomous.h here is the right integration point for the new autonomous-transaction delegation path; no issues spotted.

src/pl/plisql/src/sql/plisql_autonomous.sql (5)

7-15: LGTM: Clean test setup.

The extension installation and table creation are appropriate for the test suite. The tx_state column with a default value helps document the expected state of each test row.

---

17-131: LGTM: Comprehensive basic test coverage.

Tests 1-6 thoroughly verify core autonomous transaction functionality including parameter handling, transaction isolation, multiple data types, NULL handling, and sequential calls. The isolation test (Test 3) correctly demonstrates that autonomous commits survive outer rollbacks.

---

236-296: Known limitation properly documented.

The commented tests for explicit COMMIT/ROLLBACK within autonomous blocks correctly document the known limitation referenced in Issue #985. The comments clearly explain the expected failures and the current behavior, which aligns with the PR summary.

---

314-317: Verify error test execution behavior.

The \set ON_ERROR_STOP off/on directives are commented out, meaning Test 15 will cause the test script to fail and stop execution at the intentional error. If this test is meant to verify graceful error handling without stopping the test suite, uncomment these directives.

Do you want the error handling test to continue execution after the error, or should it stop the test suite? If it should continue, uncomment lines 315 and 317.

---

437-454: LGTM: Complete cleanup.

All created test objects (8 procedures, 9 functions, and 1 table) are properly dropped in the cleanup section.

src/pl/plisql/src/pl_autonomous.c (8)

34-66: LGTM: Correct OID caching with invalidation.

The OID caching strategy with syscache invalidation callback is correctly implemented. Both dblink_exec_oid and dblink_oid are properly reset when pg_proc changes, ensuring stale function OIDs are not used after extension drop/recreate.

---

78-103: LGTM: Robust database name retrieval.

The function correctly validates that both MyProcPort and database_name are available before use, with clear error messages explaining the context (background processes vs. missing connection info). The use of pstrdup ensures the returned string is allocated in the caller's memory context.

---

113-148: LGTM: Secure name construction with proper error handling.

The function correctly constructs a schema-qualified, SQL-injection-safe function name using quote_identifier for both components. The concurrent schema drop scenario is properly handled with a clear error message, as noted in the addressed past review comments.

---

161-183: LGTM: Proper pragma validation.

The function correctly validates placement and prevents duplicate pragmas. Deferring the dblink availability check to execution time (lines 176-180) is a sound design choice that avoids issues during procedure creation.

---

190-194: LGTM: Clean extension availability check.

Uses the standard PostgreSQL API appropriately with missing_ok=true to avoid errors when the extension is not installed.

---

210-316: LGTM: Comprehensive SQL construction with proper quoting.

The function correctly:

• Validates argument count (addresses past review comment)
• Handles NULL arguments explicitly
• Applies type-appropriate formatting (unquoted numerics, boolean conversion, quoted literals)
• Uses quote_literal_cstr for string safety
• Sets required session flags (ivorysql.compatible_mode and plisql.inside_autonomous_transaction)
• Manages memory correctly (returns sql.data for caller to free)

---

327-420: LGTM: Robust execution with proper error handling.

The function correctly implements autonomous function execution with:

• OID caching for dblink()
• Proper SPI connection tracking with spi_connected flag (addresses past review comment)
• Correct result extraction and NULL handling
• Memory context switching for pass-by-reference types to ensure data survives SPI_finish()
• Comprehensive error cleanup in the catch block

---

435-567: LGTM: Complete autonomous execution orchestration.

The function correctly:

• Builds libpq-safe connection strings with proper escaping for single quotes and backslashes
• Branches between function (SELECT via dblink + SPI) and procedure (CALL via dblink_exec) paths
• Includes comprehensive error handling with cleanup in both paths
• Returns appropriate values (actual Datum for functions, NULL with isnull=true for procedures)

The listen_addresses logic (lines 497-503) was discussed in past review comments and is reasonable for local connections with Unix socket fallback.

Reviewing files that changed from the base of the PR and between f85ece9 and 72d0173.

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

• src/pl/plisql/src/pl_exec.c (3 hunks)
• src/pl/plisql/src/sql/plisql_autonomous.sql (1 hunks)

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

src/pl/plisql/src/pl_exec.c (2)

46-46: Include of pl_autonomous.h correctly wires in autonomous-transaction support

The new include looks appropriate and localized; no issues from this file’s perspective.

---

685-711: Autonomous delegation path may bypass local SRF/OUT handling; verify full parity with normal execution

The new autonomous branch delegates cleanly to plisql_exec_autonomous_function and mirrors plugin func_end and econtext cleanup. However, verify the following behavioral guarantees:

1. Set-returning functions:
   This early return result; bypasses the later estate.retisset / ReturnSetInfo handling in plisql_exec_function. Confirm that plisql_exec_autonomous_function:

   • Fully handles fcinfo->resultinfo when func->fn_retset is true (materializes a tuplestore and fills ReturnSetInfo identically to the normal path), and
   • Returns a Datum/fcinfo->isnull pair in final form for both scalar and SRF use-cases.

   Add tests for RETURNS SETOF autonomous functions to ensure callers see identical behavior to the non-autonomous path.

2. Anonymous blocks and OUT parameters:
   This path skips the anonymous-block OUT propagation and anonymous_have_outparam logic. If PRAGMA AUTONOMOUS_TRANSACTION is forbidden for anonymous DO blocks, document that restriction; otherwise, implement corresponding OUT-parameter handling in the autonomous path or explicitly reject that combination.

3. Validation / internal calls:
   The fcinfo->flinfo->fn_oid == func->fn_oid guard prevents delegating validation contexts. Verify that:

   • All real, user-visible autonomous calls satisfy this predicate, including nested subprocedures and package-body routines.
   • All internal plumbing/validator paths never call this with matching OIDs, preventing accidental dblink recursion.

Add a comment in pl_exec.c referencing where these guarantees are upheld (e.g., in pl_autonomous.c or test suite).

src/pl/plisql/src/sql/plisql_autonomous.sql (5)

1-15: Comprehensive autonomous-transaction test suite with clear documentation. Setup is well-structured: dblink installed, test table created with appropriate schema, cleanup deferred to end. Inline comments clearly explain test purpose and expected behavior. Good coverage of parameter types, return types, and error paths.

---

168-192: Test 9 (OID invalidation) is critical for cache correctness. The test validates that the GUC-backed recursion-prevention mechanism survives extension drop/recreate. The DROP EXTENSION dblink CASCADE (line 185) is intentional here to force OID invalidation and test the syscache invalidation callback. This is a key verification for the feature's robustness, and the test design is sound.

---

235-296: Commented tests (12–14) appropriately document Issue #985 limitation. Tests for explicit COMMIT/ROLLBACK within autonomous blocks are correctly flagged as unsupported and deferred to future work. Comments clearly explain why (PL/iSQL lacks transaction-control statements), cite the issue, and provide example code for when the limitation is lifted. This is good documentation for future maintainers.

---

340-367: Test 17 (nested autonomous calls) validates multi-SPI-context correctness. Both inner_function and outer_function declare PRAGMA AUTONOMOUS_TRANSACTION, confirming that nested autonomous execution paths don't corrupt stack or SPI state. The expected return value comment (line 365) correctly traces the calculation. This is an important integration test for the feature.

---

436-453: Cleanup is complete and correct. All procedures, functions, and the test table are dropped in reverse order of creation. No dangling artifacts left in the test database. Proper tear-down for repeatable test runs.

Reviewing files that changed from the base of the PR and between 72d0173 and a580f91.

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

• src/pl/plisql/src/pl_exec.c (4 hunks)
• src/pl/plisql/src/sql/plisql_autonomous.sql (1 hunks)

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

src/pl/plisql/src/sql/plisql_autonomous.sql (4)

1-95: Setup and basic tests look sound.

Extension creation, table setup, and Tests 1–4 follow expected patterns. PRAGMA placement in the DECLARE section aligns with documented syntax.

---

171-192: OID invalidation test: verify CASCADE scope.

Line 185 uses DROP EXTENSION dblink CASCADE; to test OID invalidation and revalidation. While the extension is recreated at line 186, CASCADE could drop dependent functions or procedures if the test suite runs in a persistent environment. Ensure this test runs in isolation or document expected side effects.

---

240-296: Commented-out tests appropriately documented.

Tests 12–14 are correctly disabled with clear references to Issue #985 (lack of COMMIT/ROLLBACK support in autonomous blocks). The comments explain the limitation and expected failures well, making it easy to re-enable these tests once the issue is resolved.

---

437-454: Cleanup is thorough and well-ordered.

All test procedures, functions, and the test table are explicitly dropped. The order (procedures before functions, then table) respects dependency constraints.

src/pl/plisql/src/pl_exec.c (2)

46-46: Include of pl_autonomous.h is appropriate

The new header include is placed consistently with other PL/iSQL-specific headers and is the right way to expose plisql_exec_autonomous_function here. No issues.

---

491-515: Updated plisql_exec_function API comment now matches implementation

The expanded doc block clearly describes the function’s responsibilities, including autonomous transactions and SRF behavior, and correctly states that fcinfo must be non-NULL for executable calls, which matches all current uses and the unconditional dereferences in the body.

Reviewing files that changed from the base of the PR and between a580f91 and 48c46fc.

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

• src/pl/plisql/src/meson.build (1 hunks)
• src/pl/plisql/src/sql/plisql_autonomous.sql (1 hunks)

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

src/pl/plisql/src/meson.build (1)

13-13: pl_autonomous.c correctly wired into build

Adding 'pl_autonomous.c' to plisql_sources ensures the autonomous-transaction support is compiled into the plisql shared module and aligns with the prior review request to hook this file into Meson. No further Meson changes seem necessary here.

src/pl/plisql/src/sql/plisql_autonomous.sql (5)

435-452: Package procedure test case correctly added.

The package procedure test (lines 435–452) addresses OreoYang's request and properly validates autonomous transaction behavior within package-body routines. The test follows the same pattern as standalone procedures and verifies both the autonomous execution and data persistence.

---

236-296: Commented-out COMMIT/ROLLBACK tests provide good documentation of known limitations.

Tests 12–14 (lines 236–296) are correctly commented out and reference Issue #985, clearly documenting that explicit COMMIT/ROLLBACK within autonomous blocks is not yet supported in PL/iSQL. The comments explain the expected failure mode and set appropriate expectations for future enhancements.

---

1-15: Test setup and schema are well structured.

The setup correctly installs dblink, creates a focused test table (autonomous_test) with three columns (id, msg, tx_state), and uses clear comments to separate test sections. This design facilitates debugging and future expansion.

---

340-367: Nested autonomous function test effectively validates SPI context handling.

The nested autonomous test (inner_function and outer_function, lines 340–367) correctly exercises multiple SPI contexts and verifies that both outer and inner autonomous calls execute and commit independently. The expected result (id=60 outer, id=160 inner, return value 175) demonstrates proper isolation and value propagation across nested calls.

---

197-197: The syntax is correct for PL/iSQL. IvorySQL's PL/iSQL dialect uses RETURN type (not RETURNS type) to match Oracle PL/SQL compatibility. This is intentional and consistent with the Oracle-compatible design of the test suite.