Repository navigation
Conversation
cd088f7 to
85f7305
Compare
| if value.tzinfo is not None: | ||
| value = value.astimezone(timezone.utc) | ||
| fmt = ( | ||
| "%Y-%m-%d %H:%M:%S.%f" if value.microsecond else "%Y-%m-%d %H:%M:%S" |
There was a problem hiding this comment.
Missed this in #527: this should only apply for FB2. Fixed here.
|
| return f"'{value.strftime(fmt)}'" | ||
| # The parameter's type preserves awareness; the server casts to the target. | ||
| sql_type = "TIMESTAMPTZ" if value.utcoffset() is not None else "TIMESTAMP" | ||
| return f"{sql_type} '{value.isoformat(sep=' ')}'" |
There was a problem hiding this comment.
A datetime parameter used in a TEXT context now fails or stores a different value. Before this change it was an untyped string literal that Firebolt coerced to whatever type the context needed. Now it is a typed TIMESTAMP/TIMESTAMPTZ, so:
WHERE text_col = ?withdatetime(2024,1,1,12)now errors:operator '=' for input types (text, timestamp) not found. Before, it matched.INSERT INTO t(text_col) VALUES (?)with an aware datetime now stores'2024-01-01 06:15:00+00', rendered in the session timezone with a+00suffix. Before, it stored'2024-01-01 06:15:00'.
I checked both on Firebolt Core. Neither case is mentioned in the compatibility note in the docs.
| return f"'{value.strftime(fmt)}'" | ||
| # The parameter's type preserves awareness; the server casts to the target. | ||
| sql_type = "TIMESTAMPTZ" if value.utcoffset() is not None else "TIMESTAMP" | ||
| return f"{sql_type} '{value.isoformat(sep=' ')}'" |
There was a problem hiding this comment.
A datetime parameter compared with a DATE column now returns different rows, with no error. Before, WHERE d = ? with datetime(2024,1,1,12,0) sent '2024-01-01 12:00:00', which Firebolt casts to DATE and truncates, so it matched rows for 2024-01-01. Now the predicate becomes cast(d as timestamp) = TIMESTAMP '2024-01-01 12:00:00', which matches nothing unless the time is midnight. On Firebolt Core, the same table returned 3 rows with the old literal and 0 with the new one. This affects range filters too, such as d >= ? and d <= ?.
| return f"'{value.strftime(fmt)}'" | ||
| # The parameter's type preserves awareness; the server casts to the target. | ||
| sql_type = "TIMESTAMPTZ" if value.utcoffset() is not None else "TIMESTAMP" | ||
| return f"{sql_type} '{value.isoformat(sep=' ')}'" |
There was a problem hiding this comment.
Aware datetimes whose UTC offset has a seconds part now fail on the server. isoformat() writes offsets like +00:19:32, and Firebolt rejects them: Unable to cast TEXT '1900-01-01 00:00:00+00:19:32' to timestamptz. Invalid timestamptz constant. Python produces these offsets for zoneinfo LMT offsets on historical dates, for example datetime(1900,1,1,tzinfo=ZoneInfo("Europe/Amsterdam")), and for any timezone(timedelta(seconds=...)). The old code converted to UTC first, so these values worked. One fix is to normalize offsets that are not whole minutes, for example with value.astimezone(timezone.utc) when value.utcoffset().seconds % 60 is non-zero, before formatting.
| ) | ||
| return f"'{value.strftime(fmt)}'" | ||
| # The parameter's type preserves awareness; the server casts to the target. | ||
| sql_type = "TIMESTAMPTZ" if value.utcoffset() is not None else "TIMESTAMP" |
There was a problem hiding this comment.
An aware datetime compared with a TIMESTAMP column now casts the column, not the literal. Firebolt Core plans WHERE ts = ? as Filter (cast(rv544.ts as timestamptz) = TIMESTAMPTZ '...'). With the old untyped literal, the plan was rv544.ts = TIMESTAMP '...'. When ts is the primary index or partition column, wrapping it in a cast per row is likely to stop index/partition pruning, so the query scans the whole table. Naive datetimes compared with DATE columns get the same plan shape (cast(d as timestamp)).
| date/time fields. Timezone-aware datetimes are bound as ``TIMESTAMPTZ`` values | ||
| with an explicit UTC offset, preserving the instant. Both retain microseconds. | ||
|
|
||
| When a timezone-aware parameter is assigned to a ``TIMESTAMP`` column, Firebolt |
There was a problem hiding this comment.
This note only covers assigning an aware parameter to a TIMESTAMP column. Reads are affected too. Older SDK versions stored aware values as UTC wall-clock time in TIMESTAMP columns. In a non-UTC session, the same aware datetime used in a filter no longer matches those rows. I reproduced it on Core with timezone=Europe/Berlin: with a stored ts = 2024-01-01 12:00:00, WHERE ts = ? with datetime(2024,1,1,12,tzinfo=timezone.utc) returned 1 row with the old SDK and returns 0 now. Please document this explicitly, since applications that set a session timezone will silently get different results from existing data. It is also worth considering whether this needs a major version bump.
This branch has not been deployed
2 participants
Timezone-aware Python datetime parameters are currently converted to UTC and then formatted without an offset. Firebolt interprets that offset-free value in the session timezone, changing the instant in non-UTC sessions. For example, 12:00+02:00 becomes '10:00'; a Europe/Berlin session interprets it as 08:00 UTC instead of 10:00 UTC.
For client-side parameters, emit typed TIMESTAMP literals for datetimes without timezone information and TIMESTAMPTZ literals with an explicit offset for timezone-aware datetimes. Preserve microseconds and let Firebolt perform destination-type conversions.
This changes timezone-aware parameters assigned to TIMESTAMP columns: Firebolt now converts them to session-local date/time fields. Document that compatibility change and the explicit casts required by fb_numeric's string-valued datetime parameters. Legacy FB1 formatting remains unchanged.
Add sync/async integration coverage for inserts, comparisons, and standalone parameter expressions across datetimes with and without timezone information, TIMESTAMP/TIMESTAMPTZ destinations, UTC/Europe-Berlin sessions, and qmark/fb_numeric styles.
Validation: 794 unit tests passed (1 skipped); 18 live Core integration tests passed on Core 4.31.13, including the existing parameterized-query regressions. Pre-commit and mypy passed. Managed integration execution is pending CI.
Note
Medium Risk
Changes how datetime query parameters are serialized for V2 connections, which can alter stored instants and comparisons for timezone-aware values unless callers adapt.
Overview
V2 client-side parameter binding now emits typed
TIMESTAMP/TIMESTAMPTZliterals from Pythondatetimevalues instead of normalizing timezone-aware values to UTC and quoting a bare timestamp string. Naive datetimes stayTIMESTAMP; aware datetimes keep their offset viaisoformat, with microseconds preserved. Firebolt 1.0 formatting is unchanged via an explicit formatterversion=1branch.This is a behavior change for timezone-aware parameters written into
TIMESTAMPcolumns (session-timezone conversion on the server) and for any app that depended on the old UTC-stripping behavior—docs call out migrating by converting to UTC and droppingtzinfomanually.fb_numericstill sends datetimes as strings; docs note requiredCAST($n AS TIMESTAMP|TIMESTAMPTZ)in SQL.Adds sync/async integration tests (session timezones, aware/naive,
qmark/fb_numeric) and expands unit tests for the new literal shapes.Reviewed by Cursor Bugbot for commit 25da462. Bugbot is set up for automated code reviews on this repo. Configure here.