This is an automated email from the ASF dual-hosted git repository.
nizhikov pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ignite.git
The following commit(s) were added to refs/heads/master by this push:
new 5a3af716aed IGNITE-23515 Fix assertion error on extra column in H2
ResultInterface#currentRow (#11613)
5a3af716aed is described below
commit 5a3af716aede987ce45bee3f167fab3548c0e1c9
Author: Nikolay <[email protected]>
AuthorDate: Fri Oct 25 11:46:05 2024 +0300
IGNITE-23515 Fix assertion error on extra column in H2
ResultInterface#currentRow (#11613)
---
.../client/cache/ClientCacheFieldsQueryCursor.java | 9 +-
.../messages/GridQueryNextPageResponse.java | 8 +-
.../apache/ignite/platform/model/Department.java | 10 ++
.../processors/query/h2/opt/GridH2ProxyIndex.java | 2 +-
.../processors/query/h2/opt/H2PlainRow.java | 24 +++--
.../processors/query/h2/opt/H2PlainRowFactory.java | 15 ++-
.../query/h2/twostep/AbstractReducer.java | 6 ++
.../query/h2/twostep/GridMapQueryExecutor.java | 2 +-
.../query/h2/twostep/ReduceResultPage.java | 7 ++
.../processors/query/h2/twostep/SortedReducer.java | 2 +-
.../query/h2/twostep/UnsortedOneWayReducer.java | 2 +-
.../query/h2/twostep/UnsortedReducer.java | 2 +-
.../h2/twostep/msg/GridH2ValueMessageFactory.java | 7 +-
.../org/apache/ignite/client/ClientTestSuite.java | 2 +
.../client/thin/ExtraColumnInH2RowsTest.java | 113 +++++++++++++++++++++
15 files changed, 189 insertions(+), 22 deletions(-)
diff --git
a/modules/core/src/main/java/org/apache/ignite/internal/processors/platform/client/cache/ClientCacheFieldsQueryCursor.java
b/modules/core/src/main/java/org/apache/ignite/internal/processors/platform/client/cache/ClientCacheFieldsQueryCursor.java
index 8662e6d0325..81f9c67202d 100644
---
a/modules/core/src/main/java/org/apache/ignite/internal/processors/platform/client/cache/ClientCacheFieldsQueryCursor.java
+++
b/modules/core/src/main/java/org/apache/ignite/internal/processors/platform/client/cache/ClientCacheFieldsQueryCursor.java
@@ -44,9 +44,12 @@ class ClientCacheFieldsQueryCursor extends
ClientCacheQueryCursor<List> {
/** {@inheritDoc} */
@Override void writeEntry(BinaryRawWriterEx writer, List e) {
- assert e.size() == columnCount;
+ assert e.size() >= columnCount : "Column count less then requested: "
+ e.size() + " < " + columnCount;
- for (Object o : e)
- writer.writeObjectDetached(o);
+ // H2 engine can add extra columns at the end of result set.
+ // See, GridH2ValueMessageFactory#toMessages
+ // See ResultInterface#currentRow,
ResultInterface#getVisibleColumnCount
+ for (int i = 0; i < columnCount; i++)
+ writer.writeObjectDetached(e.get(i));
}
}
diff --git
a/modules/core/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/messages/GridQueryNextPageResponse.java
b/modules/core/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/messages/GridQueryNextPageResponse.java
index 5390c583682..59f05dcf904 100644
---
a/modules/core/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/messages/GridQueryNextPageResponse.java
+++
b/modules/core/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/messages/GridQueryNextPageResponse.java
@@ -60,14 +60,18 @@ public class GridQueryNextPageResponse implements Message {
@GridDirectCollection(Message.class)
private Collection<Message> vals;
- /** */
+ /**
+ * Note, columns count in plain row can differ from {@link #cols}.
+ * See {@code
org.apache.ignite.internal.processors.query.h2.twostep.msg.GridH2ValueMessageFactory#toMessages}.
+ * See javadoc for {@code
org.h2.result.ResultInterface#getVisibleColumnCount()} and {@code
org.h2.result.ResultInterface#currentRow()}.
+ */
@GridDirectTransient
private transient Collection<?> plainRows;
/** */
private AffinityTopologyVersion retry;
- /** Retry cause description*/
+ /** Retry cause description. */
private String retryCause;
/** Last page flag. */
diff --git
a/modules/core/src/test/java/org/apache/ignite/platform/model/Department.java
b/modules/core/src/test/java/org/apache/ignite/platform/model/Department.java
index b1f74f3731e..8c05e9a42c5 100644
---
a/modules/core/src/test/java/org/apache/ignite/platform/model/Department.java
+++
b/modules/core/src/test/java/org/apache/ignite/platform/model/Department.java
@@ -22,6 +22,16 @@ public class Department {
/** */
private String name;
+ /** */
+ public Department() {
+ // No-op.
+ }
+
+ /** */
+ public Department(String name) {
+ this.name = name;
+ }
+
/** */
public String getName() {
return name;
diff --git
a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/GridH2ProxyIndex.java
b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/GridH2ProxyIndex.java
index bb72bad37fd..e7cdf8a6084 100644
---
a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/GridH2ProxyIndex.java
+++
b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/GridH2ProxyIndex.java
@@ -191,7 +191,7 @@ public class GridH2ProxyIndex extends H2IndexCostedBase {
copyAliasColumnData(data, QueryUtils.KEY_COL,
desc.getAlternativeColumnId(QueryUtils.KEY_COL));
copyAliasColumnData(data, QueryUtils.VAL_COL,
desc.getAlternativeColumnId(QueryUtils.VAL_COL));
- return H2PlainRowFactory.create(data);
+ return H2PlainRowFactory.create(data.length, data);
}
/**
diff --git
a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/H2PlainRow.java
b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/H2PlainRow.java
index 63700e476f4..da7ac0b58fb 100644
---
a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/H2PlainRow.java
+++
b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/H2PlainRow.java
@@ -17,36 +17,48 @@
package org.apache.ignite.internal.processors.query.h2.opt;
+import java.util.Collection;
+import
org.apache.ignite.internal.processors.query.h2.twostep.msg.GridH2ValueMessageFactory;
import org.apache.ignite.internal.util.tostring.GridToStringInclude;
import org.apache.ignite.internal.util.typedef.internal.S;
+import org.h2.result.ResultInterface;
import org.h2.value.Value;
/**
* Simple array based row.
*/
public class H2PlainRow extends H2Row {
+ /** */
+ private final int colCnt;
+
/** */
@GridToStringInclude
- private Value[] vals;
+ private final Value[] vals;
/**
+ * @param colCnt Column count. H2 engine can add extra columns at the end
of result set.
* @param vals Values.
+ * @see ResultInterface#getVisibleColumnCount()
+ * @see GridH2ValueMessageFactory#toMessages(Collection, int)
+ * @see ResultInterface#currentRow()
*/
@SuppressWarnings("AssignmentOrReturnOfFieldWithMutableType")
- public H2PlainRow(Value[] vals) {
+ public H2PlainRow(int colCnt, Value[] vals) {
+ this.colCnt = colCnt;
this.vals = vals;
}
/**
- * @param len Length.
+ * @param colCnt Length.
*/
- public H2PlainRow(int len) {
- vals = new Value[len];
+ public H2PlainRow(int colCnt) {
+ this.colCnt = colCnt;
+ vals = new Value[colCnt];
}
/** {@inheritDoc} */
@Override public int getColumnCount() {
- return vals.length;
+ return colCnt;
}
/** {@inheritDoc} */
diff --git
a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/H2PlainRowFactory.java
b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/H2PlainRowFactory.java
index 22c1d4ff3d0..c9148b0093f 100644
---
a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/H2PlainRowFactory.java
+++
b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/H2PlainRowFactory.java
@@ -17,6 +17,9 @@
package org.apache.ignite.internal.processors.query.h2.opt;
+import java.util.Collection;
+import
org.apache.ignite.internal.processors.query.h2.twostep.msg.GridH2ValueMessageFactory;
+import org.h2.result.ResultInterface;
import org.h2.result.Row;
import org.h2.result.RowFactory;
import org.h2.value.Value;
@@ -34,11 +37,15 @@ public class H2PlainRowFactory extends RowFactory {
}
/**
+ * @param colCnt Requested column count. H2 engine can add extra columns
at the end of result set.
* @param data Values.
* @return Row.
+ * @see ResultInterface#getVisibleColumnCount()
+ * @see GridH2ValueMessageFactory#toMessages(Collection, int)
+ * @see ResultInterface#currentRow()
*/
- public static Row create(Value... data) {
- switch (data.length) {
+ public static Row create(int colCnt, Value... data) {
+ switch (colCnt) {
case 0:
throw new IllegalStateException("Zero columns row.");
@@ -49,12 +56,12 @@ public class H2PlainRowFactory extends RowFactory {
return new H2PlainRowPair(data[0], data[1]);
default:
- return new H2PlainRow(data);
+ return new H2PlainRow(colCnt, data);
}
}
/** {@inheritDoc} */
@Override public Row createRow(Value[] data, int memory) {
- return create(data);
+ return create(data.length, data);
}
}
diff --git
a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/AbstractReducer.java
b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/AbstractReducer.java
index 18f15c2d3d2..5e1018d2db3 100644
---
a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/AbstractReducer.java
+++
b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/AbstractReducer.java
@@ -92,6 +92,9 @@ public abstract class AbstractReducer implements Reducer {
/** */
private int pageSize;
+ /** */
+ protected int colCnt;
+
/**
* Will be r/w from query execution thread only, does not need to be
threadsafe.
*/
@@ -329,6 +332,9 @@ public abstract class AbstractReducer implements Reducer {
iter = page.rows();
+ if (iter.hasNext() && colCnt == 0)
+ colCnt = page.columnCount();
+
MTC.span().addTag(SQL_PAGE_ROWS, () ->
Integer.toString(page.rowsInPage()));
// The received iterator must be empty in the dummy last page
or on failure.
diff --git
a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/GridMapQueryExecutor.java
b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/GridMapQueryExecutor.java
index 78a0e0aeb96..7bdc767193a 100644
---
a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/GridMapQueryExecutor.java
+++
b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/GridMapQueryExecutor.java
@@ -988,7 +988,7 @@ public class GridMapQueryExecutor {
GridQueryNextPageResponse msg = new
GridQueryNextPageResponse(qr.queryRequestId(), segmentId, qry, page,
page == 0 ? res.rowCount() : -1,
res.columnCount(),
- loc ? null : toMessages(rows, new
ArrayList<>(res.columnCount()), res.columnCount()),
+ loc ? null : toMessages(rows, res.columnCount()),
loc ? rows : null,
last);
diff --git
a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/ReduceResultPage.java
b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/ReduceResultPage.java
index 2f18475159c..ead27d98d95 100644
---
a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/ReduceResultPage.java
+++
b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/ReduceResultPage.java
@@ -169,6 +169,13 @@ public class ReduceResultPage {
return r;
}
+ /**
+ * @return Column count.
+ */
+ public int columnCount() {
+ return res != null ? res.columns() : 0;
+ }
+
/**
* @return Result source node ID.
*/
diff --git
a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/SortedReducer.java
b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/SortedReducer.java
index 722c5bcd73e..4ef51f9906d 100644
---
a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/SortedReducer.java
+++
b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/SortedReducer.java
@@ -403,7 +403,7 @@ public class SortedReducer extends AbstractReducer {
if (!iter.hasNext())
return false;
- cur = H2PlainRowFactory.create(iter.next());
+ cur = H2PlainRowFactory.create(colCnt, iter.next());
return true;
}
diff --git
a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/UnsortedOneWayReducer.java
b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/UnsortedOneWayReducer.java
index c0b9bfb39eb..e8c943b8bbc 100644
---
a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/UnsortedOneWayReducer.java
+++
b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/UnsortedOneWayReducer.java
@@ -61,7 +61,7 @@ public class UnsortedOneWayReducer extends
UnsortedBaseReducer {
}
@Override public Row next() {
- return H2PlainRowFactory.create(iter.next());
+ return H2PlainRowFactory.create(colCnt, iter.next());
}
@Override public void remove() {
diff --git
a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/UnsortedReducer.java
b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/UnsortedReducer.java
index 50db44e2f2c..c19113ee8fa 100644
---
a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/UnsortedReducer.java
+++
b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/UnsortedReducer.java
@@ -53,7 +53,7 @@ public class UnsortedReducer extends UnsortedBaseReducer {
}
@Override public Row next() {
- return H2PlainRowFactory.create(iter.next());
+ return H2PlainRowFactory.create(colCnt, iter.next());
}
@Override public void remove() {
diff --git
a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/msg/GridH2ValueMessageFactory.java
b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/msg/GridH2ValueMessageFactory.java
index 58fa2fc0679..7138271f2e6 100644
---
a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/msg/GridH2ValueMessageFactory.java
+++
b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/twostep/msg/GridH2ValueMessageFactory.java
@@ -17,8 +17,10 @@
package org.apache.ignite.internal.processors.query.h2.twostep.msg;
+import java.util.ArrayList;
import java.util.Collection;
import java.util.Iterator;
+import java.util.List;
import org.apache.ignite.IgniteCheckedException;
import org.apache.ignite.internal.GridKernalContext;
import org.apache.ignite.internal.processors.query.h2.QueryTable;
@@ -72,13 +74,14 @@ public class GridH2ValueMessageFactory implements
MessageFactoryProvider {
/**
* @param src Source values.
- * @param dst Destination collection.
* @param cnt Number of columns to actually send.
* @return Destination collection.
* @throws IgniteCheckedException If failed.
*/
- public static Collection<Message> toMessages(Collection<Value[]> src,
Collection<Message> dst, int cnt)
+ public static Collection<Message> toMessages(Collection<Value[]> src, int
cnt)
throws IgniteCheckedException {
+ List<Message> dst = new ArrayList<>(src.size() * cnt);
+
for (Value[] row : src) {
assert row.length >= cnt;
diff --git
a/modules/indexing/src/test/java/org/apache/ignite/client/ClientTestSuite.java
b/modules/indexing/src/test/java/org/apache/ignite/client/ClientTestSuite.java
index 9329e2377b3..5d43639d31d 100644
---
a/modules/indexing/src/test/java/org/apache/ignite/client/ClientTestSuite.java
+++
b/modules/indexing/src/test/java/org/apache/ignite/client/ClientTestSuite.java
@@ -17,6 +17,7 @@
package org.apache.ignite.client;
+import org.apache.ignite.client.thin.ExtraColumnInH2RowsTest;
import org.apache.ignite.internal.client.thin.AffinityMetricsTest;
import org.apache.ignite.internal.client.thin.AtomicLongTest;
import org.apache.ignite.internal.client.thin.BlockingTxOpsTest;
@@ -98,6 +99,7 @@ import org.junit.runners.Suite;
ClusterGroupClusterRestartTest.class,
BlockingTxOpsTest.class,
InvokeTest.class,
+ ExtraColumnInH2RowsTest.class,
})
public class ClientTestSuite {
// No-op.
diff --git
a/modules/indexing/src/test/java/org/apache/ignite/client/thin/ExtraColumnInH2RowsTest.java
b/modules/indexing/src/test/java/org/apache/ignite/client/thin/ExtraColumnInH2RowsTest.java
new file mode 100644
index 00000000000..3ff353af72c
--- /dev/null
+++
b/modules/indexing/src/test/java/org/apache/ignite/client/thin/ExtraColumnInH2RowsTest.java
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.client.thin;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.function.Function;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.Ignition;
+import org.apache.ignite.cache.CacheMode;
+import org.apache.ignite.cache.QueryEntity;
+import org.apache.ignite.cache.affinity.rendezvous.RendezvousAffinityFunction;
+import org.apache.ignite.cache.query.SqlFieldsQuery;
+import org.apache.ignite.client.Config;
+import org.apache.ignite.client.IgniteClient;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.ClientConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.lang.IgniteBiTuple;
+import org.apache.ignite.platform.model.Department;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.h2.result.ResultInterface;
+import org.junit.Test;
+
+/**
+ * H2 engine adds extra column in results set.
+ * Test check that this scenario will be handled correctly.
+ * @see ResultInterface#currentRow()
+ * @see ResultInterface#getVisibleColumnCount()
+ */
+public class ExtraColumnInH2RowsTest extends GridCommonAbstractTest {
+ /** */
+ @Test
+ public void testExtraColumnIgnored() throws Exception {
+ try (IgniteEx ign = startGrid()) {
+ int parts = 32;
+
+ IgniteCache<Integer, Department> cache = ign.createCache(new
CacheConfiguration<Integer, Department>(DEFAULT_CACHE_NAME)
+ .setCacheMode(CacheMode.PARTITIONED)
+ .setQueryEntities(Collections.singleton(new QueryEntity()
+ .setTableName("T")
+ .setFields(new LinkedHashMap<>(Map.of("id",
Integer.class.getName(), "name", String.class.getName())))
+ .setKeyType(Integer.class.getName())
+ .setValueType(Department.class.getName())))
+ .setAffinity(new RendezvousAffinityFunction(false, parts)));
+
+ Map<Integer, IgniteBiTuple<Integer, Department>> data = new
HashMap<>();
+
+ for (int part = 0; part < parts; part++) {
+ Integer key = partitionKeys(cache, part, 1, 0).get(0);
+ Department val = new Department(String.valueOf(part));
+
+ cache.put(key, val);
+ data.put(part, F.t(key, val));
+ }
+
+ try (IgniteClient cli = Ignition.startClient(new
ClientConfiguration().setAddresses(Config.SERVER))) {
+ check(parts, qry ->
cli.cache(DEFAULT_CACHE_NAME).query(qry).getAll(), data);
+ }
+
+ check(parts, qry ->
ign.cache(DEFAULT_CACHE_NAME).query(qry).getAll(), data);
+
+ try (IgniteEx cli = startClientGrid("client")) {
+ check(parts, qry ->
cli.cache(DEFAULT_CACHE_NAME).query(qry).getAll(), data);
+ }
+ }
+ }
+
+ /** */
+ private static void check(
+ int parts,
+ Function<SqlFieldsQuery, List<List<?>>> qryExec,
+ Map<Integer, IgniteBiTuple<Integer, Department>> data
+ ) {
+ data = new HashMap<>(data);
+
+ for (int part = 0; part < parts; part++) {
+ List<List<?>> res = qryExec.apply(new SqlFieldsQuery("select _key,
_val from T order by id asc").setPartitions(part));
+
+ assertEquals(1, res.size());
+ assertEquals(2, res.get(0).size());
+
+ IgniteBiTuple<Integer, Department> t = data.remove(part);
+
+ assertEquals(t.get1(), res.get(0).get(0));
+ assertEquals(t.get2().getName(),
((Department)res.get(0).get(1)).getName());
+
+ // Check empty result set fetched OK.
+ qryExec.apply(new SqlFieldsQuery("select _key, _val from T WHERE
name IS NULL order by id asc").setPartitions(part));
+ }
+
+ assertTrue(data.isEmpty());
+ }
+}
