Skip to content

Commit ae42f77

Browse files
xiedeyantuxiedeyantu
committed
[CALCITE-4813] ANY_VALUE assumes that arguments should be comparable
1 parent 3eac1f7 commit ae42f77

File tree

3 files changed

+191
-23
lines changed

3 files changed

+191
-23
lines changed

‎core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java‎

Lines changed: 54 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2242,7 +2242,11 @@ public static boolean lt(List<?> b0, List<?> b1) {
22422242
return Functions.compareLists(b0, b1) < 0;
22432243
}
22442244

2245-
public static boolean lt(Object[] b0, Object[] b1) {
2245+
public static boolean lt(Map<?, ?> b0, Map<?, ?> b1) {
2246+
return Functions.compareMaps(b0, b1) < 0;
2247+
}
2248+
2249+
public static boolean lt(@Nullable Object @Nullable [] b0, @Nullable Object @Nullable [] b1) {
22462250
return Functions.compareObjectArrays(b0, b1) < 0;
22472251
}
22482252

@@ -2292,7 +2296,7 @@ public static boolean le(List<?> b0, List<?> b1) {
22922296
}
22932297

22942298
/** SQL <code>&le;</code> operator applied to Object[] values. */
2295-
public static boolean le(Object[] b0, Object[] b1) {
2299+
public static boolean le(@Nullable Object @Nullable [] b0, @Nullable Object @Nullable [] b1) {
22962300
return Functions.compareObjectArrays(b0, b1) <= 0;
22972301
}
22982302

@@ -2375,7 +2379,11 @@ public static boolean gt(List<?> b0, List<?> b1) {
23752379
return Functions.compareLists(b0, b1) > 0;
23762380
}
23772381

2378-
public static boolean gt(Object[] b0, Object[] b1) {
2382+
public static boolean gt(Map<?, ?> b0, Map<?, ?> b1) {
2383+
return Functions.compareMaps(b0, b1) > 0;
2384+
}
2385+
2386+
public static boolean gt(@Nullable Object @Nullable [] b0, @Nullable Object @Nullable [] b1) {
23792387
return Functions.compareObjectArrays(b0, b1) > 0;
23802388
}
23812389

@@ -2426,7 +2434,7 @@ public static boolean ge(List<?> b0, List<?> b1) {
24262434
}
24272435

24282436
/** SQL <code>&ge;</code> operator applied to Object[] values. */
2429-
public static boolean ge(Object[] b0, Object[] b1) {
2437+
public static boolean ge(@Nullable Object @Nullable [] b0, @Nullable Object @Nullable [] b1) {
24302438
return Functions.compareObjectArrays(b0, b1) >= 0;
24312439
}
24322440

@@ -4641,8 +4649,27 @@ public static double lesser(double b0, double b1) {
46414649
return b0 > b1 ? b1 : b0;
46424650
}
46434651

4644-
public static @Nullable <T extends Comparable<T>> List<T> lesser(
4645-
@Nullable List<T> b0, @Nullable List<T> b1) {
4652+
public static @Nullable List lesser(@Nullable List b0, @Nullable List b1) {
4653+
if (b0 == null) {
4654+
return b1;
4655+
}
4656+
if (b1 == null) {
4657+
return b0;
4658+
}
4659+
return lt(b0, b1) ? b0 : b1;
4660+
}
4661+
4662+
public static @Nullable Map lesser(@Nullable Map b0, @Nullable Map b1) {
4663+
if (b0 == null) {
4664+
return b1;
4665+
}
4666+
if (b1 == null) {
4667+
return b0;
4668+
}
4669+
return lt(b0, b1) ? b0 : b1;
4670+
}
4671+
4672+
public static @Nullable Object[] lesser(@Nullable Object[] b0, @Nullable Object[] b1) {
46464673
if (b0 == null) {
46474674
return b1;
46484675
}
@@ -4652,8 +4679,27 @@ public static double lesser(double b0, double b1) {
46524679
return lt(b0, b1) ? b0 : b1;
46534680
}
46544681

4655-
public static @Nullable <T extends Comparable<T>> List<T> greater(
4656-
@Nullable List<T> b0, @Nullable List<T> b1) {
4682+
public static @Nullable List greater(@Nullable List b0, @Nullable List b1) {
4683+
if (b0 == null) {
4684+
return b1;
4685+
}
4686+
if (b1 == null) {
4687+
return b0;
4688+
}
4689+
return gt(b0, b1) ? b0 : b1;
4690+
}
4691+
4692+
public static @Nullable Map greater(@Nullable Map b0, @Nullable Map b1) {
4693+
if (b0 == null) {
4694+
return b1;
4695+
}
4696+
if (b1 == null) {
4697+
return b0;
4698+
}
4699+
return gt(b0, b1) ? b0 : b1;
4700+
}
4701+
4702+
public static @Nullable Object[] greater(@Nullable Object[] b0, @Nullable Object[] b1) {
46574703
if (b0 == null) {
46584704
return b1;
46594705
}

‎core/src/test/resources/sql/blank.iq‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,4 +154,81 @@ select * from table1 where j not in (select i from table2) or j = 3;
154154

155155
!ok
156156

157+
# [CALCITE-4813] ANY_VALUE assumes that arguments should be comparable
158+
select any_value(r) over(), s from(select array[f, s] r, s from (select 1 as f, 2 as s) t) t;
159+
+--------+---+
160+
| EXPR$0 | S |
161+
+--------+---+
162+
| [1, 2] | 2 |
163+
+--------+---+
164+
(1 row)
165+
166+
!ok
167+
168+
select any_value(r) over(), s from(select map[f, s] r, s from (select 1 as f, 2 as s) t) t;
169+
+--------+---+
170+
| EXPR$0 | S |
171+
+--------+---+
172+
| {1=2} | 2 |
173+
+--------+---+
174+
(1 row)
175+
176+
!ok
177+
178+
select any_value(r) over(), s from(select row(f, s) r, s from (select 1 as f, 2 as s) t) t;
179+
+--------+---+
180+
| EXPR$0 | S |
181+
+--------+---+
182+
| {1, 2} | 2 |
183+
+--------+---+
184+
(1 row)
185+
186+
!ok
187+
188+
189+
CREATE TABLE complex_t (
190+
a INTEGER ARRAY,
191+
m MAP<VARCHAR, DOUBLE>,
192+
r ROW(r1 VARCHAR, r2 INTEGER, r3 VARCHAR)
193+
);
194+
(0 rows modified)
195+
196+
!update
197+
198+
INSERT INTO complex_t VALUES (
199+
ARRAY[1, 2, 3, 4, 5],
200+
MAP['math', 95.5, 'science', 88.0, 'english', 92.3],
201+
ROW('Alice Johnson', 30, 'a')
202+
),
203+
(
204+
ARRAY[10, 20, 30, 40, 50, 60],
205+
MAP['physics', 96.2, 'chemistry', 91.8, 'biology', 89.5, 'computer_science', 98.7],
206+
ROW('Bob Smith', 25, 'b')
207+
),
208+
(
209+
ARRAY[100, 200, 300],
210+
MAP['leadership', 88.9, 'teamwork', 94.2, 'communication', 91.5, 'problem_solving', 97.8],
211+
ROW('Charlie Chen', 35, 'c')
212+
);
213+
(3 rows modified)
214+
215+
!update
216+
217+
select
218+
max(a) as max_a,
219+
max(m) as max_m,
220+
max(r) as max_r,
221+
min(a) as min_a,
222+
min(m) as min_m,
223+
min(r) as min_r
224+
from complex_t;
225+
+-----------------+----------------------------------------------------------------------------------------------+-----------------------+-----------------+------------------------------------------------------------------------------------------+------------------------+
226+
| MAX_A | MAX_M | MAX_R | MIN_A | MIN_M | MIN_R |
227+
+-----------------+----------------------------------------------------------------------------------------------+-----------------------+-----------------+------------------------------------------------------------------------------------------+------------------------+
228+
| [100, 200, 300] | {physics =96.2, chemistry =91.8, biology =89.5, computer_science=98.7} | {Charlie Chen, 35, c} | [1, 2, 3, 4, 5] | {leadership =88.9, teamwork =94.2, communication =91.5, problem_solving=97.8} | {Alice Johnson, 30, a} |
229+
+-----------------+----------------------------------------------------------------------------------------------+-----------------------+-----------------+------------------------------------------------------------------------------------------+------------------------+
230+
(1 row)
231+
232+
!ok
233+
157234
# End blank.iq

‎linq4j/src/main/java/org/apache/calcite/linq4j/function/Functions.java‎

Lines changed: 60 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@
1616
*/
1717
package org.apache.calcite.linq4j.function;
1818

19-
import com.google.common.collect.Lists;
20-
2119
import org.checkerframework.checker.nullness.qual.Nullable;
2220
import org.checkerframework.framework.qual.DefaultQualifier;
2321
import org.checkerframework.framework.qual.TypeUseLocation;
@@ -31,6 +29,7 @@
3129
import java.util.Collections;
3230
import java.util.Comparator;
3331
import java.util.HashMap;
32+
import java.util.Iterator;
3433
import java.util.List;
3534
import java.util.Map;
3635
import java.util.Objects;
@@ -573,9 +572,7 @@ private static class NullsLastComparator
573572
} else if (o1 instanceof List && o2 instanceof List) {
574573
return compareLists((List<?>) o1, (List<?>) o2);
575574
} else if (o1 instanceof Object[] && o2 instanceof Object[]) {
576-
final List<Object> list1 = Lists.newArrayList((Object[]) o1);
577-
final List<Object> list2 = Lists.newArrayList((Object[]) o2);
578-
return compareLists(list1, list2);
575+
return compareObjectArrays((Object[]) o1, (Object[]) o2);
579576
} else {
580577
throw new IllegalArgumentException();
581578
}
@@ -601,16 +598,17 @@ private static class NullsFirstReverseComparator
601598
} else if (o1 instanceof List && o2 instanceof List) {
602599
return -compareLists((List<?>) o1, (List<?>) o2);
603600
} else if (o1 instanceof Object[] && o2 instanceof Object[]) {
604-
final List<Object> list1 = Lists.newArrayList((Object[]) o1);
605-
final List<Object> list2 = Lists.newArrayList((Object[]) o2);
606-
return -compareLists(list1, list2);
601+
return -compareObjectArrays((Object[]) o1, (Object[]) o2);
607602
} else {
608603
throw new IllegalArgumentException();
609604
}
610605
}
611606
}
612607

613608
public static int compareLists(List<?> b0, List<?> b1) {
609+
if (b0 == b1) {
610+
return 0;
611+
}
614612
if (b0.isEmpty() && b1.isEmpty()) {
615613
return 0;
616614
}
@@ -623,10 +621,46 @@ public static int compareLists(List<?> b0, List<?> b1) {
623621
return Integer.compare(b0.size(), b1.size());
624622
}
625623

624+
/**
625+
* Compares two maps.
626+
*
627+
* <p>Since maps in Calcite are implemented using {@link java.util.LinkedHashMap},
628+
* which guarantees insertion order, this method follows DuckDB's behavior by
629+
* comparing entries in order. For each entry, it first compares the key and
630+
* then the value.
631+
*/
632+
public static int compareMaps(Map<?, ?> b0, Map<?, ?> b1) {
633+
if (b0 == b1) {
634+
return 0;
635+
}
636+
final Iterator<? extends Map.Entry<?, ?>> i0 = b0.entrySet().iterator();
637+
final Iterator<? extends Map.Entry<?, ?>> i1 = b1.entrySet().iterator();
638+
while (i0.hasNext() && i1.hasNext()) {
639+
Map.Entry<?, ?> e0 = i0.next();
640+
Map.Entry<?, ?> e1 = i1.next();
641+
int c = compareListItems(e0.getKey(), e1.getKey());
642+
if (c != 0) {
643+
return c;
644+
}
645+
c = compareListItems(e0.getValue(), e1.getValue());
646+
if (c != 0) {
647+
return c;
648+
}
649+
}
650+
if (i0.hasNext()) {
651+
return 1;
652+
}
653+
if (i1.hasNext()) {
654+
return -1;
655+
}
656+
return 0;
657+
}
658+
626659
private static int compareListItems(@Nullable Object item0, @Nullable Object item1) {
627-
if (item0 == null && item1 == null) {
660+
if (item0 == item1) {
628661
return 0;
629-
} else if (item0 == null) {
662+
}
663+
if (item0 == null) {
630664
return 1;
631665
} else if (item1 == null) {
632666
return -1;
@@ -635,21 +669,32 @@ private static int compareListItems(@Nullable Object item0, @Nullable Object ite
635669
final List<?> b0ItemList = (List<?>) item0;
636670
final List<?> b1ItemList = (List<?>) item1;
637671
return compareLists(b0ItemList, b1ItemList);
672+
} else if (item0 instanceof Map && item1 instanceof Map) {
673+
return compareMaps((Map) item0, (Map) item1);
638674
} else if (item0 instanceof Object[] && item1 instanceof Object[]) {
639675
return compareObjectArrays((Object[]) item0, (Object[]) item1);
640676
} else if (item0.getClass().equals(item1.getClass()) && item0 instanceof Comparable<?>) {
641677
final Comparable b0Comparable = (Comparable) item0;
642678
final Comparable b1Comparable = (Comparable) item1;
643679
return b0Comparable.compareTo(b1Comparable);
644680
} else {
645-
throw new IllegalArgumentException("Item types do not match");
681+
throw new IllegalArgumentException("Item types do not match: "
682+
+ item0.getClass() + " vs " + item1.getClass());
646683
}
647684
}
648685

649-
public static int compareObjectArrays(Object[] b0, Object[] b1) {
650-
final List<Object> b0List = Lists.newArrayList(b0);
651-
final List<Object> b1List = Lists.newArrayList(b1);
652-
return compareLists(b0List, b1List);
686+
public static int compareObjectArrays(@Nullable Object @Nullable [] b0,
687+
@Nullable Object @Nullable [] b1) {
688+
if (b0 == b1) {
689+
return 0;
690+
}
691+
if (b0 == null) {
692+
return 1;
693+
}
694+
if (b1 == null) {
695+
return -1;
696+
}
697+
return compareLists(Arrays.asList(b0), Arrays.asList(b1));
653698
}
654699

655700
/** Nulls last reverse comparator. */

0 commit comments

Comments
 (0)