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

Updated the methods for pragma calls according to the currently #136

Merged
merged 2 commits into from
Feb 12, 2016
Merged

Updated the methods for pragma calls according to the currently #136

merged 2 commits into from
Feb 12, 2016

Conversation

aibor
Copy link
Contributor

@aibor aibor commented Jul 30, 2014

Some of the pragmas called in the Pragmas module were deprecated and new ones were missing.
So I updated the methods for pragma calls according to the currently supported pragmas listed on https://sqlite.org/pragma.html
This should fix #120

@luislavena
Copy link
Member

@aibor thank you for the pull. Can you tell us if there is a minimum version requirement with those new pragmas?

@aibor
Copy link
Contributor Author

aibor commented Jul 31, 2014

The complete set of pragmas onlyworks in recent versions. However, with older versions the then umimplemented pragmas just return nothing. So backwards compatibility is given.
I have thrown out/renamed pragmas which were changed 10 years ago (e.g. schema_cookie to schema_version).

I don't know if it is a good idea to alias these old pragma names to the new names. That might be a good idea to prevent NoMethodErrors for code which has these methods still implemented, although they haven't done anything for years now.

Edit:
Instead of removing the long gone pragmas, it might be a good idea to just keep them in for full backwards compatibility. The only thing that really changed its behaviour is auto_vacuum, which seems to have been an boolean value in the past and is now an enum value. I'm not sure when this change happened and can't find any hints in the changelogs.

@aibor
Copy link
Contributor Author

aibor commented Aug 1, 2014

After further investigating the commit history I can now tell that SQLite 3.4.0 is the minimum version for this changes to work. I have added some pragmas again for backwards compatibility.

The only thing that has changed its functionality instead of just dis-/appearing is the auto_vacuum pragma, which has changed from a boolean value to enum value with version 3.4.0.

@luislavena luislavena mentioned this pull request Oct 31, 2014
@tenderlove
Copy link
Member

Honestly I'm not a huge fan of supporting all of these since they change from version to version. It means we have to keep updating them as upstream adds more. Could we consider just providing methods to get and set pragmas, and people can pass in the pragma name they care about? Then we don't have to keep updating this stuff and I think it would be more clear to the user, and they can manipulate pragmas without waiting for us to upgrade our API.

@aibor
Copy link
Contributor Author

aibor commented Jul 20, 2015

I absolutely agree that keeping the pragma methods up to date is tedious. As the different pragma type getter and setter methods are already there, it might be desirable to make them public. So if one of the wrappers is outdated because the usage has changed or new pragmas have been added, anyone can use the generic getters and setters for these pragmas.
But I think there is no point in throwing the already present wrappers over board. At least for enum pragmas I consider them rather valuable.

@tenderlove
Copy link
Member

As the different pragma type getter and setter methods are already there, it might be desirable to make them public. So if one of the wrappers is outdated because the usage has changed or new pragmas have been added, anyone can use the generic getters and setters for these pragmas.

I agree.

But I think there is no point in throwing the already present wrappers over board. At least for enum pragmas I consider them rather valuable.

I agree with this as well. Can you change the PR to just make those getter / setter methods public?

aibor and others added 2 commits February 11, 2016 00:59
Updated the methods for pragma calls according to the currently
supported pragmas listed on https://sqlite.org/pragma.html. The previous
behaviour is maintained and extended by the new pragmas.
Only thing changed is the auto_vacuum pragma, which has changed in the
commit from 2007-04-26:
https://www.sqlite.org/cgi/src/info/f6a6d2b8872c05089810b1e095f39011f3035408

So, the minimal version for this to work is SQLite 3.4.0
As discussed in #136, the type-based getter and setter helper methods
for pragmas are public now. This allows direct usage in case there are
no specific methods for a pragma.
@aibor
Copy link
Contributor Author

aibor commented Feb 11, 2016

Type based getters/setters have been made public with ae6c84a.

I have also added methods for recently added pragmas. In order to keep the keep it tidy I have squashed the commits regarding these wrapper methods into ee010db, which is based on current master, now.

I'm sorry for the mess, but I hope this will keep the commit log sane.

@tenderlove
Copy link
Member

@aibor great, thank you!

tenderlove added a commit that referenced this pull request Feb 12, 2016
Updated the methods for pragma calls according to the currently
@tenderlove tenderlove merged commit 46ae4ee into sparklemotion:master Feb 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing pragmas
3 participants