zopefoundation / Products.ZCatalog

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

update `catalogView.dtml` to changed behavior for "empty searches" #104

Closed d-maurer closed 4 years ago

d-maurer commented 4 years ago

This fixes #102 (and #101).

It also interprets a missing or empty filterpath as "no restriction" (instead of "/"). This allows a potentially available path index to index generalized paths (not necessary starting with "/").

d-maurer commented 4 years ago

Alessandro Pisa wrote at 2020-6-18 08:49 -0700:

@ale-rt commented on this pull request.

@@ -3,8 +3,8 @@

- + Isn't this a performance issue?

This was what effectively happened previously -- before an empty search was interpreted to return nothing (instead of everything).

If the view does not present all results (I am almost sure, it batches), then the exprensive operation is the list. getAllBrains returns an iterator and it is difficult to determine "its" length. The list is essentially there to 1) make available the length and 2) support subscription, both for use by batching.

I have not checked whether the batching used by this view really requires the length. If it does, it is necessary to determine len(self._catalog.data) which can be expensive.

Using LazyMap in a new method similar to getAllBrains would allow to support subscription less costly.

d-maurer commented 4 years ago

Alessandro Pisa wrote at 2020-6-18 08:49 -0700:

@ale-rt commented on this pull request.

@@ -3,8 +3,8 @@

- + Isn't this a performance issue?

I have defined a new method searchAll. It is similar to getAllBrains but returns a data type like the other searches (LazyMap) instead of an interator. I have replaced the expensive list(getAllBrains()) with searchAll().