mihaibudiu commented on code in PR #5305:
URL: https://github.com/apache/calcite/pull/5305#discussion_r4200198078
##########
core/src/test/resources/sql/row-equality.iq:
##########
@@ -612,4 +612,93 @@ ORDER BY c;
!ok
+# [CALCITE-7836] Array functions compare ROW elements by field values.
+# Construct equal rows from separate input columns so constant folding
+# and expression reuse do not mask reference equality bugs.
+!use sparkfunc
+
+# Flat ROW values.
+SELECT ARRAY_CONTAINS(ARRAY[a], b) AS contains_row,
Review Comment:
Did you run this on another engine to validate results?
If yes, please mention it in a comment.
##########
core/src/test/resources/sql/row-equality.iq:
##########
@@ -612,4 +612,93 @@ ORDER BY c;
!ok
+# [CALCITE-7836] Array functions compare ROW elements by field values.
+# Construct equal rows from separate input columns so constant folding
+# and expression reuse do not mask reference equality bugs.
+!use sparkfunc
+
+# Flat ROW values.
+SELECT ARRAY_CONTAINS(ARRAY[a], b) AS contains_row,
Review Comment:
These are simple tests, but it's still not easy for a human to check.
##########
core/src/test/resources/sql/row-equality.iq:
##########
@@ -612,4 +612,93 @@ ORDER BY c;
!ok
+# [CALCITE-7836] Array functions compare ROW elements by field values.
Review Comment:
please use the same title as the JIRA issue.
##########
linq4j/src/main/java/org/apache/calcite/linq4j/function/Functions.java:
##########
@@ -527,115 +527,116 @@ private static class DeepEqualityComparer
@Override public int hashCode(@Nullable Object t) {
return deepHashCode(t);
}
+ }
- private static boolean deepEquals(@Nullable Object v1, @Nullable Object
v2) {
- if (v1 == v2) {
- return true;
- }
- if (v1 == null || v2 == null) {
- return false;
- }
- // Normalize both to List: an ARRAY of ROW is a List of Object[],
- // and each element is normalized in turn.
- final @Nullable List<?> list1 = asListOrNull(v1);
- final @Nullable List<?> list2 = asListOrNull(v2);
- if (list1 != null && list2 != null) {
- final int n = list1.size();
- if (n != list2.size()) {
- return false;
- }
- for (int i = 0; i < n; i++) {
- if (!deepEquals(list1.get(i), list2.get(i))) {
- return false;
- }
- }
- return true;
- }
- if (list1 != null || list2 != null) {
- return false;
- }
- if (v1 instanceof Map && v2 instanceof Map) {
- return mapDeepEquals((Map<?, ?>) v1, (Map<?, ?>) v2);
- }
- if (v1.getClass().isArray() && v2.getClass().isArray()) {
- // Primitive arrays (e.g. byte[] for BINARY values).
- return Arrays.deepEquals(new Object[] {v1}, new Object[] {v2});
- }
- return v1.equals(v2);
+ /** Compares values recursively; see {@link #deepComparer()}. */
+ public static boolean deepEquals(@Nullable Object v1, @Nullable Object v2) {
+ if (v1 == v2) {
+ return true;
}
-
- /** Compares two maps as unordered sets of entries, comparing keys and
- * values deeply.
- *
- * <p>Java {@link Map#equals} is already order-independent, but it looks a
- * key up by that key's own hashCode and equals, which matches a struct key
- * only by reference; hence the scan. */
- private static boolean mapDeepEquals(Map<?, ?> m1, Map<?, ?> m2) {
- if (m1.size() != m2.size()) {
+ if (v1 == null || v2 == null) {
+ return false;
+ }
+ // Normalize both to List: an ARRAY of ROW is a List of Object[],
+ // and each element is normalized in turn.
+ final @Nullable List<?> list1 = asListOrNull(v1);
+ final @Nullable List<?> list2 = asListOrNull(v2);
+ if (list1 != null && list2 != null) {
+ final int n = list1.size();
+ if (n != list2.size()) {
return false;
}
- // Remove on match, so that keys that are deep-equal but distinct to
- // Java, as two Object[] with the same contents are, pair up one to one.
- final List<Map.Entry<?, ?>> unmatched = new ArrayList<>(m2.entrySet());
- for (Map.Entry<?, ?> e1 : m1.entrySet()) {
- boolean found = false;
- for (int i = 0; i < unmatched.size(); i++) {
- final Map.Entry<?, ?> e2 = unmatched.get(i);
- if (deepEquals(e1.getKey(), e2.getKey())
- && deepEquals(e1.getValue(), e2.getValue())) {
- unmatched.remove(i);
- found = true;
- break;
- }
- }
- if (!found) {
+ for (int i = 0; i < n; i++) {
+ if (!deepEquals(list1.get(i), list2.get(i))) {
return false;
}
}
return true;
}
+ if (list1 != null || list2 != null) {
+ return false;
+ }
+ if (v1 instanceof Map && v2 instanceof Map) {
+ return mapDeepEquals((Map<?, ?>) v1, (Map<?, ?>) v2);
+ }
+ if (v1.getClass().isArray() && v2.getClass().isArray()) {
+ // Primitive arrays (e.g. byte[] for BINARY values).
+ return Arrays.deepEquals(new Object[] {v1}, new Object[] {v2});
+ }
+ return v1.equals(v2);
+ }
- /** Computes a hash code that is equal for values that
- * {@link #deepEquals} considers equal; in particular, an
- * {@code Object[]} and a {@link List} with equal elements hash alike. */
- private static int deepHashCode(@Nullable Object o) {
- if (o == null) {
- return 0x789d;
- }
- final @Nullable List<?> list = asListOrNull(o);
- if (list != null) {
- int h = 1;
- for (Object element : list) {
- h = 31 * h + deepHashCode(element);
- }
- return h;
- }
- if (o instanceof Map) {
- // Sum of per-entry hashes, as Map.hashCode does, so that the hash
- // ignores entry order just as mapDeepEquals does.
- int h = 0;
- for (Map.Entry<?, ?> e : ((Map<?, ?>) o).entrySet()) {
- h += deepHashCode(e.getKey()) ^ deepHashCode(e.getValue());
+ /** Compares two maps as unordered sets of entries, comparing keys and
+ * values deeply.
+ *
+ * <p>Java {@link Map#equals} is already order-independent, but it looks a
+ * key up by that key's own hashCode and equals, which matches a struct key
+ * only by reference; hence the scan. */
+ private static boolean mapDeepEquals(Map<?, ?> m1, Map<?, ?> m2) {
+ if (m1.size() != m2.size()) {
+ return false;
+ }
+ // Remove on match, so that keys that are deep-equal but distinct to
+ // Java, as two Object[] with the same contents are, pair up one to one.
+ final List<Map.Entry<?, ?>> unmatched = new ArrayList<>(m2.entrySet());
+ for (Map.Entry<?, ?> e1 : m1.entrySet()) {
+ boolean found = false;
+ for (int i = 0; i < unmatched.size(); i++) {
Review Comment:
this looks expensive. Isn't it more efficient if you iterate on the list in
the outside loop?
--
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]
