STRICT tables and type conversion

[kuhnroyal]

I started using the new STRICT table feature from sqlite3.37 and already discovered some potential errors where some columns with unknown data types made it into the database. So this is really good, especially when writing migrations with SQL.

We tend to copy/paste the SQL from drift files to migrations and adapt it, here we forget converting from BOOLEAN to INTEGER etc. but now this is prevented.

But when making tables STRICT in drift files, there are a lot of warnings about unknown types which get converted by Drift into the correct types - BOOLEAN, DATETIME, DECIMAL etc.

So maybe it would be helpful to have something similar to the MAPPED BY syntax in order to prevent these warning:

-- Generates warning
foo BOOLEAN DEFAULT FALSE,
-- Would not generate warning and can be copied to migration without changing the type
foo INTEGER DEFAULT 0 MAPPED TO BOOLEAN,

This would prevent the STRICT warning and can easier be copied to an SQL migration.

What do you think?

[simolus3]

I think the first foo should not generate a warning either. It does at the moment, but that's the implementation being overly strict. At the moment, we allow any of the following in sqlparser

• INT, INTEGER, REAL, TEXT, BLOB and ANY all the time (following sqlite docs, but support for ANY is limited at the moment)
• additionally, BOOL and DATE if the CREATE TABLE statement being analyzed comes from a drift file

So using say DECIMAL instead of REAL should be runtime error (if I read sqlite3 docs correctly) and is flagged by the static analyzer too. For tables defined in drift files, I think we can be more lenient and allow anything containing BOOL or DATE. It will be turned into an int by drift either way.

Do you copy tables from drift files into raw SQL strings that you run in a migration? I don't really see the advantage of the MAPPED TO BOOLEAN syntax except that it would be easier to see and remove in that case.

[simolus3]

Good catch, thanks!

Outside of drift files, I think DECIMAL should give an analysis error, it fails for me in sqlite3 (using 3.37.2):

sqlite> CREATE TABLE tmp (x DECIMAL) STRICT;
Error: in prepare, unknown datatype for tmp.x: "DECIMAL" (1)

The truth is that drift's generator will always pick some column type (falling back to REAL if needed). So the right behavior is probably just to completely omit the warning for drift files.

Tried to adapt this but I am also not sure about the affinity from_create_table_test:

These refer to how the type names would parse outside of drift files, and are pretty much taken from the sqlite3 documentation. The supports booleans when moor extensions are enabled test is handling the other scenario.

[kuhnroyal]

Outside of drift files, I think DECIMAL should give an analysis error

Yes, think so. I have been using this in Drift files so far but with STRICT, I will migrate to using REAL directly.

The truth is that drift's generator will always pick some column type (falling back to REAL if needed).

Yea, I saw the fallback. Maybe this should move into the _handleColumnTypeWithoutDefault function.

[kuhnroyal]

Ah nvm, you did that already :) I'll test.

[kuhnroyal]

Works! Thanks! Can you release a patch version of the sqlparser?

[simolus3]

Thanks for all your help testing! Just published version 0.19.2 of sqlparser.