vvysotskyi commented on a change in pull request #1792: DRILL-7250: Query with 
CTE fails when its name matches to the table name without access
URL: https://github.com/apache/drill/pull/1792#discussion_r285537110
 
 

 ##########
 File path: 
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/SqlConverter.java
 ##########
 @@ -280,12 +280,10 @@ protected void validateFrom(
             changeNamesIfTableIsTemporary(tempNode);
 
             // Check the schema and throw a valid SchemaNotFound exception 
instead of TableNotFound exception.
-            if (catalogReader.getTable(tempNode.names) == null) {
-              catalogReader.isValidSchema(tempNode.names);
-            }
+            catalogReader.isValidSchema(tempNode.names);
 
 Review comment:
   Good question, when it is set in the constructor or using a setter of 
`SqlIdentifier` class, it cannot be null, since there is used 
`ImmutableList.copyOf()` method for incoming lists. But this field is public 
and may be changed in some other places (I didn't find any place where it is 
set to null in the current Drill and Calcite code).
   
   Since Calcite's `SqlIdentifier` code does not assume that `names` may be 
null (there is a lot of code where this field is used without checks), I think 
we should also do not check it.

----------------------------------------------------------------
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:
[email protected]


With regards,
Apache Git Services

Reply via email to