This is an automated email from the ASF dual-hosted git repository.
ptupitsyn pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/ignite-3.git
The following commit(s) were added to refs/heads/main by this push:
new 7bdfba086a IGNITE-18925 Avoid BinaryTuple conversion from client to
storage (#2004)
7bdfba086a is described below
commit 7bdfba086af5f8071c4244d8840003645c222a44
Author: Pavel Tupitsyn <[email protected]>
AuthorDate: Wed May 3 14:28:10 2023 +0300
IGNITE-18925 Avoid BinaryTuple conversion from client to storage (#2004)
Copy client-provided `BinaryTuple` from the client as is to storage when
the conditions are met:
* Schema version matches current schema.
* There are no unset values (which require values from
`DefaultValueProvider`).
* Default value behavior is checked by
`testColumnWithDefaultValueNotSetReturnsDefault` and many other tests.
* There are no temporal types in the schema (which require precision-based
normalization).
* Normalization is checked by the following tests:
`testAllColumnsBinaryPutPojoGet`, `testAllColumnsPojoPutBinaryGet`,
`testAllColumnsBinaryPutPojoGet`, `testAllColumnsPojoPutBinaryGet`
---
.../table/MutableTupleBinaryTupleAdapter.java | 10 ++++++++
.../client/handler/requests/table/ClientTuple.java | 7 +++---
.../ignite/internal/schema/SchemaDescriptor.java | 24 ++++++++++++++++++-
.../ignite/internal/schema/row/RowAssembler.java | 18 ++++++++++++---
.../schema/marshaller/TupleMarshallerImpl.java | 27 +++++++++++++++++++++-
5 files changed, 78 insertions(+), 8 deletions(-)
diff --git
a/modules/client-common/src/main/java/org/apache/ignite/internal/client/table/MutableTupleBinaryTupleAdapter.java
b/modules/client-common/src/main/java/org/apache/ignite/internal/client/table/MutableTupleBinaryTupleAdapter.java
index 4c1cfea432..8cc2fb4f33 100644
---
a/modules/client-common/src/main/java/org/apache/ignite/internal/client/table/MutableTupleBinaryTupleAdapter.java
+++
b/modules/client-common/src/main/java/org/apache/ignite/internal/client/table/MutableTupleBinaryTupleAdapter.java
@@ -408,6 +408,16 @@ public abstract class MutableTupleBinaryTupleAdapter
implements Tuple, BinaryTup
/** {@inheritDoc} */
@Override
public @Nullable BinaryTupleReader binaryTuple() {
+ if (tuple != null) {
+ return null;
+ }
+
+ if (noValueSet != null && !noValueSet.isEmpty()) {
+ // Can't expose binary tuple if there are unset values.
+ // On the server side, DefaultValueProvider is used to replace
unset values with proper defaults.
+ return null;
+ }
+
return binaryTuple;
}
diff --git
a/modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/table/ClientTuple.java
b/modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/table/ClientTuple.java
index 6531e8200d..edfca53be7 100644
---
a/modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/table/ClientTuple.java
+++
b/modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/table/ClientTuple.java
@@ -22,6 +22,7 @@ import
org.apache.ignite.internal.binarytuple.BinaryTupleReader;
import org.apache.ignite.internal.client.table.MutableTupleBinaryTupleAdapter;
import org.apache.ignite.internal.schema.Column;
import org.apache.ignite.internal.schema.NativeTypeSpec;
+import org.apache.ignite.internal.schema.SchemaAware;
import org.apache.ignite.internal.schema.SchemaDescriptor;
import org.apache.ignite.sql.ColumnType;
import org.apache.ignite.table.Tuple;
@@ -31,7 +32,7 @@ import org.jetbrains.annotations.Nullable;
/**
* Server-side client Tuple.
*/
-class ClientTuple extends MutableTupleBinaryTupleAdapter {
+class ClientTuple extends MutableTupleBinaryTupleAdapter implements
SchemaAware {
/** Schema. */
private final SchemaDescriptor schema;
@@ -52,8 +53,8 @@ class ClientTuple extends MutableTupleBinaryTupleAdapter {
/** {@inheritDoc} */
@Override
- public @Nullable BinaryTupleReader binaryTuple() {
- return super.binaryTuple();
+ public @Nullable SchemaDescriptor schema() {
+ return binaryTuple() != null ? schema : null;
}
/** {@inheritDoc} */
diff --git
a/modules/schema/src/main/java/org/apache/ignite/internal/schema/SchemaDescriptor.java
b/modules/schema/src/main/java/org/apache/ignite/internal/schema/SchemaDescriptor.java
index 048753bbb1..95518c5e62 100644
---
a/modules/schema/src/main/java/org/apache/ignite/internal/schema/SchemaDescriptor.java
+++
b/modules/schema/src/main/java/org/apache/ignite/internal/schema/SchemaDescriptor.java
@@ -23,6 +23,7 @@ import java.util.Comparator;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Objects;
+import java.util.concurrent.atomic.AtomicBoolean;
import java.util.stream.Stream;
import org.apache.ignite.internal.schema.mapping.ColumnMapper;
import org.apache.ignite.internal.schema.mapping.ColumnMapping;
@@ -50,6 +51,9 @@ public class SchemaDescriptor {
/** Mapping 'Column name' -> Column. */
private final Map<String, Column> colMap;
+ /** Whether schema contains time or timestamp columns. */
+ private final boolean hasTemporalColumns;
+
/** Column mapper. */
private ColumnMapper colMapper = ColumnMapping.identityMapping();
@@ -82,10 +86,19 @@ public class SchemaDescriptor {
assert this.keyCols.nullMapSize() == 0 : "Primary key cannot contain
nullable column [cols=" + this.keyCols + ']';
colMap = new LinkedHashMap<>(keyCols.length + valCols.length);
+ var hasTemporalColumns = new AtomicBoolean(false);
Stream.concat(Arrays.stream(this.keyCols.columns()),
Arrays.stream(this.valCols.columns()))
.sorted(Comparator.comparingInt(Column::columnOrder))
- .forEach(c -> colMap.put(c.name(), c));
+ .forEach(c -> {
+ if (c.type() instanceof TemporalNativeType) {
+ hasTemporalColumns.set(true);
+ }
+
+ colMap.put(c.name(), c);
+ });
+
+ this.hasTemporalColumns = hasTemporalColumns.get();
// Preserving key chunk column order is not actually required.
// It is sufficient to has same column order for all nodes.
@@ -208,6 +221,15 @@ public class SchemaDescriptor {
return colMapper;
}
+ /**
+ * Get a value indicating whether schema contains temporal columns.
+ *
+ * @return {@code true} if schema contains temporal columns (time,
datetime, timestamp), {@code false} otherwise.
+ */
+ public boolean hasTemporalColumns() {
+ return hasTemporalColumns;
+ }
+
/** {@inheritDoc} */
@Override
public String toString() {
diff --git
a/modules/schema/src/main/java/org/apache/ignite/internal/schema/row/RowAssembler.java
b/modules/schema/src/main/java/org/apache/ignite/internal/schema/row/RowAssembler.java
index 6d3f1f350d..e65eec9673 100644
---
a/modules/schema/src/main/java/org/apache/ignite/internal/schema/row/RowAssembler.java
+++
b/modules/schema/src/main/java/org/apache/ignite/internal/schema/row/RowAssembler.java
@@ -665,13 +665,25 @@ public class RowAssembler {
*/
public BinaryRow build() {
boolean hasValue = flush();
-
ByteBuffer tupleBuffer = builder.build();
- ByteBuffer buffer = ByteBuffer.allocate(SCHEMA_VERSION_FLD_LEN +
HAS_VALUE_FLD_LEN + tupleBuffer.limit()).order(ORDER);
+
+ return build(tupleBuffer, schemaVersion, hasValue);
+ }
+
+ /**
+ * Builds serialized row from a BinaryTuple.
+ *
+ * @param binTupleBuffer Binary tuple buffer.
+ * @param schemaVersion Schema version.
+ * @return Created {@link BinaryRow}.
+ */
+ public static BinaryRow build(ByteBuffer binTupleBuffer, int
schemaVersion, boolean hasValue) {
+ ByteBuffer buffer = ByteBuffer.allocate(SCHEMA_VERSION_FLD_LEN +
HAS_VALUE_FLD_LEN + binTupleBuffer.limit()).order(ORDER);
buffer.putShort((short) schemaVersion);
buffer.put(hasValue ? (byte) 1 : 0);
- buffer.put(tupleBuffer);
+ buffer.put(binTupleBuffer);
buffer.position(0);
+
return new ByteBufferRow(buffer);
}
diff --git
a/modules/table/src/main/java/org/apache/ignite/internal/schema/marshaller/TupleMarshallerImpl.java
b/modules/table/src/main/java/org/apache/ignite/internal/schema/marshaller/TupleMarshallerImpl.java
index c549a6246b..d6d438cc30 100644
---
a/modules/table/src/main/java/org/apache/ignite/internal/schema/marshaller/TupleMarshallerImpl.java
+++
b/modules/table/src/main/java/org/apache/ignite/internal/schema/marshaller/TupleMarshallerImpl.java
@@ -24,6 +24,8 @@ import java.util.HashSet;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
+import org.apache.ignite.internal.binarytuple.BinaryTupleContainer;
+import org.apache.ignite.internal.binarytuple.BinaryTupleReader;
import org.apache.ignite.internal.schema.Column;
import org.apache.ignite.internal.schema.Columns;
import org.apache.ignite.internal.schema.NativeType;
@@ -64,6 +66,18 @@ public class TupleMarshallerImpl implements TupleMarshaller {
try {
SchemaDescriptor schema = schemaReg.schema();
+ if (tuple instanceof SchemaAware && tuple instanceof
BinaryTupleContainer) {
+ SchemaDescriptor tupleSchema = ((SchemaAware) tuple).schema();
+ BinaryTupleReader tupleReader = ((BinaryTupleContainer)
tuple).binaryTuple();
+
+ if (tupleSchema != null
+ && tupleReader != null
+ && tupleSchema.version() == schema.version()
+ && !binaryTupleRebuildRequired(schema)) {
+ return new Row(schema,
RowAssembler.build(tupleReader.byteBuffer(), schema.version(), true));
+ }
+ }
+
InternalTuple keyTuple0 = toInternalTuple(schema, tuple, true);
InternalTuple valTuple0 = toInternalTuple(schema, tuple, false);
@@ -317,10 +331,21 @@ public class TupleMarshallerImpl implements
TupleMarshaller {
* @param tup Internal tuple.
* @throws SchemaMismatchException If a tuple column value doesn't match
the current column type.
*/
- private void writeColumn(RowAssembler rowAsm, Column col, InternalTuple
tup) throws SchemaMismatchException {
+ private static void writeColumn(RowAssembler rowAsm, Column col,
InternalTuple tup) throws SchemaMismatchException {
RowAssembler.writeValue(rowAsm, col, tup.value(col.name()));
}
+ /**
+ * Determines whether binary tuple rebuild is required.
+ *
+ * @param schema Schema.
+ * @return True if binary tuple rebuild is required; false if the tuple
can be written to storage as is.
+ */
+ private static boolean binaryTupleRebuildRequired(SchemaDescriptor schema)
{
+ // Temporal columns require normalization according to the specified
precision.
+ return schema.hasTemporalColumns();
+ }
+
/**
* Internal tuple enriched original tuple with additional info.
*/
