Expose expression base class publicly and simplify public AST API #9045
Conversation
…r to recompilation of AST-consuming code when AST internals change.
|
@jlowe this PR addresses a number of the issues that we have discussed offline (some of which are captured in #8145). Java tests will not pass until I update those bindings (I'm happy to take a stab at it), but I'd like to reach consensus on the C++ changes first so that we can make sure we're on the same page with the API. |
Codecov Report
@@ Coverage Diff @@
## branch-21.10 #9045 +/- ##
===============================================
Coverage ? 10.71%
===============================================
Files ? 114
Lines ? 19097
Branches ? 0
===============================================
Hits ? 2046
Misses ? 17051
Partials ? 0 Continue to review full report at Codecov.
|
|
rerun tests |
|
@jlowe once this PR is approved would it be best to hold off merging until you've had a chance to update the sparks-rapids consumers of this API? |
java/src/test/java/ai/rapids/cudf/ast/CompiledExpressionTest.java
Outdated
Show resolved
Hide resolved
…add support for this in expression compilation.
|
@gpucibot merge |
This PR performs the renames
ast::expression->ast::operationandast::node->ast::expression, which more accurately reflects that literals and column references are also valid expressions. The newexpressionclass is publicly exposed, allowing client code to maintain a list of expression pointers composed of different subclasses of expression, and all APIs now accept arbitrary expressions as input so that it is possible to evaluate e.g. a raw column reference on a table. The oldnodes.hppandoperators.hppfiles have been merged into a singleexpressions.hppheader that now contains the entire public API for AST construction, but some implementation details have been moved into anexpressions.cppsource file so dependencies on detail APIs like theexpression_parserare restricted to a single TU, which dramatically simplifies recompilation of the code on small changes. Many more parameters are now accepted by (const) ref rather than by value, avoiding unnecessary copies and improving const correctness of the code.