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

44-44: Fix comment wording and document sentinel value.

“mode” → “modifier”; also clarify that -1 means “typmod not specified.”

-    int32 basetypmod;  /* base of type mode */
+    int32 basetypmod;  /* base type modifier (typmod); -1 if not specified */

src/oracle_test/regress/sql/ora_package.sql (1)

2002-2006: Spec’s return type vs rowtype: confirm intent or align for clarity.

Spec returns mds while arguments/locals use mds%rowtype. If not intentionally mixing forms, consider using mds%rowtype consistently to avoid confusion.

- function test(id integer, var2 mds%rowtype) return mds;
+ function test(id integer, var2 mds%rowtype) return mds%rowtype;

And in the body:

- function test(id integer, var2 mds%rowtype) return mds is
+ function test(id integer, var2 mds%rowtype) return mds%rowtype is

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

3909-3919: Rename to test_f2 is fine; add cleanup to future-proof the test.

If “pipelined” ever succeeds, the function will linger. Add a defensive drop after the block.

 end;
 /
 
+drop function if exists test_f2(integer);
+

src/backend/utils/cache/packagecache.c (1)

599-619: Fix typos in SYS view mapping comment

• “DATA_SCALERS” → “DATA_SCALES”
• Minor copyedit for consistency.

Apply:

- * data_scales is used for DATA_SCALERS.
+ * data_scales is used for DATA_SCALES.

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

2218-2251: Duplicate Ora-type resolution logic: factor into a small helper to reduce risk and drift.

The block mirrors the earlier function; centralize into a static helper to keep behavior consistent.

Example (outside selected range):

static void resolve_outargtype_from_typename(char *typename_txt, Oid *out_oid);

Then call it in both places.

---

2729-2735: Minor: variable name typo (prostaus_datum).

Rename to prostatus_datum for clarity. No functional impact.

