-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Extend comparison methods to accept different datetime types. #129
Changes from all commits
78576e5
9fec34f
81dff5e
54a19c8
bcdde13
701975b
e5885f0
9c65646
9eea021
174e9bd
6afc042
7225a98
1966122
e70a7de
1dd7c94
cf22dbb
48c664b
066fdae
d92fb42
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -137,6 +137,52 @@ public String toString() { | |||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||
* Implementation of a function that takes two arguments, returns a value, and | ||||||||||||||||||||||||||
* requires FunctionProperties to complete. | ||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||
* @param function {@link ExprValue} based unary function. | ||||||||||||||||||||||||||
* @param returnType return type. | ||||||||||||||||||||||||||
* @param args1Type first argument type. | ||||||||||||||||||||||||||
* @param args2Type second argument type. | ||||||||||||||||||||||||||
* @return Unary Function Implementation. | ||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||
public static SerializableFunction<FunctionName, Pair<FunctionSignature, FunctionBuilder>> | ||||||||||||||||||||||||||
implWithProperties( | ||||||||||||||||||||||||||
SerializableTriFunction<FunctionProperties, ExprValue, ExprValue, ExprValue> function, | ||||||||||||||||||||||||||
ExprType returnType, | ||||||||||||||||||||||||||
ExprType args1Type, | ||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. better to use a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could be a separate feature. We have to change all |
||||||||||||||||||||||||||
ExprType args2Type) { | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
return functionName -> { | ||||||||||||||||||||||||||
FunctionSignature functionSignature = | ||||||||||||||||||||||||||
new FunctionSignature(functionName, Arrays.asList(args1Type, args2Type)); | ||||||||||||||||||||||||||
FunctionBuilder functionBuilder = | ||||||||||||||||||||||||||
(functionProperties, arguments) -> new FunctionExpression(functionName, arguments) { | ||||||||||||||||||||||||||
@Override | ||||||||||||||||||||||||||
public ExprValue valueOf(Environment<Expression, ExprValue> valueEnv) { | ||||||||||||||||||||||||||
ExprValue arg1 = arguments.get(0).valueOf(valueEnv); | ||||||||||||||||||||||||||
ExprValue arg2 = arguments.get(1).valueOf(valueEnv); | ||||||||||||||||||||||||||
return function.apply(functionProperties, arg1, arg2); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
@Override | ||||||||||||||||||||||||||
public ExprType type() { | ||||||||||||||||||||||||||
return returnType; | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
@Override | ||||||||||||||||||||||||||
public String toString() { | ||||||||||||||||||||||||||
return String.format("%s(%s)", functionName, | ||||||||||||||||||||||||||
arguments.stream() | ||||||||||||||||||||||||||
.map(Object::toString) | ||||||||||||||||||||||||||
.collect(Collectors.joining(", "))); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||
return Pair.of(functionSignature, functionBuilder); | ||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||
* No Arg Function Implementation. | ||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||
|
@@ -317,4 +363,22 @@ public SerializableTriFunction<ExprValue, ExprValue, ExprValue, ExprValue> nullM | |||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||
* Wrapper for the ExprValue function that takes 2 arguments and is aware of FunctionProperties, | ||||||||||||||||||||||||||
* with default NULL and MISSING handling. | ||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||
public static SerializableTriFunction<FunctionProperties, ExprValue, ExprValue, ExprValue> | ||||||||||||||||||||||||||
nullMissingHandlingWithProperties( | ||||||||||||||||||||||||||
SerializableTriFunction<FunctionProperties, ExprValue, ExprValue,ExprValue> implementation) { | ||||||||||||||||||||||||||
return (functionProperties, v1, v2) -> { | ||||||||||||||||||||||||||
if (v1.isMissing() || v2.isMissing()) { | ||||||||||||||||||||||||||
return ExprValueUtils.missingValue(); | ||||||||||||||||||||||||||
} else if (v1.isNull() || v2.isNull()) { | ||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just copy-pasted from opensearch-project-sql/core/src/main/java/org/opensearch/sql/expression/function/FunctionDSL.java Lines 320 to 331 in d92fb42
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can groom all of them in scope of another task I think. |
||||||||||||||||||||||||||
return ExprValueUtils.nullValue(); | ||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||
Yury-Fridlyand marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||
return implementation.apply(functionProperties, v1, v2); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would it be better to include a
isComparable(AbstractExprValue)
function here? Overridden by child classes that return true for valid cases or are the same type?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we add a new type, we have to update that function override in all/some other types to reflect the change. It is not scalable solution.