Recursive CTEs: Stage 3 - add execution support #6

matthewgapp · 2024-01-12T04:14:15Z

WIP. Adds execution support to recursive CTEs

Todos

Fill out docs and cleanup. Add more tests.

Closes #.

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

matthewgapp · 2024-01-12T05:42:02Z

datafusion/sqllogictest/test_files/cte.slt

+        time,
+        1 as "val"
+      FROM
+        (SELECT DISTINCT "time" FROM "beg_account_balance")


This test fails. But the following changes cause the test to pass

removing time or 1 as "val" from the sub_cte projection

removing the reference to beg_account_balance in the FROM clause (effectively removing the sub-query from the sub_cte)

removing the FULL JOIN "sub_cte" on 1=1

The test fails with index out of bounds: the len is 0 but the index is 0 but other incarnations of the error after playing around with the repro were

Internal error: PhysicalExpr Column references column 'time' at index 0 (zero-based) but input schema only has 0 columns: []. This was likely caused by a bug in DataFusion's code and we would welcome that you file an bug report in our issue tracker [SQL] WITH RECURSIVE "recursive_cte" AS

External error: query failed: DataFusion error: ProjectionPushdown caused by Internal error: PhysicalExpr Column references column 'time' at index 0 (zero-based) but input schema only has 0 columns: []. This was likely caused by a bug in DataFusion's code and we would welcome that you file an bug report in our issue tracker [SQL] WITH RECURSIVE "recursive_cte" AS (

External error: query failed: DataFusion error: ProjectionPushdown caused by Internal error: PhysicalExpr Column references column 'LEAD(beg_account_balance.time,Int64(1)) ORDER BY [beg_account_balance.time ASC NULLS LAST] RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW' at index 1 (zero-based) but input schema only has 1 columns: ["LEAD(beg_account_balance.time,Int64(1)) ORDER BY [beg_account_balance.time ASC NULLS LAST] RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW"]. This was likely caused by a bug in DataFusion's code and we would welcome that you file an bug report in our issue tracker [SQL] WITH RECURSIVE "recursive_cte" AS (

Note that the columns referenced in the error messages are different from the repro because the repro at the time that I collected those errors was different.

matthewgapp · 2024-01-12T05:45:03Z

datafusion/common/src/config.rs

@@ -282,7 +282,7 @@ config_namespace! {
        /// Should DataFusion support recursive CTEs
        /// Defaults to false since this feature is a work in progress and may not
        /// behave as expected
-        pub enable_recursive_ctes: bool, default = false
+        pub enable_recursive_ctes: bool, default = true


to set back to false. Need to provide a method to set this at the root config level

* refactor * generated files * feat * feat * feat * feat * tests * clippy --------- Co-authored-by: Andrew Lamb <[email protected]>

…ion (apache#8839) * add config flag for recursive ctes update docs from script update slt test for doc change * restore testing pin * add sql -> logical plan support * impl cte as work table * move SharedState to continuance * impl WorkTableState wip: readying pr to implement only logical plan fix sql integration test wip: add sql test for logical plan wip: format test assertion * wip: remove uncessary with qualifier method some docs more docs * Add comments to `RecursiveQuery` * Update datfusion-cli Cargo.lock * Fix clippy * better errors and comments * add doc comment with rationale for create_cte_worktable method * wip: tweak --------- Co-authored-by: Andrew Lamb <[email protected]>

) * Add support for parsing FixedSizeList type * fix fmt * support cast fixedsizelist from list * clean comment * support cast between NULL and FixedSizedLisr * add test for FixedSizeList hash * add test for cast fixedsizelist

…t empty (apache#8914) * Fix aggregate_statistics * Add more test

* Support to_timestamp with chrono formatting apache#5398 * Updated user guide's to_timestamp to include chrono formatting information apache#5398 * Minor comment update. * Small documentation updates for to_timestamp functions. * Cargo fmt and clippy improvements. * Switched to assert and unwrap_err based on feedback * Fixed assert, code compiles and runs as expected now. * Fix fmt (again). * Add additional to_timestamp tests covering usage with tables with and without valid formats. * to_timestamp documentation fixes. * - Changed internal_err! -> exec_err! for unsupported data type errors. - Extracted out to_timestamp_impl method to reduce code duplication as per PR feedback. - Extracted out validate_to_timestamp_data_types to reduce code duplication as per PR feedback. - Added additional tests for argument validation and invalid arguments. - Removed unnecessary shim function 'string_to_timestamp_nanos_with_format_shim' * Resolved merge conflict, updated toStringXXX methods to reflect upstream change * prettier * Fix clippy --------- Co-authored-by: Andrew Lamb <[email protected]>

@mhilton

…value (apache#8912) * Minor: Document third argument of date_bin as optional # Rationale @mhilton noticed this as part of an internal discussion where we were confused about the output values of `date_bin` for timestamps with timezones (as the default argument is for the unix EPOCH in UTC) # Changes: Document the third parameter to `date_bin` better * prettier

Co-authored-by: Andrew Lamb <[email protected]>

apache#8829) * support array slice * fix argument * fix typo * support from and to is negative * fix conflict * modify user doc * refactor code * fix clippy * add 1-index test --------- Co-authored-by: Andrew Lamb <[email protected]>

Signed-off-by: Ruihang Xia <[email protected]>

* Fix expr partial ord test * Add comment * Resolve linter error

* Simplify windows builtin functions * add field comments

* Updated documentation concerning to_timestamp scalar function including adding missing link in the datafusion-examples/README file. Resolves apache#8980 * Fix example url. * Applied prettier.

* Add helper function for scalar function * Update datafusion/physical-expr/src/functions.rs Co-authored-by: Andrew Lamb <[email protected]> * Fix * Fix --------- Co-authored-by: Andrew Lamb <[email protected]>

* Fix optimize projections bug * Add new dataframe test

apache#8982) * optimize NOT Expr logic * fix Null type * fix test case * fmt

* Optimize MIN/MAX when relation is empty * Fix clippy

Signed-off-by: tangruilin <[email protected]>

* [task apache#8213]Part of Port tests in select.rs to sqllogictest Signed-off-by: tangruilin <[email protected]> * test large strings as well * Restore parameter tests * make test deterministic --------- Signed-off-by: tangruilin <[email protected]> Co-authored-by: Andrew Lamb <[email protected]>

* Migrate last repartition query * Add reference to issue Co-authored-by: Andrew Lamb <[email protected]> * Fix missing statement --------- Co-authored-by: Andrew Lamb <[email protected]>

* Update to sqlparser `0.42.0` * Update datafusion Cargo.lock

* Make tests deterministic * Add duplicate timestamps

* Add support for PG LIKE operators * Bump sqlparser dep from branch to merge commit

…he#8964) * fix: table alias will process uppercase alias into lowercase * test: uppercase table alias * test: add table alias sqllogictest * test: move tests to sqllogictests * test: update cte.slt * chore: add test descriptions

…pache#8986) * Document parallelism and thread scheduling in the architecture guid * Apply suggestions from code review Co-authored-by: Raphael Taylor-Davies <[email protected]> --------- Co-authored-by: Raphael Taylor-Davies <[email protected]>

* Fix none projections * Update select.slt

…type other than Int64 (apache#9001)

add config flag for recursive ctes update docs from script update slt test for doc change restore testing pin add sql -> logical plan support * impl cte as work table * move SharedState to continuance * impl WorkTableState wip: readying pr to implement only logical plan fix sql integration test wip: add sql test for logical plan wip: format test assertion wip: remove uncessary with qualifier method some docs more docs Add comments to `RecursiveQuery` Update datfusion-cli Cargo.lock Fix clippy better errors and comments add sql -> logical plan support * impl cte as work table * move SharedState to continuance * impl WorkTableState wip: readying pr to implement only logical plan fix sql integration test wip: add sql test for logical plan wip: format test assertion wip: remove uncessary with qualifier method some docs more docs impl execution support add sql -> logical plan support * impl cte as work table * move SharedState to continuance * impl WorkTableState wip: readying pr to implement only logical plan partway through porting over isidentical's work Continuing implementation with fixes and improvements Lint fixes ensure that repartitions are not added immediately after RecursiveExec in the physical-plan add trivial sqllogictest more recursive tests remove test that asserts recursive cte should fail additional cte test wip: remove tokio from physical plan dev deps format cargo tomls fix issue where CTE could not be referenced more than 1 time wip: fixes after rebase but tpcds_physical_q54 keeps overflowing its stack Impl NamedRelation as CteWorkTable * impl cte as work table * move SharedState to continuance * impl WorkTableState * upd * assign work table state * upd * upd fix min repro but still broken on larger test case set config in sql logic tests clean up cte slt tests fixes fix option add group by test case and more test case files wip add window function recursive cte example simplify stream impl for recrusive query stream add explain to trivial test case move setting of recursive ctes to slt file and add test to ensure multiple record batches are produced each iteration remove tokio dep and remove mut lint, comments and remove tokio stream update submodule pin to feat branch that contains csvs update submodule pin to feat branch that contains csvs

…ive term

matthewgapp changed the title ~~add config flag for recursive ctes~~ Recursive CTEs: Stage 3 - add execution support Jan 12, 2024

matthewgapp commented Jan 12, 2024

View reviewed changes

matthewgapp mentioned this pull request Jan 12, 2024

Add support for recursive CTEs apache/datafusion#462

Closed

matthewgapp force-pushed the matt/feat/recursive-ctes/parser branch 2 times, most recently from 98f77ff to 531d541 Compare January 12, 2024 06:08

matthewgapp force-pushed the matt/feat/recursive-ctes/execution-support branch from e0ea336 to 5257fe3 Compare January 12, 2024 06:10

matthewgapp force-pushed the matt/feat/recursive-ctes/parser branch 2 times, most recently from 9615dde to 460561c Compare January 14, 2024 00:40

matthewgapp force-pushed the matt/feat/recursive-ctes/execution-support branch 2 times, most recently from a993efb to f67763a Compare January 15, 2024 22:04

matthewgapp force-pushed the matt/feat/recursive-ctes/parser branch from 22661af to 8d38c92 Compare January 16, 2024 17:56

matthewgapp force-pushed the matt/feat/recursive-ctes/execution-support branch 2 times, most recently from 06fcbea to 68c9fe7 Compare January 18, 2024 19:56

tushushu and others added 11 commits January 18, 2024 15:24

Add serde support for Arrow FileTypeWriterOptions (apache#8850)

9b78efa

* refactor * generated files * feat * feat * feat * feat * tests * clippy --------- Co-authored-by: Andrew Lamb <[email protected]>

Improve datafusion-cli print format tests (apache#8896)

3a9e23d

remove null in array-append adn array-prepend (apache#8901)

72b81f1

aggregate_statistics should only optimize MIN/MAX when relation is no…

d0c84cc

…t empty (apache#8914) * Fix aggregate_statistics * Add more test

Minor: distinguish parquet row group pruning test type (apache#8921)

95e739c

Add hash_join_single_partition_threshold_rows config (apache#8720)

f5a97d5

Co-authored-by: Andrew Lamb <[email protected]>

Prepare 35.0.0-rc1 (apache#8924)

b7e13a0

matthewgapp force-pushed the matt/feat/recursive-ctes/execution-support branch from 68c9fe7 to b11ca8b Compare January 21, 2024 08:49

Weijun-H and others added 4 commits January 21, 2024 07:30

fix: recursive initialize method (apache#8937)

97441cc

Signed-off-by: Ruihang Xia <[email protected]>

Fix expr partial ord test (apache#8908)

c0a69a7

* Fix expr partial ord test * Add comment * Resolve linter error

Simplify windows builtin functions return type (apache#8920)

2b218be

* Simplify windows builtin functions * add field comments

Omega359 and others added 26 commits January 24, 2024 15:13

Improve to_timestamp docs (apache#8981)

d81c82d

* Updated documentation concerning to_timestamp scalar function including adding missing link in the datafusion-examples/README file. Resolves apache#8980 * Fix example url. * Applied prettier.

Add helper function for processing scalar function input (apache#8962)

d6ab343

* Add helper function for scalar function * Update datafusion/physical-expr/src/functions.rs Co-authored-by: Andrew Lamb <[email protected]> * Fix * Fix --------- Co-authored-by: Andrew Lamb <[email protected]>

Fix optimize projections bug (apache#8960)

4ac7de1

* Fix optimize projections bug * Add new dataframe test

NOT operator not return internal error when args are not boolean value (

4a3986a

apache#8982) * optimize NOT Expr logic * fix Null type * fix test case * fmt

Minor: Add new Extended ClickBench benchmark queries (apache#8950)

928162f

Minor: Add comments to MSRV CI check to help if it fails (apache#8995)

80a42bf

Minor: Document memory management design on MemoryPool (apache#8966)

7a0af5b

Fix LEAD/LAG window functions when default value null (apache#8989)

5e9c9a1

Optimize MIN/MAX when relation is empty (apache#8940)

eb6d63f

* Optimize MIN/MAX when relation is empty * Fix clippy

[task apache#8203] Port tests in joins.rs to sqllogictest (apache#8996)

b97daf7

Signed-off-by: tangruilin <[email protected]>

test: Port (last) repartition.rs query to sqllogictest (apache#8936)

6e4abf5

* Migrate last repartition query * Add reference to issue Co-authored-by: Andrew Lamb <[email protected]> * Fix missing statement --------- Co-authored-by: Andrew Lamb <[email protected]>

Update to sqlparser 0.42.0 (apache#9000)

4d02cc0

* Update to sqlparser `0.42.0` * Update datafusion Cargo.lock

Add new test (apache#8992)

8a4bad4

Make Topk aggregate tests deterministic (apache#8998)

bee7136

* Make tests deterministic * Add duplicate timestamps

Add support for Postgres LIKE operators (apache#8894)

bd38142

* Add support for PG LIKE operators * Bump sqlparser dep from branch to merge commit

Fix None Projections in Projection Pushdown (apache#9005)

ec6abec

* Fix none projections * Update select.slt

Lead and Lag window functions should support default value with data …

b3fe6aa

…type other than Int64 (apache#9001)

chore: fix license badge in README (apache#9008)

c42bf48

error if recursive ctes are nested

efed900

error if recursive cte is referenced multiple times within the recurs…

38f847d

…ive term

wip

6121248

fix rebase

80069f7

matthewgapp force-pushed the matt/feat/recursive-ctes/execution-support branch from e65caee to 80069f7 Compare January 26, 2024 17:25

matthewgapp added 3 commits January 26, 2024 14:18

move testing files into main repo

812d64f

update testing pin to main pin

39ab371

tweaks

d100913

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Recursive CTEs: Stage 3 - add execution support #6

Recursive CTEs: Stage 3 - add execution support #6

matthewgapp commented Jan 12, 2024 •

edited

Loading

matthewgapp Jan 12, 2024

matthewgapp Jan 12, 2024

matthewgapp Jan 12, 2024

matthewgapp Jan 12, 2024

Recursive CTEs: Stage 3 - add execution support #6

Are you sure you want to change the base?

Recursive CTEs: Stage 3 - add execution support #6

Conversation

matthewgapp commented Jan 12, 2024 • edited Loading

Todos

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

matthewgapp Jan 12, 2024

Choose a reason for hiding this comment

matthewgapp Jan 12, 2024

Choose a reason for hiding this comment

matthewgapp Jan 12, 2024

Choose a reason for hiding this comment

matthewgapp Jan 12, 2024

Choose a reason for hiding this comment

matthewgapp commented Jan 12, 2024 •

edited

Loading