- Datum        prostaus_datum;
+ Datum        prostatus_datum;
...
- prostaus_datum = SysCacheGetAttr(PROCOID, procTup, Anum_pg_proc_prostatus,
+ prostatus_datum = SysCacheGetAttr(PROCOID, procTup, Anum_pg_proc_prostatus,
                                   &isNull);
...
-   result = DatumGetChar(prostaus_datum);
+   result = DatumGetChar(prostatus_datum);

---

2759-2784: Skip unnecessary writes and document NA/INVALID behavior.

• If the current prostatus equals the requested value, return early to avoid catalog churn.
• The NA/INVALID early-return is surprising; add a comment explaining why refusing changes is desired (or relax it if not).

- ori_prostatus = get_func_prostatus(oldprocTup);
- if (ori_prostatus == PROSTATUS_INVALID ||
-     ori_prostatus == PROSTATUS_NA)
+ ori_prostatus = get_func_prostatus(oldprocTup);
+ /* Do not update if value is unchanged */
+ if (ori_prostatus == prostatus)
+ {
+     ReleaseSysCache(oldprocTup);
+     return;
+ }
+ /* Disallow transitions from INVALID/NA (by design) */
+ if (ori_prostatus == PROSTATUS_INVALID ||
+     ori_prostatus == PROSTATUS_NA)
 {
     ReleaseSysCache(oldprocTup);
     return;
 }

---

2039-2071: Error message accuracy.

“package, relation or view does not exist” may be misleading if a type is missing. Consider “type does not exist” or including the resolved name for better diagnostics.

Also applies to: 2218-2251

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

2-2: Typo in header comment.

“and and” duplicated.

--- -- Tests for PL/iSQL %TYPE and and %ROWTYPE Attributes
+++ -- Tests for PL/iSQL %TYPE and %ROWTYPE Attributes

---

1218-1221: Suspicious assignment of rowtype to scalar.

r.phone_number := var1; assigns a composite to a scalar field. If the intent is to test invalidation, please add a comment and -- failed marker; otherwise use var1.phone_number.

-    r.phone_number :=  var1;
+    r.phone_number :=  var1.phone_number;

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

58-62: Make prototypes consistent (add extern)
These two new declarations are missing extern unlike others in this header. Align for consistency.

-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/pl/plisql/src/pl_package.h (1)

157-161: Provide an ABI-compatibility wrapper for plisql_package_parse
No internal callers exist beyond its own implementation; add a shim matching the original signature to preserve external ABI.

src/backend/parser/gram.y (1)

919-958: Optional: fail fast if PL/iSQL tokens ever reach the core parser.

If there’s any chance the core path could inject MODE_PLISQL_* by mistake, add explicit reject branches in parse_toplevel that raise a clear error (mirrors how we gate other modes). This avoids confusing error cascades.

If desired, add guarded branches like:

 parse_toplevel:
  			stmtmulti
 ...
  			| MODE_PLPGSQL_ASSIGN3 PLAssignStmt
  				{
  					PLAssignStmt *n = (PLAssignStmt *) $2;
  
  					n->nnames = 3;
  					pg_yyget_extra(yyscanner)->parsetree =
  						list_make1(makeRawStmt((Node *) n, @2));
  				}
+ 			| MODE_PLISQL_EXPR /* not supported in core parser */
+ 			{
+ 				ereport(ERROR,
+ 					(errcode(ERRCODE_SYNTAX_ERROR),
+ 					 errmsg("PL/iSQL expressions are not supported in PostgreSQL grammar")));
+ 			}
+ 			| MODE_PLISQL_ASSIGN1
+ 			| MODE_PLISQL_ASSIGN2
+ 			| MODE_PLISQL_ASSIGN3
+ 			{
+ 				ereport(ERROR,
+ 					(errcode(ERRCODE_SYNTAX_ERROR),
+ 					 errmsg("PL/iSQL assignments are not supported in PostgreSQL grammar")));
+ 			}
 		;

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

63-67: Add brief docs and keep prototype grouping consistent

Consider adding short comments mirroring LookupTypeName/Extended docs and grouping these Ora variants adjacent to them for discoverability. No functional issues spotted.

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

202-216: Fix typos in prostatus comments

“validate te procedure” → “validate the procedure” (both occurrences).

- * validate te procedure, otherwise failed to
+ * validate the procedure, otherwise failed to

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

30-34: Remove unused includes

catalog/pg_class.h and access/table.h aren’t used in this file; consider dropping them.

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

---

909-1084: Avoid duplicating LookupTypeNameExtended logic

LookupOraTypeName(Extended) largely duplicate LookupTypeName(Extended) minus the %TYPE NOTICE. Consider a shared static helper with a “emit_pct_notice” flag to reduce drift.

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

79-79: Consider guarding Oracle-specific include

If this file is compiled in non-Oracle modes, unguarded inclusion of parser/parse_package.h could break those builds. If IvorySQL guarantees availability, ignore; otherwise wrap with a feature guard.

-#include "parser/parse_package.h"
+#ifdef USE_ORACLE_COMPAT
+#include "parser/parse_package.h"
+#endif

---

14479-14482: Prefer SQL NULLs over the literal "NULL" placeholders

Emitting text "NULL" makes downstream consumers do string checks instead of using IS NULL. Unless views are explicitly depending on this sentinel, return proper NULLs.

-		values[2] = CStringGetTextDatum("_NULL_");
-		values[3] = CStringGetTextDatum("_NULL_");
-		values[4] = CStringGetTextDatum("_NULL_");
+		nulls[2] = true;
+		nulls[3] = true;
+		nulls[4] = true;

If SYS views depend on the sentinel, keep as-is; otherwise this is a cleaner contract.

---

14260-14263: Optionally finalize tuplestore writing

Not required, but calling tuplestore_donestoring improves clarity and mirrors surrounding code patterns.

-	/* clean up and return the generated tuplestore */
-	//tuplestore_donestoring(tupstore);
+	/* clean up and return the generated tuplestore */
+	tuplestore_donestoring(tupstore);

---

14388-14427: Avoid shadowing ‘relid’ for readability

The inner declaration of relid masks the outer relid. Use a distinct name.

-			Oid 		relid;
+			Oid 		relid_local;
...
-			relid_local = RangeVarGetRelid(rel, NoLock, true);
-			attnum = get_attnum(relid_local, field);
+			relid_local = RangeVarGetRelid(rel, NoLock, true);
+			attnum = get_attnum(relid_local, field);

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

2146-2166: Add errposition and use precise message for indirection star.

Missing parser_errposition in the A_Indirection branch. Also prefer consistent wording (“row expansion via "*" is not supported here”), matching existing messages.

-                ereport(ERROR,
-                    (errcode(ERRCODE_SYNTAX_ERROR),
-                     errmsg("unsupported syntax")));
+                ereport(ERROR,
+                        (errcode(ERRCODE_SYNTAX_ERROR),
+                         errmsg("row expansion via \"*\" is not supported here"),
+                         parser_errposition(pstate, exprLocation((Node *) ind)));

---

2173-2179: Include errposition for DEFAULT rejection; clarify text.

Current error lacks cursor position.

-    if (IsA(e, SetToDefault))
-        ereport(ERROR,
-            (errcode(ERRCODE_SYNTAX_ERROR),
-             errmsg("unsupported syntax")));
+    if (IsA(e, SetToDefault))
+        ereport(ERROR,
+                (errcode(ERRCODE_SYNTAX_ERROR),
+                 errmsg("DEFAULT is not allowed in row target list"),
+                 parser_errposition(pstate, exprLocation(e))));

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

2723-2735: Comparator scope and semantics.

The comparator only sorts by objectId (descending). Given OIDs are cluster-global, dedup on objectId via qunique is safe. Consider static linkage if not used outside this TU (unless declared in a header intentionally).

---

92-95: Remove duplicate and unused includes in dependency.c

Drop the second #include "funcapi.h" (already at line 82) and remove the unused #include "commands/tablecmds.h":

src/backend/catalog/dependency.c
-#include "commands/tablecmds.h"
-#include "funcapi.h"

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

598-609: Unify PL/iSQL language checks; avoid syscache lookups in hot paths

You compare the function language to get_language_oid("plisql", true) here, but elsewhere in this file you use LANG_PLISQL_OID. Prefer the constant for consistency and to avoid repeated syscache hits.

Apply this diff in both blocks:

- if (ORA_PARSER == compatible_db &&
-     languageObjectId == get_language_oid("plisql", true))
+ if (ORA_PARSER == compatible_db &&
+     languageObjectId == LANG_PLISQL_OID)

Also applies to: 643-655

---

685-693: Skipping return-type dependency for composite types can break invalidation

You skip TypeRelationId dependency when PL/iSQL returns a composite type in Oracle mode. Ensure an equivalent dependency is recorded via the new dependency kind so DDL on the underlying relation properly invalidates the function. Otherwise, callers could observe stale prostatus or fail at runtime after ALTER TABLE.

If this is handled elsewhere, point to the object kind used and its invalidation path. If not, consider recording a weaker dependency (e.g., DEPENDENCY_NORMAL on the type oid) behind a feature flag until the new mechanism is fully wired.

---

704-717: Parameter-type transform dependencies gated by composite-type skip

Transforms for composite parameter types are skipped together with the type dependency. If a transform exists for a composite, this will silently bypass it. Confirm that’s intended (likely fine given composite transforms are uncommon) or add a comment explaining why transforms are intentionally suppressed here.

---

854-857: Use errdetail instead of errhint for the validator’s error text

The caught validator message is not a hint. Prefer errdetail("%s", edata->message) to place it correctly in the report and avoid hint misuse.

- ereport(WARNING,
-         (errmsg("compilation error"),
-          errhint("%s", edata->message),
-          errposition(edata->cursorpos)));
+ ereport(WARNING,
+         (errmsg("compilation error"),
+          errdetail("%s", edata->message),
+          errposition(edata->cursorpos)));

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

2628-2639: Use ereport() and add error code/position for ROWID restriction

elog lacks SQLSTATE and cursor position. Use ereport with a syntax or invalid-column reference code.

- if (strcmp(origTarget->name, "rowid") == 0)
-     elog(ERROR, "cannot specify system column name \"%s\"", origTarget->name);
+ if (strcmp(origTarget->name, "rowid") == 0)
+     ereport(ERROR,
+             (errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
+              errmsg("cannot specify system column name \"%s\"", origTarget->name),
+              parser_errposition(pstate, origTarget->location)));

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

14679-14688: VALUES as single-row: broaden or guard the grammar.

• Using columnref prohibits “VALUES (func())” or “VALUES CURRENT_DATE” from being treated as the row form. If that’s intentional, OK. If not, consider accepting a_expr and gate in transform.
• Columnref here yields a ColumnRefOrFuncCall node in this grammar; ensure transformValuesClause expects that node shape.

Option A (broaden):

-values_clause_no_parens:
-            VALUES columnref
+values_clause_no_parens:
+            VALUES a_expr
             {
                 SelectStmt *n = makeNode(SelectStmt);
-                n->valuesLists = list_make1(list_make1($2));
+                n->valuesLists = list_make1(list_make1($2));
                 n->valuesIsrow = true;
                 $$ = (Node *) n;
             }

Add a unit test covering:

• VALUES some_rowvar
• VALUES CURRENT_DATE
• VALUES (col1, col2) still parsed by values_clause

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

3390-3411: Error messaging: make LookupOraTypeName failure actionable

elog(ERROR, "package, relation or view does not exist") is misleading here; the failure is about resolving a type name.

Use a typed error with the offending name:

-								elog(ERROR, "package, relation or view does not exist");
+								ereport(ERROR,
+									(errcode(ERRCODE_UNDEFINED_OBJECT),
+									 errmsg("type \"%s\" does not exist",
+										TypeNameToString(tname))));

---

3795-3887: plisql_free_function: avoid ERRORs and ensure hashkey parity

• This helper should not ERROR on unsupported pseudotypes; it’s a cache eviction utility. Return quietly instead.
• Consider initializing fake_fcinfo with InitFunctionCallInfoData to avoid future API drift.

@@
-	else if (proc->prorettype != RECORDOID &&
-			 proc->prorettype != VOIDOID &&
-			 !IsPolymorphicType(proc->prorettype))
-			ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("PL/iSQL functions cannot return type %s",
-					format_type_be(proc->prorettype))));
+	else if (proc->prorettype != RECORDOID &&
+			 proc->prorettype != VOIDOID &&
+			 !IsPolymorphicType(proc->prorettype))
+			PG_RETURN_VOID(); /* nothing to free for invalid signatures */

