-
Notifications
You must be signed in to change notification settings - Fork 1
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
catalog query with sort_on="start" gives wrong order #1
Comments
Hm.... the IISet as return value seem like a bug to me. Since the DateRecurringIndex should be a drop-in replacement of DateIndex, the return values shouldn't defer. I have to dig into this. But right away, I do not have an answer to this. |
Regarding IISets: We probably should use IITreeSet instead to support get support for queryplan and the like. See Hannos answer in the recurring event index discussion: """ |
+1 for using IITreeSets. At the time I used the IISets here, there was no queryplan written and most other Indexes used IISets too. |
It appears expedient to store a tuple for reverse/unindex entry values instead of an IISet, it seems to solve the sorting problem (FWIW, KeywordIndex uses a list). In the one case it appears needed, the tuple value is cast/iterated into an IISet local at runtime. No new tests, but no regressions either. |
I am going to close this, if anyone sees need to re-open, do so and ping me. |
Added note to changelog (docs/HISTORY.rst) explaining that any user who wants the added sorting functionality working should reindex the index see commit 0428f11 |
Okay, so while this is solved, reindexing is not sufficient for migrating existing sites... there either needs to be a removal and re-add of each index using this index type, or something programmatic -- I did this in a debug session, but maybe this needs a more formal solution -- or at least better documentation of this migration quirk: >>> from zope.component.hooks import setSite
>>> import transaction
>>> site = app.Plone # use your sitename here
>>> setSite(p)
>>> app._p_jar.sync()
>>> rev_start = site.portal_catalog._catalog.indexes['start']._unindex
>>> rev_start = site.portal_catalog._catalog.indexes['start']._unindex
>>> rev_end = site.portal_catalog._catalog.indexes['end']._unindex
>>> for k,v in rev_end.items():
... rev_end[k] = tuple(v)
...
>>> for k,v in rev_start.items():
... rev_start[k] = tuple(v)
...
>>> txn = transaction.get()
>>> txn.note('/'.join(site.portal_catalog.getPhysicalPath()))
>>> txn.note('Reindexed start and end indexes (replaced reverse index values programmatically)')
>>> txn.commit() |
How should it even be possible to sort this, when Catalog and Index have no information about query params. Example: get all events with an end date in the future, sort by start date. It will put recurring events on top of the list when the first occurrence of the event was before the start date of a regular event, no matter what. |
I know two places where you have sort_on="start" in a catalog query in Plone, in events and calendar portlets.
I have a events portlet in a production site with start index of type DateRecurringIndex.
I randomly see the events in the wrong order.
My guess is that the result.sort() line at line 738 in Products/ZCatalog/Catalog.py
doesn't work with the values from DateRecurringIndex._unindex which are IISet.
With DateIndex, result contains:
with DateRecurringIndex, result contains:
I think the comparison of two IISet is undefined. I didn't find any cmp in the code.
The solution is maybe to implement the documentToKeyMap method and return a fake object implementing getitem which return the first element of the sorted IISet...
ah no, that's not right if you have recurrence. I really don't know.
For now, I simply monkey patch the events portlet renderer to sort the date a posteriori.
The text was updated successfully, but these errors were encountered: