feat: Add fluent expression builder API #11414

pedroerp · 2024-11-02T00:20:42Z

Summary:
Adding an API to allow developers to fluently create (untyped)
expression trees without having to rely on a SQL parser. Details and
extensively examples provided in code comments and unit tests

Differential Revision: D65371064

Part of #11383

facebook-github-bot · 2024-11-02T00:20:52Z

This pull request was exported from Phabricator. Differential Revision: D65371064

netlify · 2024-11-02T00:20:58Z

✅ Deploy Preview for meta-velox canceled.

Name	Link
🔨 Latest commit	`996f0af`
🔍 Latest deploy log	https://app.netlify.com/sites/meta-velox/deploys/6760f397b233e60008e25411

pedroerp · 2024-11-02T00:50:13Z

Cc: @mbasmanova @xiaoxmeng @majetideepak @kgpai @Yuhta @bikramSingh91 @rui-mo

What do you guys think about something like this as the API?

majetideepak

@pedroerp this looks nice!
I wonder if it is more convenient to use f"" for fields and l*"" for literals.
eg. f"col" and lf"1.0" for literal float, ld"1.0" for literal double.
Something like the Arrow CDataInterface.
https://arrow.apache.org/docs/format/CDataInterface.html

majetideepak · 2024-11-04T16:15:04Z

velox/parse/IExpr.h

@@ -44,7 +45,42 @@ class IExpr {
    return alias_;
  }

+  std::optional<std::string>& alias() {


nit: return string_view?

majetideepak · 2024-11-04T16:37:38Z

velox/exec/tests/utils/PlanBuilder.h

@@ -255,7 +252,7 @@ class PlanBuilder {
    }

    /// @param tableHandle Optional tableHandle. Other builder arguments such as
-    /// the subfieldFilters and remainingFilter will be ignored.
+    /// the `subfieldFilters` and `remainingFilter` will be ignored.


Nit: "``" is not used consistently in the comments throughout.

majetideepak · 2024-11-04T16:49:21Z

velox/exec/tests/utils/ExpressionBuilder.h

+  return head;
+}
+
+// Macro to reduce builerplate when overloading C++ operators. The template


typo: boilerplate

pedroerp · 2024-11-04T18:21:36Z

@majetideepak thanks for the quick review.

I wonder if it is more convenient to use f"" for fields and l*"" for literals. eg. f"col" and lf"1.0" for literal float, ld"1.0" for literal double. Something like the Arrow CDataInterface.

You mean using the same arrow identifiers as custom C++ literals? That is a great idea for consistency, but I don't think C++ let's you define the literals as a prefix; if needs to be a trailing "_xx". But we could make the trailing characters align with arrow for literals.

facebook-github-bot · 2024-11-04T18:24:00Z

This pull request was exported from Phabricator. Differential Revision: D65371064

majetideepak · 2024-11-04T18:40:10Z

@pedroerp You are right. It has to be a suffix https://en.cppreference.com/w/cpp/language/user_literal.

majetideepak · 2024-11-05T10:52:31Z

I learned from the above link that we can also have user-defined integer literals. The Arrow CDataInterface will allow us to define NULL literals as well.

mbasmanova · 2024-11-05T14:26:45Z

velox/exec/tests/utils/ExpressionBuilder.h

+/// Conjuncts and "between":
+///
+/// > ExprPtr e = "a"_f && "b"_f || "c"_f; // "a AND b OR c"
+/// > ExprPtr e = between("a"_f, 0L, 10L); // "a between 0 and 10"


What would 'in' look like?

mbasmanova · 2024-11-05T14:26:56Z

velox/exec/tests/utils/ExpressionBuilder.h

+/// execution using type binding from `core::Expressions::inferTypes()`.
+///
+/// The API provided is as close to the actual expression trees as possible.
+/// Filters, arithmetics, function calls, literals, aliases, and more are


What does 'cast' look like?

Haven't written it yet, but it would follow the exact pattern in the CastExpr node, so something like

cast(BIGINT(), "col"_f)

This would actually be better than SQL as we can be very precise about the type being cast.

mbasmanova · 2024-11-05T14:27:27Z

velox/exec/tests/utils/ExpressionBuilder.h

+/// C++ literals are automatically converted into ConstantExpr (expression
+/// literals) when part of an expression. To explicitly create a literal:
+///
+/// > ExprPtr e = literal(10.3);


Would you add some examples of complex types (arrays, maps and structs)?

Would be nice to clarify how to define literals of various integer types.

mbasmanova · 2024-11-05T14:27:48Z

velox/exec/tests/utils/ExpressionBuilder.h

+///
+/// > ExprPtr e = literal(10.3);
+/// > ExprPtr e = literal("str");
+/// > ExprPtr e = literalTimestamp("1970-01-01 01:30:00");


What would timestamp with timezone look like?

mbasmanova · 2024-11-05T14:30:11Z

velox/exec/tests/utils/ExpressionBuilder.h

+/// When building long expressions, be careful about C++ operator precedence.
+/// For example:
+///
+/// > ExprPtr e = "col"_f + 5 * 100;


Are you cautioning about plus(col, multiply(5, 100)) vs. plus(col, 500)? This wasn't obvious right away.

mbasmanova · 2024-11-05T14:31:27Z

velox/exec/tests/utils/ExpressionBuilder.h

+/// > ExprPtr e = "col"_f + 5 * literal(100);
+/// > ExprPtr e = "col"_f + literal(5) * 100;
+///
+/// Both will generate "col + 5 * 100", which is "plus(col, multiply(5, 100))".


Does this implicitly convert + to "plus" function call? Isn't this assuming "semantics"? Perhaps, list all such implicit conversions somewhere.

mbasmanova · 2024-11-05T14:32:32Z

velox/exec/tests/utils/ExpressionBuilder.h

+using core::CallExpr;
+using core::ConstantExpr;
+using core::ExprPtr;
+using core::FieldAccessExpr;


Would you add an example of accessing a field in a struct?

mbasmanova · 2024-11-05T14:35:20Z

velox/exec/tests/ExpressionBuilderTest.cpp

