Check value is not None before inserting into forward index

zopefoundation / zope.index

Indices for using with catalog like text, field, etc.

Other

10 stars 12 forks source link

Check value is not None before inserting into forward index #3

Closed vangheem closed 8 years ago

vangheem commented 9 years ago

This fixes zope.index in ZODB 4

tseaver commented 9 years ago

Thanks for the patch! A couple of issues:

This change suppresses an exception for such values on Py3k: are we sure we want to do that, rather than raising the exception even on Python2?
Whichever way that choice goes, the "value is None" case needs a new test, with appropriate assertions (either of the unchanged state of the forward index, or the always-raised exception).

vangheem commented 9 years ago

@tseaver I'm not sure this has anything to do with p2k vs p3k. In the latest BTree, which ZODB depends on, you can not have None values as keys.

tseaver commented 9 years ago

Ah, right -- I forgot that bit.

As it stands, this PR means that we won't be able to search for documents having None as their value, although they will be present in the reverse index.

vangheem commented 9 years ago

I don't really know enough about how zope.index works so this is probably short sighted. I had thought the rev_index was where the search was done on.

Do we catch the TypeError and say that None values aren't allowed with ZODB 4?

I also don't know what happens for upgraded ZODBs where there are existing None values.

tseaver commented 8 years ago

Thanks again!