This is an automated email from the ASF dual-hosted git repository. jhyde pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/calcite.git
commit f1309fa3af6826e73377e54081160228eb7ab951 Author: Julian Hyde <[email protected]> AuthorDate: Mon May 3 19:18:33 2021 -0700 [CALCITE-4594] Interpreter returns wrong result when Values has zero fields Currently, if a Values RelNode has a zero-field row type and N rows, the Interpreter returns zero rows; it should, of course, return N rows. As a project, we have not decided whether to allow zero-field row types. Unless and until we ban them (by throwing whenever we see them), the Interpreter should do its best when it sees them, not return wrong results. --- .../org/apache/calcite/interpreter/ValuesNode.java | 4 +- .../org/apache/calcite/test/InterpreterTest.java | 67 +++++++++++++++++----- 2 files changed, 55 insertions(+), 16 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/interpreter/ValuesNode.java b/core/src/main/java/org/apache/calcite/interpreter/ValuesNode.java index 6800f94..b1a8ac4 100644 --- a/core/src/main/java/org/apache/calcite/interpreter/ValuesNode.java +++ b/core/src/main/java/org/apache/calcite/interpreter/ValuesNode.java @@ -53,8 +53,8 @@ public class ValuesNode implements Node { scalar.execute(context, values); final ImmutableList.Builder<Row> rows = ImmutableList.builder(); Object[] subValues = new Object[fieldCount]; - for (int i = 0; i < values.length; i += fieldCount) { - System.arraycopy(values, i, subValues, 0, fieldCount); + for (int r = 0, n = tuples.size(); r < n; ++r) { + System.arraycopy(values, r * fieldCount, subValues, 0, fieldCount); rows.add(Row.asCopy(subValues)); } return rows.build(); diff --git a/core/src/test/java/org/apache/calcite/test/InterpreterTest.java b/core/src/test/java/org/apache/calcite/test/InterpreterTest.java index 16d590e..b4a52d7 100644 --- a/core/src/test/java/org/apache/calcite/test/InterpreterTest.java +++ b/core/src/test/java/org/apache/calcite/test/InterpreterTest.java @@ -30,6 +30,7 @@ import org.apache.calcite.rel.RelRoot; import org.apache.calcite.rel.rules.CoreRules; import org.apache.calcite.rel.type.RelDataType; import org.apache.calcite.rel.type.RelDataTypeField; +import org.apache.calcite.rex.RexLiteral; import org.apache.calcite.schema.ScalarFunction; import org.apache.calcite.schema.SchemaPlus; import org.apache.calcite.schema.TableFunction; @@ -42,11 +43,14 @@ import org.apache.calcite.sql.parser.SqlParser; import org.apache.calcite.tools.FrameworkConfig; import org.apache.calcite.tools.Frameworks; import org.apache.calcite.tools.Planner; +import org.apache.calcite.tools.RelBuilder; import org.apache.calcite.tools.RelConversionException; import org.apache.calcite.tools.ValidationException; import org.apache.calcite.util.Smalls; import org.apache.calcite.util.Util; +import com.google.common.collect.ImmutableList; + import org.checkerframework.checker.nullness.qual.Nullable; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -73,12 +77,12 @@ class InterpreterTest { /** Implementation of {@link DataContext} for executing queries without a * connection. */ private static class MyDataContext implements DataContext { - private SchemaPlus rootSchema; - private final Planner planner; + private final SchemaPlus rootSchema; + private final JavaTypeFactory typeFactory; - MyDataContext(SchemaPlus rootSchema, Planner planner) { + MyDataContext(SchemaPlus rootSchema, RelNode rel) { this.rootSchema = rootSchema; - this.planner = planner; + this.typeFactory = (JavaTypeFactory) rel.getCluster().getTypeFactory(); } public SchemaPlus getRootSchema() { @@ -86,7 +90,7 @@ class InterpreterTest { } public JavaTypeFactory getTypeFactory() { - return (JavaTypeFactory) planner.getTypeFactory(); + return typeFactory; } public @Nullable QueryProvider getQueryProvider() { @@ -103,16 +107,23 @@ class InterpreterTest { private final String sql; private final SchemaPlus rootSchema; private final boolean project; + private final Function<RelBuilder, RelNode> relFn; - Sql(String sql, SchemaPlus rootSchema, boolean project) { + Sql(String sql, SchemaPlus rootSchema, boolean project, + @Nullable Function<RelBuilder, RelNode> relFn) { this.sql = sql; this.rootSchema = rootSchema; this.project = project; + this.relFn = relFn; } @SuppressWarnings("SameParameterValue") Sql withProject(boolean project) { - return new Sql(sql, rootSchema, project); + return new Sql(sql, rootSchema, project, relFn); + } + + Sql withRel(Function<RelBuilder, RelNode> relFn) { + return new Sql(sql, rootSchema, project, relFn); } /** Interprets the sql and checks result with specified rows, ordered. */ @@ -138,14 +149,30 @@ class InterpreterTest { return Frameworks.getPlanner(config); } + /** Performs an action that requires a {@link RelBuilder}, and returns the + * result. */ + private <T> T withRelBuilder(Function<RelBuilder, T> fn) { + final FrameworkConfig config = Frameworks.newConfigBuilder() + .defaultSchema(rootSchema) + .build(); + final RelBuilder relBuilder = RelBuilder.create(config); + return fn.apply(relBuilder); + } + /** Interprets the sql and checks result with specified rows. */ private Sql returnsRows(boolean unordered, String[] rows) { try (Planner planner = createPlanner()) { - SqlNode parse = planner.parse(sql); - SqlNode validate = planner.validate(parse); - final RelRoot root = planner.rel(validate); - RelNode convert = project ? root.project() : root.rel; - final MyDataContext dataContext = new MyDataContext(rootSchema, planner); + final RelNode convert; + if (relFn != null) { + convert = withRelBuilder(relFn); + } else { + SqlNode parse = planner.parse(sql); + SqlNode validate = planner.validate(parse); + final RelRoot root = planner.rel(validate); + convert = project ? root.project() : root.rel; + } + final MyDataContext dataContext = + new MyDataContext(rootSchema, convert); assertInterpret(convert, dataContext, unordered, rows); return this; } catch (ValidationException @@ -158,7 +185,7 @@ class InterpreterTest { /** Creates a {@link Sql}. */ private Sql sql(String sql) { - return new Sql(sql, rootSchema, false); + return new Sql(sql, rootSchema, false, null); } private void reset() { @@ -476,7 +503,8 @@ class InterpreterTest { final HepPlanner hepPlanner = new HepPlanner(program); hepPlanner.setRoot(convert); final RelNode relNode = hepPlanner.findBestExp(); - final MyDataContext dataContext = new MyDataContext(rootSchema, planner); + final MyDataContext dataContext = + new MyDataContext(rootSchema, relNode); assertInterpret(relNode, dataContext, true, "[1, a]", "[3, c]"); } catch (ValidationException | SqlParseException @@ -674,4 +702,15 @@ class InterpreterTest { + "from table(\"s\".\"t\"('=100='))"; sql(sql).returnsRows("[1]", "[3]", "[100]"); } + + /** Tests projecting zero fields. */ + @Test void testZeroFields() { + final List<RexLiteral> row = ImmutableList.of(); + sql("?") + .withRel(b -> + b.values(ImmutableList.of(row, row), + b.getTypeFactory().builder().build()) + .build()) + .returnsRows("[]", "[]"); + } }
