piskvorky / sqlitedict

Persistent dict, backed by sqlite3 and pickle, multithread-safe.
Apache License 2.0
1.17k stars 131 forks source link

Pickling keys for possibility of saving any picklable objects as keys #26

Closed ziky90 closed 9 years ago

ziky90 commented 9 years ago

I had few minutes of time, so in this PR I have just tried to implement and test key pickling as @piskvorky has suggested in issue #25.

It seems that it works well, feel free to test it and report (comment here) in case of any problems found.

NOTE Backward compatibility is assured except cases where was used pickle.dumps() manually for keys.

piskvorky commented 9 years ago

@ziky90 any way to make this backward compatible?

If so, let's do it. A lot of people rely on sqlitedict, I want to avoid surprises for them.

If not, let's at least provide a tool (script) that will convert a DB in old format to a DB in new format (recode keys).

jquast commented 9 years ago

I've been thinking about it, that the key could begin with some "Magic Number", like "\xfe\xf1\x01" to say "Magic number: version 1, this is pickled"

piskvorky commented 9 years ago

Hmm. How about just unpickling and if that fails (unpickle exception), assuming the key is a string (=old style key)?

jquast commented 9 years ago

good idea, that could work pretty well, i don't think any basic str could accidentally "unpickle"

ziky90 commented 9 years ago

Hmm. How about just unpickling and if that fails (unpickle exception), assuming the key is a string (=old style key)?

I can possibly see one problem here for people who are performing pickling already in their code. It would probably fail during loading, because as I can imagine it would try to call unpickle twice, once in sqlitedict and once in their code. So this change would force them to remove unpickling in their code, which would not make sqlitedict completely backwards compatible.

piskvorky commented 9 years ago

So, do we break backward compatibility loudly and explicitly, or do we do the try: catch on unpickle, as a fallback?

I have no preference, so whatever works for you.

jquast commented 9 years ago

I've made some progress with being backwards-compatible of a branch of the same name in this repository: https://github.com/piskvorky/sqlitedict/compare/key_pickling