zstan commented on code in PR #2728:
URL: https://github.com/apache/ignite-3/pull/2728#discussion_r1368194106
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/rel/ExchangeExecutionTest.java:
##########
@@ -495,8 +497,13 @@ private RewindableAsyncRoot<Object[], Object[]>
createRootFragment(
ExchangeService exchangeService =
exchangeServices.computeIfAbsent(localNode.name(), name ->
createExchangeService(taskExecutor,
serviceFactory.forNode(localNode.name()), mailboxRegistry));
+ RowSchema schema = RowSchema.builder()
Review Comment:
this schema depends on TestDataProvider i`d like to see additional comment
here, or provider himself can return such a schema, wdyt ?
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/framework/ArrayRowHandler.java:
##########
@@ -108,17 +129,137 @@ public Object[] create(Object... fields) {
return fields;
}
- /** {@inheritDoc} */
- @Override
- public Object[] create(ByteBuffer raw) {
- return ByteUtils.fromBytes(raw.array());
- }
-
/** {@inheritDoc} */
@Override
public Object[] create(InternalTuple tuple) {
- throw new UnsupportedOperationException();
+ Object[] row = new Object[tuple.elementCount()];
+
+ for (int i = 0; i < row.length; i++) {
+ NativeType nativeType =
RowSchemaTypes.toNativeType(rowSchema.fields().get(i));
+
+ if (nativeType == null) {
+ row[i] = null;
+
+ continue;
+ }
+
+ row[i] = readValue(tuple, nativeType, i);
+ }
+
+ return row;
}
};
}
+
+ private static void appendValue(BinaryTupleBuilder builder, @Nullable
Object value) {
+ if (value == null) {
+ builder.appendNull();
+
+ return;
+ }
+
+ NativeType nativeType = NativeTypes.fromObject(value);
+
+ assert nativeType != null;
+
+ value = TypeUtils.fromInternal(value,
NativeTypeSpec.toClass(nativeType.spec(), true));
Review Comment:
1. seems we have no intergation test which touch this functionality
2. why we need such double conversion ? i comment line above and execution
tests are passed ok. Did i miss smth ?
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ScannableTableImpl.java:
##########
@@ -148,24 +147,31 @@ public <RowT> Publisher<RowT> indexLookup(
PartitionWithTerm partWithTerm,
RowFactory<RowT> rowFactory,
int indexId,
- List<String> columns, RowT key,
+ List<String> columns,
+ RowT key,
@Nullable BitSet requiredColumns
) {
-
- BinaryTupleSchema indexRowSchema =
RowConverter.createIndexRowSchema(columns, tableDescriptor);
TxAttributes txAttributes = ctx.txAttributes();
+ RowHandler<RowT> handler = rowFactory.handler();
Publisher<BinaryRow> pub;
- BinaryTuple keyTuple = toBinaryTuple(ctx, indexRowSchema, key,
rowFactory);
+ BinaryTuple keyTuple = handler.toBinaryTuple(key);
+
+ assert keyTuple.elementCount() == columns.size()
+ : format("Key should contain exactly {} fields, but was {}",
columns.size(), handler.toString(key));
if (txAttributes.readOnly()) {
+ HybridTimestamp readTime = txAttributes.time();
+
+ assert readTime != null;
+
pub = internalTable.lookup(
partWithTerm.partId(),
- txAttributes.time(),
+ readTime,
ctx.localNode(),
indexId,
keyTuple,
- requiredColumns
+ null
Review Comment:
seems all calls for such method are raised with columnsToInclude = null, do
we need them ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
