observablehq / stdlib

The Observable standard library.
https://observablehq.com/@observablehq/standard-library
ISC License
966 stars 83 forks source link

SQLiteDatabaseClient schema methods #283

Closed mbostock closed 2 years ago

mbostock commented 2 years ago

This implements the describeTables and describeColumns methods for SQLite.

Fixes #288.

mbostock commented 2 years ago

Asking @visnup for another opinion here on how to pass through the raw database type.

visnup commented 2 years ago

I like what's here plus a single key called maybe databaseType or vendorType that includes the vendor-specific name for the type. And I'd want to re-work the schema object coming from data-connector to match this too, replacing the int32: true, float32: true entries with something like databaseType: "int32"

The only other thing worth considering is calling out bigint and date since the original JSON schema specification on ajv didn't provide for either. These were {type: "string", [bigint|date]: true} since over the wire these values were serialized as strings.

It seems since then there's JSON Type Definition which seems more promising?? And this more recent JSON Schema reference seems to document types for datetime.

mbostock commented 2 years ago

I’ve populated the databaseType field. Here’s what else I would like to do, but does not appear possible with our SQLite implementation, sql.js:

We could probably fork sql.js to fix these issues.

annie commented 2 years ago

since this is a Table cell blocker, i think we should merge this once we've finalized the DatabaseClient schema API (based on Wiltse's PR) and updated this PR to match that specification (i've got a draft PR in the wings to make those updates).

it seems reasonable to me to address the query results schema and date string issues as fast follows, if that sounds good? i can create separate issues for those!

mbostock commented 2 years ago

FYI, the describeColumns query had a syntax error which I fixed in d381b56a159498f84e1e4fe61c397eb7d2732878.