Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SQLite backend sometimes infers invalid types from the Metadata #6208

Closed
radeusgd opened this issue Apr 5, 2023 · 1 comment · Fixed by #6381
Closed

SQLite backend sometimes infers invalid types from the Metadata #6208

radeusgd opened this issue Apr 5, 2023 · 1 comment · Fixed by #6381
Assignees
Labels
--bug Type: bug -libs Libraries: New libraries to be implemented l-table-column-datatypes
Milestone

Comments

@radeusgd
Copy link
Member

radeusgd commented Apr 5, 2023

To avoid the complexity of recreating the typesystem of every database engine we rely on (which are all pretty similar, but do have subtle differences), we decided to instead fetch types of expressions directly from the database backend - thus getting the most precise information.

Unfortunately, SQLite does not implement the usual typing mechanisms - all of its columns can store any type - they just have affinities which determine some type preference. This works for raw columns, but once we derive expressions from them it gets complicated. We were using a workaround that was trying to infer the types wherever possible and rely on overrides (mostly for the Boolean type which does not natively exist in SQLite and so had to be 'emulated').

After some further work (#6204), it turns out that the inference coming from the Database is unreliable - for example it may break if the first returned value is a NULL.

The conclusion is that, while for more complex Databases like Postgres, inferring types from the Metadata is the most robust way to infer operation types, this is not really viable for SQLite as that information is just wrong most of the time.

To have proper types support, we need to compute the result types locally for the SQLite backend. The other backends (Postgres) should be kept as-is.

Fortunately, this is relatively easy, since SQLite operates on only the following types:

  • TEXT (Value_Type.Char)
  • INTEGER (Value_Type.Integer)
  • FLOAT (Value_Type.Float)
  • NUMERIC (falls back to Value_Type.Float too, since it's floats, not Decimals under the hood)
  • BLOB (that we currently treat as Mixed, to be revised)
  • BOOLEAN (a fake emulated affinity to make boolean columns work - it is correctly inferred if the column was declared as BOOLEAN in CREATE TABLE, but it gets lost on derived expressions).

We only need to support these 6 types without any variants (fixed-length, int32 etc. are not present here) and the rules are rather simple.

We should adapt infer_return_type and make sure no place (other than initial table setup) is actually fetching column types from the SQLite database - since it is simply not reliable.

@radeusgd radeusgd added --bug Type: bug -libs Libraries: New libraries to be implemented l-table-column-datatypes labels Apr 5, 2023
@radeusgd radeusgd self-assigned this Apr 5, 2023
@github-project-automation github-project-automation bot moved this to ❓New in Issues Board Apr 5, 2023
@radeusgd radeusgd moved this from ❓New to 📤 Backlog in Issues Board Apr 5, 2023
@radeusgd radeusgd moved this from 📤 Backlog to 🔧 Implementation in Issues Board Apr 6, 2023
@jdunkerley jdunkerley added this to the Beta Release milestone Apr 11, 2023
@radeusgd radeusgd moved this from 🔧 Implementation to 👁️ Code review in Issues Board Apr 24, 2023
@mergify mergify bot closed this as completed in #6381 Apr 24, 2023
mergify bot pushed a commit that referenced this issue Apr 24, 2023
@github-project-automation github-project-automation bot moved this from 👁️ Code review to 🟢 Accepted in Issues Board Apr 24, 2023
@enso-bot
Copy link

enso-bot bot commented Apr 24, 2023

Radosław Waśko reports a new STANDUP for the provided date (2023-04-21):

Progress: Finished the follow up PR with typechecks for Aggregates and Cross Tab. Implemented local typechecking workaround for SQLite. Catching up bookclubs. It should be finished by 2023-04-24.

Next Day: Next day I will be working on the #6326 task. Start work on create_database_table.

Akirathan pushed a commit that referenced this issue Apr 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
--bug Type: bug -libs Libraries: New libraries to be implemented l-table-column-datatypes
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants