-
Notifications
You must be signed in to change notification settings - Fork 36
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
Support Lambda and add related array functions #864
Conversation
Signed-off-by: Heng Qian <[email protected]>
ppl-spark-integration/src/main/java/org/opensearch/sql/ast/expression/PPLLambdaFunction.java
Outdated
Show resolved
Hide resolved
ppl-spark-integration/src/main/java/org/opensearch/sql/ppl/CatalystExpressionVisitor.java
Outdated
Show resolved
Hide resolved
val logPlan = | ||
planTransformer.visit( | ||
plan(pplParser, | ||
"""source=t | eval a = json_array(1, 2, 3), b = json_array_all_match(a, x -> x > 0)""".stripMargin), |
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.
Need a test for a = json_array(json_object(..), json_object(..), json_object(..)).
For example:
a = [
{id:8, uid:1},
{id:7, uid:9},
{id:7, uid:1},
...
]
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.
Added in IT
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.
a = [ {id:8, uid:1}, {id:7, uid:9}, {id:7, uid:1}, ... ]
Could you add this json structure example to user doc ppl-lambda.md? it could help user to understand the case.
(PS: please keep review conversations open, it could help reviewers to remember the context and join conversation)
Signed-off-by: Heng Qian <[email protected]>
Signed-off-by: Heng Qian <[email protected]>
4c914cc
to
e1478f9
Compare
@@ -179,6 +185,18 @@ public interface BuiltinFunctionTransformer { | |||
args -> { | |||
return ToUTCTimestamp$.MODULE$.apply(CurrentTimestamp$.MODULE$.apply(), CurrentTimeZone$.MODULE$.apply()); | |||
}) | |||
.put( | |||
JSON_ARRAY_ALL_MATCH, | |||
args -> ArrayForAll$.MODULE$.apply(args.get(0), args.get(1)) |
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.
Can we accept both ARRAY and JSON ARRAY STRING?
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.
Em, I think we can keep this lambda functions and remove the JSON_ARRAY_
prefix since they seem common functions.
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.
Changed to use the original function name as spark
Signed-off-by: Heng Qian <[email protected]>
Signed-off-by: Heng Qian <[email protected]>
…tion # Conflicts: # ppl-spark-integration/src/main/java/org/opensearch/sql/ppl/CatalystExpressionVisitor.java
Thanks @qianheng-aws , I think we could separate the lambda functions and json functions (from tests to documentations). Another thought is will we add |
Discussed offline. Let's enhance the
|
ppl-spark-integration/src/main/java/org/opensearch/sql/ast/expression/LambdaFunction.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Heng Qian <[email protected]>
Signed-off-by: Heng Qian <[email protected]>
Signed-off-by: Heng Qian <[email protected]>
Signed-off-by: Heng Qian <[email protected]>
Signed-off-by: Heng Qian <[email protected]>
Signed-off-by: Heng Qian <[email protected]>
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.
@YANG-DB please review this PR and its description, the function names are not exactly same with the original ones in original issue. please double confirm that's fine to you.
Signed-off-by: Heng Qian <[email protected]>
* update antlr grammar for (future) P1 command syntax Signed-off-by: YANGDB <[email protected]> * add trendline command Signed-off-by: YANGDB <[email protected]> * add expand command Signed-off-by: YANGDB <[email protected]> * add geoip command Signed-off-by: YANGDB <[email protected]> * PPl `flatten` command (#784) * The flatten command implemented Signed-off-by: Lukasz Soszynski <[email protected]> * The flatten command integration tests were extended with additional checks for logical plans. Signed-off-by: Lukasz Soszynski <[email protected]> * flatten, added more tests related to plan translation and integration tests Signed-off-by: Lukasz Soszynski <[email protected]> * Flatten command added to command names list. Signed-off-by: Lukasz Soszynski <[email protected]> --------- Signed-off-by: Lukasz Soszynski <[email protected]> * Extract source table names from mv query (#854) * add sourceTables to MV index metadata properties Signed-off-by: Sean Kao <[email protected]> * parse source tables from mv query Signed-off-by: Sean Kao <[email protected]> * test cases for parse source tables from mv query Signed-off-by: Sean Kao <[email protected]> * use constant for metadata cache version Signed-off-by: Sean Kao <[email protected]> * write source tables to metadata cache Signed-off-by: Sean Kao <[email protected]> * address comment Signed-off-by: Sean Kao <[email protected]> * generate source tables for old mv without new prop Signed-off-by: Sean Kao <[email protected]> * syntax fix Signed-off-by: Sean Kao <[email protected]> --------- Signed-off-by: Sean Kao <[email protected]> * Fallback to internal scheduler when index creation failed (#850) * Fallback to internal scheduler when index creation failed Signed-off-by: Louis Chu <[email protected]> * Fix IT Signed-off-by: Louis Chu <[email protected]> * Fix IOException Signed-off-by: Louis Chu <[email protected]> --------- Signed-off-by: Louis Chu <[email protected]> * New trendline ppl command (SMA only) (#833) * WIP trendline command Signed-off-by: Kacper Trochimiak <[email protected]> * wip Signed-off-by: Kacper Trochimiak <[email protected]> * trendline supports sorting Signed-off-by: Kacper Trochimiak <[email protected]> * run scalafmtAll Signed-off-by: Kacper Trochimiak <[email protected]> * return null when there are too few data points Signed-off-by: Kacper Trochimiak <[email protected]> * sbt scalafmtAll Signed-off-by: Kacper Trochimiak <[email protected]> * Remove WMA references Signed-off-by: Hendrik Saly <[email protected]> * trendline - sortByField as Optional<Field> Signed-off-by: Kacper Trochimiak <[email protected]> * introduce TrendlineStrategy Signed-off-by: Kacper Trochimiak <[email protected]> * keywordsCanBeId -> replace SMA with trendlineType Signed-off-by: Kacper Trochimiak <[email protected]> * handle trendline alias as qualifiedName instead of fieldExpression Signed-off-by: Kacper Trochimiak <[email protected]> * Add docs Signed-off-by: Hendrik Saly <[email protected]> * Make alias optional Signed-off-by: Hendrik Saly <[email protected]> * Adapt tests for optional alias Signed-off-by: Hendrik Saly <[email protected]> * Adden logical plan unittests Signed-off-by: Hendrik Saly <[email protected]> * Add missing license headers Signed-off-by: Hendrik Saly <[email protected]> * Fix docs Signed-off-by: Hendrik Saly <[email protected]> * numberOfDataPoints must be 1 or greater Signed-off-by: Hendrik Saly <[email protected]> * Rename TrendlineStrategy to TrendlineCatalystUtils Signed-off-by: Hendrik Saly <[email protected]> * Validate TrendlineType early and pass around enum type Signed-off-by: Hendrik Saly <[email protected]> * Add trendline chaining test Signed-off-by: Hendrik Saly <[email protected]> * Fix compile errors Signed-off-by: Hendrik Saly <[email protected]> * Fix imports Signed-off-by: Hendrik Saly <[email protected]> * Fix imports Signed-off-by: Hendrik Saly <[email protected]> --------- Signed-off-by: Kacper Trochimiak <[email protected]> Signed-off-by: Hendrik Saly <[email protected]> Co-authored-by: Kacper Trochimiak <[email protected]> * update iplocation antlr Signed-off-by: YANGDB <[email protected]> * update scala fmt style Signed-off-by: YANGDB <[email protected]> * `cidrmatch` ppl command add logical tests and docs (#865) * update logical tests and docs Signed-off-by: YANGDB <[email protected]> * update scala fmt style Signed-off-by: YANGDB <[email protected]> * fix type error Signed-off-by: YANGDB <[email protected]> --------- Signed-off-by: YANGDB <[email protected]> * Support Lambda and add related array functions (#864) * json function enhancement Signed-off-by: Heng Qian <[email protected]> * Add JavaToScalaTransformer Signed-off-by: Heng Qian <[email protected]> * Apply scalafmtAll Signed-off-by: Heng Qian <[email protected]> * Address comments Signed-off-by: Heng Qian <[email protected]> * Add IT and change to use the same function name as spark Signed-off-by: Heng Qian <[email protected]> * Address comments Signed-off-by: Heng Qian <[email protected]> * Add document and separate lambda functions from json functions Signed-off-by: Heng Qian <[email protected]> * Add lambda functions transform and reduce Signed-off-by: Heng Qian <[email protected]> * polish lambda function document Signed-off-by: Heng Qian <[email protected]> * polish lambda function document Signed-off-by: Heng Qian <[email protected]> * Minor fix Signed-off-by: Heng Qian <[email protected]> * Minor change to polish the documents Signed-off-by: Heng Qian <[email protected]> --------- Signed-off-by: Heng Qian <[email protected]> --------- Signed-off-by: YANGDB <[email protected]> Signed-off-by: Lukasz Soszynski <[email protected]> Signed-off-by: Sean Kao <[email protected]> Signed-off-by: Louis Chu <[email protected]> Signed-off-by: Kacper Trochimiak <[email protected]> Signed-off-by: Hendrik Saly <[email protected]> Signed-off-by: Heng Qian <[email protected]> Co-authored-by: lukasz-soszynski-eliatra <[email protected]> Co-authored-by: Sean Kao <[email protected]> Co-authored-by: Louis Chu <[email protected]> Co-authored-by: Hendrik Saly <[email protected]> Co-authored-by: Kacper Trochimiak <[email protected]> Co-authored-by: qianheng <[email protected]>
* json function enhancement Signed-off-by: Heng Qian <[email protected]> * Add JavaToScalaTransformer Signed-off-by: Heng Qian <[email protected]> * Apply scalafmtAll Signed-off-by: Heng Qian <[email protected]> * Address comments Signed-off-by: Heng Qian <[email protected]> * Add IT and change to use the same function name as spark Signed-off-by: Heng Qian <[email protected]> * Address comments Signed-off-by: Heng Qian <[email protected]> * Add document and separate lambda functions from json functions Signed-off-by: Heng Qian <[email protected]> * Add lambda functions transform and reduce Signed-off-by: Heng Qian <[email protected]> * polish lambda function document Signed-off-by: Heng Qian <[email protected]> * polish lambda function document Signed-off-by: Heng Qian <[email protected]> * Minor fix Signed-off-by: Heng Qian <[email protected]> * Minor change to polish the documents Signed-off-by: Heng Qian <[email protected]> --------- Signed-off-by: Heng Qian <[email protected]>
Description
Support parsing lambda expression.
Add below array related function which need a lambda function as its argument:
json_array_any_match()-> exists(ARRAY, Lambda)json_array_all_match()-> forAll(ARRAY, Lambda)json_array_filter()-> filter(ARRAY, Lambda)json_array_map()-> transform(ARRAY, Lambda)json_array_reduce()-> reduce(ARRAY, start, Lambda, Lambda)Examples:
Exists:
ForAll:
Filter:
Transform:
Reduce:
Related Issues
Partial resolve #863
Check List
--signoff
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.