You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "xtern (via GitHub)" <gi...@apache.org> on 2023/08/29 13:31:41 UTC
[GitHub] [ignite-3] xtern opened a new pull request, #2512: IGNITE-19791 Introduce SqlRowHandler
xtern opened a new pull request, #2512:
URL: https://github.com/apache/ignite-3/pull/2512
Thank you for submitting the pull request.
To streamline the review process of the patch and ensure better code quality
we ask both an author and a reviewer to verify the following:
### The Review Checklist
- [ ] **Formal criteria:** TC status, codestyle, mandatory documentation. Also make sure to complete the following:
\- There is a single JIRA ticket related to the pull request.
\- The web-link to the pull request is attached to the JIRA ticket.
\- The JIRA ticket has the Patch Available state.
\- The description of the JIRA ticket explains WHAT was made, WHY and HOW.
\- The pull request title is treated as the final commit message. The following pattern must be used: IGNITE-XXXX Change summary where XXXX - number of JIRA issue.
- [ ] **Design:** new code conforms with the design principles of the components it is added to.
- [ ] **Patch quality:** patch cannot be split into smaller pieces, its size must be reasonable.
- [ ] **Code quality:** code is clean and readable, necessary developer documentation is added if needed.
- [ ] **Tests code quality:** test set covers positive/negative scenarios, happy/edge cases. Tests are effective in terms of execution time and resources.
### Notes
- [Apache Ignite Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Java+Code+Style+Guide)
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1317396164
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/framework/ArrayRowHandler.java:
##########
@@ -50,6 +52,18 @@ public Object[] concat(Object[] left, Object[] right) {
return ArrayUtils.concat(left, right);
}
+ /** {@inheritDoc} */
+ @Override
+ public Object[] map(Object[] row, int[] mapping, int offset) {
Review Comment:
Yes we can, but this will go against the description of the method in the javadoc. This implementation will only be used in tests. I'm not sure how much can we gain from such optimization. But if you insist, I can change this method to return the original array with the values replaced.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1314648207
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/RowConverter.java:
##########
@@ -108,34 +121,44 @@ public static <RowT> BinaryTuple toBinaryTuple(
assert rowColumnsCount == binarySchema.elementCount() : "Invalid lookup key.";
- if (IgniteUtils.assertionsEnabled()) {
- for (int i = 0; i < rowColumnsCount; i++) {
- if (handler.get(i, searchRow) == UNSPECIFIED_VALUE_PLACEHOLDER) {
- throw new AssertionError("Invalid lookup key.");
- }
- }
- }
-
BinaryTupleBuilder tupleBuilder = new BinaryTupleBuilder(rowColumnsCount);
- return new BinaryTuple(rowColumnsCount, toByteBuffer(ectx, binarySchema, handler, tupleBuilder, searchRow));
+ return new BinaryTuple(rowColumnsCount, toByteBuffer(binarySchema, handler, tupleBuilder, rowColumnsCount, searchRow));
+ }
+
+ /**
+ * Converts an array of objects to a byte buffer representation of a {@link BinaryTuple binary tuple}. This method uses
Review Comment:
Done, thanks.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1315438958
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/SqlRowHandler.java:
##########
@@ -0,0 +1,272 @@
+/*
+ * 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.sql.engine.exec;
+
+import java.nio.ByteBuffer;
+import java.util.List;
+import org.apache.ignite.internal.schema.BinaryTuple;
+import org.apache.ignite.internal.schema.NativeType;
+import org.apache.ignite.internal.schema.row.InternalTuple;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler.RowWrapper;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema.Builder;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchemaTypes;
+import org.apache.ignite.internal.sql.engine.exec.row.TypeSpec;
+import org.apache.ignite.internal.sql.engine.util.Commons;
+import org.apache.ignite.internal.sql.engine.util.TypeUtils;
+import org.apache.ignite.lang.IgniteStringBuilder;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Handler that uses a {@link RowWrapper wrapper} to operate with two types of row implementations.
+ *
+ * <ul>
+ * <li>{@link ObjectsArrayRowWrapper Objects array}</li>
+ * <li>{@link BinaryTupleRowWrapper Binary tuple}</li>
+ * </ul>
+ *
+ * <p>Each kind of rows is serialized to the same binary tuple format
+ * using the {@link #toByteBuffer(RowWrapper) toByteBuffer} method.
+ *
+ * <p>Factory methods {@link RowFactory#wrap(InternalTuple) wrap(InternalTuple)} and
+ * {@link RowFactory#create(ByteBuffer) create(ByteBuffer)} allow create rows without
+ * any additional conversions. But the fields in binary tuple must match the
+ * factory {@link RowSchema row schema}.
+ */
+public class SqlRowHandler implements RowHandler<RowWrapper> {
+ public static final RowHandler<RowWrapper> INSTANCE = new SqlRowHandler();
+
+ private SqlRowHandler() {
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public @Nullable Object get(int field, RowWrapper row) {
+ return row.get(field);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public void set(int field, RowWrapper row, @Nullable Object val) {
+ row.set(field, val);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper concat(RowWrapper left, RowWrapper right) {
+ int leftLen = left.columnsCount();
+ int rightLen = right.columnsCount();
+ List<TypeSpec> leftTypes = left.rowSchema().fields();
+ List<TypeSpec> rightTypes = right.rowSchema().fields();
+
+ Object[] values = new Object[leftLen + rightLen];
+ Builder schemaBuilder = RowSchema.builder();
+
+ for (int i = 0; i < leftLen; i++) {
+ values[i] = left.get(i);
+ schemaBuilder.addField(leftTypes.get(i));
+ }
+
+ for (int i = 0; i < rightLen; i++) {
+ values[leftLen + i] = right.get(i);
+ schemaBuilder.addField(rightTypes.get(i));
+ }
+
+ return new ObjectsArrayRowWrapper(schemaBuilder.build(), values);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper map(RowWrapper row, int[] mapping, int offset) {
+ Object[] fields = new Object[mapping.length];
+
+ for (int i = 0; i < mapping.length; i++) {
+ fields[i] = row.get(mapping[i] + offset);
+ }
+
+ return new ObjectsArrayRowWrapper(row.rowSchema(), fields);
+ }
+
+ @Override
+ public int columnCount(RowWrapper row) {
+ return row.columnsCount();
+ }
+
+ @Override
+ public String toString(RowWrapper row) {
+ IgniteStringBuilder buf = new IgniteStringBuilder("Row[");
+ int maxIdx = columnCount(row) - 1;
+
+ for (int i = 0; i <= maxIdx; i++) {
+ buf.app(row.get(i));
+
+ if (i != maxIdx) {
+ buf.app(", ");
+ }
+ }
+
+ return buf.app(']').toString();
+ }
+
+ @Override
+ public ByteBuffer toByteBuffer(RowWrapper row) {
+ return row.toByteBuffer();
+ }
+
+ @Override
+ public RowFactory<RowWrapper> factory(RowSchema rowSchema) {
+ int rowLen = rowSchema.fields().size();
+
+ return new RowFactory<>() {
+ /** {@inheritDoc} */
+ @Override
+ public RowHandler<RowWrapper> handler() {
+ return SqlRowHandler.this;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper create() {
+ return create(new Object[rowLen]);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper create(Object... fields) {
+ assert fields.length == rowLen;
+
+ return new ObjectsArrayRowWrapper(rowSchema, fields);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper create(ByteBuffer buf) {
+ return wrap(new BinaryTuple(rowLen, buf));
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper wrap(InternalTuple tuple) {
+ assert rowLen == tuple.elementCount();
+
+ return new BinaryTupleRowWrapper(rowSchema, tuple);
+ }
+ };
+ }
+
+ /**
+ * Provides the ability for a single {@link RowHandler} instance to interact with multiple row implementations.
+ */
+ public abstract static class RowWrapper {
Review Comment:
I would like to hide the methods of this class so that it is possible to work only through the RowHandler and not directly :thinking: :roll_eyes:
But maybe it's a bad idea.... :thinking:
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1316850230
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/row/RowSchema.java:
##########
@@ -39,6 +43,43 @@ public List<TypeSpec> fields() {
return fields;
}
+ /**
+ * Gets an object representation from a tuple's field.
+ *
+ * @param tuple Tuple to read the value from.
+ * @param fieldIndex Field index to read.
+ * @return An object representation of the value.
+ */
+ public @Nullable Object value(InternalTuple tuple, int fieldIndex) {
Review Comment:
Moved to BinaryTupleRowWrapper :thinking:
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] korlov42 commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "korlov42 (via GitHub)" <gi...@apache.org>.
korlov42 commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1315729337
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/RowHandler.java:
##########
@@ -87,5 +105,13 @@ interface RowFactory<RowT> {
* @return Instantiation defined representation.
*/
RowT create(ByteBuffer raw);
+
+ /**
+ * Wraps incoming binary tuple into a instantiation defined representation.
+ *
+ * @param tuple {@link InternalTuple} representation.
+ * @return Instantiation defined representation.
+ */
+ RowT wrap(InternalTuple tuple);
Review Comment:
let's call it `create` to be consistent with rest of the methods
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/RowHandler.java:
##########
@@ -44,6 +45,23 @@ public interface RowHandler<RowT> {
/** Concatenate two rows. */
RowT concat(RowT left, RowT right);
+ /**
+ * Creates a new row containing only the fields specified in the provided mapping.
+ *
+ * <p>For example:
+ * <pre>
+ * source row [5, 6, 7, 8] apply mapping [0, 2] with offset 1
+ * result mapping will be [1, 3]
+ * result row will be [6, 8]
+ * </pre>
+ *
+ * @param row Source row.
+ * @param mapping Target field indexes.
+ * @param offset Target field indexes offset.
+ * @return A new row with fields from the specified mapping.
+ */
+ RowT map(RowT row, int[] mapping, int offset);
Review Comment:
Firstly, this method has rather peculiar signature. Why do we need offset here? An array of integers is actually all you need to create a projection of original row.
Secondly, why do we even need this method in the first place? I see only one usage, and there it does make sense to create new row explicitly and fill it with proper values
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/row/RowSchema.java:
##########
@@ -39,6 +43,43 @@ public List<TypeSpec> fields() {
return fields;
}
+ /**
+ * Gets an object representation from a tuple's field.
+ *
+ * @param tuple Tuple to read the value from.
+ * @param fieldIndex Field index to read.
+ * @return An object representation of the value.
+ */
+ public @Nullable Object value(InternalTuple tuple, int fieldIndex) {
Review Comment:
RowSchema definitely not the proper place to put this method to
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/SqlRowHandler.java:
##########
@@ -0,0 +1,272 @@
+/*
+ * 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.sql.engine.exec;
+
+import java.nio.ByteBuffer;
+import java.util.List;
+import org.apache.ignite.internal.schema.BinaryTuple;
+import org.apache.ignite.internal.schema.NativeType;
+import org.apache.ignite.internal.schema.row.InternalTuple;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler.RowWrapper;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema.Builder;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchemaTypes;
+import org.apache.ignite.internal.sql.engine.exec.row.TypeSpec;
+import org.apache.ignite.internal.sql.engine.util.Commons;
+import org.apache.ignite.internal.sql.engine.util.TypeUtils;
+import org.apache.ignite.lang.IgniteStringBuilder;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Handler that uses a {@link RowWrapper wrapper} to operate with two types of row implementations.
+ *
+ * <ul>
+ * <li>{@link ObjectsArrayRowWrapper Objects array}</li>
+ * <li>{@link BinaryTupleRowWrapper Binary tuple}</li>
+ * </ul>
+ *
+ * <p>Each kind of rows is serialized to the same binary tuple format
+ * using the {@link #toByteBuffer(RowWrapper) toByteBuffer} method.
+ *
+ * <p>Factory methods {@link RowFactory#wrap(InternalTuple) wrap(InternalTuple)} and
+ * {@link RowFactory#create(ByteBuffer) create(ByteBuffer)} allow create rows without
+ * any additional conversions. But the fields in binary tuple must match the
+ * factory {@link RowSchema row schema}.
+ */
+public class SqlRowHandler implements RowHandler<RowWrapper> {
+ public static final RowHandler<RowWrapper> INSTANCE = new SqlRowHandler();
+
+ private SqlRowHandler() {
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public @Nullable Object get(int field, RowWrapper row) {
+ return row.get(field);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public void set(int field, RowWrapper row, @Nullable Object val) {
+ row.set(field, val);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper concat(RowWrapper left, RowWrapper right) {
+ int leftLen = left.columnsCount();
+ int rightLen = right.columnsCount();
+ List<TypeSpec> leftTypes = left.rowSchema().fields();
+ List<TypeSpec> rightTypes = right.rowSchema().fields();
+
+ Object[] values = new Object[leftLen + rightLen];
+ Builder schemaBuilder = RowSchema.builder();
+
+ for (int i = 0; i < leftLen; i++) {
+ values[i] = left.get(i);
+ schemaBuilder.addField(leftTypes.get(i));
+ }
+
+ for (int i = 0; i < rightLen; i++) {
+ values[leftLen + i] = right.get(i);
+ schemaBuilder.addField(rightTypes.get(i));
+ }
+
+ return new ObjectsArrayRowWrapper(schemaBuilder.build(), values);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper map(RowWrapper row, int[] mapping, int offset) {
+ Object[] fields = new Object[mapping.length];
+
+ for (int i = 0; i < mapping.length; i++) {
+ fields[i] = row.get(mapping[i] + offset);
+ }
+
+ return new ObjectsArrayRowWrapper(row.rowSchema(), fields);
+ }
+
+ @Override
+ public int columnCount(RowWrapper row) {
+ return row.columnsCount();
+ }
+
+ @Override
+ public String toString(RowWrapper row) {
+ IgniteStringBuilder buf = new IgniteStringBuilder("Row[");
+ int maxIdx = columnCount(row) - 1;
+
+ for (int i = 0; i <= maxIdx; i++) {
+ buf.app(row.get(i));
+
+ if (i != maxIdx) {
+ buf.app(", ");
+ }
+ }
+
+ return buf.app(']').toString();
+ }
+
+ @Override
+ public ByteBuffer toByteBuffer(RowWrapper row) {
+ return row.toByteBuffer();
+ }
+
+ @Override
+ public RowFactory<RowWrapper> factory(RowSchema rowSchema) {
+ int rowLen = rowSchema.fields().size();
+
+ return new RowFactory<>() {
+ /** {@inheritDoc} */
+ @Override
+ public RowHandler<RowWrapper> handler() {
+ return SqlRowHandler.this;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper create() {
+ return create(new Object[rowLen]);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper create(Object... fields) {
+ assert fields.length == rowLen;
+
+ return new ObjectsArrayRowWrapper(rowSchema, fields);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper create(ByteBuffer buf) {
+ return wrap(new BinaryTuple(rowLen, buf));
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper wrap(InternalTuple tuple) {
+ assert rowLen == tuple.elementCount();
+
+ return new BinaryTupleRowWrapper(rowSchema, tuple);
+ }
+ };
+ }
+
+ /**
+ * Provides the ability for a single {@link RowHandler} instance to interact with multiple row implementations.
+ */
+ public abstract static class RowWrapper {
+ abstract int columnsCount();
+
+ abstract RowSchema rowSchema();
+
+ abstract @Nullable Object get(int field);
+
+ abstract void set(int field, Object value);
+
+ abstract ByteBuffer toByteBuffer();
+ }
+
+ /**
+ * Wrapper over an array of objects.
+ */
+ private static class ObjectsArrayRowWrapper extends RowWrapper {
+ private final RowSchema rowSchema;
+ private final Object[] row;
+
+ ObjectsArrayRowWrapper(RowSchema rowSchema, Object[] row) {
+ this.rowSchema = rowSchema;
+ this.row = row;
+ }
+
+ @Override
+ int columnsCount() {
+ return row.length;
+ }
+
+ @Override
+ @Nullable Object get(int field) {
+ return row[field];
+ }
+
+ @Override
+ void set(int field, @Nullable Object value) {
+ row[field] = value;
+ }
+
+ @Override
+ ByteBuffer toByteBuffer() {
+ return RowConverter.toByteBuffer(rowSchema, row);
+ }
+
+ @Override
+ RowSchema rowSchema() {
+ return rowSchema;
+ }
+ }
+
+ /**
+ * Wrapper over an {@link BinaryTuple}.
+ *
+ * <p>Since {@link BinaryTuple binary tuple} is immutable this wrapper doesn't support {@link #set(int, Object)} operation.
+ */
+ private static class BinaryTupleRowWrapper extends RowWrapper {
+ private final RowSchema rowSchema;
+ private final InternalTuple tuple;
+
+ BinaryTupleRowWrapper(RowSchema rowSchema, InternalTuple tuple) {
+ this.rowSchema = rowSchema;
+ this.tuple = tuple;
+ }
+
+ @Override
+ int columnsCount() {
+ return tuple.elementCount();
+ }
+
+ @Override
+ @Nullable Object get(int field) {
+ Object value = rowSchema.value(tuple, field);
+
+ if (value == null) {
+ return null;
+ }
+
+ NativeType nativeType = RowSchemaTypes.toNativeType(rowSchema.fields().get(field));
+
+ return TypeUtils.toInternal(value, Commons.nativeTypeToClass(nativeType));
+ }
+
+ @Override
+ void set(int field, @Nullable Object value) {
+ throw new UnsupportedOperationException();
Review Comment:
how to make sure no one will invoke `set` for row that is wrapper around BinaryTuple?
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/RowConverter.java:
##########
@@ -108,34 +121,44 @@ public static <RowT> BinaryTuple toBinaryTuple(
assert rowColumnsCount == binarySchema.elementCount() : "Invalid lookup key.";
- if (IgniteUtils.assertionsEnabled()) {
- for (int i = 0; i < rowColumnsCount; i++) {
- if (handler.get(i, searchRow) == UNSPECIFIED_VALUE_PLACEHOLDER) {
- throw new AssertionError("Invalid lookup key.");
- }
- }
- }
-
BinaryTupleBuilder tupleBuilder = new BinaryTupleBuilder(rowColumnsCount);
- return new BinaryTuple(rowColumnsCount, toByteBuffer(ectx, binarySchema, handler, tupleBuilder, searchRow));
+ return new BinaryTuple(rowColumnsCount, toByteBuffer(binarySchema, handler, tupleBuilder, rowColumnsCount, searchRow));
+ }
+
+ /**
+ * Converts an array of objects into a {@link ByteBuffer} representation of a {@link BinaryTuple binary tuple}. This method uses
+ * {@link RowSchema} to resolve object types.
+ *
+ * @param schema Row schema.
+ * @param values Array of objects to be stored in the buffer.
+ * @return Buffer in binary tuple format.
+ */
+ static ByteBuffer toByteBuffer(RowSchema schema, Object[] values) {
Review Comment:
why do you put this method here rather than to SqlRowHandler?
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/RowHandler.java:
##########
@@ -87,5 +105,13 @@ interface RowFactory<RowT> {
* @return Instantiation defined representation.
*/
RowT create(ByteBuffer raw);
+
+ /**
+ * Wraps incoming binary tuple into a instantiation defined representation.
Review Comment:
> into a instantiation defined representation.
not sure I understand this part... Could you reword this please?
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/TableRowConverterImpl.java:
##########
@@ -53,30 +59,36 @@ public <RowT> RowT toRow(
RowHandler.RowFactory<RowT> factory,
@Nullable BitSet requiredColumns
) {
- RowHandler<RowT> handler = factory.handler();
+ Row row = schemaRegistry.resolve(binaryRow, schemaDescriptor);
- assert handler == ectx.rowHandler();
+ BinaryTupleBuilder builder = requiredColumns == null
Review Comment:
why don't return BinaryTuple from `allColumnsTuple`/`requiredColumnsTuple`?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1316845663
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/RowConverter.java:
##########
@@ -108,34 +121,44 @@ public static <RowT> BinaryTuple toBinaryTuple(
assert rowColumnsCount == binarySchema.elementCount() : "Invalid lookup key.";
- if (IgniteUtils.assertionsEnabled()) {
- for (int i = 0; i < rowColumnsCount; i++) {
- if (handler.get(i, searchRow) == UNSPECIFIED_VALUE_PLACEHOLDER) {
- throw new AssertionError("Invalid lookup key.");
- }
- }
- }
-
BinaryTupleBuilder tupleBuilder = new BinaryTupleBuilder(rowColumnsCount);
- return new BinaryTuple(rowColumnsCount, toByteBuffer(ectx, binarySchema, handler, tupleBuilder, searchRow));
+ return new BinaryTuple(rowColumnsCount, toByteBuffer(binarySchema, handler, tupleBuilder, rowColumnsCount, searchRow));
+ }
+
+ /**
+ * Converts an array of objects into a {@link ByteBuffer} representation of a {@link BinaryTuple binary tuple}. This method uses
+ * {@link RowSchema} to resolve object types.
+ *
+ * @param schema Row schema.
+ * @param values Array of objects to be stored in the buffer.
+ * @return Buffer in binary tuple format.
+ */
+ static ByteBuffer toByteBuffer(RowSchema schema, Object[] values) {
Review Comment:
> why do you put this method here
It seemed to me that this is a more suitable place, because. this class essentially does the same RowT -> BinaryTuple conversion.
anyway, moved to SqlRowHandler.
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/RowConverter.java:
##########
@@ -108,34 +121,44 @@ public static <RowT> BinaryTuple toBinaryTuple(
assert rowColumnsCount == binarySchema.elementCount() : "Invalid lookup key.";
- if (IgniteUtils.assertionsEnabled()) {
- for (int i = 0; i < rowColumnsCount; i++) {
- if (handler.get(i, searchRow) == UNSPECIFIED_VALUE_PLACEHOLDER) {
- throw new AssertionError("Invalid lookup key.");
- }
- }
- }
-
BinaryTupleBuilder tupleBuilder = new BinaryTupleBuilder(rowColumnsCount);
- return new BinaryTuple(rowColumnsCount, toByteBuffer(ectx, binarySchema, handler, tupleBuilder, searchRow));
+ return new BinaryTuple(rowColumnsCount, toByteBuffer(binarySchema, handler, tupleBuilder, rowColumnsCount, searchRow));
+ }
+
+ /**
+ * Converts an array of objects into a {@link ByteBuffer} representation of a {@link BinaryTuple binary tuple}. This method uses
+ * {@link RowSchema} to resolve object types.
+ *
+ * @param schema Row schema.
+ * @param values Array of objects to be stored in the buffer.
+ * @return Buffer in binary tuple format.
+ */
+ static ByteBuffer toByteBuffer(RowSchema schema, Object[] values) {
Review Comment:
> why do you put this method here
It seemed to me that this is a more suitable place, because. this class essentially does the same RowT -> BinaryTuple conversion.
anyway, moved to SqlRowHandler.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1314615155
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/row/RowSchemaTypes.java:
##########
@@ -82,4 +83,21 @@ public static BaseTypeSpec nativeTypeWithNullability(NativeType nativeType, bool
return new BaseTypeSpec(nativeType, nullable);
}
}
+
+ /** Returns a {@link NativeType} extracted from the row schema type. */
Review Comment:
Javadoc updated, thanks.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern merged pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern merged PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1316839526
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/RowHandler.java:
##########
@@ -87,5 +105,13 @@ interface RowFactory<RowT> {
* @return Instantiation defined representation.
*/
RowT create(ByteBuffer raw);
+
+ /**
+ * Wraps incoming binary tuple into a instantiation defined representation.
Review Comment:
Reword a bit.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1314628082
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/SqlRowHandler.java:
##########
@@ -0,0 +1,260 @@
+/*
+ * 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.sql.engine.exec;
+
+import java.nio.ByteBuffer;
+import java.util.List;
+import org.apache.ignite.internal.schema.BinaryTuple;
+import org.apache.ignite.internal.schema.NativeType;
+import org.apache.ignite.internal.schema.row.InternalTuple;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler.RowWrapper;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema.Builder;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchemaTypes;
+import org.apache.ignite.internal.sql.engine.exec.row.TypeSpec;
+import org.apache.ignite.internal.sql.engine.util.Commons;
+import org.apache.ignite.internal.sql.engine.util.TypeUtils;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Handler that uses a {@link RowWrapper wrapper} to operate with two types of row implementations.
+ *
+ * <ul>
+ * <li>{@link ObjectsArrayRowWrapper Objects array}</li>
+ * <li>{@link BinaryTupleRowWrapper Binary tuple}</li>
+ * </ul>
+ *
+ * <p>Each kind of rows is serialized to the same binary tuple format
+ * using the {@link #toByteBuffer(RowWrapper) toByteBuffer} method.
+ *
+ * <p>Factory methods {@link RowFactory#wrap(InternalTuple) wrap(InternalTuple)} and
+ * {@link RowFactory#create(ByteBuffer) create(ByteBuffer)} allow create rows without
+ * any additional conversions. But the fields in binary tuple must match the
+ * factory {@link RowSchema row schema}.
+ */
+public class SqlRowHandler implements RowHandler<RowWrapper> {
+ public static final RowHandler<RowWrapper> INSTANCE = new SqlRowHandler();
+
+ private SqlRowHandler() {
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public @Nullable Object get(int field, RowWrapper row) {
+ return row.get(field);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public void set(int field, RowWrapper row, @Nullable Object val) {
+ row.set(field, val);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper concat(RowWrapper left, RowWrapper right) {
+ int leftLen = left.columnsCount();
+ int rightLen = right.columnsCount();
+ List<TypeSpec> leftTypes = left.rowSchema().fields();
+ List<TypeSpec> rightTypes = right.rowSchema().fields();
+
+ Object[] values = new Object[leftLen + rightLen];
+ Builder schemaBuilder = RowSchema.builder();
+
+ for (int i = 0; i < leftLen; i++) {
+ values[i] = left.get(i);
+ schemaBuilder.addField(leftTypes.get(i));
+ }
+
+ for (int i = 0; i < rightLen; i++) {
+ values[leftLen + i] = right.get(i);
Review Comment:
We need two indices anyway. I don't think it worth to replace `leftLen + i` to something like `index++` and declare `index` out of cycle.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] lowka commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1314463385
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/row/SqlRowHandlerTest.java:
##########
@@ -0,0 +1,214 @@
+/*
+ * 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.sql.engine.exec.row;
+
+import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.generateValueByType;
+import static org.apache.ignite.internal.sql.engine.util.TypeUtils.columnType2NativeType;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+
+import java.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.ThreadLocalRandom;
+import java.util.stream.Stream;
+import org.apache.ignite.internal.schema.BinaryTuple;
+import org.apache.ignite.internal.schema.NativeType;
+import org.apache.ignite.internal.schema.NativeTypes;
+import org.apache.ignite.internal.sql.engine.exec.RowHandler;
+import org.apache.ignite.internal.sql.engine.exec.RowHandler.RowFactory;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler.RowWrapper;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema.Builder;
+import org.apache.ignite.internal.sql.engine.util.TypeUtils;
+import org.apache.ignite.internal.testframework.IgniteAbstractTest;
+import org.apache.ignite.sql.ColumnType;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Named;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+/**
+ * Tests for {@link SqlRowHandler}.
+ */
+public class SqlRowHandlerTest extends IgniteAbstractTest {
+ private static final RowHandler<RowWrapper> handler = SqlRowHandler.INSTANCE;
+
+ private final long seed = ThreadLocalRandom.current().nextLong();
+
+ private final Random rnd = new Random(seed);
+
+ @BeforeEach
+ void printSeed() {
+ log.info("Using seed: " + seed);
+ }
+
+ @Test
+ public void testBytebufferSerialization() {
+ List<ColumnType> columnTypes = columnTypes();
+ Object[] sourceData = values(columnTypes);
+ RowSchema schema = rowSchema(columnTypes, sourceData);
+
+ int elementsCount = schema.fields().size();
+
+ RowFactory<RowWrapper> factory = handler.factory(schema);
+ RowWrapper src = factory.create(sourceData);
+
+ // Serialization to binary tuple representation.
+ ByteBuffer buf = handler.toByteBuffer(src);
+
+ BinaryTuple tuple = new BinaryTuple(elementsCount, buf);
+ RowWrapper destWrap = factory.wrap(tuple);
+ RowWrapper dest = factory.create(buf);
+
+ for (int i = 0; i < elementsCount; i++) {
+ String msg = schema.fields().get(i).toString();
+
+ assertThat(msg, handler.get(i, src), equalTo(sourceData[i]));
+ assertThat(msg, schema.value(i, tuple), equalTo(sourceData[i]));
+
+ // Binary tuple wrapper must return data in internal format.
+ Object expected = TypeUtils.toInternal(sourceData[i]);
+
+ assertThat(msg, handler.get(i, dest), equalTo(expected));
+ assertThat(msg, handler.get(i, destWrap), equalTo(expected));
+ }
+ }
+
+ @ParameterizedTest(name = "{0} - {1}")
+ @MethodSource("concatTestArguments")
+ public void testConcat(boolean leftTupleRequired, boolean rightTupleRequired) {
+ ConcatTestParameters params = new ConcatTestParameters(leftTupleRequired, rightTupleRequired);
+
+ RowWrapper concatenated = handler.concat(params.left, params.right);
+
+ int leftLen = params.leftData.length;
+ int rightLen = params.rightData.length;
+ int totalElementsCount = leftLen + rightLen;
+
+ assertThat(handler.columnCount(concatenated), equalTo(totalElementsCount));
+
+ // Build combined schema.
+ Builder builder = RowSchema.builder();
+ params.leftSchema.fields().forEach(builder::addField);
+ params.rightSchema.fields().forEach(builder::addField);
+
+ RowSchema concatenatedSchema = builder.build();
+
+ // Serialize.
+ ByteBuffer buf = handler.toByteBuffer(concatenated);
+
+ // Wrap into row.
+ RowWrapper wrapped = handler.factory(concatenatedSchema).wrap(new BinaryTuple(totalElementsCount, buf));
+
+ for (int i = 0; i < Math.max(leftLen, rightLen); i++) {
+ if (i < leftLen) {
+ assertThat(handler.get(i, wrapped), equalTo(TypeUtils.toInternal(params.leftData[i])));
+ }
+
+ if (i < rightLen) {
+ assertThat(handler.get(leftLen + i, wrapped), equalTo(TypeUtils.toInternal(params.rightData[i])));
+ }
+ }
+ }
+
+ private static Stream<Arguments> concatTestArguments() {
+ return Stream.of(
+ Arguments.of(Named.of("array", false), Named.of("array", false)),
+ Arguments.of(Named.of("array", false), Named.of("tuple", true)),
+ Arguments.of(Named.of("tuple", true), Named.of("array", false)),
+ Arguments.of(Named.of("tuple", true), Named.of("tuple", true))
+ );
+ }
+
+ private RowSchema rowSchema(List<ColumnType> columnTypes, Object[] values) {
+ Builder schemaBuilder = RowSchema.builder();
+
+ for (int i = 0; i < values.length; i++) {
+ ColumnType type = columnTypes.get(i);
+
+ if (type == ColumnType.NULL) {
+ schemaBuilder.addField(new NullTypeSpec());
+
+ continue;
+ }
+
+ NativeType nativeType = values[i] == null
+ ? columnType2NativeType(type, 9, 3, 20)
+ : NativeTypes.fromObject(values[i]);
+
+ schemaBuilder.addField(nativeType, values[i] == null || rnd.nextBoolean());
+ }
+
+ return schemaBuilder.build();
+ }
+
+ private Object[] values(List<ColumnType> columnTypes) {
+ Object[] values = new Object[columnTypes.size()];
+ int baseValue = rnd.nextInt();
+
+ for (int i = 0; i < values.length; i++) {
+ ColumnType type = columnTypes.get(i);
+
+ values[i] = type == ColumnType.NULL || rnd.nextInt(4) == 1 ? null : generateValueByType(baseValue, type);
Review Comment:
Every fourth type (assuming that distribution is gaussian) is expected to be nullable.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1314705906
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/rel/ModifyNode.java:
##########
@@ -268,15 +268,7 @@ private void inlineUpdates(int offset, List<RowT> rows) {
RowHandler<RowT> handler = context().rowHandler();
- for (RowT row : rows) {
- for (int i = 0; i < mapping.length; i++) {
- if (offset == 0 && i == mapping[i]) {
- continue;
- }
-
- handler.set(i, row, handler.get(mapping[i] + offset, row));
- }
- }
+ rows.replaceAll(row -> handler.map(row, mapping, offset));
Review Comment:
This is because BinaryTuple is not mutable.
Those. An "honest" arbitrary change to the BinaryTuple field will result in a complete rebuild of the new BinaryTuple instance, which looks inefficient.
An alternative implementation of an arbitrary "set" operation is to add a structure to each Row that will reflect the changes made (for example, a Map or an array), but this will lead to the fact that at least a new reference to this structure will have to be added to each row instance. In addition, it would be desirable to support this during serialization.
At the moment, this is only one place where we call "set(index)" on a deserialized row.
And it seems that this "map" operation can be implemented more efficiently for a binarytuple than arbitrary set(index).
It seems to me that the question of the effective implementation of this method should be discussed separately if we come to an agreement that this method has make sense to add.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1315438958
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/SqlRowHandler.java:
##########
@@ -0,0 +1,272 @@
+/*
+ * 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.sql.engine.exec;
+
+import java.nio.ByteBuffer;
+import java.util.List;
+import org.apache.ignite.internal.schema.BinaryTuple;
+import org.apache.ignite.internal.schema.NativeType;
+import org.apache.ignite.internal.schema.row.InternalTuple;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler.RowWrapper;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema.Builder;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchemaTypes;
+import org.apache.ignite.internal.sql.engine.exec.row.TypeSpec;
+import org.apache.ignite.internal.sql.engine.util.Commons;
+import org.apache.ignite.internal.sql.engine.util.TypeUtils;
+import org.apache.ignite.lang.IgniteStringBuilder;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Handler that uses a {@link RowWrapper wrapper} to operate with two types of row implementations.
+ *
+ * <ul>
+ * <li>{@link ObjectsArrayRowWrapper Objects array}</li>
+ * <li>{@link BinaryTupleRowWrapper Binary tuple}</li>
+ * </ul>
+ *
+ * <p>Each kind of rows is serialized to the same binary tuple format
+ * using the {@link #toByteBuffer(RowWrapper) toByteBuffer} method.
+ *
+ * <p>Factory methods {@link RowFactory#wrap(InternalTuple) wrap(InternalTuple)} and
+ * {@link RowFactory#create(ByteBuffer) create(ByteBuffer)} allow create rows without
+ * any additional conversions. But the fields in binary tuple must match the
+ * factory {@link RowSchema row schema}.
+ */
+public class SqlRowHandler implements RowHandler<RowWrapper> {
+ public static final RowHandler<RowWrapper> INSTANCE = new SqlRowHandler();
+
+ private SqlRowHandler() {
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public @Nullable Object get(int field, RowWrapper row) {
+ return row.get(field);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public void set(int field, RowWrapper row, @Nullable Object val) {
+ row.set(field, val);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper concat(RowWrapper left, RowWrapper right) {
+ int leftLen = left.columnsCount();
+ int rightLen = right.columnsCount();
+ List<TypeSpec> leftTypes = left.rowSchema().fields();
+ List<TypeSpec> rightTypes = right.rowSchema().fields();
+
+ Object[] values = new Object[leftLen + rightLen];
+ Builder schemaBuilder = RowSchema.builder();
+
+ for (int i = 0; i < leftLen; i++) {
+ values[i] = left.get(i);
+ schemaBuilder.addField(leftTypes.get(i));
+ }
+
+ for (int i = 0; i < rightLen; i++) {
+ values[leftLen + i] = right.get(i);
+ schemaBuilder.addField(rightTypes.get(i));
+ }
+
+ return new ObjectsArrayRowWrapper(schemaBuilder.build(), values);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper map(RowWrapper row, int[] mapping, int offset) {
+ Object[] fields = new Object[mapping.length];
+
+ for (int i = 0; i < mapping.length; i++) {
+ fields[i] = row.get(mapping[i] + offset);
+ }
+
+ return new ObjectsArrayRowWrapper(row.rowSchema(), fields);
+ }
+
+ @Override
+ public int columnCount(RowWrapper row) {
+ return row.columnsCount();
+ }
+
+ @Override
+ public String toString(RowWrapper row) {
+ IgniteStringBuilder buf = new IgniteStringBuilder("Row[");
+ int maxIdx = columnCount(row) - 1;
+
+ for (int i = 0; i <= maxIdx; i++) {
+ buf.app(row.get(i));
+
+ if (i != maxIdx) {
+ buf.app(", ");
+ }
+ }
+
+ return buf.app(']').toString();
+ }
+
+ @Override
+ public ByteBuffer toByteBuffer(RowWrapper row) {
+ return row.toByteBuffer();
+ }
+
+ @Override
+ public RowFactory<RowWrapper> factory(RowSchema rowSchema) {
+ int rowLen = rowSchema.fields().size();
+
+ return new RowFactory<>() {
+ /** {@inheritDoc} */
+ @Override
+ public RowHandler<RowWrapper> handler() {
+ return SqlRowHandler.this;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper create() {
+ return create(new Object[rowLen]);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper create(Object... fields) {
+ assert fields.length == rowLen;
+
+ return new ObjectsArrayRowWrapper(rowSchema, fields);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper create(ByteBuffer buf) {
+ return wrap(new BinaryTuple(rowLen, buf));
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper wrap(InternalTuple tuple) {
+ assert rowLen == tuple.elementCount();
+
+ return new BinaryTupleRowWrapper(rowSchema, tuple);
+ }
+ };
+ }
+
+ /**
+ * Provides the ability for a single {@link RowHandler} instance to interact with multiple row implementations.
+ */
+ public abstract static class RowWrapper {
Review Comment:
I would like to hide the methods of this class so that it is possible to work only through the RowHandler and not directly :thinking: :roll_eyes:
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1316889031
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/SqlRowHandler.java:
##########
@@ -0,0 +1,272 @@
+/*
+ * 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.sql.engine.exec;
+
+import java.nio.ByteBuffer;
+import java.util.List;
+import org.apache.ignite.internal.schema.BinaryTuple;
+import org.apache.ignite.internal.schema.NativeType;
+import org.apache.ignite.internal.schema.row.InternalTuple;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler.RowWrapper;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema.Builder;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchemaTypes;
+import org.apache.ignite.internal.sql.engine.exec.row.TypeSpec;
+import org.apache.ignite.internal.sql.engine.util.Commons;
+import org.apache.ignite.internal.sql.engine.util.TypeUtils;
+import org.apache.ignite.lang.IgniteStringBuilder;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Handler that uses a {@link RowWrapper wrapper} to operate with two types of row implementations.
+ *
+ * <ul>
+ * <li>{@link ObjectsArrayRowWrapper Objects array}</li>
+ * <li>{@link BinaryTupleRowWrapper Binary tuple}</li>
+ * </ul>
+ *
+ * <p>Each kind of rows is serialized to the same binary tuple format
+ * using the {@link #toByteBuffer(RowWrapper) toByteBuffer} method.
+ *
+ * <p>Factory methods {@link RowFactory#wrap(InternalTuple) wrap(InternalTuple)} and
+ * {@link RowFactory#create(ByteBuffer) create(ByteBuffer)} allow create rows without
+ * any additional conversions. But the fields in binary tuple must match the
+ * factory {@link RowSchema row schema}.
+ */
+public class SqlRowHandler implements RowHandler<RowWrapper> {
+ public static final RowHandler<RowWrapper> INSTANCE = new SqlRowHandler();
+
+ private SqlRowHandler() {
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public @Nullable Object get(int field, RowWrapper row) {
+ return row.get(field);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public void set(int field, RowWrapper row, @Nullable Object val) {
+ row.set(field, val);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper concat(RowWrapper left, RowWrapper right) {
+ int leftLen = left.columnsCount();
+ int rightLen = right.columnsCount();
+ List<TypeSpec> leftTypes = left.rowSchema().fields();
+ List<TypeSpec> rightTypes = right.rowSchema().fields();
+
+ Object[] values = new Object[leftLen + rightLen];
+ Builder schemaBuilder = RowSchema.builder();
+
+ for (int i = 0; i < leftLen; i++) {
+ values[i] = left.get(i);
+ schemaBuilder.addField(leftTypes.get(i));
+ }
+
+ for (int i = 0; i < rightLen; i++) {
+ values[leftLen + i] = right.get(i);
+ schemaBuilder.addField(rightTypes.get(i));
+ }
+
+ return new ObjectsArrayRowWrapper(schemaBuilder.build(), values);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper map(RowWrapper row, int[] mapping, int offset) {
+ Object[] fields = new Object[mapping.length];
+
+ for (int i = 0; i < mapping.length; i++) {
+ fields[i] = row.get(mapping[i] + offset);
+ }
+
+ return new ObjectsArrayRowWrapper(row.rowSchema(), fields);
+ }
+
+ @Override
+ public int columnCount(RowWrapper row) {
+ return row.columnsCount();
+ }
+
+ @Override
+ public String toString(RowWrapper row) {
+ IgniteStringBuilder buf = new IgniteStringBuilder("Row[");
+ int maxIdx = columnCount(row) - 1;
+
+ for (int i = 0; i <= maxIdx; i++) {
+ buf.app(row.get(i));
+
+ if (i != maxIdx) {
+ buf.app(", ");
+ }
+ }
+
+ return buf.app(']').toString();
+ }
+
+ @Override
+ public ByteBuffer toByteBuffer(RowWrapper row) {
+ return row.toByteBuffer();
+ }
+
+ @Override
+ public RowFactory<RowWrapper> factory(RowSchema rowSchema) {
+ int rowLen = rowSchema.fields().size();
+
+ return new RowFactory<>() {
+ /** {@inheritDoc} */
+ @Override
+ public RowHandler<RowWrapper> handler() {
+ return SqlRowHandler.this;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper create() {
+ return create(new Object[rowLen]);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper create(Object... fields) {
+ assert fields.length == rowLen;
+
+ return new ObjectsArrayRowWrapper(rowSchema, fields);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper create(ByteBuffer buf) {
+ return wrap(new BinaryTuple(rowLen, buf));
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper wrap(InternalTuple tuple) {
+ assert rowLen == tuple.elementCount();
+
+ return new BinaryTupleRowWrapper(rowSchema, tuple);
+ }
+ };
+ }
+
+ /**
+ * Provides the ability for a single {@link RowHandler} instance to interact with multiple row implementations.
+ */
+ public abstract static class RowWrapper {
+ abstract int columnsCount();
+
+ abstract RowSchema rowSchema();
+
+ abstract @Nullable Object get(int field);
+
+ abstract void set(int field, Object value);
+
+ abstract ByteBuffer toByteBuffer();
+ }
+
+ /**
+ * Wrapper over an array of objects.
+ */
+ private static class ObjectsArrayRowWrapper extends RowWrapper {
+ private final RowSchema rowSchema;
+ private final Object[] row;
+
+ ObjectsArrayRowWrapper(RowSchema rowSchema, Object[] row) {
+ this.rowSchema = rowSchema;
+ this.row = row;
+ }
+
+ @Override
+ int columnsCount() {
+ return row.length;
+ }
+
+ @Override
+ @Nullable Object get(int field) {
+ return row[field];
+ }
+
+ @Override
+ void set(int field, @Nullable Object value) {
+ row[field] = value;
+ }
+
+ @Override
+ ByteBuffer toByteBuffer() {
+ return RowConverter.toByteBuffer(rowSchema, row);
+ }
+
+ @Override
+ RowSchema rowSchema() {
+ return rowSchema;
+ }
+ }
+
+ /**
+ * Wrapper over an {@link BinaryTuple}.
+ *
+ * <p>Since {@link BinaryTuple binary tuple} is immutable this wrapper doesn't support {@link #set(int, Object)} operation.
+ */
+ private static class BinaryTupleRowWrapper extends RowWrapper {
+ private final RowSchema rowSchema;
+ private final InternalTuple tuple;
+
+ BinaryTupleRowWrapper(RowSchema rowSchema, InternalTuple tuple) {
+ this.rowSchema = rowSchema;
+ this.tuple = tuple;
+ }
+
+ @Override
+ int columnsCount() {
+ return tuple.elementCount();
+ }
+
+ @Override
+ @Nullable Object get(int field) {
+ Object value = rowSchema.value(tuple, field);
+
+ if (value == null) {
+ return null;
+ }
+
+ NativeType nativeType = RowSchemaTypes.toNativeType(rowSchema.fields().get(field));
+
+ return TypeUtils.toInternal(value, Commons.nativeTypeToClass(nativeType));
+ }
+
+ @Override
+ void set(int field, @Nullable Object value) {
+ throw new UnsupportedOperationException();
Review Comment:
I agree that this method should be implemented or should not be provided at all.
From my point of view, the only right way is to replace this method with append(Object obj) and rework all related code.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1316840567
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/TableRowConverterImpl.java:
##########
@@ -53,30 +59,36 @@ public <RowT> RowT toRow(
RowHandler.RowFactory<RowT> factory,
@Nullable BitSet requiredColumns
) {
- RowHandler<RowT> handler = factory.handler();
+ Row row = schemaRegistry.resolve(binaryRow, schemaDescriptor);
- assert handler == ectx.rowHandler();
+ BinaryTupleBuilder builder = requiredColumns == null
Review Comment:
Done, thanks
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] zstan commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "zstan (via GitHub)" <gi...@apache.org>.
zstan commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1314480258
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/RowHandler.java:
##########
@@ -44,6 +45,22 @@ public interface RowHandler<RowT> {
/** Concatenate two rows. */
RowT concat(RowT left, RowT right);
+ /**
+ * Creates a new row containing only the fields specified in the provided mapping.
+ *
+ * <p>For example:
+ * <pre>
+ * source row {5, 6, 7, 8} apply mapping {1, 3}
+ * result row {6, 8}
+ * </pre>
+ *
+ * @param row Source row.
+ * @param mapping Target field indexes.
+ * @param offset Target field indexes offset.
+ * @return A new row with fields from the specified mapping.
+ */
+ RowT map(RowT row, int[] mapping, int offset);
Review Comment:
offset - is not covered by java doc, can u extend above example for _offset_ too ?
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/exp/agg/Accumulators.java:
##########
@@ -88,11 +88,11 @@ private Supplier<Accumulator> accumulatorFunctionFactory(AggregateCall call) {
case "SOME":
return minMaxFactory(false, call);
case "SINGLE_VALUE":
- return SingleVal.FACTORY;
- case "LITERAL_AGG":
- return LiteralVal.FACTORY;
+ return singleAnyValueFactory(true, call);
case "ANY_VALUE":
- return AnyVal.FACTORY;
+ return singleAnyValueFactory(false, call);
Review Comment:
i think new division by singleAnyValueFactory(true|false become less understandable ?
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/rel/ModifyNode.java:
##########
@@ -268,15 +268,7 @@ private void inlineUpdates(int offset, List<RowT> rows) {
RowHandler<RowT> handler = context().rowHandler();
- for (RowT row : rows) {
- for (int i = 0; i < mapping.length; i++) {
- if (offset == 0 && i == mapping[i]) {
- continue;
- }
-
- handler.set(i, row, handler.get(mapping[i] + offset, row));
- }
- }
+ rows.replaceAll(row -> handler.map(row, mapping, offset));
Review Comment:
previous implementation was a bit lighter, why do we need to change it ?
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/RowConverter.java:
##########
@@ -108,34 +121,44 @@ public static <RowT> BinaryTuple toBinaryTuple(
assert rowColumnsCount == binarySchema.elementCount() : "Invalid lookup key.";
- if (IgniteUtils.assertionsEnabled()) {
- for (int i = 0; i < rowColumnsCount; i++) {
- if (handler.get(i, searchRow) == UNSPECIFIED_VALUE_PLACEHOLDER) {
- throw new AssertionError("Invalid lookup key.");
- }
- }
- }
-
BinaryTupleBuilder tupleBuilder = new BinaryTupleBuilder(rowColumnsCount);
- return new BinaryTuple(rowColumnsCount, toByteBuffer(ectx, binarySchema, handler, tupleBuilder, searchRow));
+ return new BinaryTuple(rowColumnsCount, toByteBuffer(binarySchema, handler, tupleBuilder, rowColumnsCount, searchRow));
+ }
+
+ /**
+ * Converts an array of objects to a byte buffer representation of a {@link BinaryTuple binary tuple}. This method uses
Review Comment:
```suggestion
* Converts an array of objects into {@link ByteBuffer} representation of a {@link BinaryTuple binary tuple}. This method uses
```
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/SqlRowHandler.java:
##########
@@ -0,0 +1,260 @@
+/*
+ * 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.sql.engine.exec;
+
+import java.nio.ByteBuffer;
+import java.util.List;
+import org.apache.ignite.internal.schema.BinaryTuple;
+import org.apache.ignite.internal.schema.NativeType;
+import org.apache.ignite.internal.schema.row.InternalTuple;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler.RowWrapper;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema.Builder;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchemaTypes;
+import org.apache.ignite.internal.sql.engine.exec.row.TypeSpec;
+import org.apache.ignite.internal.sql.engine.util.Commons;
+import org.apache.ignite.internal.sql.engine.util.TypeUtils;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Handler that uses a {@link RowWrapper wrapper} to operate with two types of row implementations.
+ *
+ * <ul>
+ * <li>{@link ObjectsArrayRowWrapper Objects array}</li>
+ * <li>{@link BinaryTupleRowWrapper Binary tuple}</li>
+ * </ul>
+ *
+ * <p>Each kind of rows is serialized to the same binary tuple format
+ * using the {@link #toByteBuffer(RowWrapper) toByteBuffer} method.
+ *
+ * <p>Factory methods {@link RowFactory#wrap(InternalTuple) wrap(InternalTuple)} and
+ * {@link RowFactory#create(ByteBuffer) create(ByteBuffer)} allow create rows without
+ * any additional conversions. But the fields in binary tuple must match the
+ * factory {@link RowSchema row schema}.
+ */
+public class SqlRowHandler implements RowHandler<RowWrapper> {
+ public static final RowHandler<RowWrapper> INSTANCE = new SqlRowHandler();
+
+ private SqlRowHandler() {
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public @Nullable Object get(int field, RowWrapper row) {
+ return row.get(field);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public void set(int field, RowWrapper row, @Nullable Object val) {
+ row.set(field, val);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper concat(RowWrapper left, RowWrapper right) {
+ int leftLen = left.columnsCount();
+ int rightLen = right.columnsCount();
+ List<TypeSpec> leftTypes = left.rowSchema().fields();
+ List<TypeSpec> rightTypes = right.rowSchema().fields();
+
+ Object[] values = new Object[leftLen + rightLen];
+ Builder schemaBuilder = RowSchema.builder();
+
+ for (int i = 0; i < leftLen; i++) {
+ values[i] = left.get(i);
+ schemaBuilder.addField(leftTypes.get(i));
+ }
+
+ for (int i = 0; i < rightLen; i++) {
+ values[leftLen + i] = right.get(i);
Review Comment:
no need to call additional offset here : leftLen + i
you just can initialize global int i = 0; and increment it in further for loops.
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/framework/ArrayRowHandler.java:
##########
@@ -50,6 +52,18 @@ public Object[] concat(Object[] left, Object[] right) {
return ArrayUtils.concat(left, right);
}
+ /** {@inheritDoc} */
+ @Override
+ public Object[] map(Object[] row, int[] mapping, int offset) {
Review Comment:
can we gather new row without new allocations ? just using incoming row and cut all unused stuff at the end ?
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/exp/ExpressionFactoryImpl.java:
##########
@@ -277,8 +277,17 @@ public Supplier<RowT> rowSource(List<RexNode> values) {
/** {@inheritDoc} */
@Override
public <T> Supplier<T> execute(RexNode node) {
- RelDataType nodeType = node.getType();
- RowSchema rowSchema = TypeUtils.rowSchemaFromRelTypes(List.of(nodeType));
+ RelDataType exprType = node.getType();
+ List<RelDataType> typesList;
+
+ if (exprType.getSqlTypeName() == SqlTypeName.ROW) {
Review Comment:
we have no such a check previously, can you explain why do we need it for now ?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1314614388
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/row/RowSchema.java:
##########
@@ -39,6 +43,43 @@ public List<TypeSpec> fields() {
return fields;
}
+ /**
+ * Gets an object representation from a tuple's field.
+ *
+ * @param fieldIndex Field index to read.
+ * @param tuple Tuple to read the value from.
+ * @return An object representation of the value.
+ */
+ public @Nullable Object value(int fieldIndex, InternalTuple tuple) {
Review Comment:
Swapped, thanks.
p.s. I was guided by the order in which they go in RowHandler#get/set :roll_eyes:
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1314610457
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/TableRowConverterImpl.java:
##########
@@ -53,30 +55,37 @@ public <RowT> RowT toRow(
RowHandler.RowFactory<RowT> factory,
@Nullable BitSet requiredColumns
) {
- RowHandler<RowT> handler = factory.handler();
+ Row row = schemaRegistry.resolve(binaryRow, schemaDescriptor);
+ BinaryTupleSchema binarySchema = BinaryTupleSchema.createRowSchema(schemaDescriptor);
Review Comment:
Done, thanks.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1317385250
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/RowHandler.java:
##########
@@ -44,6 +45,22 @@ public interface RowHandler<RowT> {
/** Concatenate two rows. */
RowT concat(RowT left, RowT right);
+ /**
+ * Creates a new row containing only the fields specified in the provided mapping.
+ *
+ * <p>For example:
+ * <pre>
+ * source row {5, 6, 7, 8} apply mapping {1, 3}
+ * result row {6, 8}
+ * </pre>
+ *
+ * @param row Source row.
+ * @param mapping Target field indexes.
+ * @param offset Target field indexes offset.
+ * @return A new row with fields from the specified mapping.
+ */
+ RowT map(RowT row, int[] mapping, int offset);
Review Comment:
offset removed from method signature
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] lowka commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1314470533
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/TableRowConverterImpl.java:
##########
@@ -53,30 +55,37 @@ public <RowT> RowT toRow(
RowHandler.RowFactory<RowT> factory,
@Nullable BitSet requiredColumns
) {
- RowHandler<RowT> handler = factory.handler();
+ Row row = schemaRegistry.resolve(binaryRow, schemaDescriptor);
+ BinaryTupleSchema binarySchema = BinaryTupleSchema.createRowSchema(schemaDescriptor);
Review Comment:
I think we can create `BinaryTupleSchema` in constructor and reuse it.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1314614388
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/row/RowSchema.java:
##########
@@ -39,6 +43,43 @@ public List<TypeSpec> fields() {
return fields;
}
+ /**
+ * Gets an object representation from a tuple's field.
+ *
+ * @param fieldIndex Field index to read.
+ * @param tuple Tuple to read the value from.
+ * @return An object representation of the value.
+ */
+ public @Nullable Object value(int fieldIndex, InternalTuple tuple) {
Review Comment:
Done, thanks.
p.s. I was guided by the order in which they go in RowHandler#get/set :roll_eyes:
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1314634443
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/exp/agg/Accumulators.java:
##########
@@ -88,11 +88,11 @@ private Supplier<Accumulator> accumulatorFunctionFactory(AggregateCall call) {
case "SOME":
return minMaxFactory(false, call);
case "SINGLE_VALUE":
- return SingleVal.FACTORY;
- case "LITERAL_AGG":
- return LiteralVal.FACTORY;
+ return singleAnyValueFactory(true, call);
case "ANY_VALUE":
- return AnyVal.FACTORY;
+ return singleAnyValueFactory(false, call);
Review Comment:
I divided it into two factories, if I understood your comment correctly
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1314705906
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/rel/ModifyNode.java:
##########
@@ -268,15 +268,7 @@ private void inlineUpdates(int offset, List<RowT> rows) {
RowHandler<RowT> handler = context().rowHandler();
- for (RowT row : rows) {
- for (int i = 0; i < mapping.length; i++) {
- if (offset == 0 && i == mapping[i]) {
- continue;
- }
-
- handler.set(i, row, handler.get(mapping[i] + offset, row));
- }
- }
+ rows.replaceAll(row -> handler.map(row, mapping, offset));
Review Comment:
This is because BinaryTuple is not mutable.
Those. An "honest" arbitrary change to the BinaryTuple field will result in a complete rebuild of the new BinaryTuple instance, which looks inefficient.
An alternative implementation of an arbitrary "set" operation is to add a structure to each Row that will reflect the changes made (for example, a Map or an array), but this will lead to the fact that at least a new reference (field) to this structure will have to be added to each row instance. In addition, it would be desirable to support this during serialization.
At the moment, this is only one place where we call "set(index)" on a deserialized row.
And it seems that this "map" operation can be implemented more efficiently for a binarytuple than arbitrary set(index).
The question of the effective implementation of this method should be discussed separately if we come to an agreement that this method (map) has make sense to add.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1316838631
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/RowHandler.java:
##########
@@ -44,6 +45,23 @@ public interface RowHandler<RowT> {
/** Concatenate two rows. */
RowT concat(RowT left, RowT right);
+ /**
+ * Creates a new row containing only the fields specified in the provided mapping.
+ *
+ * <p>For example:
+ * <pre>
+ * source row [5, 6, 7, 8] apply mapping [0, 2] with offset 1
+ * result mapping will be [1, 3]
+ * result row will be [6, 8]
+ * </pre>
+ *
+ * @param row Source row.
+ * @param mapping Target field indexes.
+ * @param offset Target field indexes offset.
+ * @return A new row with fields from the specified mapping.
+ */
+ RowT map(RowT row, int[] mapping, int offset);
Review Comment:
offset removed from the signature.
> why do we even need this method in the first place?
It seemed to me that depending on the implementation it can be implemented more efficiently.
> it does make sense to create new row explicitly
I didn't find how to get `rowschema` :pensive: to create a new row
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/RowHandler.java:
##########
@@ -44,6 +45,23 @@ public interface RowHandler<RowT> {
/** Concatenate two rows. */
RowT concat(RowT left, RowT right);
+ /**
+ * Creates a new row containing only the fields specified in the provided mapping.
+ *
+ * <p>For example:
+ * <pre>
+ * source row [5, 6, 7, 8] apply mapping [0, 2] with offset 1
+ * result mapping will be [1, 3]
+ * result row will be [6, 8]
+ * </pre>
+ *
+ * @param row Source row.
+ * @param mapping Target field indexes.
+ * @param offset Target field indexes offset.
+ * @return A new row with fields from the specified mapping.
+ */
+ RowT map(RowT row, int[] mapping, int offset);
Review Comment:
offset removed from the signature.
> why do we even need this method in the first place?
It seemed to me that depending on the implementation it can be implemented more efficiently.
> it does make sense to create new row explicitly
I didn't find how to get `rowschema` :pensive: to create a new row
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1316839526
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/RowHandler.java:
##########
@@ -87,5 +105,13 @@ interface RowFactory<RowT> {
* @return Instantiation defined representation.
*/
RowT create(ByteBuffer raw);
+
+ /**
+ * Wraps incoming binary tuple into a instantiation defined representation.
Review Comment:
Rephrased a bit.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1315438958
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/SqlRowHandler.java:
##########
@@ -0,0 +1,272 @@
+/*
+ * 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.sql.engine.exec;
+
+import java.nio.ByteBuffer;
+import java.util.List;
+import org.apache.ignite.internal.schema.BinaryTuple;
+import org.apache.ignite.internal.schema.NativeType;
+import org.apache.ignite.internal.schema.row.InternalTuple;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler.RowWrapper;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema.Builder;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchemaTypes;
+import org.apache.ignite.internal.sql.engine.exec.row.TypeSpec;
+import org.apache.ignite.internal.sql.engine.util.Commons;
+import org.apache.ignite.internal.sql.engine.util.TypeUtils;
+import org.apache.ignite.lang.IgniteStringBuilder;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Handler that uses a {@link RowWrapper wrapper} to operate with two types of row implementations.
+ *
+ * <ul>
+ * <li>{@link ObjectsArrayRowWrapper Objects array}</li>
+ * <li>{@link BinaryTupleRowWrapper Binary tuple}</li>
+ * </ul>
+ *
+ * <p>Each kind of rows is serialized to the same binary tuple format
+ * using the {@link #toByteBuffer(RowWrapper) toByteBuffer} method.
+ *
+ * <p>Factory methods {@link RowFactory#wrap(InternalTuple) wrap(InternalTuple)} and
+ * {@link RowFactory#create(ByteBuffer) create(ByteBuffer)} allow create rows without
+ * any additional conversions. But the fields in binary tuple must match the
+ * factory {@link RowSchema row schema}.
+ */
+public class SqlRowHandler implements RowHandler<RowWrapper> {
+ public static final RowHandler<RowWrapper> INSTANCE = new SqlRowHandler();
+
+ private SqlRowHandler() {
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public @Nullable Object get(int field, RowWrapper row) {
+ return row.get(field);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public void set(int field, RowWrapper row, @Nullable Object val) {
+ row.set(field, val);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper concat(RowWrapper left, RowWrapper right) {
+ int leftLen = left.columnsCount();
+ int rightLen = right.columnsCount();
+ List<TypeSpec> leftTypes = left.rowSchema().fields();
+ List<TypeSpec> rightTypes = right.rowSchema().fields();
+
+ Object[] values = new Object[leftLen + rightLen];
+ Builder schemaBuilder = RowSchema.builder();
+
+ for (int i = 0; i < leftLen; i++) {
+ values[i] = left.get(i);
+ schemaBuilder.addField(leftTypes.get(i));
+ }
+
+ for (int i = 0; i < rightLen; i++) {
+ values[leftLen + i] = right.get(i);
+ schemaBuilder.addField(rightTypes.get(i));
+ }
+
+ return new ObjectsArrayRowWrapper(schemaBuilder.build(), values);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper map(RowWrapper row, int[] mapping, int offset) {
+ Object[] fields = new Object[mapping.length];
+
+ for (int i = 0; i < mapping.length; i++) {
+ fields[i] = row.get(mapping[i] + offset);
+ }
+
+ return new ObjectsArrayRowWrapper(row.rowSchema(), fields);
+ }
+
+ @Override
+ public int columnCount(RowWrapper row) {
+ return row.columnsCount();
+ }
+
+ @Override
+ public String toString(RowWrapper row) {
+ IgniteStringBuilder buf = new IgniteStringBuilder("Row[");
+ int maxIdx = columnCount(row) - 1;
+
+ for (int i = 0; i <= maxIdx; i++) {
+ buf.app(row.get(i));
+
+ if (i != maxIdx) {
+ buf.app(", ");
+ }
+ }
+
+ return buf.app(']').toString();
+ }
+
+ @Override
+ public ByteBuffer toByteBuffer(RowWrapper row) {
+ return row.toByteBuffer();
+ }
+
+ @Override
+ public RowFactory<RowWrapper> factory(RowSchema rowSchema) {
+ int rowLen = rowSchema.fields().size();
+
+ return new RowFactory<>() {
+ /** {@inheritDoc} */
+ @Override
+ public RowHandler<RowWrapper> handler() {
+ return SqlRowHandler.this;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper create() {
+ return create(new Object[rowLen]);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper create(Object... fields) {
+ assert fields.length == rowLen;
+
+ return new ObjectsArrayRowWrapper(rowSchema, fields);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper create(ByteBuffer buf) {
+ return wrap(new BinaryTuple(rowLen, buf));
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper wrap(InternalTuple tuple) {
+ assert rowLen == tuple.elementCount();
+
+ return new BinaryTupleRowWrapper(rowSchema, tuple);
+ }
+ };
+ }
+
+ /**
+ * Provides the ability for a single {@link RowHandler} instance to interact with multiple row implementations.
+ */
+ public abstract static class RowWrapper {
Review Comment:
I would like to hide the methods of this class so that it is possible to work only through the RowHandler and not directly :thinking: :roll_eyes:
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] lowka commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1314453635
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/row/SqlRowHandlerTest.java:
##########
@@ -0,0 +1,214 @@
+/*
+ * 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.sql.engine.exec.row;
+
+import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.generateValueByType;
+import static org.apache.ignite.internal.sql.engine.util.TypeUtils.columnType2NativeType;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+
+import java.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.ThreadLocalRandom;
+import java.util.stream.Stream;
+import org.apache.ignite.internal.schema.BinaryTuple;
+import org.apache.ignite.internal.schema.NativeType;
+import org.apache.ignite.internal.schema.NativeTypes;
+import org.apache.ignite.internal.sql.engine.exec.RowHandler;
+import org.apache.ignite.internal.sql.engine.exec.RowHandler.RowFactory;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler.RowWrapper;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema.Builder;
+import org.apache.ignite.internal.sql.engine.util.TypeUtils;
+import org.apache.ignite.internal.testframework.IgniteAbstractTest;
+import org.apache.ignite.sql.ColumnType;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Named;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+/**
+ * Tests for {@link SqlRowHandler}.
+ */
+public class SqlRowHandlerTest extends IgniteAbstractTest {
+ private static final RowHandler<RowWrapper> handler = SqlRowHandler.INSTANCE;
+
+ private final long seed = ThreadLocalRandom.current().nextLong();
+
+ private final Random rnd = new Random(seed);
+
+ @BeforeEach
+ void printSeed() {
+ log.info("Using seed: " + seed);
+ }
+
+ @Test
+ public void testBytebufferSerialization() {
+ List<ColumnType> columnTypes = columnTypes();
+ Object[] sourceData = values(columnTypes);
+ RowSchema schema = rowSchema(columnTypes, sourceData);
+
+ int elementsCount = schema.fields().size();
+
+ RowFactory<RowWrapper> factory = handler.factory(schema);
+ RowWrapper src = factory.create(sourceData);
+
+ // Serialization to binary tuple representation.
+ ByteBuffer buf = handler.toByteBuffer(src);
+
+ BinaryTuple tuple = new BinaryTuple(elementsCount, buf);
+ RowWrapper destWrap = factory.wrap(tuple);
+ RowWrapper dest = factory.create(buf);
+
+ for (int i = 0; i < elementsCount; i++) {
+ String msg = schema.fields().get(i).toString();
+
+ assertThat(msg, handler.get(i, src), equalTo(sourceData[i]));
+ assertThat(msg, schema.value(i, tuple), equalTo(sourceData[i]));
+
+ // Binary tuple wrapper must return data in internal format.
+ Object expected = TypeUtils.toInternal(sourceData[i]);
+
+ assertThat(msg, handler.get(i, dest), equalTo(expected));
+ assertThat(msg, handler.get(i, destWrap), equalTo(expected));
+ }
+ }
+
+ @ParameterizedTest(name = "{0} - {1}")
+ @MethodSource("concatTestArguments")
+ public void testConcat(boolean leftTupleRequired, boolean rightTupleRequired) {
+ ConcatTestParameters params = new ConcatTestParameters(leftTupleRequired, rightTupleRequired);
+
+ RowWrapper concatenated = handler.concat(params.left, params.right);
+
+ int leftLen = params.leftData.length;
+ int rightLen = params.rightData.length;
+ int totalElementsCount = leftLen + rightLen;
+
+ assertThat(handler.columnCount(concatenated), equalTo(totalElementsCount));
+
+ // Build combined schema.
+ Builder builder = RowSchema.builder();
+ params.leftSchema.fields().forEach(builder::addField);
+ params.rightSchema.fields().forEach(builder::addField);
+
+ RowSchema concatenatedSchema = builder.build();
+
+ // Serialize.
+ ByteBuffer buf = handler.toByteBuffer(concatenated);
+
+ // Wrap into row.
+ RowWrapper wrapped = handler.factory(concatenatedSchema).wrap(new BinaryTuple(totalElementsCount, buf));
+
+ for (int i = 0; i < Math.max(leftLen, rightLen); i++) {
+ if (i < leftLen) {
+ assertThat(handler.get(i, wrapped), equalTo(TypeUtils.toInternal(params.leftData[i])));
+ }
+
+ if (i < rightLen) {
+ assertThat(handler.get(leftLen + i, wrapped), equalTo(TypeUtils.toInternal(params.rightData[i])));
+ }
+ }
+ }
+
+ private static Stream<Arguments> concatTestArguments() {
+ return Stream.of(
+ Arguments.of(Named.of("array", false), Named.of("array", false)),
+ Arguments.of(Named.of("array", false), Named.of("tuple", true)),
+ Arguments.of(Named.of("tuple", true), Named.of("array", false)),
+ Arguments.of(Named.of("tuple", true), Named.of("tuple", true))
+ );
+ }
+
+ private RowSchema rowSchema(List<ColumnType> columnTypes, Object[] values) {
+ Builder schemaBuilder = RowSchema.builder();
+
+ for (int i = 0; i < values.length; i++) {
+ ColumnType type = columnTypes.get(i);
+
+ if (type == ColumnType.NULL) {
+ schemaBuilder.addField(new NullTypeSpec());
+
+ continue;
+ }
+
+ NativeType nativeType = values[i] == null
+ ? columnType2NativeType(type, 9, 3, 20)
+ : NativeTypes.fromObject(values[i]);
+
+ schemaBuilder.addField(nativeType, values[i] == null || rnd.nextBoolean());
+ }
+
+ return schemaBuilder.build();
+ }
+
+ private Object[] values(List<ColumnType> columnTypes) {
+ Object[] values = new Object[columnTypes.size()];
+ int baseValue = rnd.nextInt();
+
+ for (int i = 0; i < values.length; i++) {
+ ColumnType type = columnTypes.get(i);
+
+ values[i] = type == ColumnType.NULL || rnd.nextInt(4) == 1 ? null : generateValueByType(baseValue, type);
Review Comment:
> rnd.nextInt(4) == 1
Why do we need this condition ?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] lowka commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1314462191
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/row/SqlRowHandlerTest.java:
##########
@@ -0,0 +1,214 @@
+/*
+ * 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.sql.engine.exec.row;
+
+import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.generateValueByType;
+import static org.apache.ignite.internal.sql.engine.util.TypeUtils.columnType2NativeType;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+
+import java.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.ThreadLocalRandom;
+import java.util.stream.Stream;
+import org.apache.ignite.internal.schema.BinaryTuple;
+import org.apache.ignite.internal.schema.NativeType;
+import org.apache.ignite.internal.schema.NativeTypes;
+import org.apache.ignite.internal.sql.engine.exec.RowHandler;
+import org.apache.ignite.internal.sql.engine.exec.RowHandler.RowFactory;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler.RowWrapper;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema.Builder;
+import org.apache.ignite.internal.sql.engine.util.TypeUtils;
+import org.apache.ignite.internal.testframework.IgniteAbstractTest;
+import org.apache.ignite.sql.ColumnType;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Named;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+/**
+ * Tests for {@link SqlRowHandler}.
+ */
+public class SqlRowHandlerTest extends IgniteAbstractTest {
+ private static final RowHandler<RowWrapper> handler = SqlRowHandler.INSTANCE;
+
+ private final long seed = ThreadLocalRandom.current().nextLong();
+
+ private final Random rnd = new Random(seed);
+
+ @BeforeEach
+ void printSeed() {
+ log.info("Using seed: " + seed);
+ }
+
+ @Test
+ public void testBytebufferSerialization() {
+ List<ColumnType> columnTypes = columnTypes();
+ Object[] sourceData = values(columnTypes);
+ RowSchema schema = rowSchema(columnTypes, sourceData);
+
+ int elementsCount = schema.fields().size();
+
+ RowFactory<RowWrapper> factory = handler.factory(schema);
+ RowWrapper src = factory.create(sourceData);
+
+ // Serialization to binary tuple representation.
+ ByteBuffer buf = handler.toByteBuffer(src);
+
+ BinaryTuple tuple = new BinaryTuple(elementsCount, buf);
+ RowWrapper destWrap = factory.wrap(tuple);
+ RowWrapper dest = factory.create(buf);
+
+ for (int i = 0; i < elementsCount; i++) {
+ String msg = schema.fields().get(i).toString();
+
+ assertThat(msg, handler.get(i, src), equalTo(sourceData[i]));
+ assertThat(msg, schema.value(i, tuple), equalTo(sourceData[i]));
+
+ // Binary tuple wrapper must return data in internal format.
+ Object expected = TypeUtils.toInternal(sourceData[i]);
+
+ assertThat(msg, handler.get(i, dest), equalTo(expected));
+ assertThat(msg, handler.get(i, destWrap), equalTo(expected));
+ }
+ }
+
+ @ParameterizedTest(name = "{0} - {1}")
+ @MethodSource("concatTestArguments")
+ public void testConcat(boolean leftTupleRequired, boolean rightTupleRequired) {
+ ConcatTestParameters params = new ConcatTestParameters(leftTupleRequired, rightTupleRequired);
+
+ RowWrapper concatenated = handler.concat(params.left, params.right);
+
+ int leftLen = params.leftData.length;
+ int rightLen = params.rightData.length;
+ int totalElementsCount = leftLen + rightLen;
+
+ assertThat(handler.columnCount(concatenated), equalTo(totalElementsCount));
+
+ // Build combined schema.
+ Builder builder = RowSchema.builder();
+ params.leftSchema.fields().forEach(builder::addField);
+ params.rightSchema.fields().forEach(builder::addField);
+
+ RowSchema concatenatedSchema = builder.build();
+
+ // Serialize.
+ ByteBuffer buf = handler.toByteBuffer(concatenated);
+
+ // Wrap into row.
+ RowWrapper wrapped = handler.factory(concatenatedSchema).wrap(new BinaryTuple(totalElementsCount, buf));
+
+ for (int i = 0; i < Math.max(leftLen, rightLen); i++) {
+ if (i < leftLen) {
Review Comment:
I think is code too complicated for what is supposed to check - concat(row1+row2) = row(cols::row1, cols::row2)
We can just concatenate leftData with rightData and check elements of concatenated row one by one.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1314606582
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/SqlRowHandler.java:
##########
@@ -0,0 +1,260 @@
+/*
+ * 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.sql.engine.exec;
+
+import java.nio.ByteBuffer;
+import java.util.List;
+import org.apache.ignite.internal.schema.BinaryTuple;
+import org.apache.ignite.internal.schema.NativeType;
+import org.apache.ignite.internal.schema.row.InternalTuple;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler.RowWrapper;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema.Builder;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchemaTypes;
+import org.apache.ignite.internal.sql.engine.exec.row.TypeSpec;
+import org.apache.ignite.internal.sql.engine.util.Commons;
+import org.apache.ignite.internal.sql.engine.util.TypeUtils;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Handler that uses a {@link RowWrapper wrapper} to operate with two types of row implementations.
+ *
+ * <ul>
+ * <li>{@link ObjectsArrayRowWrapper Objects array}</li>
+ * <li>{@link BinaryTupleRowWrapper Binary tuple}</li>
+ * </ul>
+ *
+ * <p>Each kind of rows is serialized to the same binary tuple format
+ * using the {@link #toByteBuffer(RowWrapper) toByteBuffer} method.
+ *
+ * <p>Factory methods {@link RowFactory#wrap(InternalTuple) wrap(InternalTuple)} and
+ * {@link RowFactory#create(ByteBuffer) create(ByteBuffer)} allow create rows without
+ * any additional conversions. But the fields in binary tuple must match the
+ * factory {@link RowSchema row schema}.
+ */
+public class SqlRowHandler implements RowHandler<RowWrapper> {
+ public static final RowHandler<RowWrapper> INSTANCE = new SqlRowHandler();
+
+ private SqlRowHandler() {
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public @Nullable Object get(int field, RowWrapper row) {
+ return row.get(field);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public void set(int field, RowWrapper row, @Nullable Object val) {
+ row.set(field, val);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper concat(RowWrapper left, RowWrapper right) {
+ int leftLen = left.columnsCount();
+ int rightLen = right.columnsCount();
+ List<TypeSpec> leftTypes = left.rowSchema().fields();
+ List<TypeSpec> rightTypes = right.rowSchema().fields();
+
+ Object[] values = new Object[leftLen + rightLen];
+ Builder schemaBuilder = RowSchema.builder();
+
+ for (int i = 0; i < leftLen; i++) {
+ values[i] = left.get(i);
+ schemaBuilder.addField(leftTypes.get(i));
+ }
+
+ for (int i = 0; i < rightLen; i++) {
+ values[leftLen + i] = right.get(i);
+ schemaBuilder.addField(rightTypes.get(i));
+ }
+
+ return new ObjectsArrayRowWrapper(schemaBuilder.build(), values);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper map(RowWrapper row, int[] mapping, int offset) {
+ Object[] fields = new Object[mapping.length];
+
+ for (int i = 0; i < mapping.length; i++) {
+ fields[i] = row.get(mapping[i] + offset);
+ }
+
+ return new ObjectsArrayRowWrapper(row.rowSchema(), fields);
+ }
+
+ @Override
+ public int columnCount(RowWrapper row) {
+ return row.columnsCount();
+ }
+
+ @Override
+ public String toString(RowWrapper row) {
+ return String.valueOf(row);
+ }
+
+ @Override
+ public ByteBuffer toByteBuffer(RowWrapper row) {
+ return row.toByteBuffer();
Review Comment:
Forgot about it - thanks for pointing on this. Fixed.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] korlov42 commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "korlov42 (via GitHub)" <gi...@apache.org>.
korlov42 commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1317139058
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/rel/ModifyNode.java:
##########
@@ -268,15 +268,22 @@ private void inlineUpdates(int offset, List<RowT> rows) {
RowHandler<RowT> handler = context().rowHandler();
- for (RowT row : rows) {
- for (int i = 0; i < mapping.length; i++) {
- if (offset == 0 && i == mapping[i]) {
- continue;
- }
+ rows.replaceAll(row -> handler.map(row, applyOffset(mapping, offset)));
Review Comment:
it's better to do computation outside of the loop
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] lowka commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1314462191
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/row/SqlRowHandlerTest.java:
##########
@@ -0,0 +1,214 @@
+/*
+ * 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.sql.engine.exec.row;
+
+import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.generateValueByType;
+import static org.apache.ignite.internal.sql.engine.util.TypeUtils.columnType2NativeType;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+
+import java.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.ThreadLocalRandom;
+import java.util.stream.Stream;
+import org.apache.ignite.internal.schema.BinaryTuple;
+import org.apache.ignite.internal.schema.NativeType;
+import org.apache.ignite.internal.schema.NativeTypes;
+import org.apache.ignite.internal.sql.engine.exec.RowHandler;
+import org.apache.ignite.internal.sql.engine.exec.RowHandler.RowFactory;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler.RowWrapper;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema.Builder;
+import org.apache.ignite.internal.sql.engine.util.TypeUtils;
+import org.apache.ignite.internal.testframework.IgniteAbstractTest;
+import org.apache.ignite.sql.ColumnType;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Named;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+/**
+ * Tests for {@link SqlRowHandler}.
+ */
+public class SqlRowHandlerTest extends IgniteAbstractTest {
+ private static final RowHandler<RowWrapper> handler = SqlRowHandler.INSTANCE;
+
+ private final long seed = ThreadLocalRandom.current().nextLong();
+
+ private final Random rnd = new Random(seed);
+
+ @BeforeEach
+ void printSeed() {
+ log.info("Using seed: " + seed);
+ }
+
+ @Test
+ public void testBytebufferSerialization() {
+ List<ColumnType> columnTypes = columnTypes();
+ Object[] sourceData = values(columnTypes);
+ RowSchema schema = rowSchema(columnTypes, sourceData);
+
+ int elementsCount = schema.fields().size();
+
+ RowFactory<RowWrapper> factory = handler.factory(schema);
+ RowWrapper src = factory.create(sourceData);
+
+ // Serialization to binary tuple representation.
+ ByteBuffer buf = handler.toByteBuffer(src);
+
+ BinaryTuple tuple = new BinaryTuple(elementsCount, buf);
+ RowWrapper destWrap = factory.wrap(tuple);
+ RowWrapper dest = factory.create(buf);
+
+ for (int i = 0; i < elementsCount; i++) {
+ String msg = schema.fields().get(i).toString();
+
+ assertThat(msg, handler.get(i, src), equalTo(sourceData[i]));
+ assertThat(msg, schema.value(i, tuple), equalTo(sourceData[i]));
+
+ // Binary tuple wrapper must return data in internal format.
+ Object expected = TypeUtils.toInternal(sourceData[i]);
+
+ assertThat(msg, handler.get(i, dest), equalTo(expected));
+ assertThat(msg, handler.get(i, destWrap), equalTo(expected));
+ }
+ }
+
+ @ParameterizedTest(name = "{0} - {1}")
+ @MethodSource("concatTestArguments")
+ public void testConcat(boolean leftTupleRequired, boolean rightTupleRequired) {
+ ConcatTestParameters params = new ConcatTestParameters(leftTupleRequired, rightTupleRequired);
+
+ RowWrapper concatenated = handler.concat(params.left, params.right);
+
+ int leftLen = params.leftData.length;
+ int rightLen = params.rightData.length;
+ int totalElementsCount = leftLen + rightLen;
+
+ assertThat(handler.columnCount(concatenated), equalTo(totalElementsCount));
+
+ // Build combined schema.
+ Builder builder = RowSchema.builder();
+ params.leftSchema.fields().forEach(builder::addField);
+ params.rightSchema.fields().forEach(builder::addField);
+
+ RowSchema concatenatedSchema = builder.build();
+
+ // Serialize.
+ ByteBuffer buf = handler.toByteBuffer(concatenated);
+
+ // Wrap into row.
+ RowWrapper wrapped = handler.factory(concatenatedSchema).wrap(new BinaryTuple(totalElementsCount, buf));
+
+ for (int i = 0; i < Math.max(leftLen, rightLen); i++) {
+ if (i < leftLen) {
Review Comment:
I think is code too complicate for what is supposed to do - concat(row1+row2) = row(cols::row1, cols::row2)
We can just concatenate leftData with rightData and check elements of concatenated row one by one.
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/row/SqlRowHandlerTest.java:
##########
@@ -0,0 +1,214 @@
+/*
+ * 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.sql.engine.exec.row;
+
+import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.generateValueByType;
+import static org.apache.ignite.internal.sql.engine.util.TypeUtils.columnType2NativeType;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+
+import java.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.ThreadLocalRandom;
+import java.util.stream.Stream;
+import org.apache.ignite.internal.schema.BinaryTuple;
+import org.apache.ignite.internal.schema.NativeType;
+import org.apache.ignite.internal.schema.NativeTypes;
+import org.apache.ignite.internal.sql.engine.exec.RowHandler;
+import org.apache.ignite.internal.sql.engine.exec.RowHandler.RowFactory;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler.RowWrapper;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema.Builder;
+import org.apache.ignite.internal.sql.engine.util.TypeUtils;
+import org.apache.ignite.internal.testframework.IgniteAbstractTest;
+import org.apache.ignite.sql.ColumnType;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Named;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+/**
+ * Tests for {@link SqlRowHandler}.
+ */
+public class SqlRowHandlerTest extends IgniteAbstractTest {
+ private static final RowHandler<RowWrapper> handler = SqlRowHandler.INSTANCE;
+
+ private final long seed = ThreadLocalRandom.current().nextLong();
+
+ private final Random rnd = new Random(seed);
+
+ @BeforeEach
+ void printSeed() {
+ log.info("Using seed: " + seed);
+ }
+
+ @Test
+ public void testBytebufferSerialization() {
+ List<ColumnType> columnTypes = columnTypes();
+ Object[] sourceData = values(columnTypes);
+ RowSchema schema = rowSchema(columnTypes, sourceData);
+
+ int elementsCount = schema.fields().size();
+
+ RowFactory<RowWrapper> factory = handler.factory(schema);
+ RowWrapper src = factory.create(sourceData);
+
+ // Serialization to binary tuple representation.
+ ByteBuffer buf = handler.toByteBuffer(src);
+
+ BinaryTuple tuple = new BinaryTuple(elementsCount, buf);
+ RowWrapper destWrap = factory.wrap(tuple);
+ RowWrapper dest = factory.create(buf);
+
+ for (int i = 0; i < elementsCount; i++) {
+ String msg = schema.fields().get(i).toString();
+
+ assertThat(msg, handler.get(i, src), equalTo(sourceData[i]));
+ assertThat(msg, schema.value(i, tuple), equalTo(sourceData[i]));
+
+ // Binary tuple wrapper must return data in internal format.
+ Object expected = TypeUtils.toInternal(sourceData[i]);
+
+ assertThat(msg, handler.get(i, dest), equalTo(expected));
+ assertThat(msg, handler.get(i, destWrap), equalTo(expected));
+ }
+ }
+
+ @ParameterizedTest(name = "{0} - {1}")
+ @MethodSource("concatTestArguments")
+ public void testConcat(boolean leftTupleRequired, boolean rightTupleRequired) {
+ ConcatTestParameters params = new ConcatTestParameters(leftTupleRequired, rightTupleRequired);
+
+ RowWrapper concatenated = handler.concat(params.left, params.right);
+
+ int leftLen = params.leftData.length;
+ int rightLen = params.rightData.length;
+ int totalElementsCount = leftLen + rightLen;
+
+ assertThat(handler.columnCount(concatenated), equalTo(totalElementsCount));
+
+ // Build combined schema.
+ Builder builder = RowSchema.builder();
+ params.leftSchema.fields().forEach(builder::addField);
+ params.rightSchema.fields().forEach(builder::addField);
+
+ RowSchema concatenatedSchema = builder.build();
+
+ // Serialize.
+ ByteBuffer buf = handler.toByteBuffer(concatenated);
+
+ // Wrap into row.
+ RowWrapper wrapped = handler.factory(concatenatedSchema).wrap(new BinaryTuple(totalElementsCount, buf));
+
+ for (int i = 0; i < Math.max(leftLen, rightLen); i++) {
+ if (i < leftLen) {
Review Comment:
I think is code too complicated for what is supposed to do - concat(row1+row2) = row(cols::row1, cols::row2)
We can just concatenate leftData with rightData and check elements of concatenated row one by one.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1314619085
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/row/SqlRowHandlerTest.java:
##########
@@ -0,0 +1,214 @@
+/*
+ * 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.sql.engine.exec.row;
+
+import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.generateValueByType;
+import static org.apache.ignite.internal.sql.engine.util.TypeUtils.columnType2NativeType;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+
+import java.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.ThreadLocalRandom;
+import java.util.stream.Stream;
+import org.apache.ignite.internal.schema.BinaryTuple;
+import org.apache.ignite.internal.schema.NativeType;
+import org.apache.ignite.internal.schema.NativeTypes;
+import org.apache.ignite.internal.sql.engine.exec.RowHandler;
+import org.apache.ignite.internal.sql.engine.exec.RowHandler.RowFactory;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler.RowWrapper;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema.Builder;
+import org.apache.ignite.internal.sql.engine.util.TypeUtils;
+import org.apache.ignite.internal.testframework.IgniteAbstractTest;
+import org.apache.ignite.sql.ColumnType;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Named;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+/**
+ * Tests for {@link SqlRowHandler}.
+ */
+public class SqlRowHandlerTest extends IgniteAbstractTest {
+ private static final RowHandler<RowWrapper> handler = SqlRowHandler.INSTANCE;
+
+ private final long seed = ThreadLocalRandom.current().nextLong();
+
+ private final Random rnd = new Random(seed);
+
+ @BeforeEach
+ void printSeed() {
+ log.info("Using seed: " + seed);
+ }
+
+ @Test
+ public void testBytebufferSerialization() {
+ List<ColumnType> columnTypes = columnTypes();
+ Object[] sourceData = values(columnTypes);
+ RowSchema schema = rowSchema(columnTypes, sourceData);
+
+ int elementsCount = schema.fields().size();
+
+ RowFactory<RowWrapper> factory = handler.factory(schema);
+ RowWrapper src = factory.create(sourceData);
+
+ // Serialization to binary tuple representation.
+ ByteBuffer buf = handler.toByteBuffer(src);
+
+ BinaryTuple tuple = new BinaryTuple(elementsCount, buf);
+ RowWrapper destWrap = factory.wrap(tuple);
+ RowWrapper dest = factory.create(buf);
+
+ for (int i = 0; i < elementsCount; i++) {
+ String msg = schema.fields().get(i).toString();
+
+ assertThat(msg, handler.get(i, src), equalTo(sourceData[i]));
+ assertThat(msg, schema.value(i, tuple), equalTo(sourceData[i]));
+
+ // Binary tuple wrapper must return data in internal format.
+ Object expected = TypeUtils.toInternal(sourceData[i]);
+
+ assertThat(msg, handler.get(i, dest), equalTo(expected));
+ assertThat(msg, handler.get(i, destWrap), equalTo(expected));
+ }
+ }
+
+ @ParameterizedTest(name = "{0} - {1}")
+ @MethodSource("concatTestArguments")
+ public void testConcat(boolean leftTupleRequired, boolean rightTupleRequired) {
+ ConcatTestParameters params = new ConcatTestParameters(leftTupleRequired, rightTupleRequired);
+
+ RowWrapper concatenated = handler.concat(params.left, params.right);
+
+ int leftLen = params.leftData.length;
+ int rightLen = params.rightData.length;
+ int totalElementsCount = leftLen + rightLen;
+
+ assertThat(handler.columnCount(concatenated), equalTo(totalElementsCount));
+
+ // Build combined schema.
+ Builder builder = RowSchema.builder();
+ params.leftSchema.fields().forEach(builder::addField);
+ params.rightSchema.fields().forEach(builder::addField);
+
+ RowSchema concatenatedSchema = builder.build();
+
+ // Serialize.
+ ByteBuffer buf = handler.toByteBuffer(concatenated);
+
+ // Wrap into row.
+ RowWrapper wrapped = handler.factory(concatenatedSchema).wrap(new BinaryTuple(totalElementsCount, buf));
+
+ for (int i = 0; i < Math.max(leftLen, rightLen); i++) {
+ if (i < leftLen) {
+ assertThat(handler.get(i, wrapped), equalTo(TypeUtils.toInternal(params.leftData[i])));
+ }
+
+ if (i < rightLen) {
+ assertThat(handler.get(leftLen + i, wrapped), equalTo(TypeUtils.toInternal(params.rightData[i])));
+ }
+ }
+ }
+
+ private static Stream<Arguments> concatTestArguments() {
+ return Stream.of(
+ Arguments.of(Named.of("array", false), Named.of("array", false)),
+ Arguments.of(Named.of("array", false), Named.of("tuple", true)),
+ Arguments.of(Named.of("tuple", true), Named.of("array", false)),
+ Arguments.of(Named.of("tuple", true), Named.of("tuple", true))
+ );
+ }
+
+ private RowSchema rowSchema(List<ColumnType> columnTypes, Object[] values) {
+ Builder schemaBuilder = RowSchema.builder();
+
+ for (int i = 0; i < values.length; i++) {
+ ColumnType type = columnTypes.get(i);
+
+ if (type == ColumnType.NULL) {
+ schemaBuilder.addField(new NullTypeSpec());
+
+ continue;
+ }
+
+ NativeType nativeType = values[i] == null
+ ? columnType2NativeType(type, 9, 3, 20)
+ : NativeTypes.fromObject(values[i]);
+
+ schemaBuilder.addField(nativeType, values[i] == null || rnd.nextBoolean());
+ }
+
+ return schemaBuilder.build();
+ }
+
+ private Object[] values(List<ColumnType> columnTypes) {
+ Object[] values = new Object[columnTypes.size()];
+ int baseValue = rnd.nextInt();
+
+ for (int i = 0; i < values.length; i++) {
+ ColumnType type = columnTypes.get(i);
+
+ values[i] = type == ColumnType.NULL || rnd.nextInt(4) == 1 ? null : generateValueByType(baseValue, type);
Review Comment:
Removed this confusing `rnd.nextInt(4)`.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] lowka commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1314535154
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/SqlRowHandler.java:
##########
@@ -0,0 +1,260 @@
+/*
+ * 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.sql.engine.exec;
+
+import java.nio.ByteBuffer;
+import java.util.List;
+import org.apache.ignite.internal.schema.BinaryTuple;
+import org.apache.ignite.internal.schema.NativeType;
+import org.apache.ignite.internal.schema.row.InternalTuple;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler.RowWrapper;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema.Builder;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchemaTypes;
+import org.apache.ignite.internal.sql.engine.exec.row.TypeSpec;
+import org.apache.ignite.internal.sql.engine.util.Commons;
+import org.apache.ignite.internal.sql.engine.util.TypeUtils;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Handler that uses a {@link RowWrapper wrapper} to operate with two types of row implementations.
+ *
+ * <ul>
+ * <li>{@link ObjectsArrayRowWrapper Objects array}</li>
+ * <li>{@link BinaryTupleRowWrapper Binary tuple}</li>
+ * </ul>
+ *
+ * <p>Each kind of rows is serialized to the same binary tuple format
+ * using the {@link #toByteBuffer(RowWrapper) toByteBuffer} method.
+ *
+ * <p>Factory methods {@link RowFactory#wrap(InternalTuple) wrap(InternalTuple)} and
+ * {@link RowFactory#create(ByteBuffer) create(ByteBuffer)} allow create rows without
+ * any additional conversions. But the fields in binary tuple must match the
+ * factory {@link RowSchema row schema}.
+ */
+public class SqlRowHandler implements RowHandler<RowWrapper> {
+ public static final RowHandler<RowWrapper> INSTANCE = new SqlRowHandler();
+
+ private SqlRowHandler() {
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public @Nullable Object get(int field, RowWrapper row) {
+ return row.get(field);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public void set(int field, RowWrapper row, @Nullable Object val) {
+ row.set(field, val);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper concat(RowWrapper left, RowWrapper right) {
+ int leftLen = left.columnsCount();
+ int rightLen = right.columnsCount();
+ List<TypeSpec> leftTypes = left.rowSchema().fields();
+ List<TypeSpec> rightTypes = right.rowSchema().fields();
+
+ Object[] values = new Object[leftLen + rightLen];
+ Builder schemaBuilder = RowSchema.builder();
+
+ for (int i = 0; i < leftLen; i++) {
+ values[i] = left.get(i);
+ schemaBuilder.addField(leftTypes.get(i));
+ }
+
+ for (int i = 0; i < rightLen; i++) {
+ values[leftLen + i] = right.get(i);
+ schemaBuilder.addField(rightTypes.get(i));
+ }
+
+ return new ObjectsArrayRowWrapper(schemaBuilder.build(), values);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper map(RowWrapper row, int[] mapping, int offset) {
+ Object[] fields = new Object[mapping.length];
+
+ for (int i = 0; i < mapping.length; i++) {
+ fields[i] = row.get(mapping[i] + offset);
+ }
+
+ return new ObjectsArrayRowWrapper(row.rowSchema(), fields);
+ }
+
+ @Override
+ public int columnCount(RowWrapper row) {
+ return row.columnsCount();
+ }
+
+ @Override
+ public String toString(RowWrapper row) {
+ return String.valueOf(row);
+ }
+
+ @Override
+ public ByteBuffer toByteBuffer(RowWrapper row) {
+ return row.toByteBuffer();
Review Comment:
It would be better to provide implementation of `toString` method that is similar to ArrayRowHandler's. This one simply returns `typeName@132` or something.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] lowka commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1314582921
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/row/SqlRowHandlerTest.java:
##########
@@ -0,0 +1,212 @@
+/*
+ * 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.sql.engine.exec.row;
+
+import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.generateValueByType;
+import static org.apache.ignite.internal.sql.engine.util.TypeUtils.columnType2NativeType;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+
+import java.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.ThreadLocalRandom;
+import java.util.stream.Stream;
+import org.apache.ignite.internal.schema.BinaryTuple;
+import org.apache.ignite.internal.schema.NativeType;
+import org.apache.ignite.internal.schema.NativeTypes;
+import org.apache.ignite.internal.sql.engine.exec.RowHandler;
+import org.apache.ignite.internal.sql.engine.exec.RowHandler.RowFactory;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler.RowWrapper;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema.Builder;
+import org.apache.ignite.internal.sql.engine.util.TypeUtils;
+import org.apache.ignite.internal.testframework.IgniteAbstractTest;
+import org.apache.ignite.sql.ColumnType;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Named;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+/**
+ * Tests for {@link SqlRowHandler}.
+ */
+public class SqlRowHandlerTest extends IgniteAbstractTest {
+ private static final RowHandler<RowWrapper> handler = SqlRowHandler.INSTANCE;
+
+ private final long seed = ThreadLocalRandom.current().nextLong();
+
+ private final Random rnd = new Random(seed);
+
+ @BeforeEach
+ void printSeed() {
+ log.info("Using seed: " + seed);
+ }
+
+ @Test
+ public void testBytebufferSerialization() {
+ List<ColumnType> columnTypes = columnTypes();
+ Object[] sourceData = values(columnTypes);
+ RowSchema schema = rowSchema(columnTypes, sourceData);
+
+ int elementsCount = schema.fields().size();
+
+ RowFactory<RowWrapper> factory = handler.factory(schema);
+ RowWrapper src = factory.create(sourceData);
+
+ // Serialization to binary tuple representation.
+ ByteBuffer buf = handler.toByteBuffer(src);
+
+ BinaryTuple tuple = new BinaryTuple(elementsCount, buf);
+ RowWrapper destWrap = factory.wrap(tuple);
+ RowWrapper dest = factory.create(buf);
+
+ for (int i = 0; i < elementsCount; i++) {
+ String msg = schema.fields().get(i).toString();
+
+ assertThat(msg, handler.get(i, src), equalTo(sourceData[i]));
+ assertThat(msg, schema.value(tuple, i), equalTo(sourceData[i]));
+
+ // Binary tuple wrapper must return data in internal format.
+ Object expected = TypeUtils.toInternal(sourceData[i]);
+
+ assertThat(msg, handler.get(i, dest), equalTo(expected));
+ assertThat(msg, handler.get(i, destWrap), equalTo(expected));
+ }
+ }
+
+ @ParameterizedTest(name = "{0} - {1}")
+ @MethodSource("concatTestArguments")
+ public void testConcat(boolean leftTupleRequired, boolean rightTupleRequired) {
+ ConcatTestParameters params = new ConcatTestParameters(leftTupleRequired, rightTupleRequired);
+
+ RowWrapper concatenated = handler.concat(params.left, params.right);
+
+ int leftLen = params.leftData.length;
+ int rightLen = params.rightData.length;
+ int totalElementsCount = leftLen + rightLen;
+
+ assertThat(handler.columnCount(concatenated), equalTo(totalElementsCount));
+
+ // Build combined schema.
+ Builder builder = RowSchema.builder();
+ params.leftSchema.fields().forEach(builder::addField);
+ params.rightSchema.fields().forEach(builder::addField);
+
+ RowSchema concatenatedSchema = builder.build();
+
+ // Serialize.
+ ByteBuffer buf = handler.toByteBuffer(concatenated);
+
+ // Wrap into row.
+ RowWrapper result = handler.factory(concatenatedSchema).wrap(new BinaryTuple(totalElementsCount, buf));
+
+ for (int i = 0; i < leftLen; i++) {
+ assertThat(handler.get(i, result), equalTo(TypeUtils.toInternal(params.leftData[i])));
+ }
+
+ for (int i = 0; i < rightLen; i++) {
+ assertThat(handler.get(leftLen + i, result), equalTo(TypeUtils.toInternal(params.rightData[i])));
+ }
+ }
+
+ private static Stream<Arguments> concatTestArguments() {
+ return Stream.of(
+ Arguments.of(Named.of("array", false), Named.of("array", false)),
+ Arguments.of(Named.of("array", false), Named.of("tuple", true)),
+ Arguments.of(Named.of("tuple", true), Named.of("array", false)),
+ Arguments.of(Named.of("tuple", true), Named.of("tuple", true))
+ );
+ }
+
+ private RowSchema rowSchema(List<ColumnType> columnTypes, Object[] values) {
+ Builder schemaBuilder = RowSchema.builder();
+
+ for (int i = 0; i < values.length; i++) {
+ ColumnType type = columnTypes.get(i);
+
+ if (type == ColumnType.NULL) {
+ schemaBuilder.addField(new NullTypeSpec());
+
+ continue;
+ }
+
+ NativeType nativeType = values[i] == null
+ ? columnType2NativeType(type, 9, 3, 20)
+ : NativeTypes.fromObject(values[i]);
+
+ schemaBuilder.addField(nativeType, values[i] == null || rnd.nextBoolean());
+ }
+
+ return schemaBuilder.build();
+ }
+
+ private Object[] values(List<ColumnType> columnTypes) {
+ Object[] values = new Object[columnTypes.size()];
+ int baseValue = rnd.nextInt();
+
+ for (int i = 0; i < values.length; i++) {
+ ColumnType type = columnTypes.get(i);
+
+ values[i] = type == ColumnType.NULL ? null : generateValueByType(baseValue, type);
+ }
+
+ return values;
+ }
+
+ private List<ColumnType> columnTypes() {
+ List<ColumnType> columnTypes = new ArrayList<>(EnumSet.complementOf(EnumSet.of(ColumnType.PERIOD, ColumnType.DURATION)));
Review Comment:
it would be better to add a TODO link to an issue with INTERVAL type support https://issues.apache.org/jira/browse/IGNITE-15200
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1317398328
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/rel/ModifyNode.java:
##########
@@ -268,15 +268,22 @@ private void inlineUpdates(int offset, List<RowT> rows) {
RowHandler<RowT> handler = context().rowHandler();
- for (RowT row : rows) {
- for (int i = 0; i < mapping.length; i++) {
- if (offset == 0 && i == mapping[i]) {
- continue;
- }
+ rows.replaceAll(row -> handler.map(row, applyOffset(mapping, offset)));
Review Comment:
Done, thanks.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1314606582
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/SqlRowHandler.java:
##########
@@ -0,0 +1,260 @@
+/*
+ * 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.sql.engine.exec;
+
+import java.nio.ByteBuffer;
+import java.util.List;
+import org.apache.ignite.internal.schema.BinaryTuple;
+import org.apache.ignite.internal.schema.NativeType;
+import org.apache.ignite.internal.schema.row.InternalTuple;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler.RowWrapper;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema.Builder;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchemaTypes;
+import org.apache.ignite.internal.sql.engine.exec.row.TypeSpec;
+import org.apache.ignite.internal.sql.engine.util.Commons;
+import org.apache.ignite.internal.sql.engine.util.TypeUtils;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Handler that uses a {@link RowWrapper wrapper} to operate with two types of row implementations.
+ *
+ * <ul>
+ * <li>{@link ObjectsArrayRowWrapper Objects array}</li>
+ * <li>{@link BinaryTupleRowWrapper Binary tuple}</li>
+ * </ul>
+ *
+ * <p>Each kind of rows is serialized to the same binary tuple format
+ * using the {@link #toByteBuffer(RowWrapper) toByteBuffer} method.
+ *
+ * <p>Factory methods {@link RowFactory#wrap(InternalTuple) wrap(InternalTuple)} and
+ * {@link RowFactory#create(ByteBuffer) create(ByteBuffer)} allow create rows without
+ * any additional conversions. But the fields in binary tuple must match the
+ * factory {@link RowSchema row schema}.
+ */
+public class SqlRowHandler implements RowHandler<RowWrapper> {
+ public static final RowHandler<RowWrapper> INSTANCE = new SqlRowHandler();
+
+ private SqlRowHandler() {
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public @Nullable Object get(int field, RowWrapper row) {
+ return row.get(field);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public void set(int field, RowWrapper row, @Nullable Object val) {
+ row.set(field, val);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper concat(RowWrapper left, RowWrapper right) {
+ int leftLen = left.columnsCount();
+ int rightLen = right.columnsCount();
+ List<TypeSpec> leftTypes = left.rowSchema().fields();
+ List<TypeSpec> rightTypes = right.rowSchema().fields();
+
+ Object[] values = new Object[leftLen + rightLen];
+ Builder schemaBuilder = RowSchema.builder();
+
+ for (int i = 0; i < leftLen; i++) {
+ values[i] = left.get(i);
+ schemaBuilder.addField(leftTypes.get(i));
+ }
+
+ for (int i = 0; i < rightLen; i++) {
+ values[leftLen + i] = right.get(i);
+ schemaBuilder.addField(rightTypes.get(i));
+ }
+
+ return new ObjectsArrayRowWrapper(schemaBuilder.build(), values);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper map(RowWrapper row, int[] mapping, int offset) {
+ Object[] fields = new Object[mapping.length];
+
+ for (int i = 0; i < mapping.length; i++) {
+ fields[i] = row.get(mapping[i] + offset);
+ }
+
+ return new ObjectsArrayRowWrapper(row.rowSchema(), fields);
+ }
+
+ @Override
+ public int columnCount(RowWrapper row) {
+ return row.columnsCount();
+ }
+
+ @Override
+ public String toString(RowWrapper row) {
+ return String.valueOf(row);
+ }
+
+ @Override
+ public ByteBuffer toByteBuffer(RowWrapper row) {
+ return row.toByteBuffer();
Review Comment:
Forgot about it - thanks for pointing on this.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1314615685
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/row/SqlRowHandlerTest.java:
##########
@@ -0,0 +1,214 @@
+/*
+ * 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.sql.engine.exec.row;
+
+import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.generateValueByType;
+import static org.apache.ignite.internal.sql.engine.util.TypeUtils.columnType2NativeType;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+
+import java.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.ThreadLocalRandom;
+import java.util.stream.Stream;
+import org.apache.ignite.internal.schema.BinaryTuple;
+import org.apache.ignite.internal.schema.NativeType;
+import org.apache.ignite.internal.schema.NativeTypes;
+import org.apache.ignite.internal.sql.engine.exec.RowHandler;
+import org.apache.ignite.internal.sql.engine.exec.RowHandler.RowFactory;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler.RowWrapper;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema.Builder;
+import org.apache.ignite.internal.sql.engine.util.TypeUtils;
+import org.apache.ignite.internal.testframework.IgniteAbstractTest;
+import org.apache.ignite.sql.ColumnType;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Named;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+/**
+ * Tests for {@link SqlRowHandler}.
+ */
+public class SqlRowHandlerTest extends IgniteAbstractTest {
+ private static final RowHandler<RowWrapper> handler = SqlRowHandler.INSTANCE;
+
+ private final long seed = ThreadLocalRandom.current().nextLong();
+
+ private final Random rnd = new Random(seed);
+
+ @BeforeEach
+ void printSeed() {
+ log.info("Using seed: " + seed);
+ }
+
+ @Test
+ public void testBytebufferSerialization() {
+ List<ColumnType> columnTypes = columnTypes();
+ Object[] sourceData = values(columnTypes);
+ RowSchema schema = rowSchema(columnTypes, sourceData);
+
+ int elementsCount = schema.fields().size();
+
+ RowFactory<RowWrapper> factory = handler.factory(schema);
+ RowWrapper src = factory.create(sourceData);
+
+ // Serialization to binary tuple representation.
+ ByteBuffer buf = handler.toByteBuffer(src);
+
+ BinaryTuple tuple = new BinaryTuple(elementsCount, buf);
+ RowWrapper destWrap = factory.wrap(tuple);
+ RowWrapper dest = factory.create(buf);
+
+ for (int i = 0; i < elementsCount; i++) {
+ String msg = schema.fields().get(i).toString();
+
+ assertThat(msg, handler.get(i, src), equalTo(sourceData[i]));
+ assertThat(msg, schema.value(i, tuple), equalTo(sourceData[i]));
+
+ // Binary tuple wrapper must return data in internal format.
+ Object expected = TypeUtils.toInternal(sourceData[i]);
+
+ assertThat(msg, handler.get(i, dest), equalTo(expected));
+ assertThat(msg, handler.get(i, destWrap), equalTo(expected));
+ }
+ }
+
+ @ParameterizedTest(name = "{0} - {1}")
+ @MethodSource("concatTestArguments")
+ public void testConcat(boolean leftTupleRequired, boolean rightTupleRequired) {
+ ConcatTestParameters params = new ConcatTestParameters(leftTupleRequired, rightTupleRequired);
+
+ RowWrapper concatenated = handler.concat(params.left, params.right);
+
+ int leftLen = params.leftData.length;
+ int rightLen = params.rightData.length;
+ int totalElementsCount = leftLen + rightLen;
+
+ assertThat(handler.columnCount(concatenated), equalTo(totalElementsCount));
+
+ // Build combined schema.
+ Builder builder = RowSchema.builder();
+ params.leftSchema.fields().forEach(builder::addField);
+ params.rightSchema.fields().forEach(builder::addField);
+
+ RowSchema concatenatedSchema = builder.build();
+
+ // Serialize.
+ ByteBuffer buf = handler.toByteBuffer(concatenated);
+
+ // Wrap into row.
+ RowWrapper wrapped = handler.factory(concatenatedSchema).wrap(new BinaryTuple(totalElementsCount, buf));
+
+ for (int i = 0; i < Math.max(leftLen, rightLen); i++) {
+ if (i < leftLen) {
Review Comment:
Split into two cycles.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1314705906
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/rel/ModifyNode.java:
##########
@@ -268,15 +268,7 @@ private void inlineUpdates(int offset, List<RowT> rows) {
RowHandler<RowT> handler = context().rowHandler();
- for (RowT row : rows) {
- for (int i = 0; i < mapping.length; i++) {
- if (offset == 0 && i == mapping[i]) {
- continue;
- }
-
- handler.set(i, row, handler.get(mapping[i] + offset, row));
- }
- }
+ rows.replaceAll(row -> handler.map(row, mapping, offset));
Review Comment:
This is because BinaryTuple is not mutable.
Those. An "honest" arbitrary change to the BinaryTuple field will result in a complete rebuild of the new BinaryTuple instance, which looks inefficient.
An alternative implementation of an arbitrary "set" operation is to add a structure to each Row that will reflect the changes made (for example, a Map or an array), but this will lead to the fact that at least a new reference (field) to this structure will have to be added to each row instance. In addition, it would be desirable to support this during serialization.
At the moment, this is only one place where we call "set(index)" on a deserialized row.
And it seems that this "map" operation can be implemented more efficiently for a binarytuple than arbitrary set(index).
It seems to me that the question of the effective implementation of this method should be discussed separately if we come to an agreement that this method (map) has make sense to add.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1314705906
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/rel/ModifyNode.java:
##########
@@ -268,15 +268,7 @@ private void inlineUpdates(int offset, List<RowT> rows) {
RowHandler<RowT> handler = context().rowHandler();
- for (RowT row : rows) {
- for (int i = 0; i < mapping.length; i++) {
- if (offset == 0 && i == mapping[i]) {
- continue;
- }
-
- handler.set(i, row, handler.get(mapping[i] + offset, row));
- }
- }
+ rows.replaceAll(row -> handler.map(row, mapping, offset));
Review Comment:
This is because BinaryTuple is not mutable.
Those. An "honest" arbitrary change to the BinaryTuple field will result in a complete rebuild of the new BinaryTuple instance, which looks inefficient.
An alternative implementation of an arbitrary "set" operation is to add a structure to each Row that will reflect the changes made (for example, a Map or an array), but this will lead to the fact that at least a new reference (field) to this structure will have to be added to each row instance. In addition, it would be desirable to support this during serialization.
At the moment, this is only one place where we call "set(index)" on a deserialized row.
And it seems that this "map" operation can be implemented more efficiently for a binarytuple than arbitrary set(index).
It seems to me that the question of the effective implementation of this method should be discussed separately if we come to an agreement that this method has make sense to add.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] lowka commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1314466956
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/row/RowSchema.java:
##########
@@ -39,6 +43,43 @@ public List<TypeSpec> fields() {
return fields;
}
+ /**
+ * Gets an object representation from a tuple's field.
+ *
+ * @param fieldIndex Field index to read.
+ * @param tuple Tuple to read the value from.
+ * @return An object representation of the value.
+ */
+ public @Nullable Object value(int fieldIndex, InternalTuple tuple) {
Review Comment:
It would be better to swap arguments here. Almost every time field id follows the source data structure and not vice versa.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] lowka commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1314465396
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/row/RowSchemaTypes.java:
##########
@@ -82,4 +83,21 @@ public static BaseTypeSpec nativeTypeWithNullability(NativeType nativeType, bool
return new BaseTypeSpec(nativeType, nullable);
}
}
+
+ /** Returns a {@link NativeType} extracted from the row schema type. */
Review Comment:
I think it would be indicate in javadoc that this method returns null if is the given type spec represents null type and it throws exception if the given type spec is not `BaseTypeSpec`.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] lowka commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1314453635
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/row/SqlRowHandlerTest.java:
##########
@@ -0,0 +1,214 @@
+/*
+ * 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.sql.engine.exec.row;
+
+import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.generateValueByType;
+import static org.apache.ignite.internal.sql.engine.util.TypeUtils.columnType2NativeType;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+
+import java.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.ThreadLocalRandom;
+import java.util.stream.Stream;
+import org.apache.ignite.internal.schema.BinaryTuple;
+import org.apache.ignite.internal.schema.NativeType;
+import org.apache.ignite.internal.schema.NativeTypes;
+import org.apache.ignite.internal.sql.engine.exec.RowHandler;
+import org.apache.ignite.internal.sql.engine.exec.RowHandler.RowFactory;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler.RowWrapper;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema.Builder;
+import org.apache.ignite.internal.sql.engine.util.TypeUtils;
+import org.apache.ignite.internal.testframework.IgniteAbstractTest;
+import org.apache.ignite.sql.ColumnType;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Named;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+/**
+ * Tests for {@link SqlRowHandler}.
+ */
+public class SqlRowHandlerTest extends IgniteAbstractTest {
+ private static final RowHandler<RowWrapper> handler = SqlRowHandler.INSTANCE;
+
+ private final long seed = ThreadLocalRandom.current().nextLong();
+
+ private final Random rnd = new Random(seed);
+
+ @BeforeEach
+ void printSeed() {
+ log.info("Using seed: " + seed);
+ }
+
+ @Test
+ public void testBytebufferSerialization() {
+ List<ColumnType> columnTypes = columnTypes();
+ Object[] sourceData = values(columnTypes);
+ RowSchema schema = rowSchema(columnTypes, sourceData);
+
+ int elementsCount = schema.fields().size();
+
+ RowFactory<RowWrapper> factory = handler.factory(schema);
+ RowWrapper src = factory.create(sourceData);
+
+ // Serialization to binary tuple representation.
+ ByteBuffer buf = handler.toByteBuffer(src);
+
+ BinaryTuple tuple = new BinaryTuple(elementsCount, buf);
+ RowWrapper destWrap = factory.wrap(tuple);
+ RowWrapper dest = factory.create(buf);
+
+ for (int i = 0; i < elementsCount; i++) {
+ String msg = schema.fields().get(i).toString();
+
+ assertThat(msg, handler.get(i, src), equalTo(sourceData[i]));
+ assertThat(msg, schema.value(i, tuple), equalTo(sourceData[i]));
+
+ // Binary tuple wrapper must return data in internal format.
+ Object expected = TypeUtils.toInternal(sourceData[i]);
+
+ assertThat(msg, handler.get(i, dest), equalTo(expected));
+ assertThat(msg, handler.get(i, destWrap), equalTo(expected));
+ }
+ }
+
+ @ParameterizedTest(name = "{0} - {1}")
+ @MethodSource("concatTestArguments")
+ public void testConcat(boolean leftTupleRequired, boolean rightTupleRequired) {
+ ConcatTestParameters params = new ConcatTestParameters(leftTupleRequired, rightTupleRequired);
+
+ RowWrapper concatenated = handler.concat(params.left, params.right);
+
+ int leftLen = params.leftData.length;
+ int rightLen = params.rightData.length;
+ int totalElementsCount = leftLen + rightLen;
+
+ assertThat(handler.columnCount(concatenated), equalTo(totalElementsCount));
+
+ // Build combined schema.
+ Builder builder = RowSchema.builder();
+ params.leftSchema.fields().forEach(builder::addField);
+ params.rightSchema.fields().forEach(builder::addField);
+
+ RowSchema concatenatedSchema = builder.build();
+
+ // Serialize.
+ ByteBuffer buf = handler.toByteBuffer(concatenated);
+
+ // Wrap into row.
+ RowWrapper wrapped = handler.factory(concatenatedSchema).wrap(new BinaryTuple(totalElementsCount, buf));
+
+ for (int i = 0; i < Math.max(leftLen, rightLen); i++) {
+ if (i < leftLen) {
+ assertThat(handler.get(i, wrapped), equalTo(TypeUtils.toInternal(params.leftData[i])));
+ }
+
+ if (i < rightLen) {
+ assertThat(handler.get(leftLen + i, wrapped), equalTo(TypeUtils.toInternal(params.rightData[i])));
+ }
+ }
+ }
+
+ private static Stream<Arguments> concatTestArguments() {
+ return Stream.of(
+ Arguments.of(Named.of("array", false), Named.of("array", false)),
+ Arguments.of(Named.of("array", false), Named.of("tuple", true)),
+ Arguments.of(Named.of("tuple", true), Named.of("array", false)),
+ Arguments.of(Named.of("tuple", true), Named.of("tuple", true))
+ );
+ }
+
+ private RowSchema rowSchema(List<ColumnType> columnTypes, Object[] values) {
+ Builder schemaBuilder = RowSchema.builder();
+
+ for (int i = 0; i < values.length; i++) {
+ ColumnType type = columnTypes.get(i);
+
+ if (type == ColumnType.NULL) {
+ schemaBuilder.addField(new NullTypeSpec());
+
+ continue;
+ }
+
+ NativeType nativeType = values[i] == null
+ ? columnType2NativeType(type, 9, 3, 20)
+ : NativeTypes.fromObject(values[i]);
+
+ schemaBuilder.addField(nativeType, values[i] == null || rnd.nextBoolean());
+ }
+
+ return schemaBuilder.build();
+ }
+
+ private Object[] values(List<ColumnType> columnTypes) {
+ Object[] values = new Object[columnTypes.size()];
+ int baseValue = rnd.nextInt();
+
+ for (int i = 0; i < values.length; i++) {
+ ColumnType type = columnTypes.get(i);
+
+ values[i] = type == ColumnType.NULL || rnd.nextInt(4) == 1 ? null : generateValueByType(baseValue, type);
Review Comment:
> rnd.nextInt(4) == 1
Why do we need this condition ? The chance of this being true is practically nil.
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/row/SqlRowHandlerTest.java:
##########
@@ -0,0 +1,214 @@
+/*
+ * 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.sql.engine.exec.row;
+
+import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.generateValueByType;
+import static org.apache.ignite.internal.sql.engine.util.TypeUtils.columnType2NativeType;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+
+import java.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.ThreadLocalRandom;
+import java.util.stream.Stream;
+import org.apache.ignite.internal.schema.BinaryTuple;
+import org.apache.ignite.internal.schema.NativeType;
+import org.apache.ignite.internal.schema.NativeTypes;
+import org.apache.ignite.internal.sql.engine.exec.RowHandler;
+import org.apache.ignite.internal.sql.engine.exec.RowHandler.RowFactory;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler.RowWrapper;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema.Builder;
+import org.apache.ignite.internal.sql.engine.util.TypeUtils;
+import org.apache.ignite.internal.testframework.IgniteAbstractTest;
+import org.apache.ignite.sql.ColumnType;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Named;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+/**
+ * Tests for {@link SqlRowHandler}.
+ */
+public class SqlRowHandlerTest extends IgniteAbstractTest {
+ private static final RowHandler<RowWrapper> handler = SqlRowHandler.INSTANCE;
+
+ private final long seed = ThreadLocalRandom.current().nextLong();
+
+ private final Random rnd = new Random(seed);
+
+ @BeforeEach
+ void printSeed() {
+ log.info("Using seed: " + seed);
+ }
+
+ @Test
+ public void testBytebufferSerialization() {
+ List<ColumnType> columnTypes = columnTypes();
+ Object[] sourceData = values(columnTypes);
+ RowSchema schema = rowSchema(columnTypes, sourceData);
+
+ int elementsCount = schema.fields().size();
+
+ RowFactory<RowWrapper> factory = handler.factory(schema);
+ RowWrapper src = factory.create(sourceData);
+
+ // Serialization to binary tuple representation.
+ ByteBuffer buf = handler.toByteBuffer(src);
+
+ BinaryTuple tuple = new BinaryTuple(elementsCount, buf);
+ RowWrapper destWrap = factory.wrap(tuple);
+ RowWrapper dest = factory.create(buf);
+
+ for (int i = 0; i < elementsCount; i++) {
+ String msg = schema.fields().get(i).toString();
+
+ assertThat(msg, handler.get(i, src), equalTo(sourceData[i]));
+ assertThat(msg, schema.value(i, tuple), equalTo(sourceData[i]));
+
+ // Binary tuple wrapper must return data in internal format.
+ Object expected = TypeUtils.toInternal(sourceData[i]);
+
+ assertThat(msg, handler.get(i, dest), equalTo(expected));
+ assertThat(msg, handler.get(i, destWrap), equalTo(expected));
+ }
+ }
+
+ @ParameterizedTest(name = "{0} - {1}")
+ @MethodSource("concatTestArguments")
+ public void testConcat(boolean leftTupleRequired, boolean rightTupleRequired) {
+ ConcatTestParameters params = new ConcatTestParameters(leftTupleRequired, rightTupleRequired);
+
+ RowWrapper concatenated = handler.concat(params.left, params.right);
+
+ int leftLen = params.leftData.length;
+ int rightLen = params.rightData.length;
+ int totalElementsCount = leftLen + rightLen;
+
+ assertThat(handler.columnCount(concatenated), equalTo(totalElementsCount));
+
+ // Build combined schema.
+ Builder builder = RowSchema.builder();
+ params.leftSchema.fields().forEach(builder::addField);
+ params.rightSchema.fields().forEach(builder::addField);
+
+ RowSchema concatenatedSchema = builder.build();
+
+ // Serialize.
+ ByteBuffer buf = handler.toByteBuffer(concatenated);
+
+ // Wrap into row.
+ RowWrapper wrapped = handler.factory(concatenatedSchema).wrap(new BinaryTuple(totalElementsCount, buf));
+
+ for (int i = 0; i < Math.max(leftLen, rightLen); i++) {
+ if (i < leftLen) {
+ assertThat(handler.get(i, wrapped), equalTo(TypeUtils.toInternal(params.leftData[i])));
+ }
+
+ if (i < rightLen) {
+ assertThat(handler.get(leftLen + i, wrapped), equalTo(TypeUtils.toInternal(params.rightData[i])));
+ }
+ }
+ }
+
+ private static Stream<Arguments> concatTestArguments() {
+ return Stream.of(
+ Arguments.of(Named.of("array", false), Named.of("array", false)),
+ Arguments.of(Named.of("array", false), Named.of("tuple", true)),
+ Arguments.of(Named.of("tuple", true), Named.of("array", false)),
+ Arguments.of(Named.of("tuple", true), Named.of("tuple", true))
+ );
+ }
+
+ private RowSchema rowSchema(List<ColumnType> columnTypes, Object[] values) {
+ Builder schemaBuilder = RowSchema.builder();
+
+ for (int i = 0; i < values.length; i++) {
+ ColumnType type = columnTypes.get(i);
+
+ if (type == ColumnType.NULL) {
+ schemaBuilder.addField(new NullTypeSpec());
+
+ continue;
+ }
+
+ NativeType nativeType = values[i] == null
+ ? columnType2NativeType(type, 9, 3, 20)
+ : NativeTypes.fromObject(values[i]);
+
+ schemaBuilder.addField(nativeType, values[i] == null || rnd.nextBoolean());
+ }
+
+ return schemaBuilder.build();
+ }
+
+ private Object[] values(List<ColumnType> columnTypes) {
+ Object[] values = new Object[columnTypes.size()];
+ int baseValue = rnd.nextInt();
+
+ for (int i = 0; i < values.length; i++) {
+ ColumnType type = columnTypes.get(i);
+
+ values[i] = type == ColumnType.NULL || rnd.nextInt(4) == 1 ? null : generateValueByType(baseValue, type);
Review Comment:
> rnd.nextInt(4) == 1
Why do we need this condition ? The chance of this being true is practically nil.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] lowka commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1314582921
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/row/SqlRowHandlerTest.java:
##########
@@ -0,0 +1,212 @@
+/*
+ * 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.sql.engine.exec.row;
+
+import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.generateValueByType;
+import static org.apache.ignite.internal.sql.engine.util.TypeUtils.columnType2NativeType;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+
+import java.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.ThreadLocalRandom;
+import java.util.stream.Stream;
+import org.apache.ignite.internal.schema.BinaryTuple;
+import org.apache.ignite.internal.schema.NativeType;
+import org.apache.ignite.internal.schema.NativeTypes;
+import org.apache.ignite.internal.sql.engine.exec.RowHandler;
+import org.apache.ignite.internal.sql.engine.exec.RowHandler.RowFactory;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler.RowWrapper;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema.Builder;
+import org.apache.ignite.internal.sql.engine.util.TypeUtils;
+import org.apache.ignite.internal.testframework.IgniteAbstractTest;
+import org.apache.ignite.sql.ColumnType;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Named;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+/**
+ * Tests for {@link SqlRowHandler}.
+ */
+public class SqlRowHandlerTest extends IgniteAbstractTest {
+ private static final RowHandler<RowWrapper> handler = SqlRowHandler.INSTANCE;
+
+ private final long seed = ThreadLocalRandom.current().nextLong();
+
+ private final Random rnd = new Random(seed);
+
+ @BeforeEach
+ void printSeed() {
+ log.info("Using seed: " + seed);
+ }
+
+ @Test
+ public void testBytebufferSerialization() {
+ List<ColumnType> columnTypes = columnTypes();
+ Object[] sourceData = values(columnTypes);
+ RowSchema schema = rowSchema(columnTypes, sourceData);
+
+ int elementsCount = schema.fields().size();
+
+ RowFactory<RowWrapper> factory = handler.factory(schema);
+ RowWrapper src = factory.create(sourceData);
+
+ // Serialization to binary tuple representation.
+ ByteBuffer buf = handler.toByteBuffer(src);
+
+ BinaryTuple tuple = new BinaryTuple(elementsCount, buf);
+ RowWrapper destWrap = factory.wrap(tuple);
+ RowWrapper dest = factory.create(buf);
+
+ for (int i = 0; i < elementsCount; i++) {
+ String msg = schema.fields().get(i).toString();
+
+ assertThat(msg, handler.get(i, src), equalTo(sourceData[i]));
+ assertThat(msg, schema.value(tuple, i), equalTo(sourceData[i]));
+
+ // Binary tuple wrapper must return data in internal format.
+ Object expected = TypeUtils.toInternal(sourceData[i]);
+
+ assertThat(msg, handler.get(i, dest), equalTo(expected));
+ assertThat(msg, handler.get(i, destWrap), equalTo(expected));
+ }
+ }
+
+ @ParameterizedTest(name = "{0} - {1}")
+ @MethodSource("concatTestArguments")
+ public void testConcat(boolean leftTupleRequired, boolean rightTupleRequired) {
+ ConcatTestParameters params = new ConcatTestParameters(leftTupleRequired, rightTupleRequired);
+
+ RowWrapper concatenated = handler.concat(params.left, params.right);
+
+ int leftLen = params.leftData.length;
+ int rightLen = params.rightData.length;
+ int totalElementsCount = leftLen + rightLen;
+
+ assertThat(handler.columnCount(concatenated), equalTo(totalElementsCount));
+
+ // Build combined schema.
+ Builder builder = RowSchema.builder();
+ params.leftSchema.fields().forEach(builder::addField);
+ params.rightSchema.fields().forEach(builder::addField);
+
+ RowSchema concatenatedSchema = builder.build();
+
+ // Serialize.
+ ByteBuffer buf = handler.toByteBuffer(concatenated);
+
+ // Wrap into row.
+ RowWrapper result = handler.factory(concatenatedSchema).wrap(new BinaryTuple(totalElementsCount, buf));
+
+ for (int i = 0; i < leftLen; i++) {
+ assertThat(handler.get(i, result), equalTo(TypeUtils.toInternal(params.leftData[i])));
+ }
+
+ for (int i = 0; i < rightLen; i++) {
+ assertThat(handler.get(leftLen + i, result), equalTo(TypeUtils.toInternal(params.rightData[i])));
+ }
+ }
+
+ private static Stream<Arguments> concatTestArguments() {
+ return Stream.of(
+ Arguments.of(Named.of("array", false), Named.of("array", false)),
+ Arguments.of(Named.of("array", false), Named.of("tuple", true)),
+ Arguments.of(Named.of("tuple", true), Named.of("array", false)),
+ Arguments.of(Named.of("tuple", true), Named.of("tuple", true))
+ );
+ }
+
+ private RowSchema rowSchema(List<ColumnType> columnTypes, Object[] values) {
+ Builder schemaBuilder = RowSchema.builder();
+
+ for (int i = 0; i < values.length; i++) {
+ ColumnType type = columnTypes.get(i);
+
+ if (type == ColumnType.NULL) {
+ schemaBuilder.addField(new NullTypeSpec());
+
+ continue;
+ }
+
+ NativeType nativeType = values[i] == null
+ ? columnType2NativeType(type, 9, 3, 20)
+ : NativeTypes.fromObject(values[i]);
+
+ schemaBuilder.addField(nativeType, values[i] == null || rnd.nextBoolean());
+ }
+
+ return schemaBuilder.build();
+ }
+
+ private Object[] values(List<ColumnType> columnTypes) {
+ Object[] values = new Object[columnTypes.size()];
+ int baseValue = rnd.nextInt();
+
+ for (int i = 0; i < values.length; i++) {
+ ColumnType type = columnTypes.get(i);
+
+ values[i] = type == ColumnType.NULL ? null : generateValueByType(baseValue, type);
+ }
+
+ return values;
+ }
+
+ private List<ColumnType> columnTypes() {
+ List<ColumnType> columnTypes = new ArrayList<>(EnumSet.complementOf(EnumSet.of(ColumnType.PERIOD, ColumnType.DURATION)));
Review Comment:
it would be better to add a link to an issue with INTERVAL type support: https://issues.apache.org/jira/browse/IGNITE-15200
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1316845663
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/RowConverter.java:
##########
@@ -108,34 +121,44 @@ public static <RowT> BinaryTuple toBinaryTuple(
assert rowColumnsCount == binarySchema.elementCount() : "Invalid lookup key.";
- if (IgniteUtils.assertionsEnabled()) {
- for (int i = 0; i < rowColumnsCount; i++) {
- if (handler.get(i, searchRow) == UNSPECIFIED_VALUE_PLACEHOLDER) {
- throw new AssertionError("Invalid lookup key.");
- }
- }
- }
-
BinaryTupleBuilder tupleBuilder = new BinaryTupleBuilder(rowColumnsCount);
- return new BinaryTuple(rowColumnsCount, toByteBuffer(ectx, binarySchema, handler, tupleBuilder, searchRow));
+ return new BinaryTuple(rowColumnsCount, toByteBuffer(binarySchema, handler, tupleBuilder, rowColumnsCount, searchRow));
+ }
+
+ /**
+ * Converts an array of objects into a {@link ByteBuffer} representation of a {@link BinaryTuple binary tuple}. This method uses
+ * {@link RowSchema} to resolve object types.
+ *
+ * @param schema Row schema.
+ * @param values Array of objects to be stored in the buffer.
+ * @return Buffer in binary tuple format.
+ */
+ static ByteBuffer toByteBuffer(RowSchema schema, Object[] values) {
Review Comment:
> why do you put this method here
It seemed to me that this is a more suitable place, because. this class essentially does the same RowT -> BinaryTuple conversion.
anyway, moved to ObjectsArrayRowWrapper.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1316840282
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/RowHandler.java:
##########
@@ -87,5 +105,13 @@ interface RowFactory<RowT> {
* @return Instantiation defined representation.
*/
RowT create(ByteBuffer raw);
+
+ /**
+ * Wraps incoming binary tuple into a instantiation defined representation.
+ *
+ * @param tuple {@link InternalTuple} representation.
+ * @return Instantiation defined representation.
+ */
+ RowT wrap(InternalTuple tuple);
Review Comment:
Done
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1314619085
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/row/SqlRowHandlerTest.java:
##########
@@ -0,0 +1,214 @@
+/*
+ * 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.sql.engine.exec.row;
+
+import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.generateValueByType;
+import static org.apache.ignite.internal.sql.engine.util.TypeUtils.columnType2NativeType;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+
+import java.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.ThreadLocalRandom;
+import java.util.stream.Stream;
+import org.apache.ignite.internal.schema.BinaryTuple;
+import org.apache.ignite.internal.schema.NativeType;
+import org.apache.ignite.internal.schema.NativeTypes;
+import org.apache.ignite.internal.sql.engine.exec.RowHandler;
+import org.apache.ignite.internal.sql.engine.exec.RowHandler.RowFactory;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler.RowWrapper;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema.Builder;
+import org.apache.ignite.internal.sql.engine.util.TypeUtils;
+import org.apache.ignite.internal.testframework.IgniteAbstractTest;
+import org.apache.ignite.sql.ColumnType;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Named;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+/**
+ * Tests for {@link SqlRowHandler}.
+ */
+public class SqlRowHandlerTest extends IgniteAbstractTest {
+ private static final RowHandler<RowWrapper> handler = SqlRowHandler.INSTANCE;
+
+ private final long seed = ThreadLocalRandom.current().nextLong();
+
+ private final Random rnd = new Random(seed);
+
+ @BeforeEach
+ void printSeed() {
+ log.info("Using seed: " + seed);
+ }
+
+ @Test
+ public void testBytebufferSerialization() {
+ List<ColumnType> columnTypes = columnTypes();
+ Object[] sourceData = values(columnTypes);
+ RowSchema schema = rowSchema(columnTypes, sourceData);
+
+ int elementsCount = schema.fields().size();
+
+ RowFactory<RowWrapper> factory = handler.factory(schema);
+ RowWrapper src = factory.create(sourceData);
+
+ // Serialization to binary tuple representation.
+ ByteBuffer buf = handler.toByteBuffer(src);
+
+ BinaryTuple tuple = new BinaryTuple(elementsCount, buf);
+ RowWrapper destWrap = factory.wrap(tuple);
+ RowWrapper dest = factory.create(buf);
+
+ for (int i = 0; i < elementsCount; i++) {
+ String msg = schema.fields().get(i).toString();
+
+ assertThat(msg, handler.get(i, src), equalTo(sourceData[i]));
+ assertThat(msg, schema.value(i, tuple), equalTo(sourceData[i]));
+
+ // Binary tuple wrapper must return data in internal format.
+ Object expected = TypeUtils.toInternal(sourceData[i]);
+
+ assertThat(msg, handler.get(i, dest), equalTo(expected));
+ assertThat(msg, handler.get(i, destWrap), equalTo(expected));
+ }
+ }
+
+ @ParameterizedTest(name = "{0} - {1}")
+ @MethodSource("concatTestArguments")
+ public void testConcat(boolean leftTupleRequired, boolean rightTupleRequired) {
+ ConcatTestParameters params = new ConcatTestParameters(leftTupleRequired, rightTupleRequired);
+
+ RowWrapper concatenated = handler.concat(params.left, params.right);
+
+ int leftLen = params.leftData.length;
+ int rightLen = params.rightData.length;
+ int totalElementsCount = leftLen + rightLen;
+
+ assertThat(handler.columnCount(concatenated), equalTo(totalElementsCount));
+
+ // Build combined schema.
+ Builder builder = RowSchema.builder();
+ params.leftSchema.fields().forEach(builder::addField);
+ params.rightSchema.fields().forEach(builder::addField);
+
+ RowSchema concatenatedSchema = builder.build();
+
+ // Serialize.
+ ByteBuffer buf = handler.toByteBuffer(concatenated);
+
+ // Wrap into row.
+ RowWrapper wrapped = handler.factory(concatenatedSchema).wrap(new BinaryTuple(totalElementsCount, buf));
+
+ for (int i = 0; i < Math.max(leftLen, rightLen); i++) {
+ if (i < leftLen) {
+ assertThat(handler.get(i, wrapped), equalTo(TypeUtils.toInternal(params.leftData[i])));
+ }
+
+ if (i < rightLen) {
+ assertThat(handler.get(leftLen + i, wrapped), equalTo(TypeUtils.toInternal(params.rightData[i])));
+ }
+ }
+ }
+
+ private static Stream<Arguments> concatTestArguments() {
+ return Stream.of(
+ Arguments.of(Named.of("array", false), Named.of("array", false)),
+ Arguments.of(Named.of("array", false), Named.of("tuple", true)),
+ Arguments.of(Named.of("tuple", true), Named.of("array", false)),
+ Arguments.of(Named.of("tuple", true), Named.of("tuple", true))
+ );
+ }
+
+ private RowSchema rowSchema(List<ColumnType> columnTypes, Object[] values) {
+ Builder schemaBuilder = RowSchema.builder();
+
+ for (int i = 0; i < values.length; i++) {
+ ColumnType type = columnTypes.get(i);
+
+ if (type == ColumnType.NULL) {
+ schemaBuilder.addField(new NullTypeSpec());
+
+ continue;
+ }
+
+ NativeType nativeType = values[i] == null
+ ? columnType2NativeType(type, 9, 3, 20)
+ : NativeTypes.fromObject(values[i]);
+
+ schemaBuilder.addField(nativeType, values[i] == null || rnd.nextBoolean());
+ }
+
+ return schemaBuilder.build();
+ }
+
+ private Object[] values(List<ColumnType> columnTypes) {
+ Object[] values = new Object[columnTypes.size()];
+ int baseValue = rnd.nextInt();
+
+ for (int i = 0; i < values.length; i++) {
+ ColumnType type = columnTypes.get(i);
+
+ values[i] = type == ColumnType.NULL || rnd.nextInt(4) == 1 ? null : generateValueByType(baseValue, type);
Review Comment:
Removed this confusing `random`.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] lowka commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1314472422
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/RowConverter.java:
##########
@@ -146,4 +169,55 @@ private static <RowT> ByteBuffer toByteBuffer(
return tupleBuilder.build();
}
+
+ private static BinaryTupleBuilder appendValue(BinaryTupleBuilder builder, TypeSpec schemaType, @Nullable Object value) {
Review Comment:
I think return type is redundant here.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] lowka commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1314535154
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/SqlRowHandler.java:
##########
@@ -0,0 +1,260 @@
+/*
+ * 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.sql.engine.exec;
+
+import java.nio.ByteBuffer;
+import java.util.List;
+import org.apache.ignite.internal.schema.BinaryTuple;
+import org.apache.ignite.internal.schema.NativeType;
+import org.apache.ignite.internal.schema.row.InternalTuple;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler.RowWrapper;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema.Builder;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchemaTypes;
+import org.apache.ignite.internal.sql.engine.exec.row.TypeSpec;
+import org.apache.ignite.internal.sql.engine.util.Commons;
+import org.apache.ignite.internal.sql.engine.util.TypeUtils;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Handler that uses a {@link RowWrapper wrapper} to operate with two types of row implementations.
+ *
+ * <ul>
+ * <li>{@link ObjectsArrayRowWrapper Objects array}</li>
+ * <li>{@link BinaryTupleRowWrapper Binary tuple}</li>
+ * </ul>
+ *
+ * <p>Each kind of rows is serialized to the same binary tuple format
+ * using the {@link #toByteBuffer(RowWrapper) toByteBuffer} method.
+ *
+ * <p>Factory methods {@link RowFactory#wrap(InternalTuple) wrap(InternalTuple)} and
+ * {@link RowFactory#create(ByteBuffer) create(ByteBuffer)} allow create rows without
+ * any additional conversions. But the fields in binary tuple must match the
+ * factory {@link RowSchema row schema}.
+ */
+public class SqlRowHandler implements RowHandler<RowWrapper> {
+ public static final RowHandler<RowWrapper> INSTANCE = new SqlRowHandler();
+
+ private SqlRowHandler() {
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public @Nullable Object get(int field, RowWrapper row) {
+ return row.get(field);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public void set(int field, RowWrapper row, @Nullable Object val) {
+ row.set(field, val);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper concat(RowWrapper left, RowWrapper right) {
+ int leftLen = left.columnsCount();
+ int rightLen = right.columnsCount();
+ List<TypeSpec> leftTypes = left.rowSchema().fields();
+ List<TypeSpec> rightTypes = right.rowSchema().fields();
+
+ Object[] values = new Object[leftLen + rightLen];
+ Builder schemaBuilder = RowSchema.builder();
+
+ for (int i = 0; i < leftLen; i++) {
+ values[i] = left.get(i);
+ schemaBuilder.addField(leftTypes.get(i));
+ }
+
+ for (int i = 0; i < rightLen; i++) {
+ values[leftLen + i] = right.get(i);
+ schemaBuilder.addField(rightTypes.get(i));
+ }
+
+ return new ObjectsArrayRowWrapper(schemaBuilder.build(), values);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper map(RowWrapper row, int[] mapping, int offset) {
+ Object[] fields = new Object[mapping.length];
+
+ for (int i = 0; i < mapping.length; i++) {
+ fields[i] = row.get(mapping[i] + offset);
+ }
+
+ return new ObjectsArrayRowWrapper(row.rowSchema(), fields);
+ }
+
+ @Override
+ public int columnCount(RowWrapper row) {
+ return row.columnsCount();
+ }
+
+ @Override
+ public String toString(RowWrapper row) {
+ return String.valueOf(row);
+ }
+
+ @Override
+ public ByteBuffer toByteBuffer(RowWrapper row) {
+ return row.toByteBuffer();
Review Comment:
It would be better to provide implementation of `toString` method that is similar to ArrayRowHandler's. This one simply return `typeName@132` or something.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1314605881
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/row/SqlRowHandlerTest.java:
##########
@@ -0,0 +1,212 @@
+/*
+ * 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.sql.engine.exec.row;
+
+import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.generateValueByType;
+import static org.apache.ignite.internal.sql.engine.util.TypeUtils.columnType2NativeType;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+
+import java.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.ThreadLocalRandom;
+import java.util.stream.Stream;
+import org.apache.ignite.internal.schema.BinaryTuple;
+import org.apache.ignite.internal.schema.NativeType;
+import org.apache.ignite.internal.schema.NativeTypes;
+import org.apache.ignite.internal.sql.engine.exec.RowHandler;
+import org.apache.ignite.internal.sql.engine.exec.RowHandler.RowFactory;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler.RowWrapper;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema.Builder;
+import org.apache.ignite.internal.sql.engine.util.TypeUtils;
+import org.apache.ignite.internal.testframework.IgniteAbstractTest;
+import org.apache.ignite.sql.ColumnType;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Named;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+/**
+ * Tests for {@link SqlRowHandler}.
+ */
+public class SqlRowHandlerTest extends IgniteAbstractTest {
+ private static final RowHandler<RowWrapper> handler = SqlRowHandler.INSTANCE;
+
+ private final long seed = ThreadLocalRandom.current().nextLong();
+
+ private final Random rnd = new Random(seed);
+
+ @BeforeEach
+ void printSeed() {
+ log.info("Using seed: " + seed);
+ }
+
+ @Test
+ public void testBytebufferSerialization() {
+ List<ColumnType> columnTypes = columnTypes();
+ Object[] sourceData = values(columnTypes);
+ RowSchema schema = rowSchema(columnTypes, sourceData);
+
+ int elementsCount = schema.fields().size();
+
+ RowFactory<RowWrapper> factory = handler.factory(schema);
+ RowWrapper src = factory.create(sourceData);
+
+ // Serialization to binary tuple representation.
+ ByteBuffer buf = handler.toByteBuffer(src);
+
+ BinaryTuple tuple = new BinaryTuple(elementsCount, buf);
+ RowWrapper destWrap = factory.wrap(tuple);
+ RowWrapper dest = factory.create(buf);
+
+ for (int i = 0; i < elementsCount; i++) {
+ String msg = schema.fields().get(i).toString();
+
+ assertThat(msg, handler.get(i, src), equalTo(sourceData[i]));
+ assertThat(msg, schema.value(tuple, i), equalTo(sourceData[i]));
+
+ // Binary tuple wrapper must return data in internal format.
+ Object expected = TypeUtils.toInternal(sourceData[i]);
+
+ assertThat(msg, handler.get(i, dest), equalTo(expected));
+ assertThat(msg, handler.get(i, destWrap), equalTo(expected));
+ }
+ }
+
+ @ParameterizedTest(name = "{0} - {1}")
+ @MethodSource("concatTestArguments")
+ public void testConcat(boolean leftTupleRequired, boolean rightTupleRequired) {
+ ConcatTestParameters params = new ConcatTestParameters(leftTupleRequired, rightTupleRequired);
+
+ RowWrapper concatenated = handler.concat(params.left, params.right);
+
+ int leftLen = params.leftData.length;
+ int rightLen = params.rightData.length;
+ int totalElementsCount = leftLen + rightLen;
+
+ assertThat(handler.columnCount(concatenated), equalTo(totalElementsCount));
+
+ // Build combined schema.
+ Builder builder = RowSchema.builder();
+ params.leftSchema.fields().forEach(builder::addField);
+ params.rightSchema.fields().forEach(builder::addField);
+
+ RowSchema concatenatedSchema = builder.build();
+
+ // Serialize.
+ ByteBuffer buf = handler.toByteBuffer(concatenated);
+
+ // Wrap into row.
+ RowWrapper result = handler.factory(concatenatedSchema).wrap(new BinaryTuple(totalElementsCount, buf));
+
+ for (int i = 0; i < leftLen; i++) {
+ assertThat(handler.get(i, result), equalTo(TypeUtils.toInternal(params.leftData[i])));
+ }
+
+ for (int i = 0; i < rightLen; i++) {
+ assertThat(handler.get(leftLen + i, result), equalTo(TypeUtils.toInternal(params.rightData[i])));
+ }
+ }
+
+ private static Stream<Arguments> concatTestArguments() {
+ return Stream.of(
+ Arguments.of(Named.of("array", false), Named.of("array", false)),
+ Arguments.of(Named.of("array", false), Named.of("tuple", true)),
+ Arguments.of(Named.of("tuple", true), Named.of("array", false)),
+ Arguments.of(Named.of("tuple", true), Named.of("tuple", true))
+ );
+ }
+
+ private RowSchema rowSchema(List<ColumnType> columnTypes, Object[] values) {
+ Builder schemaBuilder = RowSchema.builder();
+
+ for (int i = 0; i < values.length; i++) {
+ ColumnType type = columnTypes.get(i);
+
+ if (type == ColumnType.NULL) {
+ schemaBuilder.addField(new NullTypeSpec());
+
+ continue;
+ }
+
+ NativeType nativeType = values[i] == null
+ ? columnType2NativeType(type, 9, 3, 20)
+ : NativeTypes.fromObject(values[i]);
+
+ schemaBuilder.addField(nativeType, values[i] == null || rnd.nextBoolean());
+ }
+
+ return schemaBuilder.build();
+ }
+
+ private Object[] values(List<ColumnType> columnTypes) {
+ Object[] values = new Object[columnTypes.size()];
+ int baseValue = rnd.nextInt();
+
+ for (int i = 0; i < values.length; i++) {
+ ColumnType type = columnTypes.get(i);
+
+ values[i] = type == ColumnType.NULL ? null : generateValueByType(baseValue, type);
+ }
+
+ return values;
+ }
+
+ private List<ColumnType> columnTypes() {
+ List<ColumnType> columnTypes = new ArrayList<>(EnumSet.complementOf(EnumSet.of(ColumnType.PERIOD, ColumnType.DURATION)));
Review Comment:
Done, thanks.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1314609653
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/RowConverter.java:
##########
@@ -146,4 +169,55 @@ private static <RowT> ByteBuffer toByteBuffer(
return tupleBuilder.build();
}
+
+ private static BinaryTupleBuilder appendValue(BinaryTupleBuilder builder, TypeSpec schemaType, @Nullable Object value) {
Review Comment:
It seemed to me that this way the code in switch-block was shorter and more concise. But since you paid attention to it, I reworked it.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1314628082
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/SqlRowHandler.java:
##########
@@ -0,0 +1,260 @@
+/*
+ * 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.sql.engine.exec;
+
+import java.nio.ByteBuffer;
+import java.util.List;
+import org.apache.ignite.internal.schema.BinaryTuple;
+import org.apache.ignite.internal.schema.NativeType;
+import org.apache.ignite.internal.schema.row.InternalTuple;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler.RowWrapper;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema.Builder;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchemaTypes;
+import org.apache.ignite.internal.sql.engine.exec.row.TypeSpec;
+import org.apache.ignite.internal.sql.engine.util.Commons;
+import org.apache.ignite.internal.sql.engine.util.TypeUtils;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Handler that uses a {@link RowWrapper wrapper} to operate with two types of row implementations.
+ *
+ * <ul>
+ * <li>{@link ObjectsArrayRowWrapper Objects array}</li>
+ * <li>{@link BinaryTupleRowWrapper Binary tuple}</li>
+ * </ul>
+ *
+ * <p>Each kind of rows is serialized to the same binary tuple format
+ * using the {@link #toByteBuffer(RowWrapper) toByteBuffer} method.
+ *
+ * <p>Factory methods {@link RowFactory#wrap(InternalTuple) wrap(InternalTuple)} and
+ * {@link RowFactory#create(ByteBuffer) create(ByteBuffer)} allow create rows without
+ * any additional conversions. But the fields in binary tuple must match the
+ * factory {@link RowSchema row schema}.
+ */
+public class SqlRowHandler implements RowHandler<RowWrapper> {
+ public static final RowHandler<RowWrapper> INSTANCE = new SqlRowHandler();
+
+ private SqlRowHandler() {
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public @Nullable Object get(int field, RowWrapper row) {
+ return row.get(field);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public void set(int field, RowWrapper row, @Nullable Object val) {
+ row.set(field, val);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper concat(RowWrapper left, RowWrapper right) {
+ int leftLen = left.columnsCount();
+ int rightLen = right.columnsCount();
+ List<TypeSpec> leftTypes = left.rowSchema().fields();
+ List<TypeSpec> rightTypes = right.rowSchema().fields();
+
+ Object[] values = new Object[leftLen + rightLen];
+ Builder schemaBuilder = RowSchema.builder();
+
+ for (int i = 0; i < leftLen; i++) {
+ values[i] = left.get(i);
+ schemaBuilder.addField(leftTypes.get(i));
+ }
+
+ for (int i = 0; i < rightLen; i++) {
+ values[leftLen + i] = right.get(i);
Review Comment:
We need two indices anyway. I don't think it worth to replace `leftLen + i` to something like 'index++' and declare `index` out of cycle.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] xtern commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1314647496
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/exp/ExpressionFactoryImpl.java:
##########
@@ -277,8 +277,17 @@ public Supplier<RowT> rowSource(List<RexNode> values) {
/** {@inheritDoc} */
@Override
public <T> Supplier<T> execute(RexNode node) {
- RelDataType nodeType = node.getType();
- RowSchema rowSchema = TypeUtils.rowSchemaFromRelTypes(List.of(nodeType));
+ RelDataType exprType = node.getType();
+ List<RelDataType> typesList;
+
+ if (exprType.getSqlTypeName() == SqlTypeName.ROW) {
Review Comment:
on the next line after this line there is a comment explaining why this condition is needed.
Previously, RowSchema was not used at all.
Currently it used to resolve object types in the row.
We faced with the problem that table function may return composite type.
As far as I understand, we do not and do not plan to support such types, and at the moment this composite type can only include one "inner" type. Therefore, we decided to convert such a composite type to single base type here.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] zstan commented on a diff in pull request #2512: IGNITE-19791 Introduce SqlRowHandler
Posted by "zstan (via GitHub)" <gi...@apache.org>.
zstan commented on code in PR #2512:
URL: https://github.com/apache/ignite-3/pull/2512#discussion_r1315387124
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/SqlRowHandler.java:
##########
@@ -0,0 +1,272 @@
+/*
+ * 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.sql.engine.exec;
+
+import java.nio.ByteBuffer;
+import java.util.List;
+import org.apache.ignite.internal.schema.BinaryTuple;
+import org.apache.ignite.internal.schema.NativeType;
+import org.apache.ignite.internal.schema.row.InternalTuple;
+import org.apache.ignite.internal.sql.engine.exec.SqlRowHandler.RowWrapper;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema.Builder;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchemaTypes;
+import org.apache.ignite.internal.sql.engine.exec.row.TypeSpec;
+import org.apache.ignite.internal.sql.engine.util.Commons;
+import org.apache.ignite.internal.sql.engine.util.TypeUtils;
+import org.apache.ignite.lang.IgniteStringBuilder;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Handler that uses a {@link RowWrapper wrapper} to operate with two types of row implementations.
+ *
+ * <ul>
+ * <li>{@link ObjectsArrayRowWrapper Objects array}</li>
+ * <li>{@link BinaryTupleRowWrapper Binary tuple}</li>
+ * </ul>
+ *
+ * <p>Each kind of rows is serialized to the same binary tuple format
+ * using the {@link #toByteBuffer(RowWrapper) toByteBuffer} method.
+ *
+ * <p>Factory methods {@link RowFactory#wrap(InternalTuple) wrap(InternalTuple)} and
+ * {@link RowFactory#create(ByteBuffer) create(ByteBuffer)} allow create rows without
+ * any additional conversions. But the fields in binary tuple must match the
+ * factory {@link RowSchema row schema}.
+ */
+public class SqlRowHandler implements RowHandler<RowWrapper> {
+ public static final RowHandler<RowWrapper> INSTANCE = new SqlRowHandler();
+
+ private SqlRowHandler() {
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public @Nullable Object get(int field, RowWrapper row) {
+ return row.get(field);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public void set(int field, RowWrapper row, @Nullable Object val) {
+ row.set(field, val);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper concat(RowWrapper left, RowWrapper right) {
+ int leftLen = left.columnsCount();
+ int rightLen = right.columnsCount();
+ List<TypeSpec> leftTypes = left.rowSchema().fields();
+ List<TypeSpec> rightTypes = right.rowSchema().fields();
+
+ Object[] values = new Object[leftLen + rightLen];
+ Builder schemaBuilder = RowSchema.builder();
+
+ for (int i = 0; i < leftLen; i++) {
+ values[i] = left.get(i);
+ schemaBuilder.addField(leftTypes.get(i));
+ }
+
+ for (int i = 0; i < rightLen; i++) {
+ values[leftLen + i] = right.get(i);
+ schemaBuilder.addField(rightTypes.get(i));
+ }
+
+ return new ObjectsArrayRowWrapper(schemaBuilder.build(), values);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper map(RowWrapper row, int[] mapping, int offset) {
+ Object[] fields = new Object[mapping.length];
+
+ for (int i = 0; i < mapping.length; i++) {
+ fields[i] = row.get(mapping[i] + offset);
+ }
+
+ return new ObjectsArrayRowWrapper(row.rowSchema(), fields);
+ }
+
+ @Override
+ public int columnCount(RowWrapper row) {
+ return row.columnsCount();
+ }
+
+ @Override
+ public String toString(RowWrapper row) {
+ IgniteStringBuilder buf = new IgniteStringBuilder("Row[");
+ int maxIdx = columnCount(row) - 1;
+
+ for (int i = 0; i <= maxIdx; i++) {
+ buf.app(row.get(i));
+
+ if (i != maxIdx) {
+ buf.app(", ");
+ }
+ }
+
+ return buf.app(']').toString();
+ }
+
+ @Override
+ public ByteBuffer toByteBuffer(RowWrapper row) {
+ return row.toByteBuffer();
+ }
+
+ @Override
+ public RowFactory<RowWrapper> factory(RowSchema rowSchema) {
+ int rowLen = rowSchema.fields().size();
+
+ return new RowFactory<>() {
+ /** {@inheritDoc} */
+ @Override
+ public RowHandler<RowWrapper> handler() {
+ return SqlRowHandler.this;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper create() {
+ return create(new Object[rowLen]);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper create(Object... fields) {
+ assert fields.length == rowLen;
+
+ return new ObjectsArrayRowWrapper(rowSchema, fields);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper create(ByteBuffer buf) {
+ return wrap(new BinaryTuple(rowLen, buf));
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public RowWrapper wrap(InternalTuple tuple) {
+ assert rowLen == tuple.elementCount();
+
+ return new BinaryTupleRowWrapper(rowSchema, tuple);
+ }
+ };
+ }
+
+ /**
+ * Provides the ability for a single {@link RowHandler} instance to interact with multiple row implementations.
+ */
+ public abstract static class RowWrapper {
Review Comment:
i see an interface, not abstract class here )
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org