This is an automated email from the ASF dual-hosted git repository.
lidavidm pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-adbc.git
The following commit(s) were added to refs/heads/main by this push:
new a1aaac2d7 fix(c/driver/postgresql): use fmt for error messages (#4627)
a1aaac2d7 is described below
commit a1aaac2d73eaa1731d66b45a1e2e6120657c2b41
Author: Jeremy Tan <[email protected]>
AuthorDate: Thu Jul 30 10:44:46 2026 +1000
fix(c/driver/postgresql): use fmt for error messages (#4627)
I noticed that this code was basically incorrect if the msg size was
larger than the reserved size of 1024:
https://github.com/apache/arrow-adbc/blob/ca0aa5e4c49d5ea2853792a8045e3310d92c3606/c/driver/postgresql/error.cc#L103-L105
To do that properly would require resizing the buffer and rerunning
vsnprintf.
But stepping back, this function seems basically pointless as MakeStatus
is already taking format args, so have just inlined the usage of this
function with direct MakeStatus calls.
---
c/driver/postgresql/connection.cc | 5 +++--
c/driver/postgresql/error.cc | 28 ----------------------------
c/driver/postgresql/error.h | 16 ----------------
c/driver/postgresql/statement.cc | 20 ++++++++++++--------
4 files changed, 15 insertions(+), 54 deletions(-)
diff --git a/c/driver/postgresql/connection.cc
b/c/driver/postgresql/connection.cc
index 1acbf3c9b..262c047d7 100644
--- a/c/driver/postgresql/connection.cc
+++ b/c/driver/postgresql/connection.cc
@@ -488,8 +488,9 @@ AdbcStatusCode PostgresConnection::Commit(struct AdbcError*
error) {
PGresult* result = PQexec(conn_, "COMMIT");
if (PQresultStatus(result) != PGRES_COMMAND_OK) {
- AdbcStatusCode code = SetError(error, result, "%s%s",
- "[libpq] Failed to commit: ",
PQerrorMessage(conn_));
+ AdbcStatusCode code =
+ MakeStatus(result, "[libpq] Failed to commit: {}",
PQerrorMessage(conn_))
+ .ToAdbc(error);
PQclear(result);
return code;
}
diff --git a/c/driver/postgresql/error.cc b/c/driver/postgresql/error.cc
index 2bd78d89e..e62be3cb5 100644
--- a/c/driver/postgresql/error.cc
+++ b/c/driver/postgresql/error.cc
@@ -17,12 +17,7 @@
#include "error.h"
-#include <stdarg.h>
-#include <cstdio>
-#include <cstring>
-#include <string>
#include <string_view>
-#include <vector>
#include <libpq-fe.h>
@@ -86,27 +81,4 @@ AdbcStatusCode ClassifySqlState(const char* sqlstate) {
return ADBC_STATUS_IO;
}
-AdbcStatusCode SetError(struct AdbcError* error, PGresult* result, const char*
format,
- ...) {
- if (error && error->release) {
- // TODO: combine the errors if possible
- error->release(error);
- }
-
- va_list args;
- va_start(args, format);
- std::string message;
- message.resize(1024);
- int chars_needed = vsnprintf(message.data(), message.size(), format, args);
- va_end(args);
-
- if (chars_needed > 0) {
- message.resize(chars_needed);
- } else {
- message.resize(0);
- }
-
- return MakeStatus(result, "{}", message).ToAdbc(error);
-}
-
} // namespace adbcpq
diff --git a/c/driver/postgresql/error.h b/c/driver/postgresql/error.h
index 07bb9f940..9dc850c85 100644
--- a/c/driver/postgresql/error.h
+++ b/c/driver/postgresql/error.h
@@ -57,22 +57,6 @@ static const std::vector<DetailField> kDetailFields = {
{PG_DIAG_TABLE_NAME, "PG_DIAG_TABLE_NAME"},
};
-// The printf checking attribute doesn't work properly on gcc 4.8
-// and results in spurious compiler warnings
-#if defined(__clang__) || (defined(__GNUC__) && __GNUC__ >= 5)
-#define ADBC_CHECK_PRINTF_ATTRIBUTE(x, y) __attribute__((format(printf, x, y)))
-#else
-#define ADBC_CHECK_PRINTF_ATTRIBUTE(x, y)
-#endif
-
-/// \brief Set an error based on a PGresult, inferring the proper ADBC status
-/// code from the PGresult. Deprecated and is currently a thin wrapper around
-/// MakeStatus() below.
-AdbcStatusCode SetError(struct AdbcError* error, PGresult* result, const char*
format,
- ...) ADBC_CHECK_PRINTF_ATTRIBUTE(3, 4);
-
-#undef ADBC_CHECK_PRINTF_ATTRIBUTE
-
AdbcStatusCode ClassifySqlState(const char* sqlstate);
template <typename... Args>
diff --git a/c/driver/postgresql/statement.cc b/c/driver/postgresql/statement.cc
index 3ea2acdd1..37e61979c 100644
--- a/c/driver/postgresql/statement.cc
+++ b/c/driver/postgresql/statement.cc
@@ -97,8 +97,9 @@ int TupleReader::GetCopyData() {
result_ = PQgetResult(conn_);
const ExecStatusType pq_status = PQresultStatus(result_);
if (pq_status != PGRES_COMMAND_OK) {
- status_ = SetError(&error_, result_, "[libpq] Execution error [%s]: %s",
- PQresStatus(pq_status),
PQresultErrorMessage(result_));
+ status_ = MakeStatus(result_, "[libpq] Execution error [{}]: {}",
+ PQresStatus(pq_status),
PQresultErrorMessage(result_))
+ .ToAdbc(&error_);
return InternalAdbcStatusCodeToErrno(status_);
} else {
return ENODATA;
@@ -441,8 +442,9 @@ AdbcStatusCode PostgresStatement::CreateBulkTable(const
std::string& current_sch
/*resultFormat=*/1 /*(binary)*/);
if (PQresultStatus(result) != PGRES_COMMAND_OK) {
AdbcStatusCode code =
- SetError(error, result, "[libpq] Failed to drop table: %s\nQuery
was: %s",
- PQerrorMessage(conn), drop.c_str());
+ MakeStatus(result, "[libpq] Failed to drop table: {}\nQuery was:
{}",
+ PQerrorMessage(conn), drop)
+ .ToAdbc(error);
PQclear(result);
return code;
}
@@ -503,8 +505,9 @@ AdbcStatusCode PostgresStatement::CreateBulkTable(const
std::string& current_sch
/*resultFormat=*/1 /*(binary)*/);
if (PQresultStatus(result) != PGRES_COMMAND_OK) {
AdbcStatusCode code =
- SetError(error, result, "[libpq] Failed to create table: %s\nQuery
was: %s",
- PQerrorMessage(conn), create.c_str());
+ MakeStatus(result, "[libpq] Failed to create table: {}\nQuery was: {}",
+ PQerrorMessage(conn), create)
+ .ToAdbc(error);
PQclear(result);
return code;
}
@@ -755,8 +758,9 @@ AdbcStatusCode PostgresStatement::ExecuteIngest(struct
ArrowArrayStream* stream,
PGresult* result = PQexec(connection_->conn(), query.c_str());
if (PQresultStatus(result) != PGRES_COPY_IN) {
AdbcStatusCode code =
- SetError(error, result, "[libpq] COPY query failed: %s\nQuery was:%s",
- PQerrorMessage(connection_->conn()), query.c_str());
+ MakeStatus(result, "[libpq] COPY query failed: {}\nQuery was:{}",
+ PQerrorMessage(connection_->conn()), query)
+ .ToAdbc(error);
PQclear(result);
return code;
}
