[CALCITE-3210] Fix the bug that RelToSqlConverter converts "cast(null as $type)" just as null by weidong3630 · Pull Request #1338 · apache/calcite

weidong3630 · 2019-07-29T07:01:15Z

Fix the bug described in https://issues.apache.org/jira/browse/CALCITE-3210?filter=-1

danny0405 · 2019-07-29T07:30:35Z

@weidong3630 You can have multiple commits to one PR and do not close the old one, the committers would rebase the code when merging into master.

weidong3630 · 2019-07-29T07:56:15Z

@weidong3630 You can have multiple commits to one PR and do not close the old one, the committers would rebase the code when merging into master.

Thanks for your advise, I just do it according to the calcite document. I will be attention to it later.

core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java

julianhyde · 2019-07-29T17:42:17Z

The first word of a commit message should generally not be "Fix". I changed the JIRA subject to "JDBC adapter should generate "CAST(NULL AS type)" rather than "NULL", so maybe use that as the commit message.

julianhyde

Spaces after '//' please.

Javadoc method comments should start with a verb, first letter upper-case, and end with a period. Rather than "Check whether null item in select need to be casted", write "Returns whether a NULL literal in the SELECT clause needs to be wrapped in a CAST." In the javadoc of that method, can you explain what the algorithm is? I guess you want to wrap if and only if the expression will end up in a SELECT clause? In which case the javadoc should be "Returns whether a NULL literal needs to be wrapped in a CAST."

weidong3630 · 2019-07-30T03:52:52Z

I've adopted your suggestions and modify my code and commit message. @julianhyde

danny0405 · 2019-07-31T01:23:56Z

core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java

+   *    because that the type of NULL can be inferred  by the target table of TableModify,
+   *    so NULL does not need to be wrapped in a CAST.
+   *    Here is an example,
+   *    "INSERT INTO t SELECT CAST(NULL AS INT) AS c1, c2 FROM t1"


What do you mean by "SingleRel", i didn't see any in your example.

Implemented done.

core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java

core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java

core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java

weidong3630 · 2019-08-06T08:10:06Z

I've solved some conflicts and rebase the commit. Who can help me to review or merge this code?

core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java

core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java

danny0405 · 2019-08-20T11:24:09Z

core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java

+    sql(expected).exec();
+  }
+
+  @Test public void testSelectNullWithInsert() {


Thanks, can we also give a test case for Join relational expression ?

Done, see testSelectNullWithInsertFromJoin.

[CALCITE-3267] Remove method SqlDataTypeSpec#deriveType(RelDataTypefa…

…er than "NULL" conditionally (Wang Weidong)

…er than "NULL" conditionally (Wang Weidong) Add test case of "testSelectNullWithInsertFromJoin".

danny0405 · 2019-08-21T09:12:01Z

core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java

+            + "\"account\"(\"account_id\",\"account_parent\",\"account_type\",\"account_rollup\")"
+            + "\tselect \"product\".\"product_id\", \n"
+            + "\tcast(NULL AS INT), \n"
+            + "\tcast(\"product\".\"product_id\" as varchar), \n"


Can we replace all these "\t" with "\n" ? Also put a "\n" at the end of every sql statement line.

…her than "NULL" conditionally (Wang Weidong) Modify format of test case.

danny0405

+1 , except some nit problems about code style, will fix when merging.

…er than "NULL" conditionally (Wang Weidong) Always wrap NULL literal in Project project list with CAST operator during rel-to-sql conversion. close apache#1338