Skip to content

Issue 5772 - ONE LEVEL search fails to return sub-suffixes #6219

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

Merged
merged 1 commit into from
Jun 13, 2024

Conversation

progier389
Copy link
Contributor

@progier389 progier389 commented Jun 11, 2024

Problem: ONE LEVEL scoped search fails to return sub-suffixes entries
Reason: When such search is done, a one level search is done on the main suffix and base search are done on any matching sub-suffix. But main suffix is processed search (to ensure that parent entries are returned before children ones when searching subtree) and ldbm_back_search change the filter to (&(parentid=xxx)old_filter) so the filter test reject the entry on the sub-suffixes.
Solution: Revert the backend list when doing one level search so that the sub-suffixes are processed first
and restore the base dn for the main suffix.
Alternative rejected: reset the filter when discivering a sub-suffix. Not so easy because filter is altered by the rewriteres.
And systematic duplication is an useless overhead if there is no matching sub-suffixes (which is the usual case)

Issue: #5772

Reviewed by: @tbordaz, @droideck (Thanks!)

Copy link
Contributor

@tbordaz tbordaz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch and nice fix.

Patch looks valid. The other approach, to not craft the filter with 'parentid', what if in build_candidate_list we are crafting the filter only if the back_entry is equal to the base search in the pblock ?

@progier389
Copy link
Contributor Author

Not sure I fully understand what you mean.
But I suspect it could lead to performance degradation in some cases:
If we craft the filter, we are back at the original problem of order
If we do not craft it, either we do not use the parentid index (but it will be inefficient for
ldapsearch -b basedn -s one (objectclass=*))
Or we do generate the parentid candidate list and merge explicitly it with the filter list but this time
ldapsearch -b basedn -s one (uid=foo)) will be slow if the basedn tree is flat
With crafted filter, the filter optimization mechanism will compute the filter efficiently in both case (starting with parentid in first case and uid in second case)

@tbordaz
Copy link
Contributor

tbordaz commented Jun 12, 2024

Not sure I fully understand what you mean. But I suspect it could lead to performance degradation in some cases: If we craft the filter, we are back at the original problem of order If we do not craft it, either we do not use the parentid index (but it will be inefficient for ldapsearch -b basedn -s one (objectclass=*)) Or we do generate the parentid candidate list and merge explicitly it with the filter list but this time ldapsearch -b basedn -s one (uid=foo)) will be slow if the basedn tree is flat With crafted filter, the filter optimization mechanism will compute the filter efficiently in both case (starting with parentid in first case and uid in second case)

Sorry I was confusing. I was wondering if something like this could help

+        slapi_pblock_get(pb, SLAPI_SEARCH_TARGET_SDN, &target_sdn);
+        if (slapi_sdn_compare(slapi_entry_get_sdn_const(e->ep_entry), target_sdn) == 0) {
+            filter_exec = create_onelevel_filter(filter, e, managedsait);
+        } else {
+            filter_exec = filter;
+        }

@progier389
Copy link
Contributor Author

There is no need to compare the sdn: if build_candidate_list scope is LDAP_SCOPE_ONELEVEL that mean that it is the
main backend (so slapi_sdn_compare will be true).
(For the sub-suffix backends the scope is LDAP_SCOPE_BASE
The problem is slapi_pblock_set(pb, SLAPI_SEARCH_FILTER, filter_exec); that modify the global pblock (and impact next backends processing)
the alternative is to reset SLAPI_SEARCH_FILTER to the SLAPI_SEARCH_FILTER_INTENDED before calling the subsuffix (but I am a bit wary because I remember that freeing the filter is a bit tricky)

Copy link
Member

@droideck droideck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rest looks good!

@progier389
Copy link
Contributor Author

Fixed the test case description

@progier389 progier389 merged commit 407bdaa into 389ds:main Jun 13, 2024
9 checks passed
progier389 added a commit that referenced this pull request Jun 13, 2024
Problem: ONE LEVEL scoped search fails to return sub-suffixes entries
Reason: When such search is done, a one level search is done on the main suffix and base search are done on any matching sub-suffix. But main suffix is processed search (to ensure that parent entries are returned before children ones when searching subtree) and ldbm_back_search change the filter to (&(parentid=xxx)old_filter) so the filter test reject the entry on the sub-suffixes.
Solution: Revert the backend list when doing one level search so that the sub-suffixes are processed first
and restore the base dn for the main suffix.
Alternative rejected: reset the filter when discivering a sub-suffix. Not so easy because filter is altered by the rewriteres.
And systematic duplication is an useless overhead if there is no matching sub-suffixes (which is the usual case)

Issue: #5772

Reviewed by: @tbordaz, @droideck (Thanks!)

(cherry picked from commit 407bdaa)
progier389 added a commit that referenced this pull request Jun 13, 2024
Problem: ONE LEVEL scoped search fails to return sub-suffixes entries
Reason: When such search is done, a one level search is done on the main suffix and base search are done on any matching sub-suffix. But main suffix is processed search (to ensure that parent entries are returned before children ones when searching subtree) and ldbm_back_search change the filter to (&(parentid=xxx)old_filter) so the filter test reject the entry on the sub-suffixes.
Solution: Revert the backend list when doing one level search so that the sub-suffixes are processed first
and restore the base dn for the main suffix.
Alternative rejected: reset the filter when discivering a sub-suffix. Not so easy because filter is altered by the rewriteres.
And systematic duplication is an useless overhead if there is no matching sub-suffixes (which is the usual case)

Issue: #5772

Reviewed by: @tbordaz, @droideck (Thanks!)

(cherry picked from commit 407bdaa)
progier389 added a commit that referenced this pull request Jun 18, 2024
Problem: ONE LEVEL scoped search fails to return sub-suffixes entries
Reason: When such search is done, a one level search is done on the main suffix and base search are done on any matching sub-suffix. But main suffix is processed search (to ensure that parent entries are returned before children ones when searching subtree) and ldbm_back_search change the filter to (&(parentid=xxx)old_filter) so the filter test reject the entry on the sub-suffixes.
Solution: Revert the backend list when doing one level search so that the sub-suffixes are processed first
and restore the base dn for the main suffix.
Alternative rejected: reset the filter when discivering a sub-suffix. Not so easy because filter is altered by the rewriteres.
And systematic duplication is an useless overhead if there is no matching sub-suffixes (which is the usual case)

Issue: #5772

Reviewed by: @tbordaz, @droideck (Thanks!)

(cherry picked from commit 407bdaa)
@progier389
Copy link
Contributor Author

d261ea2..407bdaa main --> main
baf4fd4..33e99f9 389-ds-base-3.0 --> 389-ds-base-3.0
c9d1829..dc0d818 389-ds-base-2.5 --> 389-ds-base-2.5
f27f154..9501c34 389-ds-base-2.4 --> 389-ds-base-2.4

@progier389 progier389 deleted the i5772 branch May 20, 2025 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants