search

zopefoundation / Products.ZCatalog

Zope's indexing and search solution.
Other
5 stars 22 forks source link

Fix catalog plan for not query #139

Closed mamico closed 2 years ago

mamico commented 2 years ago

The not queries produce the same key as the normal queries in the catalog plan, this is a problem because the not queries could generally be slower.

Before this PR, the queries {'UID': '1', path: '/a'} and {'UID': {'not': '1'}, path: '/a'} share the same plan, with the key ('UID', 'path'), after they will have two distinct plan with the keys ('UID', 'path') and (('UID', 'not'), 'path').

ale-rt commented 2 years ago

Disclaimer: I am not competent at all to judge on this PR, so do not expect a review from me :).

The only thing I can tell is that I am not a super fan of mixing key types (in this case string and tuples). It might backfire for whatever reason.

Maybe changing the key from ("UID", "not") to something like "!UID", "UID:not", "UID|not" or whatever might be better. I would not pick the $NAME_not option because I am pretty sure in some portal catalog in this world there is an index whose name ends with _not ;)

Feel free to discard my comment.

mamico commented 2 years ago

The only thing I can tell is that I am not a super fan of mixing key types (in this case string and tuples). It might backfire for whatever reason.

Good point ... but I've followed the same implementation (tuple with name and query) already used for valueindexes see https://github.com/zopefoundation/Products.ZCatalog/blob/master/src/Products/ZCatalog/plan.py#L246