You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/10/14 17:13:45 UTC
[GitHub] [pinot] agavra opened a new pull request, #9600: [PROTOTYPE] polymorphism in function calls
agavra opened a new pull request, #9600:
URL: https://github.com/apache/pinot/pull/9600
Some inspiration: #9397
This PR is a reference PR for a set of PRs that will follow breaking this down into less hacky, reviewable patches - it gives context in case reviewers are interested in what will come after :)
Changes:
- add backwards compatibility flags in `FunctionInfo` and `ScalarFunction` annotation to existing functions which define all the weird behaviors we have in the original function resolution logic
- adds implicit type casting based on the backwards compat flags (see logic in `FunctionRegistry`)
- convenience methods for type inference in `ExpressionTypeUtil`
- includes a hacky way to get tests to pass that didn't pass schemas into function resolution by just inferring it from the generic row
- piped down types everywhere it was necessary
This change **SHOULD NOT BE MERGED**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] agavra closed pull request #9600: [PROTOTYPE] polymorphism in function calls
Posted by GitBox <gi...@apache.org>.
agavra closed pull request #9600: [PROTOTYPE] polymorphism in function calls
URL: https://github.com/apache/pinot/pull/9600
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] walterddr commented on a diff in pull request #9600: [PROTOTYPE] polymorphism in function calls
Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9600:
URL: https://github.com/apache/pinot/pull/9600#discussion_r997134274
##########
pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java:
##########
@@ -106,12 +156,16 @@ public static void registerFunction(Method method, boolean nullableParameters) {
/**
* Registers a method with the given function name.
*/
- public static void registerFunction(String functionName, Method method, boolean nullableParameters) {
- FunctionInfo functionInfo = new FunctionInfo(method, method.getDeclaringClass(), nullableParameters);
+ public static void registerFunction(String functionName, Method method, boolean nullableParameters, boolean narrowing,
Review Comment:
qq: how do we plan to generate a signature for calcite?
- for example if a function accepts narrowing, i am not 100% sure how to tell calcite's catalog reader that a function accepts multiple parameter type (let a long the cartesian combination of all possible variations)
- it looks like we might need to use direct override of the planner function matching algorithm ?
##########
pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java:
##########
@@ -49,11 +54,53 @@ private FunctionRegistry() {
// TODO: consolidate the following 2
// This FUNCTION_INFO_MAP is used by Pinot server to look up function by # of arguments
- private static final Map<String, Map<Integer, FunctionInfo>> FUNCTION_INFO_MAP = new HashMap<>();
+ private static final Map<String, Map<List<Class<?>>, FunctionInfo>> FUNCTION_INFO_MAP = new HashMap<>();
// This FUNCTION_MAP is used by Calcite function catalog tolook up function by function signature.
private static final NameMultimap<Function> FUNCTION_MAP = new NameMultimap<>();
+ // checks casts between primitives
Review Comment:
pretty sure there's some library out there that does this in jvm. can we check if we can leverage some existing solution?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] agavra commented on a diff in pull request #9600: [PROTOTYPE] polymorphism in function calls
Posted by GitBox <gi...@apache.org>.
agavra commented on code in PR #9600:
URL: https://github.com/apache/pinot/pull/9600#discussion_r997257827
##########
pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java:
##########
@@ -106,12 +156,16 @@ public static void registerFunction(Method method, boolean nullableParameters) {
/**
* Registers a method with the given function name.
*/
- public static void registerFunction(String functionName, Method method, boolean nullableParameters) {
- FunctionInfo functionInfo = new FunctionInfo(method, method.getDeclaringClass(), nullableParameters);
+ public static void registerFunction(String functionName, Method method, boolean nullableParameters, boolean narrowing,
Review Comment:
see comment above, Calcite has a functionality to support type coercion, which will basically allow you to cast nearly any sql type to another sql type if you pass in `coerce = true`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] agavra commented on a diff in pull request #9600: [PROTOTYPE] polymorphism in function calls
Posted by GitBox <gi...@apache.org>.
agavra commented on code in PR #9600:
URL: https://github.com/apache/pinot/pull/9600#discussion_r997256909
##########
pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java:
##########
@@ -49,11 +54,53 @@ private FunctionRegistry() {
// TODO: consolidate the following 2
// This FUNCTION_INFO_MAP is used by Pinot server to look up function by # of arguments
- private static final Map<String, Map<Integer, FunctionInfo>> FUNCTION_INFO_MAP = new HashMap<>();
+ private static final Map<String, Map<List<Class<?>>, FunctionInfo>> FUNCTION_INFO_MAP = new HashMap<>();
// This FUNCTION_MAP is used by Calcite function catalog tolook up function by function signature.
private static final NameMultimap<Function> FUNCTION_MAP = new NameMultimap<>();
+ // checks casts between primitives
Review Comment:
In my latest approach I decided to leverage Calcite - it wasn't too hard and that way we'll have consistency between v1 and v2. See https://github.com/apache/calcite/blob/2c30a56158cdd351d35725006bc1f76bb6aac75b/core/src/main/java/org/apache/calcite/sql/SqlUtil.java#L635-L684
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org