[GitHub] [arrow-adbc] lidavidm commented on issue #55: [Format] Minor gaps with existing APIs

2022-08-11 Thread GitBox


lidavidm commented on issue #55:
URL: https://github.com/apache/arrow-adbc/issues/55#issuecomment-1212119635

   > Is the idea that `RowCount` would do double duty as the number of rows in 
a result set OR the number of rows affected by an update/insert?
   
   Yeah, I don't see a reason to have two separate methods.
   
   > 
   > Given the lack of reliable support, i'm fine with leaving the 
LastInsertedID out for now.
   
   Also, I would argue these sorts of use cases are mostly out of scope, though 
that's mostly my assumption.
   
   > 
   > > Returning strings from a C API is a bit annoying
   > 
   > Yea... I personally prefer the ODBC-style of passing a caller allocated 
buffer and length. But this works best when the caller can have a semblance or 
idea of how big of a buffer to allocate in the first place, so we'd have to 
either come up with a standard or some function to retrieve expected lengths of 
particular attributes so the caller can know how much to allocate, it might be 
simpler to go with the `AdbcBlob` approach you have here?
   
   It's also a little more consistent with AdbcError and the C Data Interface. 
But yeah, I don't think either is clearly better than the other?
   


-- 
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: issues-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [arrow-adbc] lidavidm commented on issue #55: [Format] Minor gaps with existing APIs

2022-08-10 Thread GitBox


lidavidm commented on issue #55:
URL: https://github.com/apache/arrow-adbc/issues/55#issuecomment-1211236715

   Returning strings from a C API is a bit annoying and I'm not sure whether 
this is preferable, or if we want to go with an ODBC-style API (pass a 
caller-allocated buffer and length and have the caller grow the buffer if too 
small)


-- 
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: issues-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [arrow-adbc] lidavidm commented on issue #55: [Format] Minor gaps with existing APIs

2022-08-10 Thread GitBox


lidavidm commented on issue #55:
URL: https://github.com/apache/arrow-adbc/issues/55#issuecomment-1211235028

   Punting on paramstyle and last inserted ID, but adding row count and current 
