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

Split the GroupColumn Implementations into smaller modules #13352

Merged
merged 1 commit into from
Nov 13, 2024

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 Physical Expressions 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 Physical Expressions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reorganize the GroupColumn implementations into more coherent modules
3 participants