feat: Add support for date.to_text for BigQuery#5712
Conversation
| use crate::{Error, Result}; | ||
|
|
||
| /// Convert a chrono format `Item` back to its strftime string representation. | ||
| fn chrono_item_to_strftime(item: &Item) -> String { |
There was a problem hiding this comment.
This is used in the BigQuery error message to make it easy to see exactly which part of the format is breaking the query:
Error:
╭─[ :6:33 ]
│
6 │ d_str = (d | date.to_text "%-m/%d/%Y")
│ ─────┬─────
│ ╰─────── format specifier `%-m` is not supported for BigQuery
───╯
otherwise it wouldn't be clear if the culprit was %-m, %d, or %Y.
I have only added this to the BigQuery error message to keep the changes in this PR to a minimum. More than happy to make this change for all dialects if you're ok with this.
segv
force-pushed
the
to-text-for-bigquery
branch
from
7513e68 to
9e97e8c
Compare
segv
force-pushed
the
to-text-for-bigquery
branch
from
9e97e8c to
3a4d00c
Compare
| // -- chrono_item_to_strftime tests -- | ||
|
|
||
| #[test] | ||
| fn chrono_item_to_strftime_numerics_zero_pad() { |
There was a problem hiding this comment.
I am not convinced these tests add a lot of value and personally I would drop 90% of them and just leave a few in as spot checks.
However the coverage tool wants high coverage, so I've just gone ahead and covered every code path. Happy to drop this if that's ok.
segv
force-pushed
the
to-text-for-bigquery
branch
from
78f6054 to
e33e4ba
Compare
|
thank you @segv ! overall this seems very reasonable. no stress on the tests; I agree re the narrow focus on coverage. but no harm in them either. |
2 participants
No description provided.