abhishekrb19 commented on code in PR #18169:
URL: https://github.com/apache/druid/pull/18169#discussion_r2162774283
##########
processing/src/main/java/org/apache/druid/segment/nested/StructuredData.java:
##########
@@ -46,7 +47,7 @@ public class StructuredData implements
Comparable<StructuredData>
private static long computeHash(StructuredData data)
{
try {
- final byte[] bytes =
ColumnSerializerUtils.SMILE_MAPPER.writeValueAsBytes(data.value);
+ final byte[] bytes =
ColumnSerializerUtils.SMILE_MAPPER.writer(SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS).writeValueAsBytes(data.value);
Review Comment:
Could you define this as a static final constant `ObjectWriter` in this
class so it can be reused if needed instead of instantiating a new object?
Also, please include a comment on why this option is needed.
##########
processing/src/test/java/org/apache/druid/segment/nested/StructuredDataTest.java:
##########
@@ -95,6 +95,18 @@ public void testCompareTo()
}
+ @Test
+ public void testCompareToWithDifferentJSONOrder()
Review Comment:
Since this change introduces a subtle behavioral difference in how JSON
columns are ingested or queried, could we also include a SQL query test? I
think one option could be to add some rows with similar values we have here to
a new column in `CalciteNestedDataQueryTest` and run a `GROUP BY` on the added
column. Prior to the sorting behavior in this patch, the `GROUP BY` results
would differ but now the results would be more inline with mysql with sorted
keys?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
