WillAyd commented on code in PR #1093:
URL: https://github.com/apache/arrow-adbc/pull/1093#discussion_r1333822111
##########
c/driver/postgresql/statement.cc:
##########
@@ -315,12 +321,13 @@ struct BindStream {
}
}
- PGresult* result = PQprepare(conn, /*stmtName=*/"", query.c_str(),
- /*nParams=*/bind_schema->n_children,
param_types.data());
- if (PQresultStatus(result) != PGRES_COMMAND_OK) {
+ const char* temp = "COPY bulk_ingest FROM STDIN WITH (FORMAT binary);";
Review Comment:
I placed this in `Prepare` because it represented a minor diff, but I'm not
sure the Prepare/Execute pattern is the same concept as a COPY. Maybe COPY
should have a dedicated ADBC method?
##########
c/driver/postgresql/statement.cc:
##########
@@ -353,10 +360,30 @@ struct BindStream {
CHECK_NA(INTERNAL, ArrowArrayViewSetArray(&array_view.value,
&array.value, nullptr),
error);
+ struct ArrowBuffer buf;
+ ArrowBufferInit(&buf);
+ ArrowBufferAppend(&buf, kPgCopyBinarySignature,
sizeof(kPgCopyBinarySignature));
Review Comment:
We could probably reserve the number of bytes needed up front
##########
c/driver/postgresql/statement.cc:
##########
@@ -500,33 +540,48 @@ struct BindStream {
std::memcpy(param_values[col] + sizeof(uint64_t), &days,
sizeof(uint32_t));
std::memcpy(param_values[col] + sizeof(uint64_t) +
sizeof(uint32_t),
&months, sizeof(uint32_t));
+ field_nbytes = 16;
break;
}
default:
SetError(error, "%s%" PRId64 "%s%s%s%s", "[libpq] Field #", col
+ 1, " ('",
bind_schema->children[col]->name,
"') has unsupported type for ingestion ",
ArrowTypeString(bind_schema_fields[col].type));
+ ArrowBufferReset(&buf);
return ADBC_STATUS_NOT_IMPLEMENTED;
}
- }
- result = PQexecPrepared(conn, /*stmtName=*/"",
- /*nParams=*/bind_schema->n_children,
param_values.data(),
- param_lengths.data(), param_formats.data(),
- /*resultFormat=*/0 /*text*/);
-
- ExecStatusType pg_status = PQresultStatus(result);
- if (pg_status != PGRES_COMMAND_OK) {
- AdbcStatusCode code = SetError(
- error, result, "[libpq] Failed to execute prepared statement: %s
%s",
- PQresStatus(pg_status), PQerrorMessage(conn));
- PQclear(result);
- return code;
+ const uint32_t net_nbytes =
ToNetworkInt32(static_cast<uint32_t>(field_nbytes));
+ ArrowBufferAppend(&buf, &net_nbytes, sizeof(net_nbytes));
+ ArrowBufferAppend(&buf, param_values[col], field_nbytes);
}
+ }
+ if (PQputCopyData(conn, reinterpret_cast<char*>(buf.data),
buf.size_bytes) <= 0) {
Review Comment:
I am not sure if the reinterpret_cast here is appropriate, or if there was a
way to construct an ArrowBufferView from the ArrowBuffer and use the union
approach
--
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]
