[
https://issues.apache.org/jira/browse/CALCITE-7557?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Ruiqi Dong updated CALCITE-7557:
--------------------------------
Priority: Major (was: Minor)
> 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
> Priority: Major
>
> *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)
