-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
SQLite3 Connection getIndexData() #6221
SQLite3 Connection getIndexData() #6221
Conversation
Got some work to do in CodeIgniter4\system\Database\SQLite3\Table Line 111 $this->keys = array_merge($this->keys, $this->formatKeys($this->db->getIndexData($table))); |
Adding SELECT `table` || '_' || `from` || '_foreign' as indexname, `from` as fieldname, 'FOREIGN' as indextype FROM pragma_foreign_key_list('" . trim($table, '`') . "') UNION ALL to _indexData query will retrieve foreign keys as well. This may be a step to far though it seems all the other databases return these with indexes. There is a separate method that will retrieve these. |
743d709
to
d4161dd
Compare
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 PR looks good, very nice tests. I really am not qualified to decide if this is a better approach to SQLite, but it seems like you are mostly expanding on what we already have so I'll give a tentative "approve". Need somebody else to chime in as well.
d4161dd
to
def6e1f
Compare
@sclubricants Thank you for the PR. |
It may be kind to add the note in https://codeigniter4.github.io/CodeIgniter4/database/metadata.html#db-getindexdata
|
What do you mean? I got the array when specifying
|
I'm not sure where I got that idea. I thought it was something that I tested but somehow I'm unable to replicate it now. After looking at the code its impossible to do because if there is an auto increment column then only the one column will be used and any others discarded. This is why the email doesn't show up in PK. I tried it in SQL and it throws an error if you try to set multiple PK with an AUTOINCREMENT column. So disregard that. |
@kenjis User Guide and Change Log updated. |
Before:
After:
|
Co-authored-by: kenjis <[email protected]>
Co-authored-by: kenjis <[email protected]>
Supersedes #6207
This is a rewrite of
system/Database/SQLite3/Connection::_indexData()
.SQLite doesn't show indexes on tables created with
INTEGER PRIMARY KEY AUTO_INCREMENT
. This PR looks for primary keys that don't appear in the normal index lookup and adds it to the return of the method. It also adds the key type: Primary, Unique, or Index.In the test database creation of the users table the current implementation won't return any index.
However if you wrote it like the following it would show up in the indexes:
With this PR both cases will return indexes for primary key.
The need is to get a comprehensive list of database constraints. This will be useful in adding
upsert()
to the database.Furthermore, this method doesn't use nested queries like the current implementation and requires only one database call.
Its important to note that something like the following would likely cause an error:
SQLite's handling of things the way they do creates a pseudo index but its not an index for the purposes of calling database commands. If you check that the index isn't name PRIMARY then the rest should be ok.
Checklist: