feat: full e2e pipeline #26

yliang412 · 2025-02-11T09:21:14Z

Problem

With the initial representation and storage added in #4 and #22, we now want to support the full pipeline going from parsing SQL, optimizing the plan using optd, and executing the query in Datafusion.

Summary of changes

Integrate all @SarveshOO7's good work in WIP Built some initial e2e datafusion infrastructure #10
Added one mock physical implementation rule + operator for each logical operator
Add physical storage tables and memo API.
Bump MSRV to 1.81.0 to be compatible with datafusion 45.0.0: Increase MSRV to 1.81.0 apache/datafusion#14330

Signed-off-by: Yuchen Liang <[email protected]>

codecov-commenter · 2025-02-11T09:26:07Z

Codecov Report

Attention: Patch coverage is 88.73500% with 122 lines in your changes missing coverage. Please review.

Project coverage is 83.5%. Comparing base (44d26e2) to head (cd7ef16).

Files with missing lines	Patch %	Lines
optd-datafusion/src/converter/into_optd.rs	69.1%	37 Missing ⚠️
optd-core/src/storage/memo.rs	91.0%	16 Missing ⚠️
optd-datafusion/src/converter/from_optd.rs	90.4%	12 Missing ⚠️
optd-datafusion/src/lib.rs	87.8%	10 Missing ⚠️
optd-core/src/operators/relational/physical/mod.rs	0.0%	8 Missing ⚠️
optd-core/src/values/mod.rs	53.3%	7 Missing ⚠️
...d-core/src/operators/relational/logical/project.rs	33.3%	6 Missing ⚠️
...-core/src/operators/relational/physical/project.rs	33.3%	6 Missing ⚠️
optd-datafusion/src/main.rs	80.7%	5 Missing ⚠️
optd-core/src/cascades/mod.rs	98.8%	4 Missing ⚠️
... and 4 more

Additional details and impacted files

Files with missing lines	Coverage Δ
...c/operators/relational/physical/scan/table_scan.rs	`50.0% <ø> (+50.0%)`	⬆️
optd-core/src/test_utils.rs	`100.0% <100.0%> (ø)`
optd-datafusion/src/converter/mod.rs	`100.0% <100.0%> (ø)`
optd-core/src/operators/relational/logical/mod.rs	`78.4% <77.7%> (-0.2%)`	⬇️
optd-core/src/operators/scalar/mod.rs	`70.3% <75.0%> (+0.8%)`	⬆️
optd-core/src/operators/scalar/and.rs	`50.0% <50.0%> (ø)`
optd-core/src/cascades/mod.rs	`98.9% <98.8%> (-0.3%)`	⬇️
optd-datafusion/src/planner.rs	`94.5% <94.5%> (ø)`
optd-datafusion/src/main.rs	`80.7% <80.7%> (ø)`
...d-core/src/operators/relational/logical/project.rs	`33.3% <33.3%> (ø)`
... and 7 more

... and 3 files with indirect coverage changes

Signed-off-by: Yuchen Liang <[email protected]>

apache/datafusion#14330 Signed-off-by: Yuchen Liang <[email protected]>

Signed-off-by: Yuchen Liang <[email protected]>

skyzh · 2025-02-11T22:31:09Z

optd-core/src/cascades/memo.rs

+    ) -> Result<Vec<(PhysicalExpressionId, Arc<PhysicalExpression>)>>;
+
+    /// Adds a physical expression to an existing group in the memo table.
+    async fn add_physical_expr_to_group(


note that it's possible for a physical expr not to belong to any group with the optd fork design, so probably we need to think twice about the physexpr related apis

skyzh · 2025-02-11T22:32:02Z

optd-core/src/cascades/mod.rs

+}
+
+#[async_recursion]
+pub async fn match_any_partial_logical_plan(


consider codegen this with the facilities in #25

skyzh · 2025-02-11T22:34:48Z

optd-core/src/operators/relational/physical/project.rs

+}
+
+/// Creates a physical project operator.
+pub fn project<Relation, Scalar>(


naming: need to distinguish from logical project?

skyzh · 2025-02-11T22:35:45Z

optd-core/src/operators/scalar/and.rs

+
+/// A scalar operator that adds two values.
+#[derive(Debug, Clone, PartialEq, Deserialize)]
+pub struct And<Scalar> {


I think we should collapse binary ops and logical ops into a single class like

pub struct LogicalOp<Scalar> { type: Or/And } pub struct BinaryOp<Scalar> { type: Add/Minus/... }

which helps reduce the num of lines of code

skyzh · 2025-02-11T22:37:03Z

optd-core/src/storage/memo.rs

+    /// Inserts an entry into the `physical_expressions` table.
+    async fn insert_into_physical_expressions(
+        txn: &mut SqliteConnection,
+        logical_expr_id: PhysicalExpressionId,


rename to physical_expr_id

skyzh · 2025-02-11T22:38:34Z

optd-core/src/storage/memo.rs

@@ -544,7 +757,9 @@ const fn get_all_scalar_exprs_in_group_query() -> &'static str {
        " UNION ALL ",
        "SELECT scalar_expression_id, json_object('Add', json_object('left', left_group_id, 'right', right_group_id)) as data FROM scalar_adds WHERE group_id = $1",
        " UNION ALL ",
-        "SELECT scalar_expression_id, json_object('Equal', json_object('left', left_group_id, 'right', right_group_id)) as data FROM scalar_equals WHERE group_id = $1"
+        "SELECT scalar_expression_id, json_object('Equal', json_object('left', left_group_id, 'right', right_group_id)) as data FROM scalar_equals WHERE group_id = $1",


I thought And will be represented as a list of expressions instead of left/right

skyzh · 2025-02-11T22:41:18Z

optd-datafusion/src/converter/into_optd.rs

+            DFLogicalPlan::Join(join) => {
+                let mut join_cond = Vec::new();
+                for (left, right) in &join.on {
+                    let left = Self::conv_df_to_optd_scalar(left, join.left.schema(), 0)?;


I would recommend make conversion + column ref rewrite into two passes because they are two different things

Otherwise every scalar conversion function has to take an offset with it. Or you can make such offset into the context.

skyzh · 2025-02-11T22:43:04Z

optd-datafusion/src/lib.rs

+pub mod converter;
+pub mod planner;
+
+pub async fn run_queries(queries: String) -> Result<()> {


Consider vendor datafusion-cli instead of implementing this by ourselves

skyzh · 2025-02-11T22:43:42Z

optd-datafusion/src/lib.rs

+}
+/// Utility function to create a session context for datafusion + optd.
+/// The allow deprecated is for the RuntimeConfig
+#[allow(deprecated)]


We should use the new API from datafusion instead of marking allow deprecated

skyzh · 2025-02-11T22:44:10Z

optd-datafusion/src/main.rs

+use optd_datafusion::run_queries;
+use std::io;
+
+#[tokio::main]


vendor datafusion-cli instead of implementing a less functional cli?

SarveshOO7 and others added 9 commits February 10, 2025 12:53

check in optd-datafusion

335c5d1

Signed-off-by: Yuchen Liang <[email protected]>

fix all compiler errors

c99428a

Signed-off-by: Yuchen Liang <[email protected]>

add back logical_project

f540ee6

Signed-off-by: Yuchen Liang <[email protected]>

add logical and

ae646b6

Signed-off-by: Yuchen Liang <[email protected]>

make scan work e2e

cac6a12

Signed-off-by: Yuchen Liang <[email protected]>

add physical operators + tests

68a82c4

Signed-off-by: Yuchen Liang <[email protected]>

make partial physical pass e2e tests

0bdb18d

Signed-off-by: Yuchen Liang <[email protected]>

e2e df now works

1f52737

Signed-off-by: Yuchen Liang <[email protected]>

Merge branch 'main' into yuchen/e2e

a8153cb

yliang412 added 3 commits February 11, 2025 04:43

rework dependencies; bump df to 45.0.0

a806f07

Signed-off-by: Yuchen Liang <[email protected]>

fix clippy

0d94f92

Signed-off-by: Yuchen Liang <[email protected]>

Bump msrv to 1.81.0

7f9449b

apache/datafusion#14330 Signed-off-by: Yuchen Liang <[email protected]>

yliang412 force-pushed the yuchen/e2e branch from 07ec3e3 to 7f9449b Compare February 11, 2025 15:54

yliang412 mentioned this pull request Feb 11, 2025

WIP Built some initial e2e datafusion infrastructure #10

Closed

fix ingestion

cd7ef16

Signed-off-by: Yuchen Liang <[email protected]>

skyzh mentioned this pull request Feb 11, 2025

feat: DSL operator codegen #25

Open

skyzh approved these changes Feb 11, 2025

View reviewed changes

yliang412 mentioned this pull request Feb 13, 2025

storage: generate schema and queries automatically #27

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: full e2e pipeline #26

feat: full e2e pipeline #26

yliang412 commented Feb 11, 2025 •

edited

Loading

codecov-commenter commented Feb 11, 2025 •

edited

Loading

skyzh Feb 11, 2025

skyzh Feb 11, 2025

skyzh Feb 11, 2025

skyzh Feb 11, 2025

skyzh Feb 11, 2025

skyzh Feb 11, 2025

skyzh Feb 11, 2025

skyzh Feb 11, 2025

skyzh Feb 11, 2025

skyzh Feb 11, 2025

skyzh Feb 11, 2025

feat: full e2e pipeline #26

Are you sure you want to change the base?

feat: full e2e pipeline #26

Conversation

yliang412 commented Feb 11, 2025 • edited Loading

Problem

Summary of changes

codecov-commenter commented Feb 11, 2025 • edited Loading

Codecov Report

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yliang412 commented Feb 11, 2025 •

edited

Loading

codecov-commenter commented Feb 11, 2025 •

edited

Loading