You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@arrow.apache.org by "kou (via GitHub)" <gi...@apache.org> on 2023/04/09 21:37:16 UTC

[GitHub] [arrow-adbc] kou opened a new issue, #581: c/driver/sqlite: Column type is always int64 with empty table

kou opened a new issue, #581:
URL: https://github.com/apache/arrow-adbc/issues/581

   With the following SQL, the SQLite driver returns `number:int64, string:int64` schema not `number:int64, string:string` schema:
   
   ```sql
   CREATE TABLE data (number int, string text);
   SELECT * FROM data;
   ```


-- 
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-unsubscribe@arrow.apache.org.apache.org

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


Re: [I] c/driver/sqlite: Column type is always int64 with empty table [arrow-adbc]

Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on issue #581:
URL: https://github.com/apache/arrow-adbc/issues/581#issuecomment-2659691292

   If we returned `null` instead of `int64` for an empty table then the custom code would be much easier (`pa.concat_xxx()` I believe will cast a `null` to anything else, but won't cast an `int64`).
   
   Other than that, I think requesting a schema would be helpful here (SQLite is not the only loosly typed data source that we might want a driver for!)


-- 
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: github-unsubscribe@arrow.apache.org

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


Re: [I] c/driver/sqlite: Column type is always int64 with empty table [arrow-adbc]

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on issue #581:
URL: https://github.com/apache/arrow-adbc/issues/581#issuecomment-2658345431

   I think ultimately, what we want is https://github.com/apache/arrow-adbc/issues/1514


-- 
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: github-unsubscribe@arrow.apache.org

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


Re: [I] c/driver/sqlite: Column type is always int64 with empty table [arrow-adbc]

Posted by "davlee1972 (via GitHub)" <gi...@apache.org>.
davlee1972 commented on issue #581:
URL: https://github.com/apache/arrow-adbc/issues/581#issuecomment-2780182171

   I think adding a schema option like how pyarrow dataset also accepts a schema would be the best approach..
   
   This could also support casting data types.. SQLite for example returns floats for numeric columns but passing in schema would allow for decimal32, 64, 128 and 256 types.


-- 
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: github-unsubscribe@arrow.apache.org

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


Re: [I] c/driver/sqlite: Column type is always int64 with empty table [arrow-adbc]

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on issue #581:
URL: https://github.com/apache/arrow-adbc/issues/581#issuecomment-2657963014

   We should add a recipe to demonstrate it, but there's an [`adbc.sqlite.query.batch_rows`](https://arrow.apache.org/adbc/current/driver/sqlite.html#type-inference-type-support) parameter that can be set on the statement to tell it to read more rows (and hence base inference on more data)


-- 
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: github-unsubscribe@arrow.apache.org

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


[GitHub] [arrow-adbc] lidavidm commented on issue #581: c/driver/sqlite: Column type is always int64 with empty table

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on issue #581:
URL: https://github.com/apache/arrow-adbc/issues/581#issuecomment-1501381444

   Yes, decltype would work. I think the hard part there will be that SQLite doesn't really check the type name so we would have to replicate 3.1.1 here: https://www.sqlite.org/datatype3.html
   
   Also it looks that decltype will fail if it's something like `x + 1` so in that case the user might still get a type that is a little unexpected. 
   
   On Mon, Apr 10, 2023, at 10:10, Sutou Kouhei wrote:
   > 
   > 
   > Ah, I thought that `sqlite3_column_type()` can work with an empty table. So I thought that we can improve the current implementation by:
   > 
   > diff --git a/c/driver/sqlite/statement_reader.c b/c/driver/sqlite/statement_reader.c
   > index 694b8cc..21fb20f 100644
   > --- a/c/driver/sqlite/statement_reader.c
   > +++ b/c/driver/sqlite/statement_reader.c
   > @@ -495,7 +495,7 @@ void StatementReaderRelease(struct ArrowArrayStream* self) {
   >  
   >  /// Initialize buffers for the first (type-inferred) batch of data.
   >  /// Use raw buffers since the types may change.
   > -AdbcStatusCode StatementReaderInitializeInfer(int num_columns, size_t infer_rows,
   > +AdbcStatusCode StatementReaderInitializeInfer(sqlite3_stmt *stmt, int num_columns, size_t infer_rows,
   >                                                struct ArrowBitmap* validity,
   >                                                struct ArrowBuffer* data,
   >                                                struct ArrowBuffer* binary,
   > @@ -507,7 +507,21 @@ AdbcStatusCode StatementReaderInitializeInfer(int num_columns, size_t infer_rows
   >      ArrowBufferInit(&data[i]);
   >      CHECK_NA(INTERNAL, ArrowBufferReserve(&data[i], infer_rows * sizeof(int64_t)), error);
   >      memset(&binary[i], 0, sizeof(struct ArrowBuffer));
   > -    current_type[i] = NANOARROW_TYPE_INT64;
   > +    int sqlite_type = sqlite3_column_type(stmt, i);
   > +    switch (sqlite_type) {
   > +    case SQLITE_FLOAT:
   > +      current_type[i] = NANOARROW_TYPE_DOUBLE;
   > +      break;
   > +    case SQLITE_TEXT:
   > +      current_type[i] = NANOARROW_TYPE_STRING;
   > +      break;
   > +    case SQLITE_BLOB:
   > +      current_type[i] = NANOARROW_TYPE_BINARY;
   > +      break;
   > +    default:
   > +      current_type[i] = NANOARROW_TYPE_INT64;
   > +      break;
   > +    }
   >    }
   >    return ADBC_STATUS_OK;
   >  }  // NOLINT(whitespace/indent)
   > @@ -835,7 +849,7 @@ AdbcStatusCode AdbcSqliteExportReader(sqlite3* db, sqlite3_stmt* stmt,
   >    enum ArrowType* current_type = malloc(num_columns * sizeof(enum ArrowType));
   >  
   >    AdbcStatusCode status = StatementReaderInitializeInfer(
   > -      num_columns, batch_size, validity, data, binary, current_type, error);
   > +      stmt, num_columns, batch_size, validity, data, binary, current_type, error);
   >    if (status == ADBC_STATUS_OK) {
   >      int64_t num_rows = 0;
   >      while (num_rows < batch_size) {
   > But `sqlite3_column_type()` always returns `SQLITE_NULL` with the case. :<
   > 
   > We may want to use `sqlite3_column_decltype()`...
   > 
   > 
   > 
   > —
   > Reply to this email directly, view it on GitHub <https://github.com/apache/arrow-adbc/issues/581#issuecomment-1501267541>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACQB36Y4GKADDJHBM6LKXLXANMXJANCNFSM6AAAAAAWYK5JDU>.
   > You are receiving this because you commented.Message ID: ***@***.***>
   > 
   > 


-- 
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: github-unsubscribe@arrow.apache.org

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


Re: [I] c/driver/sqlite: Column type is always int64 with empty table [arrow-adbc]

Posted by "miccoli (via GitHub)" <gi...@apache.org>.
miccoli commented on issue #581:
URL: https://github.com/apache/arrow-adbc/issues/581#issuecomment-2657706269

   Let me add a related problem, in which I incur very often: when the first batch of rows is all null, the driver will infer that the type is INT64, but then fail when the first non-null row is read, eg.
   ```
   [SQLite] Type mismatch in column 6: expected INT64 but got STRING/BINARY
   ```
   


-- 
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: github-unsubscribe@arrow.apache.org

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


Re: [I] c/driver/sqlite: Column type is always int64 with empty table [arrow-adbc]

Posted by "miccoli (via GitHub)" <gi...@apache.org>.
miccoli commented on issue #581:
URL: https://github.com/apache/arrow-adbc/issues/581#issuecomment-2658955277

   > [@miccoli](https://github.com/miccoli) see an example in the last commit of [#2523](https://github.com/apache/arrow-adbc/pull/2523)
   
   Thanks for clarifying this in the docs (#2523 is very informative!), but I was aware of the fact that `batch_rows` can be increased. 
   
   Unfortunately in my use case almost all values of a single column are null (and no, I do not have control on the database, which is written by another app), so there is no easy heuristics for determining the correct number of `batch_rows`. Currently I simply read all other columns with arrow-adbc and then have some custom code to create and append the missing column.
   
   So yes, #1514 would be a possible solution, but maybe also a form of (opt in) adaptive type inference could be useful:
   in the presence of null columns the type inference should be considered incomplete and retried with a bigger batch of rows.


-- 
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: github-unsubscribe@arrow.apache.org

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


Re: [I] c/driver/sqlite: Column type is always int64 with empty table [arrow-adbc]

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on issue #581:
URL: https://github.com/apache/arrow-adbc/issues/581#issuecomment-2825773353

   > I think ultimately, what we want is [#1514](https://github.com/apache/arrow-adbc/issues/1514)
   
   


-- 
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: github-unsubscribe@arrow.apache.org

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


[GitHub] [arrow-adbc] lidavidm commented on issue #581: c/driver/sqlite: Column type is always int64 with empty table

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on issue #581:
URL: https://github.com/apache/arrow-adbc/issues/581#issuecomment-1503148935

   Or really: SQLite is untyped, despite whatever it lets you say. We can sort of patch over cases like this that look like they should work, but fundamentally it's just a fiction that we're just trying to construct.


-- 
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: github-unsubscribe@arrow.apache.org

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


[GitHub] [arrow-adbc] lidavidm commented on issue #581: c/driver/sqlite: Column type is always int64 with empty table

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on issue #581:
URL: https://github.com/apache/arrow-adbc/issues/581#issuecomment-1501257240

   It's kind of a consequence of it inferring/guessing column types instead of trying to leverage metadata (since SQLite doesn't enforce types). We could have it use specified types as a starting point/as a restriction (erroring if data differs).


-- 
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: github-unsubscribe@arrow.apache.org

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


[GitHub] [arrow-adbc] kou commented on issue #581: c/driver/sqlite: Column type is always int64 with empty table

Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on issue #581:
URL: https://github.com/apache/arrow-adbc/issues/581#issuecomment-1501267541

   Ah, I thought that `sqlite3_column_type()` can work with an empty table. So I thought that we can improve the current implementation by:
   
   ```diff
   diff --git a/c/driver/sqlite/statement_reader.c b/c/driver/sqlite/statement_reader.c
   index 694b8cc..21fb20f 100644
   --- a/c/driver/sqlite/statement_reader.c
   +++ b/c/driver/sqlite/statement_reader.c
   @@ -495,7 +495,7 @@ void StatementReaderRelease(struct ArrowArrayStream* self) {
    
    /// Initialize buffers for the first (type-inferred) batch of data.
    /// Use raw buffers since the types may change.
   -AdbcStatusCode StatementReaderInitializeInfer(int num_columns, size_t infer_rows,
   +AdbcStatusCode StatementReaderInitializeInfer(sqlite3_stmt *stmt, int num_columns, size_t infer_rows,
                                                  struct ArrowBitmap* validity,
                                                  struct ArrowBuffer* data,
                                                  struct ArrowBuffer* binary,
   @@ -507,7 +507,21 @@ AdbcStatusCode StatementReaderInitializeInfer(int num_columns, size_t infer_rows
        ArrowBufferInit(&data[i]);
        CHECK_NA(INTERNAL, ArrowBufferReserve(&data[i], infer_rows * sizeof(int64_t)), error);
        memset(&binary[i], 0, sizeof(struct ArrowBuffer));
   -    current_type[i] = NANOARROW_TYPE_INT64;
   +    int sqlite_type = sqlite3_column_type(stmt, i);
   +    switch (sqlite_type) {
   +    case SQLITE_FLOAT:
   +      current_type[i] = NANOARROW_TYPE_DOUBLE;
   +      break;
   +    case SQLITE_TEXT:
   +      current_type[i] = NANOARROW_TYPE_STRING;
   +      break;
   +    case SQLITE_BLOB:
   +      current_type[i] = NANOARROW_TYPE_BINARY;
   +      break;
   +    default:
   +      current_type[i] = NANOARROW_TYPE_INT64;
   +      break;
   +    }
      }
      return ADBC_STATUS_OK;
    }  // NOLINT(whitespace/indent)
   @@ -835,7 +849,7 @@ AdbcStatusCode AdbcSqliteExportReader(sqlite3* db, sqlite3_stmt* stmt,
      enum ArrowType* current_type = malloc(num_columns * sizeof(enum ArrowType));
    
      AdbcStatusCode status = StatementReaderInitializeInfer(
   -      num_columns, batch_size, validity, data, binary, current_type, error);
   +      stmt, num_columns, batch_size, validity, data, binary, current_type, error);
      if (status == ADBC_STATUS_OK) {
        int64_t num_rows = 0;
        while (num_rows < batch_size) {
   ```
   
   But `sqlite3_column_type()` always returns `SQLITE_NULL` with the case. :<
   
   We may want to use `sqlite3_column_decltype()`...


-- 
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: github-unsubscribe@arrow.apache.org

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


[GitHub] [arrow-adbc] paleolimbot commented on issue #581: c/driver/sqlite: Column type is always int64 with empty table

Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on issue #581:
URL: https://github.com/apache/arrow-adbc/issues/581#issuecomment-1502289180

   Is there a mechanism by which a user can bind an output schema? (That would let a user get a consistent output schema if that was important for an application...short of replicating an entire SQL parser, it might be the only way to ensure that in this case?).


-- 
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: github-unsubscribe@arrow.apache.org

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


Re: [I] c/driver/sqlite: Column type is always int64 with empty table [arrow-adbc]

Posted by "davlee1972 (via GitHub)" <gi...@apache.org>.
davlee1972 commented on issue #581:
URL: https://github.com/apache/arrow-adbc/issues/581#issuecomment-2825509035

   I added a workaround to cast record batches to a schema, but it would be better if cursor() could include a schema parameter to handle casting untype NULLs to null string, null int, null float, etc..
   
   ```
                   arrow_batch_reader = cursor.fetch_record_batch()
   
                   arrow_batch_reader = pa.RecordBatchReader.from_batches(
                       schema=my_schema,
                       wrapped_reader(arrow_batch_reader, cursor),
                   )
   ```
   


-- 
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: github-unsubscribe@arrow.apache.org

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


[GitHub] [arrow-adbc] lidavidm commented on issue #581: c/driver/sqlite: Column type is always int64 with empty table

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on issue #581:
URL: https://github.com/apache/arrow-adbc/issues/581#issuecomment-1503147838

   Not really. There's also not a great way to pass in a schema currently.
   
   We don't need a SQL parser. It's just that SQLite is rather fuzzy about what type names it accepts. The rules are simple, but surprising.


-- 
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: github-unsubscribe@arrow.apache.org

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


Re: [I] c/driver/sqlite: Column type is always int64 with empty table [arrow-adbc]

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on issue #581:
URL: https://github.com/apache/arrow-adbc/issues/581#issuecomment-2660731781

   Returning `null` is also an option, I hesitate to change the behavior but admittedly `null` is more accurate perhaps than defaulting to `int64`.


-- 
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: github-unsubscribe@arrow.apache.org

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


Re: [I] c/driver/sqlite: Column type is always int64 with empty table [arrow-adbc]

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on issue #581:
URL: https://github.com/apache/arrow-adbc/issues/581#issuecomment-2658418283

   @miccoli see an example in the last commit of https://github.com/apache/arrow-adbc/pull/2523


-- 
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: github-unsubscribe@arrow.apache.org

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


Re: [I] c/driver/sqlite: Column type is always int64 with empty table [arrow-adbc]

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on issue #581:
URL: https://github.com/apache/arrow-adbc/issues/581#issuecomment-2659192892

   Hmm, an adaptive mode could be reasonable (or even having it be the normal mode)


-- 
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: github-unsubscribe@arrow.apache.org

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


Re: [I] c/driver/sqlite: Column type is always int64 with empty table [arrow-adbc]

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on issue #581:
URL: https://github.com/apache/arrow-adbc/issues/581#issuecomment-2661752875

   I filed https://github.com/apache/arrow-adbc/issues/2531 and https://github.com/apache/arrow-adbc/issues/2532


-- 
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: github-unsubscribe@arrow.apache.org

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