[jira] [Comment Edited] (PHOENIX-3355) Register Phoenix built-in functions as Calcite functions
[
https://issues.apache.org/jira/browse/PHOENIX-3355?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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);
}
[jira] [Comment Edited] (PHOENIX-3355) Register Phoenix built-in functions as Calcite functions
[
https://issues.apache.org/jira/browse/PHOENIX-3355?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15606368#comment-15606368
]
Maryann Xue edited comment on PHOENIX-3355 at 10/25/16 8:26 PM:
[~rajeshbabu], Take a closer look at the plan with default values. Calcite
interpret the default value as a special sql operator "DEFAULT()", and we did
not handle that operator in our translation logic yet. So try adding the
following to your CalciteUtils.java, and it'll work fine.
{code}
@@ -734,6 +734,18 @@ public class CalciteUtils {
}
}
});
+EXPRESSION_MAP.put(SqlKind.DEFAULT, new ExpressionFactory() {
+@SuppressWarnings("rawtypes")
+@Override
+public Expression newExpression(RexNode node,
PhoenixRelImplementor implementor) {
+PDataType targetType = relDataTypeToPDataType(node.getType());
+try {
+return LiteralExpression.newConstant(null, targetType);
+} catch (SQLException e) {
+throw new RuntimeException(e);
+}
+}
+ });
EXPRESSION_MAP.put(SqlKind.OTHER_FUNCTION, new ExpressionFactory() {
@Override
public Expression newExpression(RexNode node,
{code}
was (Author: maryannxue):
[~rajeshbabu], Take a closer look at the plan with default values. Calcite
interpret the default value as a special sql operator "DEFAULT()", and we did
not handle that operator in our translation logic yet. So try adding the
following to your CalciteUtils.java, and it'll work fine.
{code}
@@ -734,6 +734,18 @@ public class CalciteUtils {
}
}
});
+ EXPRESSION_MAP.put(SqlKind.DEFAULT, new ExpressionFactory() {
+@SuppressWarnings("rawtypes")
+@Override
+public Expression newExpression(RexNode node,
PhoenixRelImplementor implementor) {
+PDataType targetType = relDataTypeToPDataType(node.getType());
+try {
+return LiteralExpression.newConstant(null, targetType);
+} catch (SQLException e) {
+throw new RuntimeException(e);
+}
+}
+ });
EXPRESSION_MAP.put(SqlKind.OTHER_FUNCTION, new ExpressionFactory() {
@Override
public Expression newExpression(RexNode node,
{code}
> 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
>
>
> 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)
[jira] [Comment Edited] (PHOENIX-3355) Register Phoenix built-in functions as Calcite functions
[
https://issues.apache.org/jira/browse/PHOENIX-3355?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15603603#comment-15603603
]
Maryann Xue edited comment on PHOENIX-3355 at 10/24/16 11:45 PM:
-
I just suggested using a temporary map and iterate the BUILT_IN_FUNCTION_MAP
just once. The temporary map will be somewhat mapping a function name to all
its available signatures.
So if we going to register COALESCE as an UDF, we'll have to register all its
overloads. Another option is to skip registering COALESCE since calcite has
support for it already. If you take a look at line 738 of CalciteUtils.java,
you can see there are a few built-in functions already handled by this
translation logic {{EXPRESSION_MAP.put(SqlKind.OTHER_FUNCTION, new
ExpressionFactory()}}. For each of our built-in function that's also supported
in Calcite, we'll have to decide whether to use the Calcite support or override
with our UDF registration. Basically we are free to choose, if it IS
overridable, and if the signatures are identical. But let's just have a static
list, say, in CalciteUtils, "TRANSLATED_BUILT_IN_FUNCTIONS", so that your UDF
registration logic can skip them. And maybe you can COALESCE in this list.
was (Author: maryannxue):
I just suggested using a temporary map and iterate the BUILT_IN_FUNCTION_MAP
just once. The temporary map will be somewhat mapping a function name to all
its available signatures.
So if we going to register COALESCE as an UDF, we'll have to register all its
overloads. Another option is to skip registering COALESCE since calcite has
support for it already. If you take a line 738 of CalciteUtils.java, you can
see there are a few built-in functions already handled by this translation
logic {{EXPRESSION_MAP.put(SqlKind.OTHER_FUNCTION, new ExpressionFactory()}}.
For each of our built-in function that's also supported in Calcite, we'll have
to decide whether to use the Calcite support or override with our UDF
registration. Basically we are free to choose, if it IS overridable, and if the
signatures are identical. But let's just have a static list, say, in
CalciteUtils, "TRANSLATED_BUILT_IN_FUNCTIONS", so that your UDF registration
logic can skip them. And maybe you can COALESCE in this list.
> 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
>
>
> 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)
[jira] [Comment Edited] (PHOENIX-3355) Register Phoenix built-in functions as Calcite functions
[
https://issues.apache.org/jira/browse/PHOENIX-3355?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15562837#comment-15562837
]
Eric Lomore edited comment on PHOENIX-3355 at 10/10/16 5:09 PM:
Thanks [~maryannxue], the UDF approach makes sense.
{{"CREATE VIEW v as SELECT * FROM t WHERE k1=TO_DATE('2016-06-06')";}} executes
successfully
was (Author: lomoree):
{{"CREATE VIEW v as SELECT * FROM t WHERE k1=TO_DATE('2016-06-06')";}} executes
successfully
> 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
>
> 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)
