rishabhdaim commented on code in PR #3075:
URL: https://github.com/apache/jackrabbit-oak/pull/3075#discussion_r3764161091
##########
oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticOrderByTest.java:
##########
@@ -40,4 +47,39 @@ protected void createTestIndexNode() {
setTraversalEnabled(false);
}
+ /**
+ * OAK-12343: Elastic counterpart to the Lucene type=Date limitation.
Elastic sorts missing
+ * (non-date) values last instead of as the epoch, so ORDER BY returns the
valid dates in order
+ * with the non-date value last - it does not mis-order or drop rows the
way Lucene does
+ * (LuceneOrderByTest#orderByNonDateValuesWithDateTypeIsIncorrect).
+ *
+ * TODO(OAK-12344): if Lucene is aligned with this, this becomes the
shared expectation.
+ */
+ @Test
+ public void orderByNonDateValuesWithDateTypeSortsMissingLast() throws
Exception {
+ IndexDefinitionBuilder builder = createIndexDefinitionBuilder();
Review Comment:
Move the duplicated cross-backend fixture into OrderByCommonTest.
These lines duplicate the Lucene fixture, while three pass-through methods
were added to OrderByCommonTest solely to support both copies. If either
fixture changes during OAK-12344, the tests may stop comparing identical inputs
and conceal backend drift.
Please add a protected shared helper in OrderByCommonTest containing the
common index, content, commit, query, and assertion setup using indexOptions
directly. Have each backend test call it with its backend-specific expected
order, then remove the three pass-through methods and unused imports.
##########
oak-doc/src/site/markdown/query/lucene.md:
##########
@@ -503,6 +503,13 @@ type
Mostly inferred from the indexed value. However in some cases where same
property
type is not used consistently across various nodes then it would recommended
to specify the type explicitly.
+
+ Note: with `type=Date`, values that are not valid ISO-8601 dates are not
indexed as dates, so
Review Comment:
The note conflates sorting failures with result filtering, making the
failure mode and workaround difficult to understand.
The phrase 'if the query relies on that property to select nodes' suggests
that ORDER BY itself filters rows, while the actual distinction is between
sorting and using the property in a constraint. Readers may also miss which
workaround applies to which behavior.
Please replace the note with: 'Note: with `type=Date`, values that are not
valid ISO-8601 dates are omitted from the date sort field. As a result, `order
by` can produce an incorrect order; a query that uses the property in a
constraint can also return no matching nodes. For inconsistent values, omit
`type` or use `type=String`. String values sort lexicographically and match
chronological order only when timestamps use a uniform format, precision, and
zone (for example, UTC `Z`).'
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]