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

[COPY CSV] Enable TIMESTAMP_TZ formats #11711

Merged
merged 12 commits into from
Apr 18, 2024
Merged

Conversation

Tishj
Copy link
Contributor

@Tishj Tishj commented Apr 18, 2024

This PR fixes #11660

Instead of hardcoding the conversion, we now use our existing Expression framework to use the Catalog to look up the right conversion.

Previously we always used the system StrfTime, but because we now use the Catalog, we can find the overload of strftime added by the ICU extension (credit to Richard for the implementation idea)

As shown by the tests, this does break some existing behavior.
If a TIMESTAMP is cast to TIMESTAMP_TZ and it is used in a COPY to csv with ICU not loaded (and not autoinstall/loadable) it will throw a binder error.

@Tishj
Copy link
Contributor Author

Tishj commented Apr 18, 2024

@Mytherin seems to be another (unrelated) http failure https://github.com/duckdb/duckdb/actions/runs/8736103221/job/23979737940?pr=11711

@Mytherin Mytherin merged commit d0d7f7f into duckdb:main Apr 18, 2024
45 checks passed
@Mytherin
Copy link
Collaborator

Thanks!

github-actions bot pushed a commit to duckdb/duckdb-r that referenced this pull request May 1, 2024
Merge pull request duckdb/duckdb#11711 from Tishj/copy_csv_cast_rework
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.

timestampformat option for csv copy ignores configured timezone with timestamptz
2 participants