Copilot commented on code in PR #2341:
URL: https://github.com/apache/age/pull/2341#discussion_r2897209721


##########
src/backend/parser/cypher_clause.c:
##########
@@ -6583,6 +6605,59 @@ static bool clause_chain_has_dml(cypher_clause *clause)
     return false;
 }
 
+/*
+ * Build a false WHERE clause that forces a MATCH to return zero rows.
+ * Used when the MATCH pattern references a label that does not exist.
+ *
+ * When volatile_needed is false, returns a constant (true = false) that
+ * PG's planner may constant-fold — this is fine when there is no DML

Review Comment:
   These comments introduce non-ASCII em dashes (—). This repo’s C sources 
otherwise appear to be ASCII-only, and non-ASCII punctuation can cause 
encoding/tooling issues in some build environments. Consider replacing these 
with plain '-' or '--' throughout the new comments.
   ```suggestion
    * PG's planner may constant-fold -- this is fine when there is no DML
   ```



##########
src/backend/parser/cypher_clause.c:
##########
@@ -2949,6 +2952,25 @@ static Query 
*transform_cypher_match_pattern(cypher_parsestate *cpstate,
              */
             pnsi = get_namespace_item(pstate, rte);
             query->targetList = expandNSItemAttrs(pstate, pnsi, 0, true, -1);
+
+            /*
+             * Now that the predecessor chain is fully transformed and
+             * any CREATE-generated labels exist in the cache, check
+             * whether the MATCH pattern references valid labels. This
+             * deferred check is only needed when the chain has DML,
+             * since labels created by CREATE are not in the cache at
+             * the time of the early check in transform_cypher_match().
+             *
+             * We use a volatile false predicate (random() IS NULL)
+             * instead of a constant one (true = false) because PG's
+             * planner can constant-fold the latter into a One-Time
+             * Filter: false, eliminating the entire plan subtree —
+             * including the DML predecessor scan — without executing it.

Review Comment:
   The PR description says the deferred invalid-label path injects 
`makeBoolConst(false, false)`, but the implementation uses a volatile predicate 
(`random() IS NULL`) to avoid constant-fold plan elimination. Please update the 
PR description to match the actual approach so future readers don’t 
implement/debug the wrong behavior.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to