[
https://issues.apache.org/jira/browse/PHOENIX-3355?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15610029#comment-15610029
]
Eric Lomore edited comment on PHOENIX-3355 at 10/26/16 11:35 PM:
-----------------------------------------------------------------
[~maryannxue] thanks for all the help. I've attached a new patch, while it's
not fully there I think this is closer to what we want.
Few things I'm aware of and considering options for:
- We could opt to modify initBuiltinFunctionMap() to create and register the
builtin functions, essentially pushing all the work there away from
PhoenixSchema.java. No functional difference.
- I haven't modified getDataType(). For the most part this is OK, most
functions have accessible return types, and the vast majority of functions get
registered. Exhaustive list of those having issues:
{code}getDataType() error: ARRAY_CAT
getDataType() error: FLOOR
getDataType() error: ARRAY_APPEND
getDataType() error: CEIL
getDataType() error: REGEXP_REPLACE
getDataType() error: SET_BYTE
getDataType() error: ARRAY_FILL
getDataType() error: REGEXP_SUBSTR
getDataType() error: ARRAY_ELEM
getDataType() error: NOW
getDataType() error: ROUND
getDataType() error: TRUNC
getDataType() error: REGEXP_SPLIT
getDataType() error: ARRAY_PREPEND
getDataType() error: SET_BIT{code}
Many of these are translated in various parts of CalciteUtils (Floor, Ceil),
should these also be added to the TRANSLATED map?
The rest that are throwing errors I have not been able to resolve the issue of
output being dependent on input.
- I attempted making {{TRANSLATED_BUILT_IN_FUNCTIONS}} a map, and creating a
utility function to create instances of each function but since each function
requires different parameters it may not be worth it - the code would be more
breakable.
{code} static
{
TRANSLATED_BUILT_IN_FUNCTIONS.put(SqrtFunction.NAME,
SqrtFunction.class);
TRANSLATED_BUILT_IN_FUNCTIONS.put(PowerFunction.NAME,
PowerFunction.class);
TRANSLATED_BUILT_IN_FUNCTIONS.put(LnFunction.NAME, LnFunction.class);
TRANSLATED_BUILT_IN_FUNCTIONS.put(ExpFunction.NAME, ExpFunction.class);
TRANSLATED_BUILT_IN_FUNCTIONS.put(AbsFunction.NAME, AbsFunction.class);
TRANSLATED_BUILT_IN_FUNCTIONS.put(CurrentDateFunction.NAME,
CurrentDateFunction.class);
TRANSLATED_BUILT_IN_FUNCTIONS.put(CurrentTimeFunction.NAME,
CurrentTimeFunction.class);
TRANSLATED_BUILT_IN_FUNCTIONS.put(LowerFunction.NAME,
LowerFunction.class);
TRANSLATED_BUILT_IN_FUNCTIONS.put(UpperFunction.NAME,
UpperFunction.class);
TRANSLATED_BUILT_IN_FUNCTIONS.put(CoalesceFunction.NAME,
CoalesceFunction.class);
}{code}
{code} } else if
(TRANSLATED_BUILT_IN_FUNCTIONS.get(op.getName()) != null){
try {
return
TRANSLATED_BUILT_IN_FUNCTIONS.get(op.getName()).getConstructor()
.newInstance(children);
} catch(Exception e) { throw new
RuntimeException(e);}{code}
- Aggregate function TODOs noted in the patch.
was (Author: lomoree):
[~maryannxue] thanks for all the help. I've attached a new patch, while it's
not fully there I think this is closer to what we want.
Few things I'm aware of and considering options for:
- We could opt to modify initBuiltinFunctionMap() to create and register the
builtin functions, essentially pushing all the work there away from
PhoenixSchema.java. No functional difference.
- I haven't modified getDataType(). For the most part this is OK, most
functions have accessible return types, and the vast majority of functions get
registered. Exhaustive list of those having issues:
{code}getDataType() error: ARRAY_CAT
getDataType() error: FLOOR
getDataType() error: ARRAY_APPEND
getDataType() error: CEIL
getDataType() error: REGEXP_REPLACE
getDataType() error: SET_BYTE
getDataType() error: ARRAY_FILL
getDataType() error: REGEXP_SUBSTR
getDataType() error: ARRAY_ELEM
getDataType() error: NOW
getDataType() error: ROUND
getDataType() error: TRUNC
getDataType() error: REGEXP_SPLIT
getDataType() error: ARRAY_PREPEND
getDataType() error: SET_BIT{code}
Many of these are translated in CalciteUtils (Floor, Ceil), should these also
be added to the TRANSLATED map? Those that aren't have input child expressions
that aren't accessible (not coincidental that many are array functions)
- I attempted making {{TRANSLATED_BUILT_IN_FUNCTIONS}} a map, and creating a
utility function to create instances of each function but since each functions
needs a specific constructor it's likely not probable.
{code} } else if
(TRANSLATED_BUILT_IN_FUNCTIONS.get(op.getName()) != null){
try {
return
TRANSLATED_BUILT_IN_FUNCTIONS.get(op.getName()).getConstructor()
.newInstance(children);
} catch(Exception e) { throw new
RuntimeException(e);}{code}
- Aggregate function TODOs noted in the patch.
> Register Phoenix built-in functions as Calcite functions
> --------------------------------------------------------
>
> Key: PHOENIX-3355
> URL: https://issues.apache.org/jira/browse/PHOENIX-3355
> Project: Phoenix
> Issue Type: Bug
> Reporter: James Taylor
> Assignee: Eric Lomore
> Labels: calcite
> Attachments: PHOENIX-3355.wip, PHOENIX-3355.wip2
>
>
> We should register all Phoenix built-in functions that don't exist in Calcite.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