If you prefer a no-op, simply return without emitting an error.

Would you like me to wire InitFunctionCallInfoData usage here?

---

1303-1305: Inline compile: clear list memory if ever populated

You reset the globals, but if the list were somehow populated, we’d leak. Safe-guard with deep free.

-	check_referenced_objects = false;
-	plisql_referenced_objects = NIL;
+	check_referenced_objects = false;
+	if (plisql_referenced_objects != NIL)
+		list_free_deep(plisql_referenced_objects);
+	plisql_referenced_objects = NIL;

Also applies to: 1447-1449

---

547-571: Comment typos

• “allowned” -> “allowed”
• “alos” -> “also”
• “ts return type” -> “its return type”

I can batch-fix these across the file.

Also applies to: 699-707

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

14974-14976: Remove unused variable.

ObjectAddress address is never used and will trigger -Wunused-variable in many builds.

-   ObjectAddress address;

---

15129-15209: Deduplicate invalidation logic into a helper to avoid drift.

This switch is duplicated later (Lines 22687-22744). Extract to a static helper (e.g., invalidate_funpkg_array(ObjectFunOrPkg*, int)). Also fix the “funciton” typo in the nearby comment.

---

15446-15490: Strictness of “unexpected object depending on column”.

In ORA mode, only DEPENDENCY_TYPE is expected, but consider downgrading to WARNING or skipping instead of ERROR to avoid breaking on unforeseen deps.

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

