Repository navigation
feat(csharp/drivers/bigquery): add BigQuery ADBC driver - #1192
Conversation
davidhcoe
commented
Oct 11, 2023
davidhcoe
commented
- introduces ADBC support for connecting to BigQuery
- minor fixes to other projects to align with the pattern
…w-adbc into dev/snowflakedriver
|
|
…-adbc into dev/bigquerydriver
|
@lidavidm -- this is ready now. There was some strangeness with the line endings that the pre-commit check in kept fighting me on (even though the local one would pass). It seems the answer is to have the .csproj files with CRLF and the rest have LF for some reason. |
|
I had thought csproj wanted CRLF, but we can adjust that, sorry 😅 |
CurtHagenlocher
left a comment
CurtHagenlocher
left a comment
There was a problem hiding this comment.
My main feedback is about the generation of schema queries.
| public override Schema GetTableSchema(string catalog, string dbSchema, string tableName) | ||
| { | ||
| string query = string.Format("SELECT * FROM `{0}`.`{1}`.INFORMATION_SCHEMA.COLUMNS WHERE table_name = '{2}'", | ||
| catalog, dbSchema, tableName); |
There was a problem hiding this comment.
Out of curiosity (and not because it should block anything) have you investigated whether the schema hierarchy can be combined into a single query? BigQuery has pretty decent support for structured columns, so it ought to be possible to format things together in the query and reduce round trips. (My assumption is that this would be faster than non-concurrent round trips; that would need to be tested, of course.)
CurtHagenlocher
left a comment
CurtHagenlocher
left a comment
There was a problem hiding this comment.
I looked a little more closely in a few places and spotted things I missed the first time. In addition to the SQL injection risk I hinted at earlier (without explicitly calling out) I think the lack of precision and scale on numeric results makes them unusable and there are two places where null scalar values wouldn't get reported.
