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.

Sign up for GitHub

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

Jump to bottom

Support case sensitive column for with_column_renamed #7063

Merged
merged 6 commits into from
Jul 25, 2023
Merged

Support case sensitive column for with_column_renamed #7063

merged 6 commits into from
Jul 25, 2023

Conversation

comphead
Copy link
Contributor

Which issue does this PR close?

Closes #6845.

Rationale for this change

Support case sensitive column renaming for dataframe API

What changes are included in this PR?

Are these changes tested?

Yes

Are there any user-facing changes?

@github-actions github-actions bot added the core Core DataFusion crate label Jul 24, 2023
@comphead
Copy link
Contributor Author

@Jefffrey appreciate if you have time to review

Jefffrey
Jefffrey reviewed Jul 24, 2023
View reviewed changes
Copy link
Contributor

@Jefffrey Jefffrey left a comment

Choose a reason for hiding this comment

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

Looks good, with a minor nitpick

Though I'm still unsure on using the enable_ident_normalization considering its technically a SQL Parser config, could be confusing for users as if using DataFrame API they might not expect such a config to affect them?

datafusion/core/src/dataframe.rs Outdated Show resolved Hide resolved
@comphead
Copy link
Contributor Author

Looks good, with a minor nitpick

Though I'm still unsure on using the enable_ident_normalization considering its technically a SQL Parser config, could be confusing for users as if using DataFrame API they might not expect such a config to affect them?

Changed the documentation to describe the change for users

@alamb
Copy link
Contributor

alamb commented Jul 24, 2023

CI is failing on master for reasons seemingly unrelated to this PR #7078

alamb
alamb approved these changes Jul 24, 2023
View reviewed changes
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 @comphead and @Jefffrey -- looks good to me. I plan to merge this tomorrow once we have sorted out why CI is failing on master

datafusion/common/src/column.rs Show resolved Hide resolved
@alamb
Copy link
Contributor

alamb commented Jul 25, 2023

I merged this branch to main to pick up the fix for #7078. I plan to merge this PR in when CI passes

@alamb alamb merged commit 8650240 into apache:main Jul 25, 2023
20 checks passed
@comphead comphead mentioned this pull request Jan 9, 2024
Closed
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jefffrey Jefffrey Jefffrey left review comments

@alamb alamb alamb approved these changes

Assignees
No one assigned
Labels
core Core DataFusion crate
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

dataframe.with_column_renamed() does not rename properly
3 participants
@comphead @alamb @Jefffrey