Skip to content

Conversation

@J-Meyers
Copy link
Contributor

@J-Meyers J-Meyers commented Sep 5, 2025

Copy link
Contributor

@dsevilla dsevilla 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 to me taking into account the usage of double quotes for names in DuckDB.

@soren-petersen
Copy link

@J-Meyers : Thanks for helping fix this! ❤️

@soren-petersen
Copy link

I may be wrong, but is seems the patch will not work if the column name contains a double quote.

I suspect changing the following line:

cols = ",".join(f'"{col}"' for col in with_columns)

to

cols = ",".join(f'"{col.replace('\"', '\"\"')}"' for col in with_columns)

would fix this.

@J-Meyers
Copy link
Contributor Author

J-Meyers commented Sep 8, 2025

I may be wrong, but is seems the patch will not work if the column name contains a double quote.

I suspect changing the following line:

cols = ",".join(f'"{col}"' for col in with_columns)

to

cols = ",".join(f'"{col.replace('\"', '\"\"')}"' for col in with_columns)

would fix this.

@soren-petersen
Good catch, I added a test and fixed the handling of those: d1b01d8
Switched to an explicit function because python was complaining about backslashes within fstrings

@evertlammerts evertlammerts changed the base branch from v1.4-andium to main September 8, 2025 19:18
@evertlammerts evertlammerts changed the base branch from main to v1.4-andium September 8, 2025 19:18
@evertlammerts
Copy link
Collaborator

Looks good, thanks for the fix! FYI I switched the base to main and back to trigger the PR workflow, which had a bug. If the tests succeed I'll merge tomorrow and this will go into 1.4.0 (planned for Wed).

@evertlammerts evertlammerts merged commit c7b7a88 into duckdb:v1.4-andium Sep 9, 2025
33 checks passed
@J-Meyers J-Meyers deleted the column_with_space_polars branch September 9, 2025 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants