gianm commented on code in PR #5349:
URL: https://github.com/apache/calcite/pull/5349#discussion_r4406903364


##########
core/src/main/java/org/apache/calcite/sql/SqlKind.java:
##########
@@ -593,6 +595,24 @@ public enum SqlKind {
    */
   EXPLICIT_TABLE,
 
+  /**
+   * Standalone {@code FROM}, e.g. {@code FROM t}, in pipe syntax.
+   */
+  FROM_QUERY,
+
+  /**
+   * Pipe operator, e.g. {@code |> WHERE c}.
+   */
+  PIPE_OPERATOR,

Review Comment:
   Renamed it to `PIPE_CALL`.



##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -3725,6 +3747,49 @@ private void registerQuery(
     }
   }
 
+  /**
+   * Registers a query written in pipe syntax. Each is registered along with

Review Comment:
   Added it like this:
   
   ```
       // Register the query's surrogate SELECT (see
       // SqlPipeQuery#surrogateSelect) as if it were the query. This is what 
lets
       // a pipe query reuse the validation of SELECT.
       final SqlSelect surrogate = node.surrogateSelect();
   ```



##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -3725,6 +3747,49 @@ private void registerQuery(
     }
   }
 
+  /**
+   * Registers a query written in pipe syntax. Each is registered along with
+   * its {@link SqlPipeQuery#surrogateSelect()}.
+   */
+  private void registerPipe(SqlValidatorScope parentScope,
+      @Nullable SqlValidatorScope usingScope, SqlNode node,
+      SqlNode enclosingNode, @Nullable String alias, boolean forceNullable) {
+    if (node instanceof SqlPipeOperator) {
+      // PipeNamespace decides, when it is validated, what this operator sees
+      // of its input.
+      final SqlNode input = ((SqlPipeOperator) node).getQuery();
+      registerQuery(parentScope, null, input, input, null, false);
+    }
+    final SqlSelect surrogate = ((SqlPipeQuery) node).surrogateSelect();
+    registerQuery(parentScope, null, surrogate, node, null, false);
+    // The query's expressions are resolved in its surrogate's scopes, so the
+    // query itself needs only the scope it is written in.

Review Comment:
   I went into a rabbit hole on this one trying to figure out exactly what this 
is used for. For pipe operator calls in particular, as far as I can tell it is 
only be used by `validateInsert`, which needs to recover the scope for the 
query attached to an `INSERT`. The test 
`SqlValidatorTest.testPipeQueryAsInsertSource` seems to pass equally well 
whether `parentScope` or `getSelectScope(surrogate)` is written here.
   
   The latter seems more principled given the javadoc for the `scopes` map, so 
I updated it. Now this reads:
   
   ```
       // Associate this node with the scope created for its surrogate, so it
       // can be found by e.g. validateInsert.
       scopes.put(node, getSelectScope(surrogate));
   ```



-- 
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