lidavidm commented on code in PR #955:
URL: https://github.com/apache/arrow-adbc/pull/955#discussion_r1282376426
##########
c/driver/sqlite/sqlite.c:
##########
@@ -1025,6 +1027,50 @@ AdbcStatusCode SqliteStatementInitIngest(struct
SqliteStatement* stmt,
goto cleanup;
}
+ int status =
+ ArrowSchemaViewInit(&view, stmt->binder.schema.children[i],
&arrow_error);
+ if (status != 0) {
+ SetError(error, "Failed to parse schema for column %d: %s (%d): %s", i,
+ strerror(status), status, arrow_error.message);
+ code = ADBC_STATUS_INTERNAL;
+ goto cleanup;
+ }
+
+ switch (view.type) {
+ case NANOARROW_TYPE_UINT8:
+ case NANOARROW_TYPE_UINT16:
+ case NANOARROW_TYPE_UINT32:
+ case NANOARROW_TYPE_UINT64:
+ case NANOARROW_TYPE_INT8:
+ case NANOARROW_TYPE_INT16:
+ case NANOARROW_TYPE_INT32:
+ case NANOARROW_TYPE_INT64:
+ sqlite3_str_appendf(create_query, " INTEGER");
+ break;
+ case NANOARROW_TYPE_FLOAT:
+ case NANOARROW_TYPE_DOUBLE:
+ sqlite3_str_appendf(create_query, " REAL");
+ break;
+ case NANOARROW_TYPE_STRING:
+ case NANOARROW_TYPE_LARGE_STRING:
+ sqlite3_str_appendf(create_query, " TEXT");
+ break;
+ case NANOARROW_TYPE_DATE32:
+ sqlite3_str_appendf(create_query, " INTEGER");
Review Comment:
This is wrong, dates end up as strings.
##########
c/driver/sqlite/sqlite.c:
##########
@@ -1025,6 +1027,50 @@ AdbcStatusCode SqliteStatementInitIngest(struct
SqliteStatement* stmt,
goto cleanup;
}
+ int status =
+ ArrowSchemaViewInit(&view, stmt->binder.schema.children[i],
&arrow_error);
+ if (status != 0) {
+ SetError(error, "Failed to parse schema for column %d: %s (%d): %s", i,
+ strerror(status), status, arrow_error.message);
+ code = ADBC_STATUS_INTERNAL;
+ goto cleanup;
+ }
+
+ switch (view.type) {
+ case NANOARROW_TYPE_UINT8:
+ case NANOARROW_TYPE_UINT16:
+ case NANOARROW_TYPE_UINT32:
+ case NANOARROW_TYPE_UINT64:
+ case NANOARROW_TYPE_INT8:
+ case NANOARROW_TYPE_INT16:
+ case NANOARROW_TYPE_INT32:
+ case NANOARROW_TYPE_INT64:
+ sqlite3_str_appendf(create_query, " INTEGER");
+ break;
+ case NANOARROW_TYPE_FLOAT:
+ case NANOARROW_TYPE_DOUBLE:
+ sqlite3_str_appendf(create_query, " REAL");
+ break;
+ case NANOARROW_TYPE_STRING:
+ case NANOARROW_TYPE_LARGE_STRING:
+ sqlite3_str_appendf(create_query, " TEXT");
+ break;
+ case NANOARROW_TYPE_DATE32:
+ sqlite3_str_appendf(create_query, " INTEGER");
+ break;
+ case NANOARROW_TYPE_BINARY:
+ sqlite3_str_appendf(create_query, " BLOB");
+ break;
+ case NANOARROW_TYPE_TIMESTAMP:
+ sqlite3_str_appendf(create_query, " TEXT");
+ break;
+ default:
+ SetError(error, "[SQLite] Field #%d ('%s') has unsupported type for
ingestion",
+ i + 1, stmt->binder.schema.children[i]->name);
+ code = ADBC_STATUS_NOT_IMPLEMENTED;
+ goto cleanup;
Review Comment:
Just `break;` here.
##########
c/driver/sqlite/sqlite.c:
##########
@@ -1025,6 +1027,46 @@ AdbcStatusCode SqliteStatementInitIngest(struct
SqliteStatement* stmt,
goto cleanup;
}
+ int status =
+ ArrowSchemaViewInit(&view, stmt->binder.schema.children[i],
&arrow_error);
+ if (status != 0) {
+ SetError(error, "Failed to parse schema for column %d: %s (%d): %s", i,
+ strerror(status), status, arrow_error.message);
+ code = ADBC_STATUS_INTERNAL;
+ goto cleanup;
+ }
Review Comment:
Why are you initializing it against the dictionary...? You would just
initialize it against `stmt->binder.schema`.
##########
c/driver/sqlite/sqlite.c:
##########
@@ -1025,6 +1027,50 @@ AdbcStatusCode SqliteStatementInitIngest(struct
SqliteStatement* stmt,
goto cleanup;
}
+ int status =
+ ArrowSchemaViewInit(&view, stmt->binder.schema.children[i],
&arrow_error);
+ if (status != 0) {
+ SetError(error, "Failed to parse schema for column %d: %s (%d): %s", i,
+ strerror(status), status, arrow_error.message);
+ code = ADBC_STATUS_INTERNAL;
+ goto cleanup;
+ }
+
+ switch (view.type) {
+ case NANOARROW_TYPE_UINT8:
+ case NANOARROW_TYPE_UINT16:
+ case NANOARROW_TYPE_UINT32:
+ case NANOARROW_TYPE_UINT64:
+ case NANOARROW_TYPE_INT8:
+ case NANOARROW_TYPE_INT16:
+ case NANOARROW_TYPE_INT32:
+ case NANOARROW_TYPE_INT64:
+ sqlite3_str_appendf(create_query, " INTEGER");
+ break;
+ case NANOARROW_TYPE_FLOAT:
+ case NANOARROW_TYPE_DOUBLE:
+ sqlite3_str_appendf(create_query, " REAL");
+ break;
+ case NANOARROW_TYPE_STRING:
+ case NANOARROW_TYPE_LARGE_STRING:
+ sqlite3_str_appendf(create_query, " TEXT");
+ break;
+ case NANOARROW_TYPE_DATE32:
+ sqlite3_str_appendf(create_query, " INTEGER");
+ break;
+ case NANOARROW_TYPE_BINARY:
+ sqlite3_str_appendf(create_query, " BLOB");
+ break;
+ case NANOARROW_TYPE_TIMESTAMP:
+ sqlite3_str_appendf(create_query, " TEXT");
Review Comment:
This is arguably misleading, too. I would rather we just don't insert a type
here, or use `TIMESTAMP`.
--
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]
