felipecrv commented on code in PR #3871:
URL: https://github.com/apache/arrow-adbc/pull/3871#discussion_r2715044853
##########
c/include/arrow-adbc/adbc.h:
##########
@@ -973,6 +991,113 @@ struct AdbcPartitions {
/// @}
+/// \defgroup adbc-statement-multi Multiple Result Set Execution
+/// Some databases support executing a statement that returns multiple
+/// result sets. This section defines the API for working with such
+/// statements and result sets.
+/// @{
+
+/// \brief A struct for handling a potentially multi-result set execution
+///
+/// This struct is populated by AdbcStatementExecuteMulti and can be used to
iterate
+/// through the result sets of the execution. The caller can use the
NextResultSet or
+/// NextResultSetPartitions functions on the AdbcResultSet struct to iterate
through the
+/// result sets. The caller is responsible for calling the release function
when finished
+/// with the result set.
+///
+/// \since ADBC API revision 1.2.0
+struct ADBC_EXPORT AdbcResultSet {
+ /// \brief opaque implementation-defined state
+ void* private_data;
+
+ /// \brief The associated driver
+ struct AdbcDriver* private_driver;
+};
+
+/// \brief Release the AdbcResultSet and any associated resources.
+///
+/// \since ADBC API revision 1.2.0
+///
+/// If all the result sets have not been completely consumed, then the driver
+/// should cancel any remaining work if this is called.
+///
+/// \param[in] result_set The result set to release.
+/// \param[out] error An optional location to return an error message if
necessary.
+///
+/// \return ADBC_STATUS_OK on success or an appropriate error code.
+AdbcStatusCode AdbcResultSetRelease(struct AdbcResultSet* result_set,
+ struct AdbcError* error);
+
+/// \brief Get the next result set from a multi-result-set execution.
Review Comment:
What you mean here is `Get the next *ArrowArrayStream* ( our implementation
of "result set") from a multi-result-set execution result that we call
ResultSet", am I right?
##########
c/include/arrow-adbc/adbc.h:
##########
@@ -973,6 +991,113 @@ struct AdbcPartitions {
/// @}
+/// \defgroup adbc-statement-multi Multiple Result Set Execution
+/// Some databases support executing a statement that returns multiple
+/// result sets. This section defines the API for working with such
+/// statements and result sets.
+/// @{
+
+/// \brief A struct for handling a potentially multi-result set execution
+///
+/// This struct is populated by AdbcStatementExecuteMulti and can be used to
iterate
+/// through the result sets of the execution. The caller can use the
NextResultSet or
+/// NextResultSetPartitions functions on the AdbcResultSet struct to iterate
through the
+/// result sets. The caller is responsible for calling the release function
when finished
+/// with the result set.
+///
+/// \since ADBC API revision 1.2.0
+struct ADBC_EXPORT AdbcResultSet {
Review Comment:
Shouldn't this be called `AdbcMultiResultSet`? Very famous database APIs
like Java's [1] call `ResultSet` the set of ROWS returned after the execution
of a single query/statement. Whereas here, AFAIU, this is supposed to be the
thing that gives me different "results sets" (implemented as
`ArrowArrayStream`).
By introducing `struct AdbcResultSet` for this use-case you might confuse
people expecting this to be the equivalent of a classic database "result set".
[1] https://docs.oracle.com/javase/8/docs/api/java/sql/ResultSet.html
##########
c/include/arrow-adbc/adbc.h:
##########
@@ -973,6 +991,113 @@ struct AdbcPartitions {
/// @}
+/// \defgroup adbc-statement-multi Multiple Result Set Execution
+/// Some databases support executing a statement that returns multiple
+/// result sets. This section defines the API for working with such
+/// statements and result sets.
+/// @{
+
+/// \brief A struct for handling a potentially multi-result set execution
+///
+/// This struct is populated by AdbcStatementExecuteMulti and can be used to
iterate
+/// through the result sets of the execution. The caller can use the
NextResultSet or
+/// NextResultSetPartitions functions on the AdbcResultSet struct to iterate
through the
+/// result sets. The caller is responsible for calling the release function
when finished
+/// with the result set.
+///
+/// \since ADBC API revision 1.2.0
+struct ADBC_EXPORT AdbcResultSet {
+ /// \brief opaque implementation-defined state
+ void* private_data;
+
+ /// \brief The associated driver
+ struct AdbcDriver* private_driver;
+};
+
+/// \brief Release the AdbcResultSet and any associated resources.
+///
+/// \since ADBC API revision 1.2.0
+///
+/// If all the result sets have not been completely consumed, then the driver
+/// should cancel any remaining work if this is called.
+///
+/// \param[in] result_set The result set to release.
+/// \param[out] error An optional location to return an error message if
necessary.
+///
+/// \return ADBC_STATUS_OK on success or an appropriate error code.
+AdbcStatusCode AdbcResultSetRelease(struct AdbcResultSet* result_set,
+ struct AdbcError* error);
+
+/// \brief Get the next result set from a multi-result-set execution.
+///
+/// \since ADBC API revision 1.2.0
+///
+/// The AdbcResultSet must outlive the returned ArrowArrayStream.
Review Comment:
Otherwise what? The `ArrowArrayStream` returns cancellation errors if polled
again?
##########
c/include/arrow-adbc/adbc.h:
##########
@@ -1135,6 +1261,40 @@ struct ADBC_EXPORT AdbcDriver {
struct AdbcError*);
/// @}
+
+ /// \defgroup adbc-1.2.0 ADBC API Revision 1.2.0
+ ///
+ /// Functions added in ADBC 1.2.0. For backwards compatibility,
+ /// these members must not be accessed unless the version passed to
+ /// the AdbcDriverInitFunc is greater than or equal to
+ /// ADBC_VERSION_1_2_0.
+ ///
+ /// When a driver implementing an older spec is loaded by a newer
+ /// driver manager, the newer manager will allocate the new, expanded
+ /// AdbcDriver struct and attempt to have the driver initialize it with
+ /// the newer version. This must return an error, after which the driver
+ /// will try again with successively older versions all the way back to
+ /// ADBC_VERSION_1_0_0. The driver must not access the new fields,
+ /// which will carry undefined values.
Review Comment:
Shouldn't the manager populate them with functions that fail immediately?
##########
c/include/arrow-adbc/adbc.h:
##########
@@ -973,6 +991,113 @@ struct AdbcPartitions {
/// @}
+/// \defgroup adbc-statement-multi Multiple Result Set Execution
+/// Some databases support executing a statement that returns multiple
+/// result sets. This section defines the API for working with such
+/// statements and result sets.
+/// @{
+
+/// \brief A struct for handling a potentially multi-result set execution
+///
+/// This struct is populated by AdbcStatementExecuteMulti and can be used to
iterate
+/// through the result sets of the execution. The caller can use the
NextResultSet or
+/// NextResultSetPartitions functions on the AdbcResultSet struct to iterate
through the
+/// result sets. The caller is responsible for calling the release function
when finished
+/// with the result set.
+///
+/// \since ADBC API revision 1.2.0
+struct ADBC_EXPORT AdbcResultSet {
+ /// \brief opaque implementation-defined state
+ void* private_data;
+
+ /// \brief The associated driver
+ struct AdbcDriver* private_driver;
+};
+
+/// \brief Release the AdbcResultSet and any associated resources.
+///
+/// \since ADBC API revision 1.2.0
+///
+/// If all the result sets have not been completely consumed, then the driver
+/// should cancel any remaining work if this is called.
Review Comment:
To be non error-prone, wrapper APIs will have to keep a strong reference to
the `AdbcResultSet *` alive inside every Arrow stream produced by this
`AdbcResultSet`. Do you agree? If that is the case, it would be better to
handle the ref-counting inside the driver IMO.
--
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]
