This is an automated email from the ASF dual-hosted git repository.
libenchao pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/main by this push:
new 5ee1b1ba2c [CALCITE-5553] `RelStructuredTypeFlattener` produces bad
plan for single field struct
5ee1b1ba2c is described below
commit 5ee1b1ba2c9bd347f098bca758fd3032541db5e9
Author: Andrew Pilloud <apill...@google.com>
AuthorDate: Mon Mar 6 13:31:36 2023 -0800
[CALCITE-5553] `RelStructuredTypeFlattener` produces bad plan for single
field struct
Close apache/calcite#3092
---
.../sql2rel/RelStructuredTypeFlattener.java | 15 ++++++++++++--
.../apache/calcite/sql/test/SqlAdvisorTest.java | 1 +
.../apache/calcite/test/SqlToRelConverterTest.java | 6 ++++++
.../apache/calcite/test/SqlToRelConverterTest.xml | 14 +++++++++++++
.../org/apache/calcite/test/catalog/Fixture.java | 3 +++
.../test/catalog/MockCatalogReaderSimple.java | 23 +++++++++++-----------
6 files changed, 48 insertions(+), 14 deletions(-)
diff --git
a/core/src/main/java/org/apache/calcite/sql2rel/RelStructuredTypeFlattener.java
b/core/src/main/java/org/apache/calcite/sql2rel/RelStructuredTypeFlattener.java
index 3c3ec316ba..1d0570eb7a 100644
---
a/core/src/main/java/org/apache/calcite/sql2rel/RelStructuredTypeFlattener.java
+++
b/core/src/main/java/org/apache/calcite/sql2rel/RelStructuredTypeFlattener.java
@@ -337,8 +337,19 @@ public class RelStructuredTypeFlattener implements
ReflectiveVisitor {
for (RelNode input : inputs) {
fieldCnt += input.getRowType().getFieldCount();
if (fieldCnt > fieldIdx) {
- return getNewForOldRel(input).getRowType().getFieldList().size()
- == input.getRowType().getFieldList().size();
+ List<RelDataTypeField> newTypeFields =
getNewForOldRel(input).getRowType().getFieldList();
+ List<RelDataTypeField> inputTypeFields =
input.getRowType().getFieldList();
+ if (newTypeFields.size() != inputTypeFields.size()) {
+ return false;
+ }
+ // Ensure single field nested structs aren't flattened
+ for (int i = 0; i < newTypeFields.size(); ++i) {
+ if (newTypeFields.get(i).getType().isStruct()
+ != inputTypeFields.get(i).getType().isStruct()) {
+ return false;
+ }
+ }
+ return true;
}
}
return false;
diff --git a/core/src/test/java/org/apache/calcite/sql/test/SqlAdvisorTest.java
b/core/src/test/java/org/apache/calcite/sql/test/SqlAdvisorTest.java
index 7a2cd77358..e9443a0626 100644
--- a/core/src/test/java/org/apache/calcite/sql/test/SqlAdvisorTest.java
+++ b/core/src/test/java/org/apache/calcite/sql/test/SqlAdvisorTest.java
@@ -90,6 +90,7 @@ class SqlAdvisorTest extends SqlValidatorTestCase {
"TABLE(CATALOG.SALES.EMPTY_PRODUCTS)",
"TABLE(CATALOG.SALES.EMP_ADDRESS)",
"TABLE(CATALOG.SALES.DEPT)",
+ "TABLE(CATALOG.SALES.DEPT_SINGLE)",
"TABLE(CATALOG.SALES.DEPT_NESTED)",
"TABLE(CATALOG.SALES.DEPT_NESTED_EXPANDED)",
"TABLE(CATALOG.SALES.BONUS)",
diff --git
a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
index 60215c3e06..7541d68e27 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
@@ -3715,6 +3715,12 @@ class SqlToRelConverterTest extends SqlToRelTestBase {
sql(sql).ok();
}
+ @Test void testNestedStructSingleFieldAccessWhere() {
+ final String sql = "select dn.skill\n"
+ + "from sales.dept_single dn WHERE dn.skill.type = ''";
+ sql(sql).ok();
+ }
+
@Test void testFunctionWithStructInput() {
final String sql = "select json_type(skill)\n"
+ "from sales.dept_nested";
diff --git
a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
index 1636bc7c17..25c1a3c302 100644
--- a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
@@ -4776,6 +4776,20 @@ from sales.dept_nested dn]]>
<![CDATA[
LogicalProject(EXPR$0=[$2.OTHERS.A])
LogicalTableScan(table=[[CATALOG, SALES, DEPT_NESTED]])
+]]>
+ </Resource>
+ </TestCase>
+ <TestCase name="testNestedStructSingleFieldAccessWhere">
+ <Resource name="sql">
+ <![CDATA[select dn.skill
+from sales.dept_single dn WHERE dn.skill.type = '']]>
+ </Resource>
+ <Resource name="plan">
+ <![CDATA[
+LogicalProject(SKILL=[ROW($0)])
+ LogicalFilter(condition=[=($0, '')])
+ LogicalProject(TYPE=[$0.TYPE])
+ LogicalTableScan(table=[[CATALOG, SALES, DEPT_SINGLE]])
]]>
</Resource>
</TestCase>
diff --git a/testkit/src/main/java/org/apache/calcite/test/catalog/Fixture.java
b/testkit/src/main/java/org/apache/calcite/test/catalog/Fixture.java
index 8a527b4b84..f1e97dc469 100644
--- a/testkit/src/main/java/org/apache/calcite/test/catalog/Fixture.java
+++ b/testkit/src/main/java/org/apache/calcite/test/catalog/Fixture.java
@@ -68,6 +68,9 @@ final class Fixture extends AbstractFixture {
.build())
.kind(StructKind.PEEK_FIELDS_NO_EXPAND)
.build();
+ final RelDataType singleRecordType = typeFactory.builder()
+ .add("TYPE", varchar10Type)
+ .build();
final RelDataType abRecordType = typeFactory.builder()
.add("A", varchar10Type)
.add("B", varchar10Type)
diff --git
a/testkit/src/main/java/org/apache/calcite/test/catalog/MockCatalogReaderSimple.java
b/testkit/src/main/java/org/apache/calcite/test/catalog/MockCatalogReaderSimple.java
index 6171e69c7e..4e60734f72 100644
---
a/testkit/src/main/java/org/apache/calcite/test/catalog/MockCatalogReaderSimple.java
+++
b/testkit/src/main/java/org/apache/calcite/test/catalog/MockCatalogReaderSimple.java
@@ -158,6 +158,12 @@ public class MockCatalogReaderSimple extends
MockCatalogReader {
deptTable.addColumn("NAME", fixture.varchar10Type);
registerTable(deptTable);
+ // Register "DEPT_SINGLE" table.
+ MockTable deptSingleTable =
+ MockTable.create(this, salesSchema, "DEPT_SINGLE", false, 4);
+ deptSingleTable.addColumn("SKILL", fixture.singleRecordType);
+ registerTable(deptSingleTable);
+
// Register "DEPT_NESTED" table.
MockTable deptNestedTable =
MockTable.create(this, salesSchema, "DEPT_NESTED", false, 4);
@@ -291,13 +297,10 @@ public class MockCatalogReaderSimple extends
MockCatalogReader {
registerTable(suppliersTable);
// Register "EMP_20" and "EMPNULLABLES_20 views.
- // Same columns as "EMP" amd "EMPNULLABLES",
- // but "DEPTNO" not visible and set to 20 by default
- // and "SAL" is visible but must be greater than 1000,
- // which is the equivalent of:
+ // Same columns as "EMP" amd "EMPNULLABLES", but "DEPTNO" not visible and
set to 20 by default
+ // and "SAL" is visible but must be greater than 1000, which is the
equivalent of:
// SELECT EMPNO, ENAME, JOB, MGR, HIREDATE, SAL, COMM, SLACKER
- // FROM EMP
- // WHERE DEPTNO = 20 AND SAL > 1000
+ // FROM EMP WHERE DEPTNO = 20 AND SAL > 1000
final ImmutableIntList m0 = ImmutableIntList.of(0, 1, 2, 3, 4, 5, 6, 8);
MockTable emp20View =
new MockViewTable(this, salesSchema.getCatalogName(),
salesSchema.getName(),
@@ -412,12 +415,8 @@ public class MockCatalogReaderSimple extends
MockCatalogReader {
registerTable(structNullableTypeTable);
// Register "STRUCT.T_10" view.
- // Same columns as "STRUCT.T",
- // but "F0.C0" is set to 10 by default,
- // which is the equivalent of:
- // SELECT *
- // FROM T
- // WHERE F0.C0 = 10
+ // Same columns as "STRUCT.T", but "F0.C0" is set to 10 by default, which
is the equivalent of:
+ // SELECT * FROM T WHERE F0.C0 = 10
// This table uses MockViewTable which does not populate the constrained
columns with default
// values on INSERT.
final ImmutableIntList m1 = ImmutableIntList.of(0, 1, 2, 3, 4, 5, 6, 7, 8);