+TEST(ExpressionBuilderTest, fieldAccess) {
+  ExprWrapper result;
+
+  result = field("col");


I noticed that applications using Velox library often do not have the luxury of using Velox symbols unqualified and forced to use velox::BIGINT() vs. just BIGINT().

Since this API is all about brevity and readability, how do you think applications can avoid having to prefix all these names with velox::expr::builder?

velox::expr::builder::field("f") vs. field("f")

Good question. We can naturally move this to the top-level namespace. My concern was to prevent inadvertent symbols clashes by requiring API users to explicitly import namespace. For example, maybe there could be other velox::field() symbols defined somewhere else? This was out of abundant caution though; more likely than not moving to top-level should be fine.

Curious what is the reason preventing applications from doing using namespace velox::expr::builder. using namespace velox is probably a bit too broad but using namespace velox::expr::builder probably is fine.

In the worst case they can also do namespace VEB = velox::expr::builder and use VEB::field()

mbasmanova · 2024-11-05T14:37:22Z

velox/exec/tests/ExpressionBuilderTest.cpp

+TEST(ExpressionBuilderTest, combined) {
+  ExprWrapper result;
+
+  result = 10L * "c1"_f > call("func", 3.4, "g"_f / "h"_f, call("j"));


This expression is not nearly as complex as expressions commonly found in production, yet, even that is notably less readable as compared to SQL.

I think it is useful to have this API, but, perhaps, not as a replacement for SQL.

pedroerp · 2024-11-05T16:04:13Z

@mbasmanova thanks for the comments. Will address them in the next few days.

Overall I just wanted to prototype how this API could look like and get a feeling about how the community feels about it, but if we are aligned that this might be useful, I'm happy to complete it add support for the missing expressions you pointed out (complex types, casts, lambdas, etc)

Yuhta · 2024-11-05T16:43:40Z

velox/exec/tests/utils/ExpressionBuilder.h

+  }
+
+// Define C++ operator overload for comparisons (filters).
+VELOX_EXPR_BUILDER_OPERATOR(operator==, "eq");


We have namespace prefix for functions (e.g. presto.default.eq) so this need to be configurable. Maybe a global variable for prefix is enough. But I would guess there will be other things people want to configure in the future. So a global singleton of config object probably is better.

+1. Besides the prefix, I believe Spark functions have different names, e.g. 'eq' in Presto is called 'equalto' in Spark.

Yuhta · 2024-11-05T16:48:31Z

velox/exec/tests/ExpressionBuilderTest.cpp

+TEST(ExpressionBuilderTest, fieldAccess) {
+  ExprWrapper result;
+
+  result = field("col");


Curious what is the reason preventing applications from doing using namespace velox::expr::builder. using namespace velox is probably a bit too broad but using namespace velox::expr::builder probably is fine.

In the worst case they can also do namespace VEB = velox::expr::builder and use VEB::field()

rui-mo · 2024-11-06T02:16:07Z

velox/exec/tests/utils/ExpressionBuilder.h

+
+/// Field access (references to columns).
+inline ExprWrapper field(const std::string& name) {
+  return {std::make_shared<FieldAccessExpr>(name, std::nullopt)};


The field access could contain 'input', which makes it a dereference expression. Do we need to support it? Thanks.

Summary: Adding an API to allow developers to fluently create (untyped) expression trees without having to rely on a SQL parser. Details and extensively examples provided in code comments and unit tests. Part of facebookincubator#11383 Differential Revision: D65371064

facebook-github-bot · 2024-12-17T03:44:57Z

This pull request was exported from Phabricator. Differential Revision: D65371064

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 2, 2024

facebook-github-bot added the fb-exported label Nov 2, 2024

pedroerp requested review from majetideepak, mbasmanova and xiaoxmeng November 2, 2024 00:21

majetideepak reviewed Nov 4, 2024

View reviewed changes

pedroerp force-pushed the export-D65371064 branch from 5acbf33 to b2d958d Compare November 4, 2024 18:23

mbasmanova reviewed Nov 5, 2024

View reviewed changes

Yuhta reviewed Nov 5, 2024

View reviewed changes

rui-mo reviewed Nov 6, 2024

View reviewed changes

pedroerp force-pushed the export-D65371064 branch from b2d958d to 996f0af Compare December 17, 2024 03:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add fluent expression builder API #11414

feat: Add fluent expression builder API #11414

pedroerp commented Nov 2, 2024 •

edited

Loading

facebook-github-bot commented Nov 2, 2024

netlify bot commented Nov 2, 2024 •

edited

Loading

pedroerp commented Nov 2, 2024

majetideepak left a comment

majetideepak Nov 4, 2024

majetideepak Nov 4, 2024

majetideepak Nov 4, 2024

pedroerp commented Nov 4, 2024

facebook-github-bot commented Nov 4, 2024

majetideepak commented Nov 4, 2024

majetideepak commented Nov 5, 2024

mbasmanova Nov 5, 2024

mbasmanova Nov 5, 2024

pedroerp Nov 5, 2024 •

edited

Loading

mbasmanova Nov 5, 2024

mbasmanova Nov 5, 2024

mbasmanova Nov 5, 2024

mbasmanova Nov 5, 2024

mbasmanova Nov 5, 2024

mbasmanova Nov 5, 2024

pedroerp Nov 5, 2024

Yuhta Nov 5, 2024 •

edited

Loading

mbasmanova Nov 5, 2024

pedroerp commented Nov 5, 2024

Yuhta Nov 5, 2024

rui-mo Nov 6, 2024

Yuhta Nov 5, 2024 •

edited

Loading

rui-mo Nov 6, 2024

facebook-github-bot commented Dec 17, 2024

feat: Add fluent expression builder API #11414

Are you sure you want to change the base?

feat: Add fluent expression builder API #11414

Conversation

pedroerp commented Nov 2, 2024 • edited Loading

facebook-github-bot commented Nov 2, 2024

netlify bot commented Nov 2, 2024 • edited Loading

✅ Deploy Preview for meta-velox canceled.

pedroerp commented Nov 2, 2024

majetideepak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pedroerp commented Nov 4, 2024

facebook-github-bot commented Nov 4, 2024

majetideepak commented Nov 4, 2024

majetideepak commented Nov 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pedroerp Nov 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yuhta Nov 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pedroerp commented Nov 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yuhta Nov 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

facebook-github-bot commented Dec 17, 2024

pedroerp commented Nov 2, 2024 •

edited

Loading

netlify bot commented Nov 2, 2024 •

edited

Loading

pedroerp Nov 5, 2024 •

edited

Loading

Yuhta Nov 5, 2024 •

edited

Loading

Yuhta Nov 5, 2024 •

edited

Loading