Wrong Expected Outcome for Sort with Duplicate Elements

[cmoesel]

The test CqlListOperatorsTest.Sort.simpleSortAsc specifies the following CQL:

({4, 5, 1, 6, 2, 1}) sL sort asc

and expected result {1, 1, 2, 4, 5, 6}.

Similarly, the test CqlListOperatorsTest.Sort.simpleSortDesc specifies the following CQL:

({4, 5, 1, 6, 2, 1}) sL sort desc

and expected result {6, 5, 4, 2, 1, 1}.

These are both incorrect since the CQL spec says:

By default, a query returns a list of distinct results, suppressing duplicates.

As such, the expected outputs should be {1, 2, 4, 5, 6} and {6, 5, 4, 2, 1} respectively. Either updated the expected outcomes or add the all keyword to the queries so that duplicates are not suppressed.

[raleigh-g-thompson]

Agree with your analysis for the CQL spec and that the test is incorrect.

I'll modify the test to remove the duplicates. I'll also add sorting tests using the all keyword.

[raleigh-g-thompson]

@cmoesel On further review, I'm not convinced that sort by itself should return distinct values.

The logical spec has a section about sorting that states
The Sort operator returns a list with all the elements in source, sorted as described by the by element.

The shaping link you reference above, https://cql.hl7.org/02-authorsguide.html#shaping, applies to the return clause. I agree a return clause should result in distinct values.

The tests are using just the $cql operation, so I believe the return clause doesn't apply to these tests.

From this point of view, the tests appear to be correct.

If a user of sort by itself wants unique values, then the distinct qualifier must be used.
distinct ({4, 5, 1, 6, 2, 1}) sL sort desc

[cmoesel]

@raleigh-g-thompson - It's the default behavior of the query itself to remove duplicates (not the sort). The CQL spec says:

By default, a query returns a list of distinct results, suppressing duplicates.

Although that's in the "shaping" part of the author's guide, it's talking about the default behavior of queries in general. That said the Developer's Guide states it more clearly:

In addition, the default for return clauses is distinct, as opposed to all, so if no return clause is specified, duplicates will be eliminated from the result.

That above quote from the developer's guide happens to be in the multi-source queries section of the spec, but again, it's intended to apply to all queries.

In the specific test this issue is discussing, ({4, 5, 1, 6, 2, 1}) sL sort desc is a query. We know it is a query because it has a query alias sL. So... the default behavior is to remove duplicates, as specified above.

Feel free to reach out to @brynrhodes if you want to double-check this interpretation of the spec.

[brynrhodes]

Agree with @cmoesel here, and I think the spec is clear as well:

By default, a query returns a list of distinct results, suppressing duplicates

This applies whether there is an explicit return clause or not. In other words, queries always have a "return", authors just don't have to say it if they just want the default behavior.

If you think there is a need for clarification in the spec on this point, I'm happy to try to add that, but please suggest language and a location in the spec where it should be.

So yes, @raleigh-g-thompson , agree with your original take on this, we should update the current tests to remove the duplicates, and add tests that include an all.

[brynrhodes]

@cmoesel , digging even further into this, while I agree that the spec was clear on this point, at least the Java engine did not implement things this way. Rather, the Java engine only eliminates duplicates if the return clause is present and either doesn't include a qualifier or includes a qualifier of distinct.

Does the JavaScript engine follow the spec here and eliminate duplicates from a query result even if a return clause is not present?

[cmoesel]

@brynrhodes - Yes, the JavaScript engine always removes duplicates from a query result unless the query specifies to return all. I found this issue in the tests because the JavaScript engine removed duplicates and thus failed the expectation.