SHIVANSH-ux-ys commented on PR #51133:
URL: https://github.com/apache/arrow/pull/51133#issuecomment-5647545135
Hi @ennuite!
Thanks for the feedback and guidance!
To answer your questions directly:
1. **Goal of these tests & protocol alignment**:
The Flight SQL protocol update adds an optional `is_update` boolean field
(#4) to `ActionCreatePreparedStatementResult`. Because existing servers
deployed in the wild will not populate this field, the client must treat it as
optional (`std::optional<bool>`) rather than assuming a default `false`.
The unit tests verify:
- For query prepared statements (`SELECT`), `is_update()` returns
`std::optional<bool>(false)`.
- For modification prepared statements (`INSERT`/`UPDATE`), `is_update()`
returns `std::optional<bool>(true)`.
- For backwards compatibility, when a server leaves `is_update` unset (or
for custom/legacy servers), `ActionCreatePreparedStatementResult::is_update` is
`std::nullopt`, omitted from serialization, and
`PreparedStatement::is_update()` evaluates to `std::nullopt`.
I just pushed commit `fb2cd35` which cleans up the unit test in
`server_test.cc` (`TestCommandPreparedStatementUnsetIsUpdate`) so it compiles
cleanly and directly verifies this unset behavior.
2. **Arrow-Go PR reference**:
Yes! The Go Flight SQL client (apache/arrow-go#176 / apache/arrow#43414)
represents this via `(bool, bool)` / optional semantics to distinguish between
"field not provided by server" vs "explicitly set". This C++ PR brings the C++
`FlightSqlClient` API to parity with that same protocol specification.
Appreciate your guidance on my first C++ Flight SQL contribution! Ready for
re-review whenever convenient.
--
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]