464-467: Case-insensitive TYPE_OBJECT_TYPE check can be simplified and made consistent.

Use UPPER(..) IN (...) instead of four equality checks.

-    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'
+    WHEN UPPER(ARG.TYPE_OBJECT_TYPE::VARCHAR2) IN ('TABLE','VIEW')
       THEN 'PL/SQL RECORD'

---

544-548: Avoid sentinel 'NULL' string checks; use NULLIF/COALESCE to express absence.

This reduces string-logic and edge cases.

-  CASE
-     WHEN ARG.TYPE_NAME != '_NULL_'
-          THEN SYS.ORA_CASE_TRANS(ARG.TYPE_NAME::VARCHAR2)::VARCHAR2(128)
-     ELSE
-          NULL
-  END::VARCHAR2(128) AS TYPE_NAME,
+  NULLIF(SYS.ORA_CASE_TRANS(ARG.TYPE_NAME::VARCHAR2), '_NULL_')::VARCHAR2(128) AS TYPE_NAME,

-  CASE
-     WHEN ARG.TYPE_SUBNAME != '_NULL_'
-          THEN SYS.ORA_CASE_TRANS(ARG.TYPE_SUBNAME::VARCHAR2)::VARCHAR2(128)
-     ELSE
-          NULL
-  END::VARCHAR2(128) AS TYPE_SUBNAME,
+  NULLIF(SYS.ORA_CASE_TRANS(ARG.TYPE_SUBNAME::VARCHAR2), '_NULL_')::VARCHAR2(128) AS TYPE_SUBNAME,

