alex-plekhanov commented on code in PR #12113: URL: https://github.com/apache/ignite/pull/12113#discussion_r2215766713
########## modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/metadata/IgniteMdRowCount.java: ########## @@ -282,9 +288,21 @@ private static IntMap<KeyColumnOrigin> findOrigins(RelMetadataQuery mq, RelNode if (table == null) continue; - int keyPos = keyColumns(table).indexOf(origin.getOriginColumnOrdinal()); + int srcKeyColIdx = origin.getOriginColumnOrdinal(); + + RelDataType insertRowType = table.descriptor().insertRowType(Commons.typeFactory(joinInput)); + RelDataType curRowType = origin.getOriginTable().getRowType(); + + assert curRowType.getFieldCount() >= insertRowType.getFieldCount(); + + if (curRowType.getFieldCount() > insertRowType.getFieldCount()) { + /** Current row type probably contains {@link QueryUtils#KEY_FIELD_NAME} and {@link QueryUtils#VAL_FIELD_NAME}. */ + srcKeyColIdx -= curRowType.getFieldCount() - insertRowType.getFieldCount(); + } Review Comment: 1. insertRowType can contain _KEY/_VAL fields as well (see DistributedJoinIntegrationTest#testTrimExchange for example) 2. Join can be on _KEY field, in this case srcKeyColIdx will be negative 3. Current implementation is wrong. Implementation without shift is even more correct (but doesn't take into account key field aliases and proxy PK index). Foe example any two fields PK index is not working: ``` sql("CREATE TABLE t1(id1 INT, id2 INT, val VARCHAR, PRIMARY KEY(id1, id2))"); sql("CREATE TABLE t2(id1 INT, id2 INT, val VARCHAR, PRIMARY KEY(id1, id2))"); sql("SELECT * FROM t1 JOIN t2 USING(id1, id2)"); ``` 4. There are a lot of errors in this part of code already. Can we introduce some unit test to check this code explicitely? Because current tests doesn't check this code at all. ########## modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/metadata/IgniteMdRowCount.java: ########## @@ -149,4 +263,195 @@ public double getRowCount(IgniteAggregate rel, RelMetadataQuery mq) { public double getRowCount(IgniteLimit rel, RelMetadataQuery mq) { return rel.estimateRowCount(mq); } + + /** */ + private static IntMap<KeyColumnOrigin> findOrigins(RelMetadataQuery mq, RelNode joinInput, ImmutableIntList keys) { + IntMap<KeyColumnOrigin> res = new IntRWHashMap<>(); Review Comment: No concurrency here, IntHashMap can be used. Maybe even array (KeyColumnOrigin[]) since key is limited -- 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org