Skip to content

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 Changes to the physical-expr crates 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
@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 Changes to the physical-expr crates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants