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]

Reply via email to