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

3909-3919: Add teardown for test hygiene

Drop test_f2 after the block to avoid leaking objects into later tests that run in the same database.

Example to append after creation:

DROP FUNCTION test_f2(integer);

Please confirm no other tests rely on test_f2 persisting beyond this script.

src/backend/commands/functioncmds.c (1)

1681-1686: Redundant initialization code.

This loop initializes the arrays that should have been initialized at declaration (as mentioned in the earlier comment). If you add the initialization at declaration, this loop becomes redundant.

If the arrays are initialized at declaration, this loop can be removed to avoid duplicate initialization.

src/include/parser/parser.h (1)

46-50: Enum extended without doc update — clarify PL/iSQL modes.

Please extend the comment above RawParseMode to describe the PL/iSQL variants for parity with PL/pgSQL.

Apply this doc tweak:

  * RAW_PARSE_PLPGSQL_ASSIGNn: parse a PL/pgSQL assignment statement,
  * and return a one-element List containing a RawStmt node.  "n"
  * gives the number of dotted names comprising the target ColumnRef.
+ *
+ * RAW_PARSE_PLISQL_EXPR: parse a PL/iSQL expression; returns a one-element
+ * List containing a RawStmt.
+ *
+ * RAW_PARSE_PLISQL_ASSIGNn: parse a PL/iSQL assignment statement; returns a
+ * one-element List containing a RawStmt. "n" is the dotted target length.

src/include/parser/parse_target.h (1)

58-62: Use extern for consistency with this header’s prototypes

Every other declaration in this header uses extern; keep style consistent.

-List *transformRowExpression(ParseState *pstate, List *exprlist,
-					ParseExprKind exprKind, bool allowDefault);
-List *transformRowTargetList(ParseState *pstate, List *targetlist,
-					ParseExprKind exprKind);
+extern List *transformRowExpression(ParseState *pstate, List *exprlist,
+									ParseExprKind exprKind, bool allowDefault);
+extern List *transformRowTargetList(ParseState *pstate, List *targetlist,
+									ParseExprKind exprKind);

src/include/catalog/dependency.h (2)

152-153: Expose comparator — document order

Comparator returns -1 when a.objectId > b.objectId (descending). Add a brief comment here to record sort order for callers relying on it.

---

101-111: Rename flags to classId and document the first‐field invariant

File: src/include/catalog/dependency.h (lines 101–111)

• Change the field

  Oid flags;  /* flags used to identify function, package and package body */

  to

  Oid classId;  /* OID of pg_proc/pg_package/pg_package_body */

  since it isn’t a bitmask but the catalog‐relation OID.
• Replace the vague “MUST BE FIRST” comment with a clear explanation—or better, add

  static_assert(offsetof(ObjectFunOrPkg, objectId) == 0,
                "ObjectFunOrPkg.objectId must be first for sort/dedup logic");

  to document why objectId must remain at offset 0 for the comparator and qunique calls.
• No other code relies on this struct’s layout beyond the palloc/repalloc, pg_qsort and qunique usage.

src/include/catalog/pg_proc.h (2)

202-216: Fix typos and tighten comments for consistency; keep style to single-line summaries.

Current text has typos (“te”), repetition, and verbose phrasing. Suggest:

-#define PROSTATUS_NA		'n'	/* procedure status is unknown,
-					 * must call ALTER FUNCTION COMPILE statement to
-					 * validate te procedure, otherwise failed to
-					 * call the procedure
-					 */
-#define PROSTATUS_VALID		'v'	/* procedure can be called */
-#define PROSTATUS_INVALID	'i'	/* procedure status is invalid,
-					 * must call ALTER FUNCTION COMPILE statement to
-					 * validate te procedure, otherwise failed to
-					 * call the procedure
-					 */
+#define PROSTATUS_NA		'n'	/* unknown/unvalidated; run ALTER FUNCTION ... COMPILE before calls succeed */
+#define PROSTATUS_VALID		'v'	/* procedure can be called */
+#define PROSTATUS_INVALID	'i'	/* invalid; run ALTER FUNCTION ... COMPILE to revalidate before calls succeed */

---

220-248: Confirm how prostatus is assigned during CREATE/REPLACE and PL/iSQL validation. Consider explicit param only if needed.

If ProcedureCreate does not expose a prostatus parameter, ensure it initializes to PROSTATUS_VALID and PL/iSQL paths subsequently flip to NA/INVALID as appropriate. If multiple callers need different initial states, consider adding a prostatus arg to ProcedureCreate to avoid post-hoc UPDATEs.

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

3828-3929: Row-target dispatcher: polish booleans and fast-paths; leverage transformed TList.

• Use bool for flags; initialize attrrowtype to InvalidOid.
• Avoid duplicate transformTargetList calls by transforming once and reusing.
• Minor correctness: treat ROW identifier case-insensitively consistently (you used pg_strcasecmp here; elsewhere strcmp).

