anton-vinogradov commented on code in PR #13236:
URL: https://github.com/apache/ignite/pull/13236#discussion_r3520273738
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/query/schema/operation/SchemaAddQueryEntityOperation.java:
##########
@@ -71,42 +66,34 @@ public SchemaAddQueryEntityOperation(
this.entities = entities;
this.qryParallelism = qryParallelism;
this.sqlEscape = sqlEscape;
+
+ entitiesMsgs = F.viewReadOnly(entities, this::makeEntityMessage);
Review Comment:
`F.viewReadOnly` is a lazy view: the closure runs again on every iteration,
and the view keeps references to the source collection and (via
`this::makeEntityMessage`) to the operation itself. Two consequences:
- the message is not a snapshot — if the caller mutates `entities` after the
operation is constructed, the wire content silently changes;
- every serialization re-creates all `QueryEntityMessage`s (plus the nested
index messages).
`F.transform` is the eager twin (copies into an `ArrayList`):
```suggestion
entitiesMsgs = F.transform(entities, this::makeEntityMessage);
```
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/query/schema/operation/SchemaAddQueryEntityOperation.java:
##########
@@ -71,42 +66,34 @@ public SchemaAddQueryEntityOperation(
this.entities = entities;
this.qryParallelism = qryParallelism;
this.sqlEscape = sqlEscape;
+
+ entitiesMsgs = F.viewReadOnly(entities, this::makeEntityMessage);
}
- /**
- * @return Collection of query entities.
- */
+ /** @return Collection of query entities. */
public Collection<QueryEntity> entities() {
+ if (entities == null)
+ entities = F.viewReadOnly(entitiesMsgs,
QueryEntityMessage::toEntity);
Review Comment:
This caches the view object, but not the conversion:
`TransformCollectionView` re-runs `toEntity()` on every iteration and hands out
fresh `QueryEntity` instances each time. On the receiving node
`GridQueryProcessor` (L1153 + L1159) converts the whole set twice, and
`CacheConfiguration.setQueryEntities` (reached via
`CU.patchCacheConfiguration`) iterates it inside a nested loop — O(n²)
`toEntity()` calls.
Materializing once keeps the getter contract (multiple reads stay possible)
while doing the conversion a single time:
```suggestion
entities = F.transform(entitiesMsgs,
QueryEntityMessage::toEntity);
```
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/query/schema/message/QueryEntityMessage.java:
##########
@@ -0,0 +1,154 @@
+/*
+ * 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.internal.processors.query.schema.message;
+
+import java.util.Collection;
+import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.cache.QueryEntity;
+import org.apache.ignite.internal.MarshallableMessage;
+import org.apache.ignite.internal.Order;
+import org.apache.ignite.internal.cache.query.QueryIndexMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.marshaller.Marshaller;
+
+/**
+ * Message for {@link QueryEntity} transfer.
+ */
+public class QueryEntityMessage implements MarshallableMessage {
+ /** Key type. */
+ @Order(0)
+ String keyType;
+
+ /** Value type. */
+ @Order(1)
+ String valType;
+
+ /** Key name. */
+ @Order(2)
+ String keyFieldName;
+
+ /** Value name. */
+ @Order(3)
+ String valFieldName;
+
+ /** Fields available for query. */
+ @Order(4)
+ LinkedHashMap<String, String> fields;
+
+ /** Set of field names that belong to the key. */
+ @Order(5)
+ String[] keyFields;
+
+ /** Aliases. */
+ @Order(6)
+ Map<String, String> aliases;
+
+ /** Collection of query indexes. */
+ @Order(7)
+ Collection<QueryIndexMessage> idxs;
+
+ /** Table name. */
+ @Order(8)
+ String tableName;
+
+ /**
+ * Fields that must have non-null value.
+ * NB: Used in serde of both QueryEntity and QueryEntityEx in order to
avoid duplication.
+ */
+ @Order(9)
+ Set<String> notNullFields;
+
+ /** Fields default values. */
+ Map<String, Object> dfltFieldValues;
+
+ /** Serialized form of {@link #dfltFieldValues}. */
+ @Order(10)
+ byte[] dfltFieldValuesBytes;
+
+ /** Precision(Maximum length) for fields. */
+ @Order(11)
+ Map<String, Integer> fieldsPrecision;
+
+ /** Scale for fields. */
+ @Order(12)
+ Map<String, Integer> fieldsScale;
+
+ /** */
+ public QueryEntityMessage() { }
+
+ /** @param qryEntity Original {@link QueryEntity}. */
+ public QueryEntityMessage(QueryEntity qryEntity) {
+ keyType = qryEntity.getKeyType();
+ valType = qryEntity.getValueType();
+
+ keyFieldName = qryEntity.getKeyFieldName();
+ valFieldName = qryEntity.getValueFieldName();
+
+ fields = qryEntity.getFields();
+ aliases = qryEntity.getAliases();
+
+ if (!F.isEmpty(qryEntity.getKeyFields()))
+ keyFields = qryEntity.getKeyFields().toArray(U.EMPTY_STRS);
+
+ if (!F.isEmpty(qryEntity.getIndexes()))
+ idxs = F.viewReadOnly(qryEntity.getIndexes(),
QueryIndexMessage::new);
Review Comment:
Same lazy-view issue on the index write side: the message keeps a view over
the source entity's index collection, so mutations of the source leak into the
wire representation, and every write re-creates the `QueryIndexMessage`s.
```suggestion
idxs = F.transform(qryEntity.getIndexes(),
QueryIndexMessage::new);
```
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/query/schema/message/QueryEntityMessage.java:
##########
@@ -0,0 +1,154 @@
+/*
+ * 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.internal.processors.query.schema.message;
+
+import java.util.Collection;
+import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.cache.QueryEntity;
+import org.apache.ignite.internal.MarshallableMessage;
+import org.apache.ignite.internal.Order;
+import org.apache.ignite.internal.cache.query.QueryIndexMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.marshaller.Marshaller;
+
+/**
+ * Message for {@link QueryEntity} transfer.
+ */
+public class QueryEntityMessage implements MarshallableMessage {
+ /** Key type. */
+ @Order(0)
+ String keyType;
+
+ /** Value type. */
+ @Order(1)
+ String valType;
+
+ /** Key name. */
+ @Order(2)
+ String keyFieldName;
+
+ /** Value name. */
+ @Order(3)
+ String valFieldName;
+
+ /** Fields available for query. */
+ @Order(4)
+ LinkedHashMap<String, String> fields;
+
+ /** Set of field names that belong to the key. */
+ @Order(5)
+ String[] keyFields;
+
+ /** Aliases. */
+ @Order(6)
+ Map<String, String> aliases;
+
+ /** Collection of query indexes. */
+ @Order(7)
+ Collection<QueryIndexMessage> idxs;
+
+ /** Table name. */
+ @Order(8)
+ String tableName;
+
+ /**
+ * Fields that must have non-null value.
+ * NB: Used in serde of both QueryEntity and QueryEntityEx in order to
avoid duplication.
+ */
+ @Order(9)
+ Set<String> notNullFields;
+
+ /** Fields default values. */
+ Map<String, Object> dfltFieldValues;
+
+ /** Serialized form of {@link #dfltFieldValues}. */
+ @Order(10)
+ byte[] dfltFieldValuesBytes;
+
+ /** Precision(Maximum length) for fields. */
+ @Order(11)
+ Map<String, Integer> fieldsPrecision;
+
+ /** Scale for fields. */
+ @Order(12)
+ Map<String, Integer> fieldsScale;
+
+ /** */
+ public QueryEntityMessage() { }
+
+ /** @param qryEntity Original {@link QueryEntity}. */
+ public QueryEntityMessage(QueryEntity qryEntity) {
+ keyType = qryEntity.getKeyType();
+ valType = qryEntity.getValueType();
+
+ keyFieldName = qryEntity.getKeyFieldName();
+ valFieldName = qryEntity.getValueFieldName();
+
+ fields = qryEntity.getFields();
+ aliases = qryEntity.getAliases();
+
+ if (!F.isEmpty(qryEntity.getKeyFields()))
+ keyFields = qryEntity.getKeyFields().toArray(U.EMPTY_STRS);
+
+ if (!F.isEmpty(qryEntity.getIndexes()))
+ idxs = F.viewReadOnly(qryEntity.getIndexes(),
QueryIndexMessage::new);
+
+ tableName = qryEntity.getTableName();
+
+ notNullFields = qryEntity.getNotNullFields();
+ dfltFieldValues = qryEntity.getDefaultFieldValues();
+ fieldsPrecision = qryEntity.getFieldsPrecision();
+ fieldsScale = qryEntity.getFieldsScale();
+ }
+
+ /** @return Original {@link QueryEntity}. */
+ public QueryEntity toEntity() {
+ return new QueryEntity()
+ .setKeyType(keyType)
+ .setValueType(valType)
+ .setKeyFieldName(keyFieldName)
+ .setValueFieldName(valFieldName)
+ .setFields(fields)
+ .setKeyFields(!F.isEmpty(keyFields) ? new
LinkedHashSet<>(List.of(keyFields)) : null)
+ .setAliases(aliases)
+ .setIndexes(!F.isEmpty(idxs) ? F.viewReadOnly(idxs,
QueryIndexMessage::queryIndex) : null)
Review Comment:
...and the read side: every iteration over the restored entity's indexes
re-creates the `QueryIndex` objects through the view.
```suggestion
.setIndexes(!F.isEmpty(idxs) ? F.transform(idxs,
QueryIndexMessage::queryIndex) : null)
```
##########
modules/core/src/test/java/org/apache/ignite/internal/processors/query/schema/message/QueryEntityMessageSerializationTest.java:
##########
@@ -0,0 +1,193 @@
+/*
+ * 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.internal.processors.query.schema.message;
+
+import java.io.Externalizable;
+import java.io.Serializable;
+import java.lang.reflect.Modifier;
+import java.math.BigDecimal;
+import java.nio.ByteBuffer;
+import java.util.Arrays;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import org.apache.ignite.cache.QueryEntity;
+import org.apache.ignite.cache.QueryIndex;
+import org.apache.ignite.cache.QueryIndexType;
+import org.apache.ignite.internal.CoreMessagesProvider;
+import org.apache.ignite.internal.direct.DirectMessageReader;
+import org.apache.ignite.internal.direct.DirectMessageWriter;
+import
org.apache.ignite.internal.managers.communication.IgniteMessageFactoryImpl;
+import org.apache.ignite.internal.processors.query.QueryEntityEx;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.marshaller.Marshaller;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.apache.ignite.plugin.extensions.communication.MessageFactory;
+import
org.apache.ignite.plugin.extensions.communication.MessageFactoryProvider;
+import org.apache.ignite.plugin.extensions.communication.MessageSerializer;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+
+import static org.apache.ignite.marshaller.Marshallers.jdk;
+import static
org.apache.ignite.spi.communication.tcp.TcpCommunicationSpi.makeMessageType;
+
+/** Test for serialization round-trip of {@link QueryEntityMessage} and {@link
QueryEntityExMessage}. */
+public class QueryEntityMessageSerializationTest extends
GridCommonAbstractTest {
+ /** Error suffix. */
+ public static final String ERROR_SUFFIX = " count is not equal to the
expected fields count. " +
+ "Has the number of fields in the `QueryEntity` or `QueryEntityEx`
classes changed?";
Review Comment:
Companion to the chunked round-trip suggestion below (`writeAndReadBack`):
```suggestion
/** Error suffix. */
public static final String ERROR_SUFFIX = " count is not equal to the
expected fields count. " +
"Has the number of fields in the `QueryEntity` or `QueryEntityEx`
classes changed?";
/** Chunk size, small enough to exercise partial writes and reads. */
private static final int CHUNK_SIZE = 16;
```
##########
modules/core/src/test/java/org/apache/ignite/internal/processors/query/schema/message/QueryEntityMessageSerializationTest.java:
##########
@@ -0,0 +1,193 @@
+/*
+ * 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.internal.processors.query.schema.message;
+
+import java.io.Externalizable;
+import java.io.Serializable;
+import java.lang.reflect.Modifier;
+import java.math.BigDecimal;
+import java.nio.ByteBuffer;
+import java.util.Arrays;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import org.apache.ignite.cache.QueryEntity;
+import org.apache.ignite.cache.QueryIndex;
+import org.apache.ignite.cache.QueryIndexType;
+import org.apache.ignite.internal.CoreMessagesProvider;
+import org.apache.ignite.internal.direct.DirectMessageReader;
+import org.apache.ignite.internal.direct.DirectMessageWriter;
+import
org.apache.ignite.internal.managers.communication.IgniteMessageFactoryImpl;
+import org.apache.ignite.internal.processors.query.QueryEntityEx;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.marshaller.Marshaller;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.apache.ignite.plugin.extensions.communication.MessageFactory;
+import
org.apache.ignite.plugin.extensions.communication.MessageFactoryProvider;
+import org.apache.ignite.plugin.extensions.communication.MessageSerializer;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+
+import static org.apache.ignite.marshaller.Marshallers.jdk;
+import static
org.apache.ignite.spi.communication.tcp.TcpCommunicationSpi.makeMessageType;
+
+/** Test for serialization round-trip of {@link QueryEntityMessage} and {@link
QueryEntityExMessage}. */
+public class QueryEntityMessageSerializationTest extends
GridCommonAbstractTest {
+ /** Error suffix. */
+ public static final String ERROR_SUFFIX = " count is not equal to the
expected fields count. " +
+ "Has the number of fields in the `QueryEntity` or `QueryEntityEx`
classes changed?";
+
+ /** */
+ private final Marshaller marsh = jdk();
+
+ /** */
+ private final MessageFactory msgFactory = new IgniteMessageFactoryImpl(
+ new MessageFactoryProvider[] {new CoreMessagesProvider(marsh, marsh,
U.gridClassLoader())});
+
+ /** */
+ @Test
+ public void testQueryEntity() throws Exception {
+ QueryEntity entity = queryEntity();
+
+ assertEquals(entity, serializeAndDeserialize(entity,
serializableFieldsCount(QueryEntity.class)));
+ }
+
+ /** */
+ @Test
+ public void testQueryEntityEx() throws Exception {
+ QueryEntityEx entity = queryEntityEx();
+
+ long expReadWriteCnt = serializableFieldsCount(QueryEntityEx.class) -
+ /* QueryEntityEx duplicates 'notNullFields', but
'QueryEntityExMessage' doesn't */ 1;
+
+ QueryEntity res = serializeAndDeserialize(entity, expReadWriteCnt);
+
+ assertTrue(res instanceof QueryEntityEx);
+ assertEquals(entity, res);
+
+ // Not part of QueryEntityEx.equals(), so assert it explicitly.
+ assertEquals(entity.fillAbsentPKsWithDefaults(),
((QueryEntityEx)res).fillAbsentPKsWithDefaults());
+ }
+
+ /**
+ * @param src Source entity.
+ * @param expReadsWritesCnt Expected count of field reads and writes.
+ *
+ * @return Entity read during a full serde round-trip.
+ */
+ private QueryEntity serializeAndDeserialize(QueryEntity src, long
expReadsWritesCnt) {
+ QueryEntityMessage msg = src instanceof QueryEntityEx
+ ? new QueryEntityExMessage((QueryEntityEx)src) : new
QueryEntityMessage(src);
+
+ return writeAndReadBack(msg, expReadsWritesCnt).toEntity();
+ }
+
+ /**
+ * @param cls Class of an object.
+ */
+ private long serializableFieldsCount(Class<?> cls) {
+ if (cls == Object.class)
+ return 0;
+
+ assertTrue("Not a serializable class: " + cls,
Serializable.class.isAssignableFrom(cls));
+ assertFalse("Should not be Externalizable:" + cls,
Externalizable.class.isAssignableFrom(cls));
+
+ return serializableFieldsCount(cls.getSuperclass()) +
Arrays.stream(cls.getDeclaredFields())
+ .filter(f -> !Modifier.isStatic(f.getModifiers()) &&
!Modifier.isTransient(f.getModifiers()))
+ .count();
+ }
+
+ /**
+ * @param msg Message to write and read back through {@link
DirectMessageWriter}/{@link DirectMessageReader}.
+ * @param expReadsWritesCnt Expected count of field reads and writes.
+ * @param <T> Type of Message.
+ *
+ * @return Restored message.
+ */
+ private <T extends Message> T writeAndReadBack(T msg, long
expReadsWritesCnt) {
+ ByteBuffer buf = ByteBuffer.allocate(64 * 1024);
+
+ MessageSerializer<T> serde =
(MessageSerializer<T>)msgFactory.serializer(msg.directType());
+
+ DirectMessageWriter writer = new DirectMessageWriter(msgFactory);
+ writer.setBuffer(buf);
+
+ assertTrue(serde.writeTo(msg, writer));
+ assertEquals("Writes" + ERROR_SUFFIX,
+ expReadsWritesCnt, writer.state());
+
+ buf.flip();
+
+ DirectMessageReader reader = new DirectMessageReader(msgFactory, null);
+ reader.setBuffer(buf);
+
+ T res = (T)msgFactory.create(makeMessageType(buf.get(), buf.get()));
+
+ assertTrue(serde.readFrom(res, reader));
+ assertEquals("Reads" + ERROR_SUFFIX,
+ expReadsWritesCnt, reader.state());
+
+ return res;
+ }
Review Comment:
The 64K buffer writes the whole message in one shot, so the
partial-write/resume state machine — exactly where collection- and
nested-message serialization bugs hide — is never exercised. A chunked variant
in the style of `DirectMarshallingMessagesTest#doMarshalUnmarshalChunked`,
keeping your field-count assertions (needs the `CHUNK_SIZE` constant from the
suggestion above; verified locally, both tests green):
```suggestion
private <T extends Message> T writeAndReadBack(T msg, long
expReadsWritesCnt) {
ByteBuffer buf = ByteBuffer.allocate(64 * 1024);
MessageSerializer<T> serde =
(MessageSerializer<T>)msgFactory.serializer(msg.directType());
DirectMessageWriter writer = new DirectMessageWriter(msgFactory);
boolean written = false;
while (!written) {
ByteBuffer chunk = ByteBuffer.allocate(CHUNK_SIZE);
writer.setBuffer(chunk);
written = serde.writeTo(msg, writer);
chunk.flip();
buf.put(chunk);
}
assertEquals("Writes" + ERROR_SUFFIX,
expReadsWritesCnt, writer.state());
byte[] bytes = new byte[buf.position()];
buf.flip();
buf.get(bytes);
DirectMessageReader reader = new DirectMessageReader(msgFactory,
null);
T res = (T)msgFactory.create(makeMessageType(bytes[0], bytes[1]));
int pos = 2;
boolean read = false;
while (!read) {
ByteBuffer chunk = ByteBuffer.allocate(Math.min(CHUNK_SIZE,
bytes.length - pos));
chunk.put(bytes, pos, chunk.capacity());
chunk.flip();
reader.setBuffer(chunk);
read = serde.readFrom(res, reader);
pos += chunk.position();
}
assertEquals("Reads" + ERROR_SUFFIX,
expReadsWritesCnt, reader.state());
return res;
}
```
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/query/schema/operation/SchemaAddQueryEntityOperation.java:
##########
@@ -71,42 +66,34 @@ public SchemaAddQueryEntityOperation(
this.entities = entities;
this.qryParallelism = qryParallelism;
this.sqlEscape = sqlEscape;
+
+ entitiesMsgs = F.viewReadOnly(entities, this::makeEntityMessage);
}
- /**
- * @return Collection of query entities.
- */
+ /** @return Collection of query entities. */
public Collection<QueryEntity> entities() {
+ if (entities == null)
+ entities = F.viewReadOnly(entitiesMsgs,
QueryEntityMessage::toEntity);
+
return entities;
}
- /**
- * @return Query parallelism.
- */
+ /** @return Query parallelism. */
public int queryParallelism() {
return qryParallelism;
}
- /**
- * @return Sql escape flag.
- */
+ /** @return Sql escape flag. */
public boolean isSqlEscape() {
return sqlEscape;
}
- /** {@inheritDoc} */
- @Override public void prepareMarshal(Marshaller marsh) throws
IgniteCheckedException {
- if (entities != null)
- qryEntitiesBytes = U.marshal(marsh, entities);
- }
-
- /** {@inheritDoc} */
- @Override public void finishUnmarshal(Marshaller marsh, ClassLoader
clsLdr) throws IgniteCheckedException {
- if (qryEntitiesBytes != null) {
- entities = U.unmarshal(marsh, qryEntitiesBytes, clsLdr);
-
- qryEntitiesBytes = null;
- }
+ /**
+ * @param qryEntity Query entity.
+ * @return The appropriate query entity message.
+ */
+ private QueryEntityMessage makeEntityMessage(QueryEntity qryEntity) {
Review Comment:
Optional, non-blocking: this factory could live next to the messages as
`QueryEntityMessage.of(QueryEntity)` — symmetric to `toEntity()`, reusable by
other IEP-132 operations, and it keeps the `instanceof` dispatch (which would
silently drop fields if a third `QueryEntity` subclass ever appears) in one
place:
```java
// In QueryEntityMessage (+ import
o.a.i.internal.processors.query.QueryEntityEx):
public static QueryEntityMessage of(QueryEntity qryEntity) {
return qryEntity instanceof QueryEntityEx ? new
QueryEntityExMessage((QueryEntityEx)qryEntity)
: new QueryEntityMessage(qryEntity);
}
```
Here the constructor becomes `entitiesMsgs = F.transform(entities,
QueryEntityMessage::of);`, this method goes away, and the
`QueryEntityEx`/`QueryEntityExMessage` imports can be dropped. Compiles and
passes the serialization test locally.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/query/schema/message/QueryEntityExMessage.java:
##########
@@ -0,0 +1,94 @@
+/*
+ * 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.internal.processors.query.schema.message;
+
+import org.apache.ignite.cache.QueryEntity;
+import org.apache.ignite.internal.Order;
+import org.apache.ignite.internal.processors.query.QueryEntityEx;
+
+/** Message for {@link QueryEntityEx} transfer. */
+public class QueryEntityExMessage extends QueryEntityMessage {
+ /** Whether to preserve order specified by 'keyFields' or not. */
+ @Order(0)
+ boolean preserveKeysOrder;
+
+ /** Whether a primary key should be autocreated or not. */
+ @Order(1)
+ boolean implicitPk;
+
+ /** Whether absent PK parts should be filled with defaults or not. */
+ @Order(2)
+ boolean fillAbsentPKsWithDefaults;
+
+ /** INLINE_SIZE for PK index. */
+ @Order(3)
+ int pkInlineSize;
+
+ /** INLINE_SIZE for affinity field index. */
+ @Order(4)
+ int affKeyInlineSize;
+
+ /** Whether query entity was created by SQL. */
+ @Order(5)
+ boolean sql;
+
+ /** */
+ public QueryEntityExMessage() { }
+
+ /**
+ * @param qryEntity Original {@link QueryEntity}.
+ */
+ public QueryEntityExMessage(QueryEntityEx qryEntity) {
+ super(qryEntity);
+
+ preserveKeysOrder = qryEntity.isPreserveKeysOrder();
+ implicitPk = qryEntity.implicitPk();
+ fillAbsentPKsWithDefaults = qryEntity.fillAbsentPKsWithDefaults();
+
+ pkInlineSize = qryEntity.getPrimaryKeyInlineSize() != null ?
qryEntity.getPrimaryKeyInlineSize() : -1;
+
+ affKeyInlineSize = qryEntity.getAffinityKeyInlineSize() != null ?
qryEntity.getAffinityKeyInlineSize() : -1;
+
+ sql = qryEntity.sql();
+ }
+
+ /** {@inheritDoc} */
+ @Override public QueryEntity toEntity() {
+ QueryEntity baseEntity = super.toEntity();
+
+ // We should nullify '_notNullFields' field of base entity,
+ // because 'QueryEntityEx' uses extra 'notNullFields' field for
storing of not null fields.
+ baseEntity.setNotNullFields(null);
Review Comment:
Optional, non-blocking idea to retire the nullify trick entirely: extract
the base-field filling (everything except `notNullFields`) into a protected
method, then build the `QueryEntityEx` directly — no intermediate base entity,
no shadow field to null out:
```java
// QueryEntityMessage:
public QueryEntity toEntity() {
return fillBase(new QueryEntity()).setNotNullFields(notNullFields);
}
protected QueryEntity fillBase(QueryEntity target) {
return target
.setKeyType(keyType)
...
.setFieldsScale(fieldsScale); // everything except notNullFields
}
// QueryEntityExMessage:
@Override public QueryEntity toEntity() {
QueryEntityEx qryEntity = new QueryEntityEx();
fillBase(qryEntity);
qryEntity.setNotNullFields(notNullFields);
// ... the rest as is
}
```
Verified locally: compiles, round-trip test stays green (the
`base._notNullFields == null` invariant your test checks is preserved by
construction).
--
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]
