src/oracle_test/modules/test_package_reset/expected/test_reset_defaults.out (1)

84-108: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a dynamic-default or init-block assertion here.

This expected output only proves that literal defaults and NULL come back after reset. It would still pass if the implementation restored a saved snapshot instead of re-running default expressions / the init block, which is the behavior this PR is explicitly trying to preserve. Adding a value derived from a changing expression or an init-block side effect would close that gap. As per path instructions, "**/expected/*.out: These are expected test outputs from pg_regress. Review for: ... Edge case coverage ... Consistency with corresponding .sql test files`."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/oracle_test/modules/test_package_reset/expected/test_reset_defaults.out`
around lines 84 - 108, Add an assertion in the pkg_defaults_test reset
expectations that exercises a dynamic default or init block, not just literal
defaults and NULL, so the pg_regress output verifies re-execution after reset.
Update the corresponding expected output for test_reset_defaults to include a
value from a changing expression or init-block side effect, using the existing
test_package_reset.reset_all_packages and pkg_defaults_test getters as anchors
to keep coverage aligned with the .sql test.

Source: Path instructions

src/oracle_test/modules/test_package_reset/sql/test_reset_defaults.sql (1)

5-15: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add a dynamic-default / init-block assertion here.

This only proves literal defaults are restored. The PR contract also says reset must re-evaluate dynamic defaults and rerun the package init block lazily, but neither behavior is exercised here, so a cached-default or skipped-init regression would still pass. As per path instructions, **/sql/*.sql should ensure comprehensive coverage of features.

Also applies to: 61-67

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/oracle_test/modules/test_package_reset/sql/test_reset_defaults.sql`
around lines 5 - 15, Add coverage in the pkg_defaults_test setup to assert both
a dynamic default and the package init block are re-evaluated after reset, not
just literal defaults. Update the SQL test around pkg_defaults_test by
introducing a dynamic-default-backed symbol and an initialization side effect,
then verify via the existing reset flow that the dynamic value changes and the
init block runs lazily again on first access after reset. Use the
pkg_defaults_test package and its reset-defaults test case to locate the
assertions and extend them rather than creating a separate test.

Source: Path instructions

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

contrib/ivorysql_ora/sql/dbms_session.sql (1)

118-153: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add coverage that RESET_PACKAGE leaves other session state untouched.

This section validates package-state reset, but doesn't verify dbms_session.reset_package() is scoped to packages only. Per PR objectives, the reset must not "impact other sessions or non-package session state." Consider setting a DBMS_SESSION application-context value before calling reset_package() and asserting it's still readable afterward.

💡 Suggested addition

+-- Verify RESET_PACKAGE does not disturb unrelated session state (application context)
+call dbms_session.set_context('rp_isolation', 'k', 'v');
+call dbms_session.reset_package();
+select sys_context('rp_isolation', 'k') as context_survives_reset;
+
 DROP PACKAGE rp_pkg;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@contrib/ivorysql_ora/sql/dbms_session.sql` around lines 118 - 153, The
RESET_PACKAGE smoke test in dbms_session.sql only verifies package state, but it
should also prove non-package session state is preserved. Extend the existing
reset_package scenario by using dbms_session to set a
session/application-context value before the call and asserting it remains
readable afterward, while keeping the current rp_pkg package-state checks
intact.

Source: Path instructions

src/oracle_test/modules/test_package_reset/sql/test_reset_defaults.sql (1)

104-105: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

set_elem mutation is never independently verified.

set_elem(2, 99) at line 104 is immediately overwritten by set_arr(ARRAY[10, 20, 30, 40]) at line 105 with no intermediate SELECT. If set_elem were a no-op, the test would still pass, leaving the element-update code path untested.

Add a SELECT between the two CALL statements to verify the element mutation took effect, and update the expected output accordingly.

♻️ Proposed fix

 CALL pkg_arr_defaults.set_elem(2, 99);
+SELECT pkg_arr_defaults.get_arr_len AS len, pkg_arr_defaults.get_arr_first AS first;
 CALL pkg_arr_defaults.set_arr(ARRAY[10, 20, 30, 40]);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/oracle_test/modules/test_package_reset/sql/test_reset_defaults.sql`
around lines 104 - 105, The pkg_arr_defaults.set_elem mutation is overwritten by
pkg_arr_defaults.set_arr before it is verified, so the element-update path is
not actually tested. Insert a SELECT of the package array state between the
set_elem and set_arr calls in test_reset_defaults.sql, using the existing
pkg_arr_defaults symbols so the mutation is asserted independently, and update
the expected output for that added verification before continuing with the array
reset.