Skip to content

Support case sensitive column for with_column_renamed - #7063

Merged
alamb merged 6 commits into
apache:mainfrom
comphead:dev
Jul 25, 2023
Merged

alamb merged 6 commits into
apache:mainfrom
comphead:dev

Conversation

@comphead

Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

@Jefffrey appreciate if you have time to review

@Jefffrey Jefffrey left a comment

Copy link
Copy Markdown
Contributor

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?

Comment thread datafusion/core/src/dataframe.rs Outdated
@comphead

Copy link
Copy Markdown
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

alamb commented Jul 24, 2023

Copy link
Copy Markdown
Contributor

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

@alamb alamb left a comment

Copy link
Copy Markdown
Contributor

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

Comment thread datafusion/common/src/column.rs
@alamb

alamb commented Jul 25, 2023

Copy link
Copy Markdown
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

dataframe.with_column_renamed() does not rename properly

3 participants