Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove RowAccumulators and datafusion-row #6968

Merged
merged 4 commits into from
Jul 20, 2023

Conversation

alamb
Copy link
Contributor

@alamb alamb commented Jul 14, 2023

Which issue does this PR close?

Part of #6798
Part of #6889

Rationale for this change

The new grouping introduced in #6904 is faster than RowAccumulators and thus we do not need RowAccumulator code anymore

The RowAccumulator were the last user of the datafusion-row crate -- we now fully use the upstream arrow_row crate https://docs.rs/arrow-row/43.0.0/arrow_row/index.html

What changes are included in this PR?

  1. Remove RowAccumulators
  2. Remove datafusion-row crate

Are these changes tested?

existing coverage

Are there any user-facing changes?

No

@github-actions github-actions bot added physical-expr Physical Expressions core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) labels Jul 14, 2023
@alamb alamb force-pushed the alamb/remove_row_accumulator branch from 50ea550 to f871f56 Compare July 14, 2023 19:34
@alamb alamb changed the title Remove RowAccumulators Remove RowAccumulators and datafusion-row Jul 14, 2023
@alamb alamb force-pushed the alamb/remove_row_accumulator branch from d3f441c to 22370ea Compare July 19, 2023 11:00
@github-actions github-actions bot removed the sqllogictest SQL Logic Tests (.slt) label Jul 19, 2023
@alamb alamb marked this pull request as ready for review July 19, 2023 11:01
@alamb
Copy link
Contributor Author

alamb commented Jul 19, 2023

FYI @yjshen and @yahoNanJing -- I plan to merge this in a few days unless I hear otherwise

@yahoNanJing
Copy link
Contributor

I'm free to remove this. However, I hope to implement multiple row format in arrow-row:

  • For sorting
  • For just comparison used in the aggregation operator
    • Extract the null bit indicator together to the frontend for both variable width and fixed width

@alamb alamb merged commit 7e2cca8 into apache:main Jul 20, 2023
20 checks passed
@alamb alamb deleted the alamb/remove_row_accumulator branch July 20, 2023 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core DataFusion crate physical-expr Physical Expressions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants