Skip to content

Conversation

@jiashenC
Copy link
Contributor

Which issue does this PR close?

Closes #13262.

What changes are included in this PR?

  • Introduce a new sub-directory: multi_column under group_value.
  • Rename column.rs.
  • Move implementation in group_column.rs to primitive.rs, bytes.rs, and bytes_view.rs.

Are these changes tested?

Using original tests.

Are there any user-facing changes?

No

@github-actions github-actions bot added the physical-expr Changes to the physical-expr crates label Nov 11, 2024
@jiashenC
Copy link
Contributor Author

@alamb Can you please take a look at this PR? Thanks!

Copy link
Contributor

@jonathanc-n jonathanc-n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This lgtm! Thanks @jiashenC. After this gets merged we can maybe open a pr for #13263

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great -- thank you @jiashenC and @jonathanc-n

@alamb alamb changed the title Reorganize the GroupColumn Implemenations Split the GroupColumn Implementations into smaller modules Nov 13, 2024
@alamb
Copy link
Contributor

alamb commented Nov 13, 2024

@alamb Can you please take a look at this PR? Thanks!

Thank you very much for this PR -- I am sorry for the delay in reviewing (I was away for a few days).

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @jiashenC and @jonathanc-n -- this looks great to me

@alamb alamb merged commit f894c7d into apache:main Nov 13, 2024
26 checks passed
alamb pushed a commit to alamb/datafusion that referenced this pull request Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

physical-expr Changes to the physical-expr crates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reorganize the GroupColumn implementations into more coherent modules

3 participants