[CALCITE-6550] Improve SQL function overloading#3954
[CALCITE-6550] Improve SQL function overloading#3954normanj-bitquill wants to merge 2 commits intoapache:mainfrom
Conversation
| * the SqlOperator to select the correct operator since the hash of operators | ||
| * may not be unique. | ||
| */ | ||
| private static class OpImplementatorPair { |
There was a problem hiding this comment.
Could you use Pair<SqlOperator, RexCallImplementor> instead? I don't think hashCode or equals are ever called on the values in a map or array-list-multimap.
There was a problem hiding this comment.
Or, maybe better, change scalarMap to type Map<SqlOperator, ImmutablePairList<SqlOperator, RexCallImplementor>>.
There was a problem hiding this comment.
I have rebased this on top of your change to refactor RexImpTable. I am now using ImmutablePairList.
normanj-bitquill
force-pushed
the
calcite-6550
branch
2 times, most recently
from
97bf906 to
5909e8b
Compare
Split class Builder into subclass and superclass to prevent code in populate() from accessing map directly.
* RexImpTable can better handle collisions in the scalar function map * Return the implementor if only one implementor is found for an operator key * If there are multiple implementors for an operator key, look for one with the exact same operator * An operator key is the operator name and kind
normanj-bitquill
force-pushed
the
calcite-6550
branch
from
5909e8b to
cc6ec96
Compare
|
There was a problem hiding this comment.
I will test it locally in the next few days to see if there are any other problems. Overall, it is good.
@normanj-bitquill If I test next week this week, I think we can merge this PR next week without introducing more conflicts
|
@julianhyde you asked for changes, please approve if you are satisfied. |
Close apache#3954
Use forbiddenApis to prevent use of Locale.setDefault; if you really need it, call Unsafe.setDefaultLocale. Close apache#3954
Remove usage of Locale.setDefault in tests where possible. Add Locale.setDefault to forbiddenApis, add method Unsafe.setDefaultLocale(Locale), and change the remaining uses of Locale.setDefault to go via Unsafe. The implementation of the Postgres TO_CHAR function (and related functions) now get Locale and TimeZone from DataContext. Rework unit tests for PostgresqlDateTimeFormatter to be less repetitive, more readable. Minimize usage of ZoneId.systemDefault(). Close apache#3954
4 participants
The main idea here is to allow collisions in the scalar function map. The collisions are resolved as described above. Much of the change here is managing the scalar function map in
RexImpTable.Alternate solutions:
SqlBasicFunctionoptions. There will still be some collisions inRexImpTable, since a function could be defined multiple times (ie it is in multiple function libraries with different implementations).RexImpTable. We should only need the functions for selected libraries at any given time.