-
Notifications
You must be signed in to change notification settings - Fork 235
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
Add optional projection via AST expression evaluation #3165
Conversation
Signed-off-by: Jason Lowe <jlowe@nvidia.com>
build |
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.
Really just some nits and doc improvements. At some point we have to think about documentation too. It is getting to be very clear that the lambda vs non-lambda distinction in Spark will not map one to one to the AST work. At some point we need to rip out the Lambda doc generation code entirely, and replace it with something that explains how we do it for various operators and shows AST support. Could you file a follow on issue for us to figure that out?
sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuBoundAttribute.scala
Show resolved
Hide resolved
sql-plugin/src/main/scala/com/nvidia/spark/rapids/basicPhysicalOperators.scala
Outdated
Show resolved
Hide resolved
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.
At some point we need to rip out the Lambda doc generation code entirely, and replace it with something that explains how we do it for various operators and shows AST support. Could you file a follow on issue for us to figure that out?
Filed #3172 to track.
The other review comments should be addressed.
sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuBoundAttribute.scala
Show resolved
Hide resolved
build |
Adds the ability to implement a projection via cudf AST expression evaluation if the expressions are supported by cudf AST. The
RapidsMeta
andGpuExpression
objects are reused, extended with newtagForAst
andconvertToAst
methods, respectively.I covered as many unary and binary AST operators as I could get to be compatible. Many of the missing AST operators cannot be supported to to lacking expressiveness in the AST for NaN handling, null literals, conditional execution, etc.