-    int target_has_row = false;
-    bool    cols_has_row = false;
-    int col_is_complex = false;
-    int row_no;
-    Oid attrrowtype;
+    bool target_has_row = false;
+    bool cols_has_row = false;
+    bool col_is_complex = false;
+    int  row_no = 0;
+    Oid  attrrowtype = InvalidOid;

[ suggest_nitpick ]

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

2069-2093: Star handling messages should be specific.

“unsupported syntax” is vague. Prefer messages similar to existing code (“row expansion via "*" is not supported here” or “invalid user.table.column, table.column, or column specification”) with errposition.

src/backend/catalog/dependency.c (2)

92-95: Remove duplicate include of funcapi.h.

funcapi.h is already included at Line 82. Redundant include increases rebuild churn.

Apply:

 #include "utils/packagecache.h"
-#include "commands/tablecmds.h"
-#include "funcapi.h"
-#include "lib/qunique.h"
+#include "commands/tablecmds.h"
+#include "lib/qunique.h"

---

927-929: Unnecessary classid==0 check.

pg_depend.classid should always be valid. If you intend to future-proof, add a comment; otherwise remove for clarity.

src/backend/catalog/pg_proc.c (1)

598-610: DRY the prostatus assignment (minor).

The prostatus initialization for plisql/ORA mode is duplicated. Compute a bool is_plisql once and reuse.

Example:

+	bool is_plisql = (ORA_PARSER == compatible_db &&
+					  languageObjectId == get_language_oid("plisql", true));
 ...
-		if (ORA_PARSER == compatible_db &&
-			languageObjectId == get_language_oid("plisql", true))
+		if (is_plisql)
 		{
 			if (check_function_bodies)
 				values[Anum_pg_proc_prostatus - 1] = CharGetDatum(PROSTATUS_VALID);
 			else
 				values[Anum_pg_proc_prostatus - 1] = CharGetDatum(PROSTATUS_NA);
 		}
 ...
-		if (ORA_PARSER == compatible_db &&
-			languageObjectId == get_language_oid("plisql", true))
+		if (is_plisql)

Also applies to: 643-655

src/backend/oracle_parser/ora_gram.y (1)

14592-14614: End-to-end verification checklist for %ROWTYPE and VALUES changes.

Before merge, please validate:

• %ROWTYPE types flow: arguments, RETURNS, dependency recording, and error positions.
• SETOF %ROWTYPE (if added) parses and transforms.
• No bison conflicts; %expect 0 holds.
• No regression in INSERT … VALUES syntax.
• Views/rules/util code (ruleutils.c) handle valuesIsrow appropriately.

Would you like me to draft a minimal SQL test matrix to cover these paths?

Also applies to: 9339-9350, 14004-14005, 892-896

src/backend/commands/tablecmds.c (4)

13550-13552: FYI: Watch for unused locals.

ObjectAddress address looks unused in the provided context. If truly unused, drop it to avoid warnings.

---

13705-13786: Minor fixes and dedup suggestion.

• Typo: “funciton” → “function”.
• Error message wording: prefer “failed to lookup package body …” for consistency (also elsewhere).
• The invalidation switch (function/package/package body) is duplicated here and in check_function_dependency_internal; extract a tiny static helper to reduce duplication and drift.

- * Now find out the dependent funciton which parameters datatype
+ * Now find out the dependent function whose parameters datatype

---

14022-14066: Error handling branch is fine; keep DEPENDENCY_TYPE guard narrow.

Flow is correct: block AT_AlterColumnType for non-ORA paths; collect dependents only in ORA mode. Keep the elog(ERROR, "unexpected object…") conservative—consider LOG/DEBUG1 instead to avoid hard errors if new dep types appear.

---

13725-13735: Optional: extract common invalidation into a helper to avoid drift.

Both blocks do the same three-way switch. A static invalidate_fun_or_pkg(ObjectFunOrPkg *entry) reduces duplication.

Also applies to: 21703-21716

contrib/ivorysql_ora/src/sysview/sysview--1.0.sql (3)

464-467: Simplify and harden PL/SQL RECORD detection (case-insensitive, NULL-safe).

Use lower(COALESCE(...)) IN ('table','view') to avoid four equality checks and handle NULLs cleanly.

-       WHEN ARG.TYPE_OBJECT_TYPE::VARCHAR2 = 'table' OR ARG.TYPE_OBJECT_TYPE::VARCHAR2 = 'TABLE'
-         OR ARG.TYPE_OBJECT_TYPE::VARCHAR2 = 'view' OR ARG.TYPE_OBJECT_TYPE::VARCHAR2 = 'VIEW'
-         THEN 'PL/SQL RECORD'
+       WHEN lower(COALESCE(ARG.TYPE_OBJECT_TYPE::text, '')) IN ('table','view')
+         THEN 'PL/SQL RECORD'

