[GitHub] [calcite] my7ym commented on a change in pull request #1104: [CALCITE-2900] RewriteRexShuttle generates wrong type during flatten for structured type when visitInputRef

[GitBox <gi...@apache.org>]

my7ym commented on a change in pull request #1104: [CALCITE-2900] RewriteRexShuttle generates wrong type during flatten for structured type when visitInputRef
URL: https://github.com/apache/calcite/pull/1104#discussion_r266268800
 
 

 ##########
 File path: core/src/test/java/org/apache/calcite/tools/PlannerTest.java
 ##########
 @@ -996,6 +997,66 @@ private void checkBushy(String sql, String expected) throws Exception {
     assertThat(toString(transform), containsString(expected));
   }
 
+  private void checkFlattenTypesOnNestedColumn(String sql, String expected) throws Exception {
 
 Review comment:
   I could change the name checkQueryPlan or some other better names you could think of.
   
   I actually wanted to merge the checkBushy and the new helper together. But actually it may be cleaner to keep them both because 
   1. FrameworkConfig is different because default will only identify table `BOOKSTORE` but not `bookstore` and similar things.
   2. defaultSchema is different.
   3. More importantly, flow is different. checkBushy needs `project`, convert traitSet and transform while my test do not need them.
   
   If I force to merge them, it won't necessarily make it better. That's why I did not merge them.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services