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

Support sqlite query parameters #2053

Merged
merged 4 commits into from
Jan 31, 2022
Merged

Support sqlite query parameters #2053

merged 4 commits into from
Jan 31, 2022

Conversation

swiffer
Copy link
Contributor

@swiffer swiffer commented Dec 31, 2021

Adds support for the cache query parameter - the same has been added in CakePHP already and e.g. allows using the same in memory database when running a CakePHP App TestSuite via Migrator.

CakePHP also adds support for a named inmemory database via the mode query parameter. Adding support for this one needs some discussion in #2052

Something similar has been done for CakePHP already here: cakephp/cakephp#16189

@MasterOdin MasterOdin changed the title support cache query parameter Support sqlite query parameters Jan 3, 2022
@dereuromark
Copy link
Member

@MasterOdin Is this OK to be merged as is?

@MasterOdin
Copy link
Member

@dereuromark It would be good to come up with a way to test this that isn't just try opening a bunch of sqlite connections with different parameters on 8.1+. Could we expose a public function getDsn() function for all adapters? Could also make the function private, and then test it via reflection, but either way, I think just making sure we're putting the string together correctly is going to be necessary to make sure we don't have regressions here as the functionality is somewhat complicated.

@dereuromark
Copy link
Member

Sure, we can also check non private ways for the new 0.13 branch/release.
And keep things simple for last minor version of 0.12

@swiffer
Copy link
Contributor Author

swiffer commented Jan 24, 2022

@dereuromark It would be good to come up with a way to test this that isn't just try opening a bunch of sqlite connections with different parameters on 8.1+. Could we expose a public function getDsn() function for all adapters? Could also make the function private, and then test it via reflection, but either way, I think just making sure we're putting the string together correctly is going to be necessary to make sure we don't have regressions here as the functionality is somewhat complicated.

is someone from your end picking this up or can you give a little more guidenance on the way this should be implemented?

@MasterOdin
Copy link
Member

is someone from your end picking this up or can you give a little more guidenance on the way this should be implemented?

This would be done in a follow-up PR, no changes necessary from you here.

@dereuromark dereuromark merged commit 1e8bfd7 into cakephp:master Jan 31, 2022
@swiffer swiffer deleted the sqlite-params branch January 31, 2022 15:52
aspirantzhang added a commit to aspirantzhang/phinx that referenced this pull request Feb 2, 2022
cakephp#2053 (Support sqlite query and mode parameters)
cakephp#2066 (doc(postgres): Add GIN index documentation)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants