asolimando commented on code in PR #4139: URL: https://github.com/apache/calcite/pull/4139#discussion_r1915469267
########## core/src/test/java/org/apache/calcite/test/TableInRootSchemaTest.java: ########## @@ -134,16 +118,84 @@ protected RowTable() { } } - /** Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-6764">[CALCITE-6764] + /** Helper class for the test for [CALCITE-6764] below. */ + private static class TableWithNullableRowInMap extends EmptyTable { + protected TableWithNullableRowInMap() { + super(); + } + + @Override public RelDataType getRowType(RelDataTypeFactory typeFactory) { + final PairList<String, RelDataType> columnDesc = PairList.withCapacity(1); + // Schema contains a column whose type is MAP<VARCHAR, ROW(VARCHAR)>, but + // the ROW type can be nullable. This can conceivably be created by a + // declaration such as + // CREATE TABLE T(p MAP<VARCHAR, ROW(k VARCHAR)>); + final RelDataType colType = + typeFactory.createMapType(typeFactory.createSqlType(SqlTypeName.VARCHAR), + new RelRecordType( + StructKind.PEEK_FIELDS, + ImmutableList.of( + new RelDataTypeFieldImpl("K", 0, + typeFactory.createSqlType(SqlTypeName.VARCHAR))), true)); + columnDesc.add("P", colType); + return typeFactory.createStructType(columnDesc); + } + } + + /** Helper class for the test for [CALCITE-6764] below. */ + private static class TableWithNullableRowToplevel extends EmptyTable { + protected TableWithNullableRowToplevel() { + super(); + } + + @Override public RelDataType getRowType(RelDataTypeFactory typeFactory) { + final PairList<String, RelDataType> columnDesc = PairList.withCapacity(1); + // This table can conceivably be created by a declaration such as + // CREATE TABLE T(p ROW(k VARCHAR) NULL); + final RelDataType colType = + new RelRecordType( + StructKind.PEEK_FIELDS, ImmutableList.of( + new RelDataTypeFieldImpl( + "K", 0, typeFactory.createSqlType(SqlTypeName.VARCHAR))), + true); + columnDesc.add("P", colType); + return typeFactory.createStructType(columnDesc); + } + } + + /** Test case for + * <a href="https://issues.apache.org/jira/browse/CALCITE-6764">[CALCITE-6764] * Field access from a nullable ROW should be nullable</a>. */ @Test void testNullableValue() throws Exception { Connection connection = DriverManager.getConnection("jdbc:calcite:"); CalciteConnection calciteConnection = connection.unwrap(CalciteConnection.class); - calciteConnection.getRootSchema().add("T", new RowTable()); + calciteConnection.getRootSchema().add("T", new TableWithNullableRowInMap()); Statement statement = calciteConnection.createStatement(); + // Without the fix to this issue the Validator crashes with an AssertionFailure: + // java.lang.RuntimeException: java.lang.AssertionError: + // Conversion to relational algebra failed to preserve datatypes: + // validated type: ResultSet resultSet = statement.executeQuery("SELECT P['a'].K FROM T"); resultSet.close(); statement.close(); connection.close(); } + + /** Test case for + * <a href="https://issues.apache.org/jira/browse/CALCITE-6764">[CALCITE-6764] + * Field access from a nullable ROW should be nullable</a>. */ + @Test void testNullableValue2() throws Exception { Review Comment: ```suggestion @Test void testNullableRowToplevel() throws Exception { ``` ########## core/src/test/java/org/apache/calcite/test/TableInRootSchemaTest.java: ########## @@ -134,16 +118,84 @@ protected RowTable() { } } - /** Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-6764">[CALCITE-6764] + /** Helper class for the test for [CALCITE-6764] below. */ + private static class TableWithNullableRowInMap extends EmptyTable { + protected TableWithNullableRowInMap() { + super(); + } + + @Override public RelDataType getRowType(RelDataTypeFactory typeFactory) { + final PairList<String, RelDataType> columnDesc = PairList.withCapacity(1); + // Schema contains a column whose type is MAP<VARCHAR, ROW(VARCHAR)>, but + // the ROW type can be nullable. This can conceivably be created by a + // declaration such as + // CREATE TABLE T(p MAP<VARCHAR, ROW(k VARCHAR)>); + final RelDataType colType = + typeFactory.createMapType(typeFactory.createSqlType(SqlTypeName.VARCHAR), + new RelRecordType( + StructKind.PEEK_FIELDS, + ImmutableList.of( + new RelDataTypeFieldImpl("K", 0, + typeFactory.createSqlType(SqlTypeName.VARCHAR))), true)); + columnDesc.add("P", colType); + return typeFactory.createStructType(columnDesc); + } + } + + /** Helper class for the test for [CALCITE-6764] below. */ + private static class TableWithNullableRowToplevel extends EmptyTable { + protected TableWithNullableRowToplevel() { + super(); + } + + @Override public RelDataType getRowType(RelDataTypeFactory typeFactory) { + final PairList<String, RelDataType> columnDesc = PairList.withCapacity(1); + // This table can conceivably be created by a declaration such as + // CREATE TABLE T(p ROW(k VARCHAR) NULL); + final RelDataType colType = + new RelRecordType( + StructKind.PEEK_FIELDS, ImmutableList.of( + new RelDataTypeFieldImpl( + "K", 0, typeFactory.createSqlType(SqlTypeName.VARCHAR))), + true); + columnDesc.add("P", colType); + return typeFactory.createStructType(columnDesc); + } + } + + /** Test case for + * <a href="https://issues.apache.org/jira/browse/CALCITE-6764">[CALCITE-6764] * Field access from a nullable ROW should be nullable</a>. */ @Test void testNullableValue() throws Exception { Review Comment: ```suggestion @Test void testNullableRowInMap() throws Exception { ``` -- 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: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org