-  CASE
-     WHEN ARG.TYPE_OBJECT_TYPE != '_NULL_'
-          THEN SYS.ORA_CASE_TRANS(ARG.TYPE_OBJECT_TYPE::VARCHAR2)::VARCHAR2(128)
-     ELSE
-          NULL
-  END::VARCHAR2(128) AS TYPE_OBJECT_TYPE,
+  NULLIF(SYS.ORA_CASE_TRANS(ARG.TYPE_OBJECT_TYPE::VARCHAR2), '_NULL_')::VARCHAR2(128) AS TYPE_OBJECT_TYPE,

Also applies to: 550-554, 557-561

---

679-682: Duplicate case-insensitive checks (ALL_ARGUMENTS) — same simplification as 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'
+    WHEN UPPER(ARG.TYPE_OBJECT_TYPE::VARCHAR2) IN ('TABLE','VIEW')
       THEN 'PL/SQL RECORD'

---

759-763: Prefer NULLIF over sentinel comparison in ALL_ARGUMENTS too.

-  CASE
-     WHEN ARG.TYPE_NAME != '_NULL_'
-          THEN SYS.ORA_CASE_TRANS(ARG.TYPE_NAME::VARCHAR2)::VARCHAR2(128)
-     ELSE
-          NULL
-  END::VARCHAR2(128) AS TYPE_NAME,
+  NULLIF(SYS.ORA_CASE_TRANS(ARG.TYPE_NAME::VARCHAR2), '_NULL_')::VARCHAR2(128) AS TYPE_NAME,

-  CASE
-     WHEN ARG.TYPE_SUBNAME != '_NULL_'
-          THEN SYS.ORA_CASE_TRANS(ARG.TYPE_SUBNAME::VARCHAR2)::VARCHAR2(128)
-     ELSE
-          NULL
-  END::VARCHAR2(128) AS TYPE_SUBNAME,
+  NULLIF(SYS.ORA_CASE_TRANS(ARG.TYPE_SUBNAME::VARCHAR2), '_NULL_')::VARCHAR2(128) AS TYPE_SUBNAME,

-  CASE
-     WHEN ARG.TYPE_OBJECT_TYPE != '_NULL_'
-          THEN SYS.ORA_CASE_TRANS(ARG.TYPE_OBJECT_TYPE::VARCHAR2)::VARCHAR2(128)
-     ELSE
-          NULL
-  END::VARCHAR2(128) AS TYPE_OBJECT_TYPE,
+  NULLIF(SYS.ORA_CASE_TRANS(ARG.TYPE_OBJECT_TYPE::VARCHAR2), '_NULL_')::VARCHAR2(128) AS TYPE_OBJECT_TYPE,

Also applies to: 765-769, 772-776

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

910-997: Optional: eliminate per-object scans with a single pass + de-dup in-memory.

If many referenced objects exist, scanning pg_depend per-object is O(N) index probes. Consider collecting all candidate (refclassid, refobjid, refobjsubid) into a temp hash and then a single syscache scan per (refclassid, refobjid) to mark duplicates.

Also applies to: 1206-1285

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.