Support fetching TIMESTAMPTZ as SQL_TYPE_TIMESTAMP #127
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
staticlibs
force-pushed
the
timestamp_tz_fetch
branch
from
37198b1 to
98b1ff6
Compare
This change implements support for returning variable-length data to client with multiple subsequent calls to `SQLGetData`. The ODBC spec logic for buffer/write/return sizing is relatively straightforward, but there is a number of corner cases. Also the logic for char, widechar and binary data differs slightly (different null-termination handling). This change unifies this logic for all variable-length types in one place. Testing: new test is added that covers retrieving data by parts for char, widechar and binary data. PyODBC test suite is extended to check the retrieval of overly long strings and also enabled on CI on Windows (to check the retrieval through the Windows Driver Manager). Fixes: duckdb#121
staticlibs
force-pushed
the
timestamp_tz_fetch
branch
from
98b1ff6 to
8dbf0ac
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
DuckDB supports
TIMESTAMP WITH TIME ZONEdata type that stores UTC dates (with specified time zone already applied on DB insert). ODBC does not have a notion of time zones. When client app reads suchTIMESTAMP WITH TIME ZONEit needs to be converted intoTIMESTAMP WITHOUT TIME ZONEwith client-local time zone applied. And such converted timestamp must represent the same moment of time as stored UTC timestamp.Applying client-local time zone to timestamp is a non-trivial operation
There are two options to get the effective offset for the specified UTC timestamp in the specified time zone:
Variant with ICU extension appeared to be problematic, as we would like to not link neither to ICU extension (C++ API, not in DuckDB C API) nor to ICU itself (C API exists, but symbols include ICU major version that can change). We can do ICU call from ODBC by creating SQL string and executing it. But this conversion must be performed for every value in every row. And there no reasonbly-easy ways to cache the results. Thus the idea of SQL calls was rejected.
Variant with OS API matches the approach used in JDBC (see duckdb/duckdb-java#166) where JVM-configured time zone is used (invariant 1 there).
The problem with accessing OS API is that
<ctime>utilities are not thread-safe untilC++20. Thus Windows<timezoneapi.h>and POSIX<time.h>API are used directly. And<ctime>approach is used in tests for cross-checking.Testing: new test added that fetches
TIMESTAMP WITH TIME ZONEvalue asSQL_C_TYPE_TIMESTAMPand checks that it is shifted correctly.