[jira] [Comment Edited] (CALCITE-2928) Make UDF lookup default to case insensitive
[
https://issues.apache.org/jira/browse/CALCITE-2928?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16803610#comment-16803610
]
Danny Chan edited comment on CALCITE-2928 at 3/28/19 6:09 AM:
--
[~julianhyde] i have fire a
[PR#1137|https://github.com/apache/calcite/pull/1137]
Initially i wanna change both builtin operators and UDFs case sensitive
internal, and control the case-sensitiveness through config parser config[2].
The problems i encounter when doing this patch:
1.There is a SqlAbstractParserImpl which extended by all the SqlParser, so
there is no change to see the case-sensitivless when parsing the builtin
operators in SqlStdOperatorTable[1], and i have to hard code the case
sensitiveness to be false.
2. Because of 1 i have to make the
ReflectiveSqlOperatorTable#lookupOperatorOverloads case- insensitive for
SqlStdOperatorTable(use instance of to detect).
So with this patch, we can control both table/column/UDF name with
SqlParser.Config.caseSensitive(this flag can be configured through connection
property or FrameworkConfig).
But for builtins, they behave all case-insensitively, just like before.
BTW, this patch add the flag in SqlOperatorTable#lookupOperatorOverloads, so
many codes are touched, i kind of thought the SqlOperatorTable is also
plugable, if we create a case-insensitive ListSqlOperatorTable can also make
another choice for users. And this will make minor change.
[1]
[link1|https://github.com/apache/calcite/blob/3124a85b93ff2f1b79484c7bd4cc41835d4f1920/core/src/main/java/org/apache/calcite/sql/parser/SqlAbstractParserImpl.java#L395]
[2]
[link2|https://github.com/apache/calcite/blob/3124a85b93ff2f1b79484c7bd4cc41835d4f1920/core/src/main/java/org/apache/calcite/sql/parser/SqlParser.java#L221]
was (Author: danny0405):
[~julianhyde] i have fire a
[PR#1137|https://github.com/apache/calcite/pull/1137]
Initially i wanna change both builtin operators and UDFs case sensitive
internal, and control the case-sensitiveness through config parser config[2].
The problems i encounter when doing this patch:
1.There is a SqlAbstractParserImpl which extended by all the SqlParser, so
there is no change to see the case-sensitivless when parsing the builtin
operators in SqlStdOperatorTable[1], and i have to hard code the case
sensitiveness to be false.
2. Because of 1 i have to make the
ReflectiveSqlOperatorTable#lookupOperatorOverloads case- insensitive for
SqlStdOperatorTable(use instance of to detect).
So with this patch, we can control both table/column/UDF name with
SqlParser.Config.caseSensitive(this flag can be configured through connection
property or FrameworkConfig).
But for builtins, they behave all case-insensitively, just like before.
BTW, this patch add the flag in SqlOperatorTable#lookupOperatorOverloads, so
many codes are touched, i kind of thought the SqlOperatorTable is also
plugable, if we create a case-insensitive ListSqlOperatorTable can also make
another choice for users. And this will make minor change.
[1]
[link1|https://github.com/apache/calcite/blob/3124a85b93ff2f1b79484c7bd4cc41835d4f1920/core/src/main/java/org/apache/calcite/sql/parser/SqlAbstractParserImpl.java#L395]
[2][link2|https://github.com/apache/calcite/blob/3124a85b93ff2f1b79484c7bd4cc41835d4f1920/core/src/main/java/org/apache/calcite/sql/parser/SqlParser.java#L221]
> Make UDF lookup default to case insensitive
> ---
>
> Key: CALCITE-2928
> URL: https://issues.apache.org/jira/browse/CALCITE-2928
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Affects Versions: 1.19.0
>Reporter: Danny Chan
>Assignee: Danny Chan
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.20.0
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> Now for Calcite, we make default parser config unquotedCasing to
> Lex.ORACLE.unquotedCasing(to uppercase)[1], and caseSensitive to
> Lex.ORACLE.caseSensitive(case sensitive true).
> So if we have a UDAF named my_func and query with sql like:
> {code:java}
> select f0, my_func(f1) from table1 group by f0;
> {code}
> We would got a unparsed sql:
> {code:java}
> SELECT F0, MY_FUNC(F1) FROM TABLE1 GROUP BY F0;
> {code}
> For CalciteCatalogReader we hard code the function look up to case sensitive
> true[2],
> For ListSqlOperatorTable we make the operator name lookup case sensitive
> true[3].
> For ReflectiveSqlOperatorTable, we make built-in operators
> case-insensitively[4].
> For most of the cases, we use ListSqlOperatorTable to register our UDFs[5]
> chained with SqlStdOperatorTable(which composite a ChainedSqlOperatorTable),
> which finally passed to CalciteCatalogReader for validation.
> So there are some questions i have:
> 1. Why we make built-in operators look
[jira] [Comment Edited] (CALCITE-2928) Make UDF lookup default to case insensitive
[
https://issues.apache.org/jira/browse/CALCITE-2928?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16803610#comment-16803610
]
Danny Chan edited comment on CALCITE-2928 at 3/28/19 6:08 AM:
--
[~julianhyde] i have fire a
[PR#1137|https://github.com/apache/calcite/pull/1137]
Initially i wanna change both builtin operators and UDFs case sensitive
internal, and control the case-sensitiveness through config parser config[2].
The problems i encounter when doing this patch:
1.There is a SqlAbstractParserImpl which extended by all the SqlParser, so
there is no change to see the case-sensitivless when parsing the builtin
operators in SqlStdOperatorTable[1], and i have to hard code the case
sensitiveness to be false.
2. Because of 1 i have to make the
ReflectiveSqlOperatorTable#lookupOperatorOverloads case- insensitive for
SqlStdOperatorTable(use instance of to detect).
So with this patch, we can control both table/column/UDF name with
SqlParser.Config.caseSensitive(this flag can be configured through connection
property or FrameworkConfig).
But for builtins, they behave all case-insensitively, just like before.
BTW, this patch add the flag in SqlOperatorTable#lookupOperatorOverloads, so
many codes are touched, i kind of thought the SqlOperatorTable is also
plugable, if we create a case-insensitive ListSqlOperatorTable can also make
another choice for users. And this will make minor change.
[1]
[link1|https://github.com/apache/calcite/blob/3124a85b93ff2f1b79484c7bd4cc41835d4f1920/core/src/main/java/org/apache/calcite/sql/parser/SqlAbstractParserImpl.java#L395]
[2][link2|https://github.com/apache/calcite/blob/3124a85b93ff2f1b79484c7bd4cc41835d4f1920/core/src/main/java/org/apache/calcite/sql/parser/SqlParser.java#L221]
was (Author: danny0405):
[~julianhyde] i have fire a
[PR#1137|https://github.com/apache/calcite/pull/1137]
Initially i wanna change both builtin operators and UDFs case sensitive
internal, and control the case-sensitiveness through config parser config[2].
The problems i encounter when doing this patch:
1.There is a SqlAbstractParserImpl which extended by all the SqlParser, so
there is no change to see the case-sensitivless when parsing the builtin
operators in SqlStdOperatorTable[1], and i have to hard code the case
sensitiveness to be false.
2. Because of 1 i have to make the
ReflectiveSqlOperatorTable#lookupOperatorOverloads case- insensitive for
SqlStdOperatorTable(use instance of to detect).
So with this patch, we can control both table/column/UDF name with
SqlParser.Config.caseSensitive(this flag can be configured through connection
property or FrameworkConfig).
But for builtins, they behave all case-insensitively, just like before.
BTW, this patch add the flag in SqlOperatorTable#lookupOperatorOverloads, so
many codes are touched, i kind of thought the SqlOperatorTable is also
plugable, if we create a case-insensitive ListSqlOperatorTable can also make
another choice for users. And this will make minor change.
[1]https://github.com/apache/calcite/blob/3124a85b93ff2f1b79484c7bd4cc41835d4f1920/core/src/main/java/org/apache/calcite/sql/parser/SqlAbstractParserImpl.java#L395
[2]https://github.com/apache/calcite/blob/3124a85b93ff2f1b79484c7bd4cc41835d4f1920/core/src/main/java/org/apache/calcite/sql/parser/SqlParser.java#L221
> Make UDF lookup default to case insensitive
> ---
>
> Key: CALCITE-2928
> URL: https://issues.apache.org/jira/browse/CALCITE-2928
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Affects Versions: 1.19.0
>Reporter: Danny Chan
>Assignee: Danny Chan
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.20.0
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> Now for Calcite, we make default parser config unquotedCasing to
> Lex.ORACLE.unquotedCasing(to uppercase)[1], and caseSensitive to
> Lex.ORACLE.caseSensitive(case sensitive true).
> So if we have a UDAF named my_func and query with sql like:
> {code:java}
> select f0, my_func(f1) from table1 group by f0;
> {code}
> We would got a unparsed sql:
> {code:java}
> SELECT F0, MY_FUNC(F1) FROM TABLE1 GROUP BY F0;
> {code}
> For CalciteCatalogReader we hard code the function look up to case sensitive
> true[2],
> For ListSqlOperatorTable we make the operator name lookup case sensitive
> true[3].
> For ReflectiveSqlOperatorTable, we make built-in operators
> case-insensitively[4].
> For most of the cases, we use ListSqlOperatorTable to register our UDFs[5]
> chained with SqlStdOperatorTable(which composite a ChainedSqlOperatorTable),
> which finally passed to CalciteCatalogReader for validation.
> So there are some questions i have:
> 1. Why we make built-in operators look up
[jira] [Comment Edited] (CALCITE-2928) Make UDF lookup default to case insensitive
[
https://issues.apache.org/jira/browse/CALCITE-2928?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16797394#comment-16797394
]
Julian Hyde edited comment on CALCITE-2928 at 3/20/19 6:00 PM:
---
So, you're saying that Oracle uses the same case-sensitivity policy for tables
and UDFs. Calcite does that, and I think we should continue.
I agree it's a little weird that we look up built-in operators
case-insensitively. Possibly because we didn't have a good way to quote, until
CALCITE-2674. We can revisit that decision.
I disagree that users always want their UDFs case-insensitive. For example, if
a user wants to supply a Java class and have all static methods become UDFs,
then it's possible that there are clashes if we use case-insensitive matching,
because Java is case-sensitive, and {{myFun(int)}} is different from
{{myfun(int)}}. So I think we should continue to support both case-sensitive
and case-insensitive UDFs.
The implementation of both case-sensitive and case-insensitive might use
case-sensitive maps, because maps are case-sensitive by default in Java. If the
UDF is called "myFun" and the user wrote "select MYFUN", after we've resolved
it (case-insensitively) to "myFun" we can thereafter use case-sensitive
matching to find the operator by name. If there's a bug, let me know the SQL or
test case.
Did I answer your questions 1 and 2?
was (Author: julianhyde):
So, you're saying that Oracle uses the same case-sensitivity policy for tables
and UDFs. Calcite does that, and I think we should continue.
I agree it's a little weird that we look up built-in operators
case-insensitively. Possibly because we didn't have a good way to quote, until
CALCITE-2674. We can revisit that decision.
I disagree that users always want their UDFs case-insensitive. For example, if
a user wants to supply a Java class and have all static methods become UDFs,
then it's possible that there are clashes if we use case-insensitive matching,
because Java is case-sensitive, and {{myFun(int)}} is different from
{{myfun(int)}}. So I think we should continue to support both case-sensitive
and case-insensitive UDFs.
The implementation of both case-sensitive and case-insensitive might use
case-sensitive maps, because maps are case-sensitive by default in Java. If the
UDF is called "myFun" and the user wrote "select MYFUN", after we've resolved
it (case-insensitively) to "myFun" we can thereafter use case-sensitive
matching to find the operator by name. If there's a bug, let me know.
> Make UDF lookup default to case insensitive
> ---
>
> Key: CALCITE-2928
> URL: https://issues.apache.org/jira/browse/CALCITE-2928
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Affects Versions: 1.19.0
>Reporter: Danny Chan
>Assignee: Danny Chan
>Priority: Major
> Fix For: 1.20.0
>
>
> Now for Calcite, we make default parser config unquotedCasing to
> Lex.ORACLE.unquotedCasing(to uppercase)[1], and caseSensitive to
> Lex.ORACLE.caseSensitive(case sensitive true).
> So if we have a UDAF named my_func and query with sql like:
> {code:java}
> select f0, my_func(f1) from table1 group by f0;
> {code}
> We would got a unparsed sql:
> {code:java}
> SELECT F0, MY_FUNC(F1) FROM TABLE1 GROUP BY F0;
> {code}
> For CalciteCatalogReader we hard code the function look up to case sensitive
> true[2],
> For ListSqlOperatorTable we make the operator name lookup case sensitive
> true[3].
> For ReflectiveSqlOperatorTable, we make built-in operators
> case-insensitively[4].
> For most of the cases, we use ListSqlOperatorTable to register our UDFs[5]
> chained with SqlStdOperatorTable(which composite a ChainedSqlOperatorTable),
> which finally passed to CalciteCatalogReader for validation.
> So there are some questions i have:
> 1. Why we make built-in operators look up case-insensitively while
> ListSqlOperatorTable(for UDFs) case-sensitively, with default unquotedCasing
> of TO_UPPERCASE.
> 2. What is the usage of CalciteCatalogReader#lookupOperatorOverloads i only
> saw it used in a unit test LookupOperatorOverloadsTest.
> It seems that make UDF look up case-sensitively does not make any sense,
> users will never distinguish their function with just word cases. And i
> checked also MYSQL, ORACLE, POSTGRES, their UDFs are all registered
> case-insensitively.
> [1]
> https://github.com/apache/calcite/blob/ffca956be03a99cd11e440d652b09674aaa727e6/core/src/main/java/org/apache/calcite/sql/parser/SqlParser.java#L231
> [2]
> https://github.com/apache/calcite/blob/ffca956be03a99cd11e440d652b09674aaa727e6/core/src/main/java/org/apache/calcite/prepare/CalciteCatalogReader.java#L166
> [3]
>
