[unparser] Custom timestamp format for DuckDB #17653
Conversation
alamb
changed the title
|
@phillipleblanc or @sgrebnov could you help review this PR? |
| @@ -50,6 +50,7 @@ recursive_protection = ["dep:recursive"] | |||
| [dependencies] | |||
| arrow = { workspace = true } | |||
| bigdecimal = { workspace = true } | |||
| chrono = { workspace = true } | |||
There was a problem hiding this comment.
Why is dependency required?
There was a problem hiding this comment.
Because I needed to import it in datafusion-sql:
https://github.com/apache/datafusion/pull/17653/files#diff-305e66c0cc21e69a7c113f9f404dce0ee7be8b45f923177893965ec64310a183R26
|
The change makes sense to me/ looks good 👍 I would ideally like to have this new override method be more generic and support more types (at least timestamp w/o tz), but I synched w/ @krinart on this and we have not found any good/elegant solution for this. Thus current implementation looks good to me. It also seems something we can potentially try to improve in DuckDB as well 😉 |
|
Thank you @krinart @sgrebnov and @phillipleblanc ! |
Which issue does this PR close?
Required for spiceai/spiceai#7055
Rationale for this change
Most databases use following format for timestamps :
2025-09-15 11:00:00 +00:00But duckdb fails due to extra space:
What changes are included in this PR?
Dialectwithtimestamp_with_tz_to_stringand default implementationtimestamp_with_tz_to_stringimplementation forDuckDBDialectAre these changes tested?
Manually + unit tests
Are there any user-facing changes?
No