DRILL-4682: Allow full schema identifier in SELECT clause#549
DRILL-4682: Allow full schema identifier in SELECT clause#549vdiravka wants to merge 2 commits intoapache:masterfrom
Conversation
vdiravka
commented
Jul 20, 2016
vdiravka
commented
- changed SqlBracketlessSyntax logic in the DrillCompoundIdentifier;
- added checking of using full schema name in column names;
- added unit test testColumnNamesWithSchemaName;
| } No newline at end of file | ||
| @Test // DRILL-4682 | ||
| public void testColumnNamesWithSchemaName() throws Exception { | ||
| test("SELECT cp.`employee.json`.department_id FROM cp.`employee.json` ORDER BY cp.`employee.json`.department_id"); |
There was a problem hiding this comment.
first comment. Please use TestBuilder to verify some query result. Using test() only verifies query is completed.
There was a problem hiding this comment.
Done in the new commit.
- changed SqlBracketlessSyntax logic in the DrillCompoundIdentifier; - added checking of using full schema name in column names; - added unit test testColumnNamesWithSchemaName;
vdiravka
force-pushed
the
DRILL-4682
branch
4 times, most recently
from
d9cb47e to
2c505a3
Compare
|
@jinfengni Could you please review the changes |
|
|
||
| @Test | ||
| @Ignore | ||
| public void testNestedQueryInWhereStatement() throws Exception { |
There was a problem hiding this comment.
I'm not fully convinced that the current patch is ready to merge, before we could resolve the case where full-schema qualified names appear in a subquery.
With your patch, looks like we can only allow fully-schema qualified name in a top-level query, which is kind of restricted use case. It's hard to tell Drill user that they only can use one special case, but not for other cases.
I would suggest we look into the Calcite in more detail, to understand the problem more.
|
I feel it might be better to work on this issue after Drill rebases Calcite. I debugged a bit in SqlValidator, and seem that the code in Drill's forked Calcite is different from what Calcite master has. It's possible that some of patches (star column) in Drill's forked Calcite changes the behavior, and hence run into problem with the subquery cases. Since star column logic has been merged to Calcite master, it's better to try with this patch after rebasing on top of Calcite master, in stead of coming up with a temporary solution with restrictions. |
|
@jinfengni Agree. It will be better to merge this patch after rebasing to the lastest version of calcite. Because currently I'm not sure how to solve this task in different manner than mine. |
|
@vdiravka Are you planning to complete this change? |
4 participants
