This would be because of our shortcuts in idl and idlset. When you have sn=user8, you get the empty idl, so you return nothing.

But when you have objectClass=tombstone, you would then hit an unindexed case, so this would return with the allids from nstombstone and request a filter test.

Really, the issue is we are trying to hide these entries, but then relying on the same search mechanisms.

What we really need to resolve this is a flag on the filter that says "tombstone true/false", and similar to how we inject a referral clause, we need to inject a !(oc=nstombstone) by default.

This way we can have the same indexes for tombstone and normal objects, and in the normal case we just exclude them. If we have a request for a tombstone, we can just request them as normal.

This is also a reason to mark tombstones as ldapsubentries, because that has the same result for us here - alternately, having them in a hidden suffix (rather than in the main tree).

What we really need to resolve this is a flag on the filter that says "tombstone true/false", and similar to how we inject a referral clause, we need to inject a !(oc=nstombstone) by default.

we have this flag and evalute it:

  (slapi_entry_flag_is_set(e->ep_entry,SLAPI_ENTRY_FLAG_TOMBSTONE) &&
                  ((!isroot && !filter_flag_is_set(filter, SLAPI_FILTER_RUV)) ||
                   !filter_flag_is_set(filter, SLAPI_FILTER_TOMBSTONE))))

so the hiding works. The problem is that when a tombstone is requested we get inconsistent results

This way we can have the same indexes for tombstone and normal objects, and in the normal case we just exclude them. If we have a request for a tombstone, we can just request them as normal.

yes, that was my request in this ticket, we just need to verify we don't break anything

I think having the same indexes for tombstones as normal entries would simplify the code.
Do you have an idea why it was decided to remove the index values from tombstone. If it is only for reducing footprint of tombstones we can offer keeping those values.

I don't know why it was implemented like this, I can only guess:
- maybe a first assumption was that removing them from the indexes would hide tombstones from normal searches. This is true if the ancestorid index is not allids, then scoping would just remove entries from the result list
- maybe it was to avoid includeing ids in idlists which would later be removed.
I think it was a mixture between help in hiding and hope for optimization, but I think the overhead would not be that much and including tombstones in indexes would make tombstone searches correct

My suggestion is to use the flag filter_flag_is_set(filter, SLAPI_FILTER_TOMBSTONE) to inject a search term into the filter (similar to how we handle referrals.

So when you do:

ldapsearch '(&(objectClass=account)(uid=william))'

Right now this is converted to:

(|(objectClass=referral)(&(objectClass=account)(uid=william))

I would propose that we change this to default inject:

(&(!(objectClass=nsTombStone))(|(objectClass=referral)(&(objectClass=account)(uid=william)))

Optimally, we'd change this to:

(&(|(objectClass=referral)(&(objectClass=account)(uid=william))(!(objectClass=nsTombStone)))

```

Because this means if the first (|) term hit test threshold, we never acces the OC index (we apply the regex to assert not nstombstone.

This would really simplify the index code (we just index everything), and would not affect internal searches like uid uniq by default (ie, uid uniq when it checks for (uid=x), it wouldn't matter if a nsTombstone shared uid=x, because we exclude it anyway). It means that the whole logic of what is a tombstone is only known to the front end code, never to the backends, the backends just see "objects".

Finally, my only concern - request is that in the process of this, we look into improving the performance of the (!) filter type, because if we plan to use it more in this style, we should make sure that it's performant. @lkrispen has some comments about this during the idlset patch, so this could be a good time to review and update this.

Does this sound like a reasonable approach to the problem?

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/2414

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.