Ruiqi Dong created CALCITE-7557:
-----------------------------------
Summary: Linq4j.ListEnumerable.take(int) / skip(int) diverge from
EnumerableDefaults on negative counts
Key: CALCITE-7557
URL: https://issues.apache.org/jira/browse/CALCITE-7557
Project: Calcite
Issue Type: Bug
Components: linq4j
Affects Versions: 1.41.0
Reporter: Ruiqi Dong
*Summary*
Linq4j.asEnumerable(list) exposes list-specialized take(int) and skip(int)
implementations that are not behaviorally equivalent to the generic
EnumerableDefaults versions for negative counts. For the same input enumerable
and the same negative count: * EnumerableDefaults.take(source, -1) returns an
empty enumerable, but ListEnumerable.take(-1) throws
* EnumerableDefaults.skip(source, -1) returns the original sequence, but
ListEnumerable.skip(-1) throws
This is a clean semantic divergence between the generic and optimized paths of
the same API.
*Affected code*
File: linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java
{code:java}
public static <TSource> Enumerable<TSource> skip(Enumerable<TSource> source,
final int count) {
return skipWhile(source, (v1, v2) -> {
return v2 < count;
});
}
public static <TSource> Enumerable<TSource> take(Enumerable<TSource> source,
final int count) {
return takeWhile(
source, (v1, v2) -> {
return v2 < count;
});
} {code}
File: linq4j/src/main/java/org/apache/calcite/linq4j/Linq4j.java
{code:java}
@Override public Enumerable<T> skip(int count) {
final List<T> list = toList();
if (count >= list.size()) {
return Linq4j.emptyEnumerable();
}
return new ListEnumerable<>(list.subList(count, list.size()));
}
@Override public Enumerable<T> take(int count) {
final List<T> list = toList();
if (count >= list.size()) {
return this;
}
return new ListEnumerable<>(list.subList(0, count));
}{code}
*Reproducer*
Add the following test to
linq4j/src/test/java/org/apache/calcite/linq4j/test/Linq4jTest.java
{code:java}
@Test void testTakeListEnumerableNegativeSize() {
final List<Integer> values = Arrays.asList(1, 2, 3);
assertThat(EnumerableDefaults.take(Linq4j.asEnumerable(values), -1).toList(),
is(empty()));
assertThat(Linq4j.asEnumerable(values).take(-1).toList(), is(empty()));
}
@Test void testSkipListEnumerableNegativeSize() {
final List<Integer> values = Arrays.asList(1, 2, 3);
assertThat(EnumerableDefaults.skip(Linq4j.asEnumerable(values), -1).toList(),
is(values));
assertThat(Linq4j.asEnumerable(values).skip(-1).toList(), is(values));
}{code}
Run:
{code:java}
./gradlew :linq4j:test \
--tests
org.apache.calcite.linq4j.test.Linq4jTest.testTakeListEnumerableNegativeSize \
--tests
org.apache.calcite.linq4j.test.Linq4jTest.testSkipListEnumerableNegativeSize
{code}
Observed behavior:
The optimized list path throws in both methods instead of matching the generic
enumerable semantics
{code:java}
java.lang.IllegalArgumentException: fromIndex(0) > toIndex(-1)
at java.base/java.util.AbstractList.subListRangeCheck(AbstractList.java:509)
at java.base/java.util.AbstractList.subList(AbstractList.java:497)
at org.apache.calcite.linq4j.Linq4j$ListEnumerable.take(Linq4j.java:599)
java.lang.IndexOutOfBoundsException: fromIndex = -1
at java.base/java.util.AbstractList.subListRangeCheck(AbstractList.java:505)
at java.base/java.util.AbstractList.subList(AbstractList.java:497)
at org.apache.calcite.linq4j.Linq4j$ListEnumerable.skip(Linq4j.java:591)
{code}
Expected behavior:
Negative counts should behave the same regardless of whether the source
enumerable happens to be backed by a List. * Negative `take` should return an
empty enumerable, matching EnumerableDefaults.take
* Negative `skip` should return the original sequence, matching
EnumerableDefaults.skip
This is an optimization-path semantic regression. The same logical operation
has two implementations in Calcite, but the list-specialized path is not
behaviorally equivalent to the generic enumerable contract for negative counts.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
