lidavidm commented on code in PR #868:
URL: https://github.com/apache/arrow-adbc/pull/868#discussion_r1255902661
##########
c/driver/postgresql/statement.cc:
##########
@@ -145,6 +145,9 @@ struct BindStream {
// XXX: this assumes fixed-length fields only - will need more
// consideration to deal with variable-length fields
+ bool has_tz_field = false;
+ std::string tz_setting;
Review Comment:
agh, I was going to ask for this to be `std::optional`...but we can't do that
##########
c/driver/postgresql/statement.cc:
##########
@@ -255,6 +252,45 @@ struct BindStream {
AdbcStatusCode Prepare(PGconn* conn, const std::string& query,
struct AdbcError* error) {
+ // tz-aware timestamps require special handling to set the timezone to UTC
+ // prior to sending over the binary protocol; must be reset after execute
+ for (int64_t col = 0; col < bind_schema->n_children; col++) {
+ if ((bind_schema_fields[col].type ==
ArrowType::NANOARROW_TYPE_TIMESTAMP) &&
+ (strcmp("", bind_schema_fields[col].timezone))) {
+ has_tz_field = true;
+
+ PGresult* begin_result = PQexec(conn, "BEGIN");
Review Comment:
+how does this interact with auto-commit off?
##########
c/driver/postgresql/statement.cc:
##########
@@ -255,6 +252,45 @@ struct BindStream {
AdbcStatusCode Prepare(PGconn* conn, const std::string& query,
struct AdbcError* error) {
+ // tz-aware timestamps require special handling to set the timezone to UTC
+ // prior to sending over the binary protocol; must be reset after execute
+ for (int64_t col = 0; col < bind_schema->n_children; col++) {
+ if ((bind_schema_fields[col].type ==
ArrowType::NANOARROW_TYPE_TIMESTAMP) &&
+ (strcmp("", bind_schema_fields[col].timezone))) {
+ has_tz_field = true;
+
+ PGresult* begin_result = PQexec(conn, "BEGIN");
Review Comment:
Don't we need to hoist this out of the loop? Or else, only do this if
`has_tz_field` was previously false. Otherwise, it looks like we'll try to
begin the transaction twice.
(Also: I wonder if we shouldn't always begin/end a transaction when dealing
with multiple bind parameters...)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
