[CALCITE-7415] CalciteCatalogReader.lookupOperatorOverloads keeps original function identifier casing instead of resolved schema-path casing#4794
Conversation
zzwqqq
force-pushed
the
fix_func_identifier
branch
from
89bd682 to
63c10f1
Compare
…ginal function identifier casing instead of resolved schema-path casing
zzwqqq
force-pushed
the
fix_func_identifier
branch
from
63c10f1 to
9e9a8f4
Compare
| return functions2; | ||
| } | ||
|
|
||
| private static SqlIdentifier resolvedFunctionIdentifier(CalciteSchema schema, |
There was a problem hiding this comment.
I don't really see what this has anything to do with functions. This function seems to be very generic, and work for all all identifiers. Could it be called "createResolvedIdentifier"?
There was a problem hiding this comment.
I have rename resolvedFunctionIdentifier to createResolvedIdentifier. Thanks.
| resolvedNames.addAll( | ||
| schemaPath.subList(schemaPath.size() - resolvedQualifierCount, schemaPath.size())); | ||
| resolvedNames.add(functionName); | ||
| return new SqlIdentifier(resolvedNames, SqlParserPos.ZERO); |
There was a problem hiding this comment.
How about passing the position of the original identifier too?
Positions can be very useful for error reporting.
There was a problem hiding this comment.
Thanks for catching this.
I have updated to keep the original identifier position (opName.getParserPosition()) when creating the resolved identifier, instead of using SqlParserPos.ZERO
| checkInternal(false); | ||
| } | ||
|
|
||
| @Test void testLookupQualifiedNameUsesResolvedCase() { |
There was a problem hiding this comment.
Could you add a comment for each of these summarizing what is being tested?
E.g., "lookup MyCatalog.MySchema.MyFUNC as myfunc"
There was a problem hiding this comment.
I have added short comments above each new test to summarize the exact lookup scenario being verified (including concrete identifier examples). Thanks.
…riginal SqlParserPos andd add comments for tests
2 participants
see https://issues.apache.org/jira/projects/CALCITE/issues/CALCITE-7415