paleolimbot commented on code in PR #3872:
URL: https://github.com/apache/arrow-adbc/pull/3872#discussion_r2838223112
##########
c/include/arrow-adbc/adbc.h:
##########
@@ -1612,6 +1627,24 @@ ADBC_EXPORT
AdbcStatusCode AdbcConnectionRelease(struct AdbcConnection* connection,
struct AdbcError* error);
+/// \brief Set a warning handler.
+///
+/// May be set before or after AdbcConnectionInit.
+///
+/// \since ADBC API revision 1.2.0
Review Comment:
This may be a future clarification, but as somebody setting a warning
handler it would probably be good to have some expectation about how frequently
this would be called, or perhaps some guidance as a driver author about how
frequently it should be called. For example, if this is somehow used to warn
about a lossy conversion from the internal database wire format to Arrow,
driver authors should probably just warn about the first lossy value to be
polite (this makes implementing the handler a bit easier).
It would probably be a good idea to explicitly state the thread safety of
the handler (i.e., is the expectation that this handler can be called from any
thread at any time, or is it the driver's responsibility to serialize warning
calls?). It would be easier to implement a handler if it were the driver's
responsibility to serialize calls.
##########
c/include/arrow-adbc/adbc.h:
##########
@@ -1092,6 +1090,18 @@ AdbcStatusCode AdbcMultiResultSetNextPartitions(struct
AdbcMultiResultSet* resul
struct AdbcError* error);
/// @}
+/// \brief A warning handler function.
+///
+/// The handler must not block and must not call any other ADBC functions
+/// (besides releasing the warning). The warning does not need to be released
+/// before returning.
+///
+/// \param[in] warning The warning information. The application is
+/// responsible for releasing the warning, but the warning pointer itself
+/// may not be valid after the handler returns.
+/// \param[in] user_data The user_data pointer.
+typedef void (*AdbcWarningHandler)(const struct AdbcError* warning, void*
user_data);
Review Comment:
I think this is simpler and the right way to go, but just pointing out that
it implies that the lifecycle of `user_data` must outlive the connection (i.e.,
if you want to do something like log to a file you're managing that file handle
on your own).
--
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]
