c/driver/sqlite: Use of implicit "main" schema and/or table names that require quoting broken for SQLite in 0.6.x

[alexander-beedie]

It seems something significant has changed between the 0.5.x and 0.6.x releases, as the standard main¹ db schema can no longer be referenced in SQLite queries, and neither can table names that need to be quoted.

The following minimal test-cases demonstrate the issues:

• Setup

  from adbc_driver_sqlite.dbapi import connect
  import pyarrow as pa
  
  data = pa.Table.from_pydict({
      "key": ["aa", "bb", "cc"], 
      "value": [10, 20, 30], 
  })

• Use of main with a simple table name...

  with connect() as conn, conn.cursor() as crsr: 
      crsr.adbc_ingest( "main.test_data", data, "create" )
      conn.commit()

  ...now raises:

  adbc_driver_manager._lib.InternalError: ADBC_STATUS_INTERNAL (9): 
  [SQLite] Failed to prepare statement: no such table: main.test_data 
  (executed 'INSERT INTO main.test_data VALUES (?, ?)')
  

• Use of a table name that requires quoting...

  with connect() as conn, conn.cursor() as crsr:
      crsr.adbc_ingest( '"test-data"', data, "create" )
      conn.commit()

  ...now raises:

  adbc_driver_manager._lib.InternalError: ADBC_STATUS_INTERNAL (9): 
  [SQLite] Failed to prepare statement: no such table: test-data 
  (executed 'INSERT INTO "test-data" VALUES (?, ?)')
  

In the previous release (0.5.x) these examples both work correctly, as expected 👍

Footnotes

1. Note that main should always be valid, as it refers to the primary db being connected to (other names can be brought in via ATTACH DATABASE). ↩

[lidavidm]

Hmm. Thanks for the report. Maybe we should do a 0.6.1...

[alexander-beedie]

Hmm. Thanks for the report. Maybe we should do a 0.6.1...

That would be much appreciated ;)

[lidavidm]

Ah, we started quoting the table name and columns for you because of #905. But we didn't quote it in the INSERT, only in the CREATE (d'oh). We should fix that.

However, that leaves question of how to handle the catalog name ambiguous. Right now we would create a table "main.test_data". The next release will add functions to explicitly set/get the 'active' catalog/schema, but the SQLite driver doesn't implement all that (and I don't really plan on implementing it for the time being: see #924). Or I suppose we could try to parse this out for you, but then there wouldn't be a way to actually make a table called "main.test_data" if you wanted to...

[alexander-beedie]

Curious why the same issue doesn't hit the PostgreSQL driver (I just tested); maybe some parsing/quoting logic can be lifted from there?

I'm not sure that quoting table names by default is a good idea though (or at least not those containing "."), as access to main and temp (which are always available/valid), and any other names established by ATTACH DATABASE is fundamental, whereas creating tables containing a "." in the name is merely A Very Bad Idea™ ;)

You can determine recognised database names in SQLite using PRAGMA database_list or SELECT * FROM pragma_database_list (I don't think the C API offers an equivalent function). Perhaps use that to determine what is/isn't a db prefix and require the user to quote if they really have managed to create a standalone column called "main.something"?

[lidavidm]

We might not have implemented the escaping for Postgres.

In general the problem is that we only have a table name, and we didn't provide for a catalog/schema name. We could add parameters to set those separately?

[lidavidm]

Though again, setting the active catalog/schema also covers that (it's not that we can't support it in SQLite, just a question of maintainer time)

[lidavidm]

I merged the first fix, but going to leave this open for further discussion.

I think adding explicit parameters for the target catalog/schema would make the most sense, and would save you from having to know about quoting rules for specific backends entirely.

[alexander-beedie]

I think adding explicit parameters for the target catalog/schema would make the most sense, and would save you from having to know about quoting rules for specific backends entirely.

Indeed, otherwise it's going to be a can of worms explicitly handling every flavour of quoting (backtick, double-quote, square bracket). It's important to be able to do so though, as demonstrated above 👍