-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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 partial indexes support #6643
base: 4.3.x
Are you sure you want to change the base?
Conversation
src/Schema/SQLiteSchemaManager.php
Outdated
if ($tableIndex['partial'] === 1) { | ||
$idx['where'] = $this->connection->fetchOne( | ||
<<<'SQL' | ||
SELECT SUBSTR(sql, INSTR(sql, 'WHERE') + 6) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SELECT SUBSTR(sql, INSTR(sql, 'WHERE') + 6) | |
SELECT SUBSTR(sql, INSTR(sql, 'WHERE') + LENGTH('WHERE ')) |
Question: Is this query somehow affected by the collation of a database so that an all uppercase WHERE would end up in a wrong result?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe i do not understand correctly, but if we are referring to the output of sql_master table it's documented here: https://www.sqlite.org/schematab.html#interpretation_of_the_schema_table it mentions it does some spaces conversion and keyword conversion. So I did some testing and if the partial is created with a lowercase where:
CREATE UNIQUE INDEX simple_partial_index ON person (id, name) where (id IS NULL)
instead of
CREATE UNIQUE INDEX simple_partial_index ON person (id, name) WHERE (id IS NULL)
it would output the lowercase where, since it was created that way.
SELECT SUBSTR(sql, INSTR(lower(sql), 'where') + LENGTH('where ')) FROM sqlite_master WHERE type = 'index'` AND name = (?)
would cover both cases. Since we are only checking in sqlite_master if we are dealing with a partial index.
I can modify the implementation but do we need an additional test which proves this? I can add it if we want it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason why I stuck to uppercase is because dbal itself uses the where in uppercases, but it can be used in a context where the table has been created outside of dbal, which may slightly differ.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added new commits which should resolve this
…ex functionality for SQLite does not work
Queries the sqlite_master instead of using PRAGMA because PRAGMA doesn't store the definition
0774f2c
to
8b2e5a5
Compare
Summary
This adds support for partial indexes to the SQLitePlatform. Similar as the PostgresPlatform provides.