I think the telephone number sorting is wrong, in the telephonenumber syntax spaces and hyphens are insignificant and sorting should be based on the normalized values, so the order should be 12 123 13.

Is the combined sorting for other attrs like -S "sn givenname" working as expected?

Replying to [comment:1 lkrispen]:

I think the telephone number sorting is wrong, in the telephonenumber syntax spaces and hyphens are insignificant and sorting should be based on the normalized values, so the order should be 12 123 13.

Is the combined sorting for other attrs like -S "sn givenname" working as expected?

Sorry for the late reply.

I tried sorting by removing the spaces in telephonenumber. As the "sn" values are same for all the entries, the expected results for sorting with "telephonenumber sn" and "telephonenumber" should be same. But the search results are different for sorting with "telephonenumber sn" and "telephonenumber". For sorting with "telephonenumber sn" the order is 12 13 123 and with "telephonenumber" the order is 12 123 13.

I also checked the combined sorting for attributes sn and givenname. And I have made "sn" values of all the entries same. Here also the order for sorting with "givenname sn" and "givenname" are different.

In the case of ldapsearch sort with two attributes like telephonenumber and sn, the sort control will take first two entries and compare their “telephonenumber” values. If “telephonenumber” values are same then it will compare the “sn” values for the same entries.

When I have gone through the code, I found that for each attribute value type, there are separate functions to compare the values. For example tel_compare function will be called to compare two telephonenumber values and cis_compare function will be called to compare two sn values.

The values of two entries are compared in the function '''sort_attr_compare()''', input to the fumction are the two values to be compared and the compare function (eg.tel_compare).While calling the function sort_attr_compare, '''s->compare_fn''' is passed instead of using the local pointer '''this_one->compare_fn'''. I have provided the file name and the other details below. Can you please verify and give a clarification whether the below problem will create incorrect results in sorting ?

In the file /ldap/servers/slapd/back-ldbm/sort.c

static int compare_entries_sv(ID id_a, ID id_b, sort_spec s,baggage_carrier bc, int error)
{
.
.
for (this_one = (sort_spec_thing)s; this_one ; this_one = this_one->next)
{

/ s – linked list which contains the sort attributes and address of their corresponding compare function in compare_fn
this_one – local pointer to move through the linked list /
.
.
.
result = sort_attr_compare(value_a, value_b, s->compare_fn); / '''this_one->compare_fn''' should come instead of '''s->compare_fn''' /
.
.
}
.
.
}

Any update on this ?

result = sort_attr_compare(value_a, value_b, s->compare_fn); / this_one->compare_fn should come instead of s->compare_fn /

A good catch! You are right. We are using the wrong syntax there...

BTW, the code you mentioned is the implementation for the server side sorting. This command-line you are running does NOT use the server side sorting functionality. This sorting is done in the ldapsearch client code.
$ ldapsearch -h dirsrv12-VirtualBox? -p 389 -b "ou=people,dc=asiapacific,dc=hpqcorp,dc=net" -s one -S "telephonenumber sn" -x "(objectclass=inetorgperson)" sn telephonenumber

To enable the server side sorting, please use the search extension:
$ man ldapsearch
-E [!]ext[=extparam]
"[!]sss=[-]<attr[:OID]>[/[-]<attr[:OID]>...] (server side sorting)
I.e., this command-line would do the job.
$ ldapsearch -h dirsrv12-VirtualBox? -p 389 -b "ou=people,dc=asiapacific,dc=hpqcorp,dc=net" -s one -E "sss=telephonenumber/sn" -x "(objectclass=inetorgperson)" sn telephonenumber

I'm attaching the patch next...

Bug description: In the server side sorting compare function
compare_entries_sv, if multiple attribute types are specified,
the sort spec for each attribute is scanned one by one in the
for loop. In the for loop, instead of using the "current"
spec, the first spec is kept using. If the attribute types
have different syntaxes (e.g., cis, tel), then the first
syntax is unexpectedly selected for the second syntax.

Fix description: This patch correctly uses the current spec
in the for loop.

git patch file (master)
0001-Ticket-543-Sorting-with-attributes-in-ldapsearch-giv.patch

Reviewed by Nathan (Thank you!!)

Pushed to master: commit 95f425d

Ticket has been cloned to Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=918712

In the file /ldap/servers/slapd/back-ldbm/sort.c

The behaviour which was seen in the function '''compare_entries_sv''' also exists in this function '''compare_entries'''. If multiple attribute types are specified, the sort spec for each attribute is scanned one by one in the for loop. In the for loop, instead of using the "current" spec, the first spec is kept using.

Line no: 584 and 587

static int compare_entries(ID id_a, ID id_b, sort_spec s,baggage_carrier bc, int error)
{
.
.
.
for (this_one = (sort_spec_thing)s; this_one ; this_one = this_one->next) {
.
.
.
if (!order) {
result = sort_attr_compare(value_a, value_b, '''s->compare_fn''');
} else {
/ If reverse, invert the sense of the comparison /
result = sort_attr_compare(value_b, value_a, '''s->compare_fn''');
}
.
.
}
.
.
.
}

Can someone verify this ?

In the source code (all the active branches), the function "compare_entries" is in "#if 0" with this comment:
/ USE THE _SV VERSION NOW /

So, the problematic function is not being used. Sorry about the confusion. We could fix the line even in the comment since it could be resurrected some time in the future.

Thank you for the report!

Note: the unused "compare_entries" in "#if 0" was removed to reduce confusion.

Pushed to 389-ds-base-1.2.11:
a682fe5..855d289 389-ds-base-1.2.11 -> 389-ds-base-1.2.11
commit 855d289

Pushed to 389-ds-base-1.3.0:
6d6354f..8356792 389-ds-base-1.3.0 -> 389-ds-base-1.3.0
commit 8356792

1.3.1 and newer already have this fix.

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/543

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.