---

679-682: Apply the same PL/SQL RECORD detection simplification in ALL_ARGUMENTS.

Keep logic consistent with DBA_ARGUMENTS.

-       WHEN ARG.TYPE_OBJECT_TYPE::VARCHAR2 = 'table' OR ARG.TYPE_OBJECT_TYPE::VARCHAR2 = 'TABLE'
-         OR ARG.TYPE_OBJECT_TYPE::VARCHAR2 = 'view' OR ARG.TYPE_OBJECT_TYPE::VARCHAR2 = 'VIEW'
-         THEN 'PL/SQL RECORD'
+       WHEN lower(COALESCE(ARG.TYPE_OBJECT_TYPE::text, '')) IN ('table','view')
+         THEN 'PL/SQL RECORD'

---

759-776: Unify TYPE_ null handling via NULLIF across all argument views*

• Replace each

     CASE  
       WHEN ARG.TYPE_<X> != '_NULL_'  
         THEN SYS.ORA_CASE_TRANS(ARG.TYPE_<X>::VARCHAR2)::VARCHAR2(128)  
       ELSE NULL  
     END::VARCHAR2(128) AS TYPE_<X>

  with

     SYS.ORA_CASE_TRANS(NULLIF(ARG.TYPE_<X>, '_NULL_')::VARCHAR2)::VARCHAR2(128) AS TYPE_<X>

  for TYPE_NAME, TYPE_SUBNAME and TYPE_OBJECT_TYPE in ALL_ARGUMENTS, USER_ARGUMENTS and DBA_ARGUMENTS.

• PLS_TYPE logic (WHEN (T.TYPTYPE = 'b' OR 'c') AND ARG.PLS_TYPE IS NOT NULL → use ARG.PLS_TYPE; WHEN TYPTYPE='b' AND ARG.PLS_TYPE IS NULL → fallback to T.OID; ELSE NULL) is consistent and requires no change.

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

1281-1283: No state leak: globals are reset and nested contexts handled. check_referenced_objects and plisql_referenced_objects are set to NIL at the start of each compile (in both pl_comp.c and pl_package.c), freed at the end, and saved/restored for nested calls—no cross-session leaks or thread‐safety issues in the single‐process backend. Optional: encapsulate these fields into a compile‐context struct for clarity.

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

1511-1548: Extended reload-check for %TYPE/%ROWTYPE looks correct

The deeper comparison (pctrowtypname presence, pct_type/row_type flags, name list case-insensitive match) makes overload resolution robust for %TYPE/%ROWTYPE. Consider early-continue after name-list length mismatch to skip the loop and save cycles.

src/backend/catalog/namespace.c (1)

1602-1618: Optional: deduplicate Ora-type fallback logic

The positional branch duplicates the package/Ora fallback. Consider extracting a small helper to resolve one arg-type name (handles pkg type, Ora type, validity notice) and call it from both branches. This reduces drift and future bugs.

src/backend/utils/fmgr/funcapi.c (1)

2716-2739: Typos and naming consistency in prostatus helper.

Minor: fix typos in comment (“catalog”), and variable prostaus_datum → prostatus_datum for clarity.

Apply this diff:

- * get function prostatus from the pg_proc catatlog
+ * get function prostatus from the pg_proc catalog
@@
-    Datum      prostaus_datum;
+    Datum      prostatus_datum;
@@
-    prostaus_datum = SysCacheGetAttr(PROCOID, procTup, Anum_pg_proc_prostatus,
-                            &isNull);
+    prostatus_datum = SysCacheGetAttr(PROCOID, procTup,
+                                      Anum_pg_proc_prostatus, &isNull);
@@
-        result = DatumGetChar(prostaus_datum);
+        result = DatumGetChar(prostatus_datum);

src/backend/utils/cache/lsyscache.c (2)

1676-1681: Comment typos and clarity.

Fix grammar: “dropped”, “really”. This improves developer comprehension.

Apply this diff:

- * If A function returns a variable in a package, 
- * the variable may be changed with package. If A function's 
- * returned datatype is tablename.columnname%TYPE, the tablename
- * or the columnname can be changed or droped. 
- * so, this function created to get the realy type.
+ * If a function returns a variable in a package,
+ * the variable may change with the package. If a function's
+ * returned datatype is tablename.columnname%TYPE, the table name
+ * or the column name can be changed or dropped.
+ * This function gets the real underlying type.

---

2005-2018: Free arg type-name strings to avoid leaking memory.

argtypenames entries come from TextDatumGetCString and should be freed after use.

Outside the shown hunk, add before return desc; in build_function_result_tupdesc_d:

if (argtypenames)
{
    for (i = 0; i < numargs; i++)
        if (argtypenames[i])
            pfree(argtypenames[i]);
    pfree(argtypenames);
}

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

55-63: Stability: proname LIKE filters can be ambiguous.

SELECTs on pg_proc using proname LIKE may match multiple versions across schemas or after redefinitions, causing flaky counts or statuses.

Use exact match in current schema:

SELECT prostatus
FROM pg_proc p
JOIN pg_namespace n ON n.oid = p.pronamespace
WHERE p.proname = 'fun1' AND n.nspname = current_schema;

Also applies to: 69-76, 154-175, 261-275, 281-292, 324-331, 154-157, 185-187, 298-303, 324-331, 518-522, 686-692, 707-714, 880-885, 896-903, 1556-1611, 2143-2146, 2373-2384, 2395-2415

src/backend/parser/parse_type.c (2)

30-34: Remove unused includes to keep compile tight.

access/table.h and catalog/pg_class.h appear unused in this TU. If not required by macros, drop them.

-#include "catalog/pg_class.h"
-#include "access/table.h"

---

912-918: Code duplication between LookupTypeNameExtended and LookupOraTypeNameExtended.

The two functions are nearly identical except for the %TYPE notice behavior. Consider factoring into a single internal helper with a “emit_pct_type_notice” boolean to avoid drift.

static Type
LookupTypeNameExtendedInternal(ParseState*, const TypeName*, int32*, bool temp_ok, bool missing_ok, bool emit_pct_notice);

Also applies to: 924-1084

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

1149-1149: Nit: fix typo in comment.

“alreay” → “already”.

Apply:

-                /* Check duplicate dependencies that alreay been built in CREATE FUNCTION/PROCEDURE statment */
+                /* Check duplicate dependencies that already were built in CREATE FUNCTION/PROCEDURE statement */

---

2597-2604: Minor API consistency and immutability notes.

• pctrowtypname is stored by pointer; ensure it lives in a long-lived context.
• Keep PLiSQL_type immutable after creation; all per-variable attributes should be on the var.

Also applies to: 2669-2671, 3031-3037

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

941-998: Dup-dependency scan is repeated and heavy; consider factoring.

The pg_depend scan logic is duplicated in spec/body compiles. Factor into a small helper to DRY and reduce future divergence.

I can factor this into static void record_pct_dependencies(ObjectAddress *myself) shared by both callers.

Also applies to: 1218-1284

---

3226-3240: Minor: free temporary arrays after tuplestore_putvalues.

The container arrays (oids, paramModes, etc.) can be pfree’d after constructing ArrayTypes to reduce peak memory.

Also applies to: 3270-3280, 3415-3476

---

454-456: Typos in DEBUG messages.

“realy” → “really”.

Apply similar replacements across the file.

Also applies to: 574-577, 585-587, 469-471, 579-581

src/backend/utils/adt/ruleutils.c (5)

80-80: Dependency layering: parse_package.h include in ruleutils.c

Pulling parser/parse_package.h into ruleutils.c couples deparsing with Oracle package-type lookup. Prefer moving the tiny helper you need behind a utils-facing API (e.g., a small wrapper in utils/ or parser/ gate-kept by ORA_PARSER) to avoid spreading parser headers across utils/adt. If we keep this, add a short comment explaining why the include is safe here.

---

546-554: Forward-decl and fixed column count: hide magic 9

NUM_RETURN_COLUMNS is fine, but consider an enum for column indexes (OID_COL, POSITION_COL, …) to prevent accidental mismatch between writers and consumers. Also, declare the helper with static inline comments on each column’s meaning to make maintenance easier.

---

14036-14044: RELKIND coverage for composite types

You only classify RELKIND_RELATION and RELKIND_VIEW/MATVIEW. Composite types created via CREATE TYPE AS (relkind == RELKIND_COMPOSITE_TYPE) will be mislabelled. Consider setting type_object_type = "type" for RELKIND_COMPOSITE_TYPE.

---

13853-13858: Random-access flag should be boolean

Passing (rsinfo->allowedModes & SFRM_Materialize_Random) to tuplestore_begin_heap works, but is a bitwise mask; wrap it as a boolean cast for clarity: (rsinfo->allowedModes & SFRM_Materialize_Random) != 0.

Apply this small change:

-  tupstore =
-    tuplestore_begin_heap(rsinfo->allowedModes & SFRM_Materialize_Random,
+  tupstore =
+    tuplestore_begin_heap((rsinfo->allowedModes & SFRM_Materialize_Random) != 0,
                           false, work_mem);

---

13959-14033: Error positioning helper with NULL pstate

parser_errposition(NULL, typeName->location) will always yield 0; if you want a hint without pstate, consider errhint with NameListToString(...) and keep location-less errors, or thread a ParseState only when available. At minimum, drop parser_errposition(NULL, ...) to avoid confusion.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.