catalog:
   
   ```diff
   commit 50b2e40d727c0a51029d7f5506c0696b3a19a3b9
   Author: David Li 
   Date:   Wed Aug 10 16:29:27 2022 -0400
   
   [Format][C][Java] Add methods to get row count, current catalog
   
   diff --git a/adbc.h b/adbc.h
   index e7d9d51..5535193 100644
   --- a/adbc.h
   +++ b/adbc.h
   @@ -248,6 +248,18 @@ struct ADBC_EXPORT AdbcError {
  void (*release)(struct AdbcError* error);
};

   +/// \brief A driver-allocated blob.
   +struct ADBC_EXPORT AdbcBlob {
   +  char* value;
   +  size_t length;
   +
   +  /// \brief Release the contained error.
   +  ///
   +  /// Unlike other structures, this is an embedded callback to make it
   +  /// easier for the driver manager and driver to cooperate.
   +  void (*release)(struct AdbcError* error);
   +};
   +
/// }@

/// \brief Canonical option value for enabling an option.
   @@ -342,6 +354,21 @@ ADBC_EXPORT
AdbcStatusCode AdbcConnectionRelease(struct AdbcConnection* connection,
 struct AdbcError* error);

   +/// \defgroup adbc-connection-attributes Connection Attributes
   +/// Functions for retrieving metadata about the connection.
   +///
   +/// @{
   +
   +/// \brief Get the value of a string attribute.
   +ADBC_EXPORT
   +AdbcStatusCode AdbcConnectionGetAttr(struct AdbcConnection* connection, 
const char* key,
   + struct AdbcBlob* blob, struct 
AdbcError* error);
   +
   +/// \brief The name of the attribute for the current catalog (database).
   +#define ADBC_CONNECTION_ATTR_CURRENT_CATALOG 
"adbc.connection.current_catalog"
   +
   +/// }@
   +
/// \defgroup adbc-connection-metadata Metadata
/// Functions for retrieving metadata about the database.
///
   @@ -746,6 +773,26 @@ AdbcStatusCode AdbcStatementBindStream(struct 
AdbcStatement* statement,
   struct ArrowArrayStream* values,
   struct AdbcError* error);

   +/// \brief Get the number of rows affected by the most recent query.
   +///
   +/// Not supported by all drivers. If the query returns a result set,
   +/// this returns (if possible) the number of rows in the result
   +/// set. Else, this returns (if possible) the number of rows
   +/// inserted/updated.
   +///
   +/// This method can be called only after AdbcStatementExecute.  It may
   +/// not be called if any of the partitioning methods have been called
   +/// (see below).
   +///
   +/// \param[in] statement The statement.
   +/// \param[out] row_count The count of rows affected, or -1 if not
   +///   known.
   +/// \param[out] error An optional location to return an error message
   +///   if necessary.
   +ADBC_EXPORT
   +AdbcStatusCode AdbcStatementGetRowCount(struct AdbcStatement* statement,
   +int64_t* row_count, struct 
AdbcError* error);
   +
/// \brief Read the result of a statement.
///
/// This method can be called only once per execution of the
   @@ -882,6 +929,8 @@ struct ADBC_EXPORT AdbcDriver {
  AdbcStatusCode (*DatabaseInit)(struct AdbcDatabase*, struct AdbcError*);
  AdbcStatusCode (*DatabaseRelease)(struct AdbcDatabase*, struct 
AdbcError*);

   +  AdbcStatusCode (*AdbcConnectionGetAttr)(struct AdbcConnection*, const 
char*,
   +  struct AdbcBlob*, struct 
AdbcError*);
  AdbcStatusCode (*ConnectionGetInfo)(struct AdbcConnection*, uint32_t*, 
size_t,
  struct AdbcStatement*, struct 
AdbcError*);
  AdbcStatusCode (*ConnectionNew)(struct AdbcConnection*, struct 
AdbcError*);
   @@ -924,6 +973,8 @@ struct ADBC_EXPORT AdbcDriver {
  struct AdbcError*);
  AdbcStatusCode (*StatementGetPartitionDesc)(struct AdbcStatement*, 
uint8_t*,
  struct AdbcError*);
   +  AdbcStatusCode (*StatementGetRowCount)(struct AdbcStatement*, int64_t*,
   + struct AdbcError*);
  AdbcStatusCode (*StatementSetOption)(struct AdbcStatement*, const char*, 
const char*,
   struct AdbcError*);
  AdbcStatusCode (*StatementSetSqlQuery)(struct AdbcStatement*, const char*,
   diff --git 
a/java/core/src/main/java/org/apache/arrow/adbc/core/AdbcConnection.java 
b/java/core/src/main/java/org/apache/arrow/adbc/core/AdbcConnection.java
   index 3e3fe6f..cbea438 100644
   --- a/java/core/src/main/java/org/apache/arrow/adbc/core/AdbcConnection.java
   +++ b/java/core/src/main/java/org/apache/arrow/adbc/core/AdbcConnection.java
   @@ -54,8 +54,53 @@ public interface AdbcConnection extends 

[GitHub] [arrow-adbc] lidavidm commented on issue #55: [Format] Minor gaps with existing APIs

2022-08-10 Thread GitBox


lidavidm commented on issue #55:
URL: https://github.com/apache/arrow-adbc/issues/55#issuecomment-1211184131

   So looking into it
   - rowcount is easy to bind, but hard to support (lots of things don't 
support it or only support it for inserts) - that's OK. Flight SQL only exposes 
it for updates.
   - last inserted ID is harder to bind; JDBC requires you to specify that you 
want to get the information up front, while things like SQLite make it hard to 
get a reliable value. Flight SQL doesn't expose this.
   - paramstyle can just go in GetInfo but I think I'd rather punt on that sort 
of thing for now
   - current catalog (and current schema) are easily accessible from the 
connection JDBC. I don't immediately see how to get this for ODBC. Flight SQL 
doesn't really expose this.


-- 
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: issues-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org