mihaibudiu commented on code in PR #5305:
URL: https://github.com/apache/calcite/pull/5305#discussion_r4188879434
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -7334,17 +7343,77 @@ public static List arrayExcept(List list1, List list2) {
/** Support the ARRAY_INTERSECT function. */
public static List arrayIntersect(List list1, List list2) {
- final Set result = new LinkedHashSet<>(list1);
- result.retainAll(list2);
- return new ArrayList<>(result);
+ final Set<@Nullable Object> result = toDeepEqualitySet(list1);
+ result.retainAll(toDeepEqualitySet(list2));
+ return unwrapDeepEqualitySet(result);
}
/** Support the ARRAY_UNION function. */
public static List arrayUnion(List list1, List list2) {
- final Set result = new LinkedHashSet<>();
- result.addAll(list1);
- result.addAll(list2);
- return new ArrayList<>(result);
+ final Set<@Nullable Object> result = toDeepEqualitySet(list1);
+ for (Object element : list2) {
+ result.add(deepKey(element));
+ }
+ return unwrapDeepEqualitySet(result);
+ }
+
+ /** Creates keys in encounter order, retaining the first equal value. */
+ private static Set<@Nullable Object> toDeepEqualitySet(List<?> list) {
+ final Set<@Nullable Object> keys = new LinkedHashSet<>();
+ for (Object value : list) {
+ keys.add(deepKey(value));
+ }
+ return keys;
+ }
+
+ /** Wraps composite values, which may contain ROW values, but leaves scalar
+ * values and null unchanged. */
+ private static @Nullable Object deepKey(@Nullable Object value) {
+ if (value == null) {
+ return null;
+ } else if (value instanceof List || value instanceof Map ||
value.getClass().isArray()) {
+ return new DeepKey(value);
+ }
+ return value;
+ }
+
+ /** Returns the original values, without exposing the internal keys. */
+ private static List<@Nullable Object> unwrapDeepEqualitySet(Set<@Nullable
Object> keys) {
+ final List<@Nullable Object> values = new ArrayList<>(keys.size());
+ for (Object key : keys) {
+ values.add(key instanceof DeepKey ? ((DeepKey) key).value : key);
+ }
+ return values;
+ }
+
+ /** A key for deep equality in array functions. The value must not change
+ * while the key is in use. */
+ private static final class DeepKey {
+ private final Object value;
+ // Benign races may compute the same hash more than once.
+ private int hashCode;
+
+ private DeepKey(Object value) {
+ this.value = value;
+ }
+
+ @Override public boolean equals(@Nullable Object obj) {
+ return this == obj
+ || obj instanceof DeepKey
+ && Functions.deepEquals(value, ((DeepKey) obj).value);
+ }
+
+ @Override public int hashCode() {
+ int hash = hashCode;
+ if (hash == 0) {
Review Comment:
If the hash is 0 this will always recompute.
Could make it an Integer.
I don't know if it's worth.
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -7334,17 +7343,77 @@ public static List arrayExcept(List list1, List list2) {
/** Support the ARRAY_INTERSECT function. */
public static List arrayIntersect(List list1, List list2) {
- final Set result = new LinkedHashSet<>(list1);
- result.retainAll(list2);
- return new ArrayList<>(result);
+ final Set<@Nullable Object> result = toDeepEqualitySet(list1);
+ result.retainAll(toDeepEqualitySet(list2));
+ return unwrapDeepEqualitySet(result);
}
/** Support the ARRAY_UNION function. */
public static List arrayUnion(List list1, List list2) {
- final Set result = new LinkedHashSet<>();
- result.addAll(list1);
- result.addAll(list2);
- return new ArrayList<>(result);
+ final Set<@Nullable Object> result = toDeepEqualitySet(list1);
+ for (Object element : list2) {
+ result.add(deepKey(element));
+ }
+ return unwrapDeepEqualitySet(result);
+ }
+
+ /** Creates keys in encounter order, retaining the first equal value. */
Review Comment:
I am not sure what "keys" are.
This really creates sets.
I would not call the elements of a set "keys".
Why does the order matter?
##########
core/src/test/java/org/apache/calcite/test/JdbcTest.java:
##########
@@ -8261,6 +8261,35 @@ private void
testConvertOracleInternal(CalciteAssert.AssertThat with) {
+ "JavaType(class java.lang.String CHARACTER SET \"GBK\") to
type VARCHAR NOT NULL");
}
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-7836";>[CALCITE-7836]
+ * Array functions return incorrect results for ROW elements</a>. */
+ @Test void testArrayFunctionsWithRows() {
Review Comment:
We prefer quidem tests to jdbc tests (in .iq files), they are much easier to
read and write.
Maybe you can convert this one.
--
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]
