contrib/tsm_system_rows/sql/tsm_system_rows.sql (1)

39-39: Fix punctuation/grammar in trailing comment.

Use “extension, but succeeds …” (comma placement and verb form).

-DROP EXTENSION tsm_system_rows;  -- fail, view depends on extension ,but success in Oracle because of FORCE VIEW
+DROP EXTENSION tsm_system_rows;  -- fail, view depends on extension, but succeeds in Oracle because of FORCE VIEW

contrib/tsm_system_time/sql/tsm_system_time.sql (1)

51-51: Fix punctuation/grammar in trailing comment.

-DROP EXTENSION tsm_system_time;  -- fail, view depends on extension ,but success in Oracle because of FORCE VIEW
+DROP EXTENSION tsm_system_time;  -- fail, view depends on extension, but succeeds in Oracle because of FORCE VIEW

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

595-595: Replace curly apostrophe with ASCII to avoid encoding surprises.

--- a
+++ b
--- this perhaps should be rejected, but it isn’t:
+-- this perhaps should be rejected, but it isn't:

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

1091-1194: Avoid triple calling pg_get_viewdef per row (perf).

Compute once via LATERAL and reuse; apply similarly to ALL_VIEWS and USER_VIEWS.

 SELECT
@@
-    LENGTH(pg_get_viewdef(C.OID))::NUMBER(38) AS TEXT_LENGTH,
-    SYS.ORA_CASE_TRANS(pg_get_viewdef(C.OID)::VARCHAR2)::TEXT AS TEXT,
-    SYS.ORA_CASE_TRANS(pg_get_viewdef(C.OID)::VARCHAR2)::VARCHAR2(4000) AS TEXT_VC,
+    LENGTH(v.vdef)::NUMBER(38) AS TEXT_LENGTH,
+    SYS.ORA_CASE_TRANS(v.vdef::VARCHAR2)::TEXT AS TEXT,
+    SYS.ORA_CASE_TRANS(v.vdef::VARCHAR2)::VARCHAR2(4000) AS TEXT_VC,
@@
-FROM
-    PG_CLASS AS C
+FROM
+    PG_CLASS AS C
+    CROSS JOIN LATERAL (SELECT pg_get_viewdef(C.OID) AS vdef) v
 WHERE
     C.RELKIND = 'v';

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

22-22: Includes look OK; minor style nit.

access/relation.h and commands/view.h are justified. Nit: keep consistent spacing in conditionals (add a space after if).

Also applies to: 29-29

src/backend/rewrite/rewriteHandler.c (1)

2571-2582: Improve error reporting for force views in get_view_query().

Good guard to avoid NULL rd_rules deref. Prefer ereport with SQLSTATE and translatable message, and mention the relation name for diagnosability.

-    if (ORA_PARSER == compatible_db && rel_is_force_view(RelationGetRelid(view)))
-        elog(ERROR, "force view does not have a Query tree");
+    if (ORA_PARSER == compatible_db &&
+        rel_is_force_view(RelationGetRelid(view)))
+        ereport(ERROR,
+                (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+                 errmsg("force view \"%s\" does not have a rewrite rule",
+                        RelationGetRelationName(view)),
+                 errdetail("Compile the force view before accessing its query tree.")));

src/oracle_test/regress/sql/ora_force_view.sql (3)

5-7: Typo in comments.

“defination” → “definition”.

-select pg_get_viewdef('v_bug193');        -- force view defination not change
+select pg_get_viewdef('v_bug193');        -- force view definition not change
...
-select pg_get_viewdef('v_bug193');        -- force view defination be changed
+select pg_get_viewdef('v_bug193');        -- force view definition be changed

---

74-83: Optional: assert expected compile behavior.

Consider adding explicit checks (e.g., selecting from dba_views/user_views or a helper to show invalid/valid state) after ALTER VIEW ... COMPILE to make the test more robust.

---

114-124: Clarity nit on “error” cases.

Mark the intended error-producing statements with a comment like “-- should error” consistently (you already do elsewhere) to aid expected output maintenance.

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

1531-1563: Parse-time compile path: switch to ereport and reopen by OID to avoid name/path races.

• Use ereport with an SQLSTATE instead of elog(ERROR).
• After compilation, reopen the same object by OID to avoid search_path/rename races during the unlock window.

Apply:

-            /* reopen the view */
-            lockmode = isLockedRefname(pstate, refname) ? RowShareLock : AccessShareLock;
-            rel = parserOpenTable(pstate, relation, lockmode);
+            /* reopen the same view by OID to avoid name/path races */
+            lockmode = isLockedRefname(pstate, refname) ? RowShareLock : AccessShareLock;
+            rel = relation_open(viewoid, lockmode);
...
-            /* ORA-04063: view "xxx.xxx" has errors */
-            elog(ERROR, "view \"%s.%s\" has errors", nspname, viewname);
+            /* ORA-04063: view "xxx.xxx" has errors */
+            ereport(ERROR,
+                    (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                     errmsg("view \"%s.%s\" has errors", nspname, viewname)));

src/include/commands/view.h (1)

25-32: Public force-view prototypes look consistent with view.c.

Consider documenting ownership/lifetime of returned char* from get_force_view_def (pfree by caller) in a follow-up comment.

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

72-75: Typo/consistency in identifier-case constant.

“UNDEFINE” → “UNDEFINED” for clarity (grep for usages before changing).

-#define          IDENT_CASE_UNDEFINE        'u'     /* UNDEFINE */
+#define          IDENT_CASE_UNDEFINED       'u'     /* UNDEFINED */

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

420-442: Avoid double AcquireDeletionLock on rewalk.

Locks were already taken before the first walk; re-acquiring is redundant. Not harmful but can be dropped.

-           AcquireDeletionLock(thisobj, flags);
+           /* Lock already held from the first pass; no need to reacquire here. */

---

1114-1116: Typo in comment.

“funciton” → “function”.

-    * Find out the dependent funciton which uses %TYPE or %ROWTYPE
+    * Find out the dependent function which uses %TYPE or %ROWTYPE

src/backend/commands/view.c (8)

295-368: Improve function documentation and clarify behavior

The function documentation mentions updating metadata when replace is true for existing force views, but the actual implementation at lines 320-323 only sets *need_update = replace without performing any updates. This could be misleading.

Update the comment to clarify that this function only creates placeholders and flags whether updates are needed:

 /*
  * CreateForceVirtualPlaceholder
  *
- * This function creates a placeholder for a force view. If a view with the same
- * name already exists and is a force view, it simply returns its address. If
- * "replace" is specified, it marks the metadata for update. If the existing view
- * is a regular view, it can be invalidated if "replace" is true, otherwise it
- * does nothing. If no view exists, a new placeholder view is created.
+ * This function creates a placeholder for a force view. If a view with the same
+ * name already exists and is a force view, it simply returns its address and
+ * sets need_update flag to indicate whether the caller should update metadata.
+ * If the existing view is a regular view, it can be invalidated if "replace" 
+ * is true. If no view exists, a new placeholder view is created.
+ *
+ * The need_update output parameter indicates whether the caller should update
+ * the pg_force_view catalog entry.
  */

---

699-700: Fix spacing issue in function parameter

There's a missing space after the comma in the function parameter list.

-StoreForceViewQuery(Oid viewOid, bool replace, char ident_case ,const char *queryString)
+StoreForceViewQuery(Oid viewOid, bool replace, char ident_case, const char *queryString)

---

768-778: Remove commented-out debug code

The commented-out error handling code (lines 768-771) should be removed if it's not needed, or uncommented if it represents a valid error condition.

Either remove the commented code or restore it if the error check is valid:

-    #if 0
-    if (!HeapTupleIsValid(tup))
-        elog(ERROR, "cache lookup failed for force view %u", forceviewid);
-    #endif
-
     if (!HeapTupleIsValid(tup))
     {
         table_close(force_view_rel, RowExclusiveLock);
         return;	
     }

---

792-808: Optimize syscache lookups in rel_is_force_view

The function performs two syscache lookups when it could potentially be optimized. Also, the logic assumes that a view without a _RETURN rule is a force view, which might not always be accurate.

Consider adding a more explicit check or documenting the assumption:

 bool
 rel_is_force_view(Oid relid)
 {
+    /* 
+     * A force view is identified by being a view relation without 
+     * a _RETURN rule in pg_rewrite catalog.
+     */
     if (IsUnderPostmaster &&
         get_rel_relkind(relid) == RELKIND_VIEW)
     {
         HeapTuple	tuple;
 
         tuple = SearchSysCache2(RULERELNAME, ObjectIdGetDatum(relid), CStringGetDatum("_RETURN"));
         if (HeapTupleIsValid(tuple))
         {
             ReleaseSysCache(tuple);
             return false;
         }
         else
             return true;
     }
     else
         return false;
 }

---

1036-1042: Fix typo in comment

There's a minor typo in the comment.

-/* 
+/*
  * make_view_invalid
  *
- *		Make a view to force view, which is an invalid state.
+ *		Convert a view to a force view, which is an invalid state.
  *		Currently, the invalidation of a view cannot be issued explicitly. 
  *		In most cases, it is caused by the deletion of dependent objects.
  */

---

1055-1057: Early return can bypass necessary resource cleanup

The early return at line 1056 when the view is already a force view bypasses any necessary cleanup or state validation.

Consider whether any validation or logging should occur even when the view is already invalid:

 if (rel_is_force_view(viewid))
+{
+    elog(DEBUG1, "View %u is already a force view", viewid);
     return true;
+}

---

1163-1165: Redundant check can be removed

The check at line 1163 is redundant since the caller already verified this is a force view.

Consider removing the redundant check or converting it to an assertion:

-if (!rel_is_force_view(viewOid))
-    return true;
+Assert(rel_is_force_view(viewOid));

---

1122-1137: Document and justify IDENT_CASE_UNDEFINE usage

Expand the existing comment to state explicitly that IDENT_CASE_UNDEFINE intentionally defers quoting/case interpretation to pg_get_viewdef (so the stored view SQL may differ from the user-entered quoting) and list compatibility trade-offs; optionally add a code path or GUC to preserve original identifier-case handling when Oracle-compatibility is required.

Location: src/backend/commands/view.c (lines 1122–1137)

• Optional refactor: preserve original identifier case or derive behavior from a DB compatibility setting instead of always using IDENT_CASE_UNDEFINE.

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

2377-2385: ALTER VIEW … COMPILE accepted but consider IF EXISTS parity

The new branch handles ALTER VIEW COMPILE cleanly. For UX parity with other ALTER VIEW branches in this file, consider adding an IF EXISTS form (even if it maps to the same execution), or explicitly documenting why it’s intentionally unsupported.

-       |   ALTER VIEW qualified_name ora_alter_view_cmds
+       |   ALTER VIEW qualified_name ora_alter_view_cmds
         {
           ...
         }
+       |   ALTER VIEW IF_P EXISTS qualified_name ora_alter_view_cmds
+           {
+               AlterTableStmt *n = makeNode(AlterTableStmt);
+               n->relation = $5;
+               n->cmds = $6;
+               n->objtype = OBJECT_VIEW;
+               n->missing_ok = true;
+               $$ = (Node *)n;
+           }

---

2478-2492: ora_alter_view_cmds allows repeated COMPILE without separator

As written, “ALTER VIEW v COMPILE COMPILE” parses into two cmds. If multiple COMPILEs are meaningless, simplify to a single-command nonterminal (or require commas) to avoid surprising accepts.

-ora_alter_view_cmds:
-           ora_alter_view_cmd                              { $$ = list_make1($1); }
-           | ora_alter_view_cmds ora_alter_view_cmd        { $$ = lappend($1, $2); }
-       ;
+ora_alter_view_cmds:
+           ora_alter_view_cmd                              { $$ = list_make1($1); }
+       ;

---

12550-12554: OptViewForce keyword shape is OK; consider Oracle-only gating

If FORCE/NO FORCE should be accepted only in Oracle-compat mode, guard in semantic phase or add a compatibility check to reduce dialect leakage when running in pure PG mode.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro