Implement basic unnest function #6796

jayzhan211 · 2023-06-29T05:44:51Z

Which issue does this PR close?

Ref #6555

Closes #.

Rationale for this change

Support unnest for arrays

What changes are included in this PR?

select unnest()
select * from unnest()
unnest alias
unnest multidimensional arrays
unnest args more than 2
select partial columns
deterministic result for datafusion.execution.target_partitions > 1 (We might not need this?)

Are these changes tested?

Are there any user-facing changes?

jayzhan211 · 2023-06-29T05:56:38Z

Current implementation.

There are two workflows for unnest

unnest_arrays is the core function for building an Unnest Logical Plan.

Workflow 1: select unnest(), we will get statements from select.projection. SQL::Function unnest is then transformed to Expr::Unnest. We will then build the LogicalPlan in optimize phase. In optimize rule, when we see Expr::Unnest, build the Unnest Logical Plan with unnest_arrays.

Workflow 2:select * from unnest(), we can get the table relation Unnest directly. In this case, we can build an Unnest Logical Plan with unnest_arrays without Expr::Unnest involved.

jayzhan211 · 2023-06-29T07:59:45Z

Code cleanup is done, and ready for review.
@izveigor @alamb

datafusion/core/tests/sqllogictests/test_files/array.slt

izveigor · 2023-06-29T14:07:34Z

datafusion/core/tests/sqllogictests/test_files/array.slt

+set datafusion.execution.target_partitions = 1;
+
+query ?
+select unnest(make_array(1,2,3,4,5))


I also recommend to create test with one of aggregate functions as stated in the description of the issue: #6555

SELECT sum(a) AS total FROM (SELECT unnest(make_array(3, 5, 6) AS a) AS b; ---- 14

This one also. Add to task list

jayzhan211 · 2023-07-02T06:55:55Z

datafusion/sql/src/planner.rs

-        )?);
-
-        self.apply_expr_alias(apply_name_plan, alias.columns)
+        let plan = self.apply_expr_alias(plan, alias.columns)?;


The order is changed so that we can get either expr or table. expr.

Example in array.slt
table.expr: SELECT sum(data.a), sum(data.b) FROM unnest(make_array(1, 2, 3), make_array(4, 5, 6, 7)) as data(a, b);
expr: SELECT sum(a), sum(b) FROM unnest(make_array(1, 2, 3), make_array(4, 5, 6, 7)) as data(a, b);

jayzhan211 · 2023-07-03T03:42:42Z

Wait for sqlparser 0.36

alamb · 2023-07-03T10:05:36Z

I was planning to release sqlparser in a few weeks -- but if it is blocking this PR I can find time to make a release sooner

jayzhan211 · 2023-07-03T13:03:15Z

I was planning to release sqlparser in a few weeks -- but if it is blocking this PR I can find time to make a release sooner

I hope sqlparser can be released soon! Thanks!

alamb · 2023-07-03T19:21:45Z

Next release is tracked by apache/datafusion-sqlparser-rs#905

I will try and prioritize it but this week is short for me because of the US holiday (and I am spending most of my DataFusion time on #6800)

jayzhan211 · 2023-07-16T14:36:27Z

@alamb Kindly remind you. :)

jayzhan211 · 2023-07-28T07:09:24Z

Possible improvement:
Utilize the existing unnest_columns function instead building a new one.

TODO in another PR

alamb

Thank you @jayzhan211 -- this is getting there but I don't think it is ready to merge yet (for reasons I left in comments below)

My major concerns are that we aren't using the LogicalPlan::Unnest and UnnestExec physical operator -- doing so would mean that improvements we plan (like making UnnestExec more efficient) will not apply to using unnest as a function

As this PR is large and I would like to move it along, I suggest

Remove flatten (we can put that into another PR)
Make a PR with just the Expr::Unnest -- maybe this could simply create a LogicalPlan::Unnest?
Make a PR to add any additional features needed for unnest (like multiple column support, for example)

What do you think?

cc @izveigor @vincev and @smiklos

alamb · 2023-08-07T20:51:38Z

datafusion/core/tests/sqllogictests/test_files/array.slt

+# TODO: Unnest columns fails
+query error DataFusion error: SQL error: TokenizerError\("Unterminated string literal at Line: 2, Column 95"\)
+caused by
+Internal error: UNNEST only supports list type\. 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


this appears still to be NotImplemented

datafusion/expr/src/expr.rs

alamb · 2023-08-07T20:55:54Z

datafusion/expr/src/expr.rs

+                    args: flatten_args,
+                }))
+            }
+            _ => Err(DataFusionError::Internal(format!(


Suggested change

_ => Err(DataFusionError::Internal(format!(

_ => Err(DataFusionError::NotYetImplemented(format!(

The updated version is in https://github.com/apache/arrow-datafusion/pull/7461/files

datafusion/expr/src/expr_schema.rs

alamb · 2023-08-07T20:58:19Z

datafusion/expr/src/logical_plan/builder.rs

@@ -1038,6 +1042,114 @@ impl LogicalPlanBuilder {
    pub fn unnest_column(self, column: impl Into<Column>) -> Result<Self> {
        Ok(Self::from(unnest(self.plan, column.into())?))
    }
+
+    pub fn join_unnest_plans(


Is it possible to use https://docs.rs/datafusion/latest/datafusion/physical_plan/unnest/struct.UnnestExec.html directly rather than building up a special LogicalPlan? It seems like UnnestExec already does this

datafusion/expr/src/logical_plan/builder.rs

jayzhan211 · 2023-08-08T00:54:39Z

As this PR is large and I would like to move it along, I suggest

Remove flatten (we can put that into another PR)

Make a PR with just the Expr::Unnest -- maybe this could simply create a LogicalPlan::Unnest?

Make a PR to add any additional features needed for unnest (like multiple column support, for example)

Let me work on #6995 first while thinking about step 2, since I don't think simply creating LogicalPlan from Expr::Unnest is trivial.

alamb · 2023-08-16T18:52:53Z

Marking as draft to signify it is not waiting on review

jayzhan211 · 2023-08-30T14:48:13Z

Make a PR with just the Expr::Unnest -- maybe this could simply create a LogicalPlan::Unnest?

To create LogicalPlan::Unnest, it seems we can only do that in the optimization step. But this can't avoid schema change, due to name unnest([1,2,3]) -> [1,2,3] and type list(i64) -> i64.

Convert expr to logical plan in select_to_plan works, cool.

jayzhan211 · 2023-09-12T12:51:57Z

The Unnest function is now processing in the planning step, converting Expr::Unnest to LogicalPlan::Unnest in this phase. Nothing to do in the optimize phase and only the existing UnnestExec is processed in the PhysicalPlan phase. Not sure if this is a good design but it works :)

Maybe we can review the overall design, and then I will address the comments and resolve conflicts.

alamb · 2023-09-12T14:20:32Z

I will try and review this more carefully later today and provide feedback. Thank you @jayzhan211

jayzhan211 · 2023-10-05T00:14:14Z

datafusion/expr/src/expr_schema.rs

+                    .collect::<Result<Vec<_>>>()?;
+
+                if data_types.is_empty() {
+                    return Err(DataFusionError::NotImplemented(


TODO: use macro

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

alamb · 2023-10-30T20:31:27Z

Marking this one as draft as it has bitrotted for a while and I think we need to work on simplifing the array code we have before we add more

alamb · 2024-04-08T21:06:35Z

I think DataFusion now supports UNNEST -- is this PR still being worked on?

jayzhan211 · 2024-04-08T23:41:12Z

I think DataFusion now supports UNNEST -- is this PR still being worked on?

We support single columns only, the challenging one multiple columns has not yet been supported, I remember @jonahgao is working on it. I forgot which PR you mentioned.

jonahgao · 2024-04-09T01:53:22Z

I think DataFusion now supports UNNEST -- is this PR still being worked on?

We support single columns only, the challenging one multiple columns has not yet been supported, I remember @jonahgao is working on it. I forgot which PR you mentioned.

Not started yet. I just discovered that it hasn't been implemented in #9592 (comment), but I can start working on it from now on.

jonahgao · 2024-04-09T02:07:39Z

I think I might have misunderstood. Neither of the two queries below has been implemented, the one discussed in this PR seems to be the latter.

DataFusion CLI v37.0.0
❯ select unnest(ARRAY[1,2]), unnest(ARRAY[3,4,5]);
This feature is not implemented: Only support single unnest expression for now

❯ select * from unnest(ARRAY[1,2], ARRAY[2,3]);
This feature is not implemented: unnest() does not support multiple arguments yet

I plan to try to implement them both.

github-actions bot added core Core DataFusion crate logical-expr Logical plan and expressions optimizer Optimizer rules physical-expr Physical Expressions sql SQL Planner sqllogictest SQL Logic Tests (.slt) labels Jun 29, 2023

github-actions bot removed the physical-expr Physical Expressions label Jun 29, 2023

jayzhan211 marked this pull request as ready for review June 29, 2023 06:46

jayzhan211 force-pushed the unnest_array branch from 84a2da7 to c5dc0ac Compare June 29, 2023 07:57

izveigor reviewed Jun 29, 2023

View reviewed changes

datafusion/core/tests/sqllogictests/test_files/array.slt Outdated Show resolved Hide resolved

izveigor reviewed Jun 29, 2023

View reviewed changes

jayzhan211 force-pushed the unnest_array branch from c5dc0ac to 5835d99 Compare July 1, 2023 06:17

jayzhan211 marked this pull request as draft July 2, 2023 01:32

jayzhan211 force-pushed the unnest_array branch from 5835d99 to 1ff8bde Compare July 2, 2023 05:53

jayzhan211 marked this pull request as ready for review July 2, 2023 06:52

jayzhan211 commented Jul 2, 2023

View reviewed changes

jayzhan211 marked this pull request as draft July 3, 2023 02:50

jayzhan211 force-pushed the unnest_array branch 2 times, most recently from 9127da7 to e55a2fb Compare July 28, 2023 04:27

jayzhan211 marked this pull request as ready for review July 28, 2023 07:09

jayzhan211 marked this pull request as draft July 28, 2023 07:10

jayzhan211 marked this pull request as ready for review July 28, 2023 07:24

jayzhan211 mentioned this pull request Aug 7, 2023

Implement flatten function #6995

Closed

alamb reviewed Aug 7, 2023

View reviewed changes

alamb marked this pull request as draft August 16, 2023 18:52

korowa mentioned this pull request Aug 24, 2023

fix: inconsistent scalar types in DistinctArrayAggAccumulator state #7385

Merged

jayzhan211 force-pushed the unnest_array branch from 7c56682 to cfcb8b3 Compare August 28, 2023 13:04

jayzhan211 mentioned this pull request Sep 1, 2023

Implement flatten for MakeArray #7461

Closed

jayzhan211 marked this pull request as ready for review September 12, 2023 12:52

jayzhan211 force-pushed the unnest_array branch from 72273a5 to 907cc71 Compare October 5, 2023 00:08

jayzhan211 requested a review from alamb October 5, 2023 00:09

jayzhan211 commented Oct 5, 2023

View reviewed changes

jayzhan211 added 4 commits October 9, 2023 19:59

rebase with squash

c321084

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

rebase with squash

388d619

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

fix rebase error

cf685a8

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

update flatten

199f770

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

jayzhan211 force-pushed the unnest_array branch from 907cc71 to 199f770 Compare October 9, 2023 12:01

alamb marked this pull request as draft October 30, 2023 20:31

jayzhan211 mentioned this pull request Jan 30, 2024

Unnest with single expression #9069

Merged

jayzhan211 mentioned this pull request Feb 26, 2024

Unnest function doesn't work as expected with additional columns #9349

Closed

jonahgao mentioned this pull request Apr 11, 2024

feat: support unnest multiple arrays #10044

Merged

alamb closed this in #10044 Apr 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement basic unnest function #6796

Implement basic unnest function #6796

jayzhan211 commented Jun 29, 2023 •

edited

Loading

jayzhan211 commented Jun 29, 2023 •

edited

Loading

jayzhan211 commented Jun 29, 2023 •

edited

Loading

izveigor Jun 29, 2023

jayzhan211 Jun 30, 2023 •

edited

Loading

jayzhan211 Jul 2, 2023

jayzhan211 commented Jul 3, 2023

alamb commented Jul 3, 2023

jayzhan211 commented Jul 3, 2023

alamb commented Jul 3, 2023

jayzhan211 commented Jul 16, 2023

jayzhan211 commented Jul 28, 2023 •

edited

Loading

alamb left a comment

alamb Aug 7, 2023

alamb Aug 7, 2023

jayzhan211 Oct 5, 2023

alamb Aug 7, 2023

jayzhan211 commented Aug 8, 2023 •

edited

Loading

alamb commented Aug 16, 2023

jayzhan211 commented Aug 30, 2023 •

edited

Loading

jayzhan211 commented Sep 12, 2023 •

edited

Loading

alamb commented Sep 12, 2023

jayzhan211 Oct 5, 2023

alamb commented Oct 30, 2023

alamb commented Apr 8, 2024

jayzhan211 commented Apr 8, 2024

jonahgao commented Apr 9, 2024

jonahgao commented Apr 9, 2024

	_ => Err(DataFusionError::Internal(format!(
	_ => Err(DataFusionError::NotYetImplemented(format!(

Implement basic unnest function #6796

Implement basic unnest function #6796

Conversation

jayzhan211 commented Jun 29, 2023 • edited Loading

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

jayzhan211 commented Jun 29, 2023 • edited Loading

jayzhan211 commented Jun 29, 2023 • edited Loading

izveigor Jun 29, 2023

Choose a reason for hiding this comment

jayzhan211 Jun 30, 2023 • edited Loading

Choose a reason for hiding this comment

jayzhan211 Jul 2, 2023

Choose a reason for hiding this comment

jayzhan211 commented Jul 3, 2023

alamb commented Jul 3, 2023

jayzhan211 commented Jul 3, 2023

alamb commented Jul 3, 2023

jayzhan211 commented Jul 16, 2023

jayzhan211 commented Jul 28, 2023 • edited Loading

alamb left a comment

Choose a reason for hiding this comment

alamb Aug 7, 2023

Choose a reason for hiding this comment

alamb Aug 7, 2023

Choose a reason for hiding this comment

jayzhan211 Oct 5, 2023

Choose a reason for hiding this comment

alamb Aug 7, 2023

Choose a reason for hiding this comment

jayzhan211 commented Aug 8, 2023 • edited Loading

alamb commented Aug 16, 2023

jayzhan211 commented Aug 30, 2023 • edited Loading

jayzhan211 commented Sep 12, 2023 • edited Loading

alamb commented Sep 12, 2023

jayzhan211 Oct 5, 2023

Choose a reason for hiding this comment

alamb commented Oct 30, 2023

alamb commented Apr 8, 2024

jayzhan211 commented Apr 8, 2024

jonahgao commented Apr 9, 2024

jonahgao commented Apr 9, 2024

jayzhan211 commented Jun 29, 2023 •

edited

Loading

jayzhan211 commented Jun 29, 2023 •

edited

Loading

jayzhan211 commented Jun 29, 2023 •

edited

Loading

jayzhan211 Jun 30, 2023 •

edited

Loading

jayzhan211 commented Jul 28, 2023 •

edited

Loading

jayzhan211 commented Aug 8, 2023 •

edited

Loading

jayzhan211 commented Aug 30, 2023 •

edited

Loading

jayzhan211 commented Sep 12, 2023 •

edited

Loading