gianm commented on code in PR #5349: URL: https://github.com/apache/calcite/pull/5349#discussion_r4406901503
########## core/src/main/java/org/apache/calcite/sql/SqlPipeCall.java: ########## @@ -0,0 +1,114 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.calcite.sql; + +import org.apache.calcite.sql.parser.SqlParserPos; +import org.apache.calcite.sql.validate.SqlConformance; + +import org.checkerframework.checker.nullness.qual.Nullable; + +import static java.util.Objects.requireNonNull; + +/** + * Base class for parse tree nodes that represent operators in SQL pipe + * syntax; for example, {@code FROM emp |> WHERE deptno = 10}. + * + * <p>A pipe operator stays in the parse tree through validation. It is + * validated and converted as its {@link #surrogateSelect}, which reads the + * operator's input through a {@link SqlPipeInput}. For example, the + * surrogate of {@code |> WHERE c} is {@code SELECT * FROM input WHERE c}. + * + * @see SqlConformance#isPipeSyntaxAllowed() + * @see SqlKind#PIPE_OPERATOR + */ +public abstract class SqlPipeOperator extends SqlPipeQuery { + /** The {@code |>} token, with which each operator's name starts. */ + public static final String PIPE = "|>"; + + protected SqlNode query; + + //~ Constructors ----------------------------------------------------------- + + protected SqlPipeOperator(SqlParserPos pos, SqlNode query) { + super(pos); + this.query = requireNonNull(query, "query"); + } + + //~ Methods ---------------------------------------------------------------- + + /** Creates a {@code SELECT} for {@link #surrogateSelect} to return, which + * reads this operator's input through a {@link SqlPipeInput}. */ + protected SqlSelect surrogate(@Nullable SqlNodeList keywordList, + SqlNodeList selectList) { + return surrogate(keywordList, selectList, SqlPipeInput.of(this)); + } + + /** Returns the query that this pipe operator applies to. */ + public SqlNode getQuery() { + return query; + } + + @Override public void setOperand(int i, @Nullable SqlNode operand) { + if (i == 0) { + query = requireNonNull(operand, "operand"); + } else { + super.setOperand(i, operand); + } + } + + @Override public final SqlKind getKind() { + return SqlKind.PIPE_OPERATOR; + } + + /** Definition of a pipe operator. Unparses the input query, then the + * operator's keyword, then a tail supplied by the sub-class. */ + public abstract static class Operator extends SqlPipeQuery.Operator { + protected Operator(String name) { + super(name, SqlKind.PIPE_OPERATOR); + } + + @Override public SqlSyntax getSyntax() { + return SqlSyntax.POSTFIX; + } + + @Override public final void unparse(SqlWriter writer, SqlCall call, + int leftPrec, int rightPrec) { + final SqlPipeOperator pipe = (SqlPipeOperator) call; + // An ORDER_BY frame starts each operator on a new line, unindented. + final SqlWriter.Frame frame = + writer.startList(SqlWriter.FrameTypeEnum.ORDER_BY); + final SqlNode query = pipe.query; + if (query instanceof SqlWith + || query instanceof SqlOrderBy + && ((SqlOrderBy) query).query instanceof SqlWith) { + // Without parentheses, the operator would be re-parsed into the WITH. + final SqlWriter.Frame frame1 = writer.startList("(", ")"); + query.unparse(writer, 0, 0); + writer.endList(frame1); + } else { + query.unparse(writer, 0, 0); + } + writer.sep(getName()); + unparseTail(writer, pipe); + writer.endList(frame); + } + + /** Unparses whatever follows the operator's keyword. */ + protected abstract void unparseTail(SqlWriter writer, Review Comment: It will end up having one implementation per pipe operator (SELECT, EXTEND, AGGREGATE, etc). ########## core/src/main/java/org/apache/calcite/sql/SqlPipeQuery.java: ########## @@ -0,0 +1,75 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.calcite.sql; + +import org.apache.calcite.sql.parser.SqlParserPos; +import org.apache.calcite.sql.util.SqlBasicVisitor; +import org.apache.calcite.sql.util.SqlVisitor; +import org.apache.calcite.sql.validate.SqlConformance; +import org.apache.calcite.sql.validate.SqlValidator; +import org.apache.calcite.sql.validate.SqlValidatorScope; + +import org.checkerframework.checker.nullness.qual.Nullable; + +/** + * Base class for parse tree nodes that represent a query written in SQL pipe + * syntax: a standalone {@code FROM} query, which is the head of a pipeline, + * or a {@link SqlPipeOperator} applied to a query. The query an operator is Review Comment: It seemed like it might be useful context for a reader. I trimmed it down to just: ``` /** * Base class for parse tree nodes that represent a query written in SQL pipe * syntax. Each is validated and converted through its surrogate SELECT; see * {@link #surrogateSelect()}. * * @see SqlConformance#isPipeSyntaxAllowed() * @see SqlKind#PIPE_QUERY */ public abstract class SqlPipeQuery extends SqlCall { ``` ########## core/src/main/java/org/apache/calcite/sql/SqlPipeOperator.java: ########## @@ -0,0 +1,114 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.calcite.sql; + +import org.apache.calcite.sql.parser.SqlParserPos; +import org.apache.calcite.sql.validate.SqlConformance; + +import org.checkerframework.checker.nullness.qual.Nullable; + +import static java.util.Objects.requireNonNull; + +/** + * Base class for parse tree nodes that represent operators in SQL pipe + * syntax; for example, {@code FROM emp |> WHERE deptno = 10}. + * + * <p>A pipe operator stays in the parse tree through validation. It is + * validated and converted as its {@link #surrogateSelect}, which reads the + * operator's input through a {@link SqlPipeInput}. For example, the + * surrogate of {@code |> WHERE c} is {@code SELECT * FROM input WHERE c}. + * + * @see SqlConformance#isPipeSyntaxAllowed() + * @see SqlKind#PIPE_OPERATOR + */ +public abstract class SqlPipeOperator extends SqlPipeQuery { Review Comment: I renamed it to `SqlPipeCall`. (I suppose this makes sense because it represents a call of a pipe operator.) ########## core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java: ########## @@ -1924,6 +1934,95 @@ private void checkCallTypesConsistent(String sql) { sql("table emp").ok(); } + @Test void testFromQuery() { + sql("from emp").withConformance(SqlConformanceEnum.LENIENT).ok(); + } + + /** Tests that a pipeline over a streaming query is still streaming. */ + @Test void testPipeWhereOverStream() { + sql("select stream * from orders |> where productid = 1") + .withConformance(SqlConformanceEnum.LENIENT).ok(); + } + + /** As {@link #testPipeWhereOverStream()}, but the pipeline starts from a + * WITH whose body is the streaming query. */ + @Test void testPipeWhereOverStreamingWith() { + sql("(with d as (select * from dept)\n" + + " select stream * from orders) |> where productid = 1") + .withConformance(SqlConformanceEnum.LENIENT).ok(); + } + + @Test void testPipeWhere() { + sql("from emp |> where deptno = 10") + .withConformance(SqlConformanceEnum.LENIENT).ok(); + } + + /** Tests that {@code |> WHERE} keeps the order of its input. The input's + * sort key, which is not one of its columns, stays as a hidden column. */ + @Test void testPipeWhereKeepsOrder() { + final SqlToRelFixture f = + sql("select ename, deptno from emp order by sal desc\n" + + "|> where deptno = 10") + .withConformance(SqlConformanceEnum.LENIENT); + f.ok(); + assertThat(f.toRoot().collation, hasToString("[2 DESC]")); + } + + /** Tests a correlated sub-query in a {@code |> WHERE} whose input has a + * hidden column for its sort key. */ + @Test void testPipeWhereCorrelatedHiddenSortKey() { + sql("select ename, deptno as dno from emp order by sal + 1 desc\n" + + "|> where exists (select 1 from dept as d where d.deptno = dno)") + .withConformance(SqlConformanceEnum.LENIENT) + .withDecorrelate(false).ok(); + } + + /** Tests a pipe query used as an expression sub-query, correlated on a + * table alias of the enclosing pipeline. */ + @Test void testPipeQueryAsCorrelatedSubQuery() { + sql("from emp as e\n" + + "|> where exists (from dept |> where deptno = e.deptno)") + .withConformance(SqlConformanceEnum.LENIENT).ok(); + } + + /** Tests that implicit type coercion casts the column of a pipe query that + * is an {@code IN} sub-query, as it would for a SELECT. */ + @Test void testPipeWhereInSubQueryCoercion() { + sql("select * from emp\n" Review Comment: The plan has a `CAST($0):BIGINT NOT NULL` in it, which is what this test is ultimately verifying. I added a comment about it here. ########## core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java: ########## @@ -1924,6 +1934,95 @@ private void checkCallTypesConsistent(String sql) { sql("table emp").ok(); } + @Test void testFromQuery() { + sql("from emp").withConformance(SqlConformanceEnum.LENIENT).ok(); + } + + /** Tests that a pipeline over a streaming query is still streaming. */ + @Test void testPipeWhereOverStream() { + sql("select stream * from orders |> where productid = 1") + .withConformance(SqlConformanceEnum.LENIENT).ok(); + } + + /** As {@link #testPipeWhereOverStream()}, but the pipeline starts from a + * WITH whose body is the streaming query. */ + @Test void testPipeWhereOverStreamingWith() { + sql("(with d as (select * from dept)\n" + + " select stream * from orders) |> where productid = 1") + .withConformance(SqlConformanceEnum.LENIENT).ok(); + } + + @Test void testPipeWhere() { + sql("from emp |> where deptno = 10") + .withConformance(SqlConformanceEnum.LENIENT).ok(); + } + + /** Tests that {@code |> WHERE} keeps the order of its input. The input's + * sort key, which is not one of its columns, stays as a hidden column. */ + @Test void testPipeWhereKeepsOrder() { + final SqlToRelFixture f = + sql("select ename, deptno from emp order by sal desc\n" + + "|> where deptno = 10") + .withConformance(SqlConformanceEnum.LENIENT); + f.ok(); + assertThat(f.toRoot().collation, hasToString("[2 DESC]")); + } + + /** Tests a correlated sub-query in a {@code |> WHERE} whose input has a + * hidden column for its sort key. */ + @Test void testPipeWhereCorrelatedHiddenSortKey() { + sql("select ename, deptno as dno from emp order by sal + 1 desc\n" + + "|> where exists (select 1 from dept as d where d.deptno = dno)") + .withConformance(SqlConformanceEnum.LENIENT) + .withDecorrelate(false).ok(); + } + + /** Tests a pipe query used as an expression sub-query, correlated on a + * table alias of the enclosing pipeline. */ + @Test void testPipeQueryAsCorrelatedSubQuery() { + sql("from emp as e\n" + + "|> where exists (from dept |> where deptno = e.deptno)") + .withConformance(SqlConformanceEnum.LENIENT).ok(); + } + + /** Tests that implicit type coercion casts the column of a pipe query that + * is an {@code IN} sub-query, as it would for a SELECT. */ + @Test void testPipeWhereInSubQueryCoercion() { + sql("select * from emp\n" + + "where cast(deptno as bigint) in\n" + + " (select deptno from dept |> where deptno > 0)") + .withConformance(SqlConformanceEnum.LENIENT) + .withExpand(false) + .ok(); + } + + @Test void testPipeWhereWrapsSelect() { + sql("select ename, sal from emp |> where sal > 100") + .withConformance(SqlConformanceEnum.LENIENT).ok(); + } + + @Test void testPipeWhereAfterUnion() { + sql("select deptno from emp union all select deptno from dept\n" + + "|> where deptno = 10") + .withConformance(SqlConformanceEnum.LENIENT).ok(); + } + + @Test void testPipeWhereAfterWith() { + // The operator applies to the body of the WITH, above its ORDER BY. Review Comment: The `|> WHERE`. Updated the comment. -- 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]
