alex-plekhanov commented on code in PR #12926:
URL: https://github.com/apache/ignite/pull/12926#discussion_r3021882365


##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/schema/SchemaHolderImpl.java:
##########
@@ -473,4 +493,19 @@ private void rebuild() {
 
         calciteSchema = newCalciteSchema;
     }
+
+    /** */
+    private static RelCollation deriveKeyFieldIndexCollation(IgniteCacheTable 
tbl) {
+        ColumnDescriptor desc = 
tbl.descriptor().columnDescriptor(QueryUtils.KEY_FIELD_NAME);
+        assert desc != null : String.format(
+            "cacheName=%s, schemaName=%s, tableName=%s",
+            tbl.descriptor().typeDescription().cacheName(),
+            tbl.descriptor().typeDescription().tableName(),

Review Comment:
   schemaName



##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/IndexWrappedKeyScan.java:
##########
@@ -0,0 +1,101 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.processors.query.calcite.exec;
+
+import java.util.Map;
+import org.apache.calcite.util.ImmutableBitSet;
+import org.apache.calcite.util.ImmutableIntList;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.internal.cache.query.index.sorted.IndexKeyDefinition;
+import org.apache.ignite.internal.cache.query.index.sorted.IndexKeyType;
+import org.apache.ignite.internal.cache.query.index.sorted.IndexPlainRowImpl;
+import org.apache.ignite.internal.cache.query.index.sorted.IndexRow;
+import 
org.apache.ignite.internal.cache.query.index.sorted.InlineIndexRowHandler;
+import org.apache.ignite.internal.cache.query.index.sorted.inline.InlineIndex;
+import org.apache.ignite.internal.cache.query.index.sorted.keys.IndexKey;
+import 
org.apache.ignite.internal.cache.query.index.sorted.keys.IndexKeyFactory;
+import org.apache.ignite.internal.processors.query.QueryUtils;
+import 
org.apache.ignite.internal.processors.query.calcite.exec.exp.RangeIterable;
+import 
org.apache.ignite.internal.processors.query.calcite.schema.CacheTableDescriptor;
+import 
org.apache.ignite.internal.processors.query.calcite.schema.ColumnDescriptor;
+import org.apache.ignite.internal.processors.query.calcite.util.TypeUtils;
+import org.jetbrains.annotations.Nullable;
+
+/** Extension for column {@value QueryUtils#KEY_FIELD_NAME} in case of 
composite primary key. */
+public class IndexWrappedKeyScan<Row> extends IndexScan<Row> {
+    /** */
+    public IndexWrappedKeyScan(
+        ExecutionContext<Row> ectx,
+        CacheTableDescriptor desc,
+        InlineIndex idx,
+        ImmutableIntList idxFieldMapping,
+        int[] parts,
+        RangeIterable<Row> ranges,
+        @Nullable ImmutableBitSet requiredColumns
+    ) {
+        super(ectx, desc, idx, idxFieldMapping, parts, ranges, 
requiredColumns);
+    }
+
+    /** */
+    @Override protected IndexRow row2indexRow(Row bound) {
+        if (bound == null)
+            return null;
+
+        RowHandler<Row> rowHnd = ectx.rowHandler();
+
+        Object key = rowHnd.get(QueryUtils.KEY_COL, bound);
+        assert key != null : String.format("idxName=%s, bound=%s", idx.name(), 
rowHnd.toString(bound));
+
+        if (key instanceof BinaryObject)
+            return binaryObject2indexRow((BinaryObject)key);
+
+        // TODO: IGNITE-28374 Implement for POJO key class

Review Comment:
   Let's remove this TODO. In fact changes related to this ticket can be made 
not in this method. 



##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/schema/SchemaHolderImpl.java:
##########
@@ -301,12 +311,22 @@ private static Object 
affinityIdentity(CacheConfiguration<?, ?> ccfg) {
         IndexDescriptor idxDesc
     ) {
         IgniteCacheTable tbl = table(schemaName, tblName);
-        assert tbl != null;
+        assert tbl != null : String.format("schemaName=%s, tableName=%s, 
idxName=%s", schemaName, tblName, idxName);
 
         RelCollation idxCollation = deriveSecondaryIndexCollation(idxDesc, 
tbl);
 
         IgniteIndex idx = new CacheIndexImpl(idxCollation, idxName, 
idxDesc.index(), tbl);
         tbl.addIndex(idx);
+
+        // For a composite PK index, we need to create another proxy index 
that will expand the passed boundaries into
+        // index keys for BinaryObject/Key classes. That is, _key -> 
idx_field_0, idx_field_1, etc.
+        if (idxDesc.isPk() && idxDesc.isComposite()) {
+            RelCollation keyFieldCollation = deriveKeyFieldIndexCollation(tbl);

Review Comment:
   Collation for wrapped key index should always be empty, since order by 
binary object is not the same as order by unwrapped fields. 



##########
modules/calcite/src/test/java/org/apache/ignite/testsuites/IntegrationTestSuite.java:
##########
@@ -174,7 +175,8 @@
     QueryEntityValueColumnAliasTest.class,
     CacheStoreTest.class,
     MultiDcQueryMappingTest.class,
-    TxWithExceptionalInterceptorTest.class
+    TxWithExceptionalInterceptorTest.class,
+    SelectByKeyFieldTest.class

Review Comment:
   Please add comma at the end of line (this will reduce merge conflicts in the 
future).



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/query/schema/management/IndexDescriptor.java:
##########
@@ -141,4 +141,9 @@ public IndexDescriptor targetIdx() {
     public TableDescriptor table() {
         return tbl;
     }
+
+    /** */
+    public boolean isComposite() {

Review Comment:
   Used only once. Maybe inline?



##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/RowHandler.java:
##########
@@ -40,6 +39,9 @@ public interface RowHandler<Row> {
     /** */
     int columnCount(Row row);
 
+    /** */
+    String toString(Row row);

Review Comment:
   Not sure it's a right place for this method. Maybe move it to some utils 
class or remove at all (used only once in assertion)



##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/IndexWrappedKeyFirstLastScan.java:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.processors.query.calcite.exec;
+
+import org.apache.calcite.util.ImmutableBitSet;
+import org.apache.calcite.util.ImmutableIntList;
+import org.apache.ignite.internal.cache.query.index.sorted.IndexRow;
+import 
org.apache.ignite.internal.cache.query.index.sorted.inline.IndexQueryContext;
+import org.apache.ignite.internal.cache.query.index.sorted.inline.InlineIndex;
+import 
org.apache.ignite.internal.processors.query.calcite.schema.CacheTableDescriptor;
+import org.jetbrains.annotations.Nullable;
+
+/** Like {@link IndexFirstLastScan} but expanding {@link IndexWrappedKeyScan}. 
*/
+public class IndexWrappedKeyFirstLastScan<Row> extends 
IndexWrappedKeyScan<Row> {

Review Comment:
   There should not be used first/last scan for wrappped key index, since order 
by binary object is not the same as order by unwrapped fields and this 
first/last scan will produce the wrong result. 
   Empty collation will protect from creating first/last scan for this index.



##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/schema/CacheIndexImpl.java:
##########
@@ -197,4 +178,59 @@ public Index queryIndex() {
 
         return true;
     }
+
+    /** */
+    protected List<SearchBounds> buildSearchBounds(
+        RelOptCluster cluster,
+        RelCollation collation,
+        @Nullable RexNode cond,
+        RelDataType rowType,
+        @Nullable ImmutableBitSet requiredColumns
+    ) {
+        return RexUtils.buildSortedSearchBounds(
+            cluster,
+            collation,
+            cond,
+            rowType,
+            requiredColumns
+        );
+    }
+
+    /** */
+    protected <Row> IndexScan<Row> createIndexScan(
+        ExecutionContext<Row> ectx,
+        ColocationGroup grp,
+        RangeIterable<Row> ranges,
+        @Nullable ImmutableBitSet requiredColumns
+    ) {
+        return new IndexScan<>(
+            ectx,
+            tbl.descriptor(),
+            idx.unwrap(InlineIndex.class),
+            collation.getKeys(),
+            grp.partitions(ectx.localNodeId()),
+            ranges,
+            requiredColumns
+        );
+    }
+
+    /** */
+    protected <Row> IndexScan<Row> createIndexFirstLastScan(
+        boolean first, ExecutionContext<Row> ectx, ColocationGroup grp, 
@Nullable ImmutableBitSet requiredColumns
+    ) {
+        return new IndexFirstLastScan<>(
+            first,
+            ectx,
+            tbl.descriptor(),
+            idx.unwrap(InlineIndexImpl.class),
+            collation.getKeys(),
+            grp.partitions(ectx.localNodeId()),
+            requiredColumns
+        );
+    }
+
+    /** */
+    protected CacheIndexImpl copyWithNewTable(IgniteCacheTable newTbl) {

Review Comment:
   Maybe `clone(IgniteCacheTable tbl)` and `clone(RelCollation collation)` 
(identical to IgniteRel) 



##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/schema/SchemaHolderImpl.java:
##########
@@ -301,12 +311,22 @@ private static Object 
affinityIdentity(CacheConfiguration<?, ?> ccfg) {
         IndexDescriptor idxDesc
     ) {
         IgniteCacheTable tbl = table(schemaName, tblName);
-        assert tbl != null;
+        assert tbl != null : String.format("schemaName=%s, tableName=%s, 
idxName=%s", schemaName, tblName, idxName);
 
         RelCollation idxCollation = deriveSecondaryIndexCollation(idxDesc, 
tbl);
 
         IgniteIndex idx = new CacheIndexImpl(idxCollation, idxName, 
idxDesc.index(), tbl);
         tbl.addIndex(idx);
+
+        // For a composite PK index, we need to create another proxy index 
that will expand the passed boundaries into
+        // index keys for BinaryObject/Key classes. That is, _key -> 
idx_field_0, idx_field_1, etc.
+        if (idxDesc.isPk() && idxDesc.isComposite()) {
+            RelCollation keyFieldCollation = deriveKeyFieldIndexCollation(tbl);
+
+            tbl.addIndex(new CacheWrappedKeyIndexImpl(
+                keyFieldCollation, idxName + "_proxy", idxDesc.index(), tbl, 
idxCollation

Review Comment:
   SchemaManager#generateProxyIdxName



##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/SelectByKeyFieldTest.java:
##########
@@ -0,0 +1,250 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.processors.query.calcite.integration;
+
+import java.util.List;
+import java.util.Objects;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.calcite.CalciteQueryEngineConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.configuration.SqlConfiguration;
+import org.apache.ignite.indexing.IndexingQueryEngineConfiguration;
+import org.apache.ignite.internal.processors.query.QueryUtils;
+import org.apache.ignite.internal.processors.query.calcite.QueryChecker;
+import org.apache.ignite.internal.util.tostring.GridToStringInclude;
+import org.apache.ignite.internal.util.typedef.internal.S;
+import org.jetbrains.annotations.Nullable;
+import org.junit.Ignore;
+import org.junit.Test;
+
+/**
+ * Checks that using {@link QueryUtils#KEY_FIELD_NAME} in condition will use
+ * {@link QueryUtils#PRIMARY_KEY_INDEX pk index}.
+ */
+public class SelectByKeyFieldTest extends AbstractBasicIntegrationTest {
+    /** {@inheritDoc} */
+    @Override protected int nodeCount() {
+        return 1;
+    }
+
+    /** {@inheritDoc} */
+    @Override protected IgniteConfiguration getConfiguration(String 
igniteInstanceName) throws Exception {
+        SqlConfiguration sqlCfg = new 
SqlConfiguration().setQueryEnginesConfiguration(
+            new CalciteQueryEngineConfiguration().setDefault(true),
+            new IndexingQueryEngineConfiguration()
+        );
+
+        return super.getConfiguration(igniteInstanceName)
+            .setSqlConfiguration(sqlCfg);
+    }
+
+    /** */
+    @Test
+    public void testSimplePk() {
+        checkSimplePk(null);
+    }
+
+    /** */
+    @Test
+    public void testSimplePkAfterAddColumn() {
+        checkSimplePk(this::executeAlterTableAddColumn);
+    }
+
+    /** */
+    @Test
+    public void testSimplePkAfterDropColumn() {
+        checkSimplePk(this::executeAlterTableDropColumn);
+    }
+
+    /** */
+    @Test
+    public void testCompositePk() {
+        checkCompositePk(false, true, null);
+    }
+
+    /** */
+    @Test
+    public void testCompositePkAfterAddColumn() {
+        checkCompositePk(false, true, this::executeAlterTableAddColumn);
+    }
+
+    /** */
+    @Test
+    public void testCompositePkAfterDropColumn() {
+        checkCompositePk(false, true, this::executeAlterTableDropColumn);
+    }
+
+    /** */
+    @Test
+    public void testCompositePkWithKeyTypeAndBinaryObject() {
+        checkCompositePk(true, true, null);
+    }
+
+    /** */
+    @Test
+    @Ignore("https://issues.apache.org/jira/browse/IGNITE-28374";)
+    public void testCompositePkWithKeyTypeAndPersonCompositeKey() {
+        checkCompositePk(true, false, null);
+    }
+
+    /** */
+    private void checkSimplePk(@Nullable Runnable executeBeforeChecks) {
+        sql("create table PUBLIC.PERSON(id int primary key, name varchar, 
surname varchar, age int)");
+
+        for (int i = 0; i < 10; i++) {
+            sql(
+                "insert into PUBLIC.PERSON(id, name, surname, age) values (?, 
?, ?, ?)",
+                i, "foo" + i, "bar" + i, 18 + i
+            );
+        }
+
+        List<List<?>> sqlRs = sql("select _key, id from PUBLIC.PERSON order by 
id");
+        int _key = (Integer)sqlRs.get(7).get(0);
+        int id = (Integer)sqlRs.get(7).get(1);
+
+        assertEquals(7, _key);
+        assertEquals(7, id);
+
+        if (executeBeforeChecks != null)
+            executeBeforeChecks.run();
+
+        assertQuery("select id, name, age, _key from PUBLIC.PERSON where _key 
= ?")
+            .withParams(_key)
+            .matches(QueryChecker.containsIndexScan("PUBLIC", "PERSON", 
QueryUtils.PRIMARY_KEY_INDEX))
+            .columnNames("ID", "NAME", "AGE", QueryUtils.KEY_FIELD_NAME)
+            .returns(id, "foo7", 25, _key)
+            .check();
+
+        // Let's check with a smaller number of columns.
+        assertQuery("select id, age, _key from PUBLIC.PERSON where _key = ?")
+            .withParams(_key)
+            .matches(QueryChecker.containsIndexScan("PUBLIC", "PERSON", 
QueryUtils.PRIMARY_KEY_INDEX))
+            .columnNames("ID", "AGE", QueryUtils.KEY_FIELD_NAME)
+            .returns(id, 25, _key)
+            .check();
+
+        // Let's just make sure that PK search is not broken.
+        assertQuery("select id, name, age, _key from PUBLIC.PERSON where id = 
?")
+            .withParams(id)
+            .matches(QueryChecker.containsIndexScan("PUBLIC", "PERSON", 
QueryUtils.PRIMARY_KEY_INDEX))
+            .columnNames("ID", "NAME", "AGE", QueryUtils.KEY_FIELD_NAME)
+            .returns(id, "foo7", 25, _key)
+            .check();
+    }
+
+    /** */
+    private void checkCompositePk(
+        boolean setKeyTypeToCreateTblDdl, boolean useBinaryObject, @Nullable 
Runnable executeBeforeChecks
+    ) {
+        if (setKeyTypeToCreateTblDdl) {
+            // Order of the primary key columns has been deliberately changed.
+            sql(String.format(
+                "create table PUBLIC.PERSON(id int, name varchar, surname 
varchar, age int, primary key(name, id)) " +
+                    "with \"key_type=%s\"",
+                PersonCompositeKey.class.getName()
+            ));
+        }
+        else
+            sql("create table PUBLIC.PERSON(id int, name varchar, surname 
varchar, age int, primary key(id, name))");
+
+        for (int i = 0; i < 10; i++) {
+            sql(
+                "insert into PUBLIC.PERSON(id, name, surname, age) values (?, 
?, ?, ?)",
+                i, "foo" + i, "bar" + i, 18 + i
+            );
+        }
+
+        List<List<?>> sqlRs = sql("select _key, id, name from PUBLIC.PERSON 
order by id");
+        BinaryObject _key = (BinaryObject)sqlRs.get(6).get(0);
+        int id = (Integer)sqlRs.get(6).get(1);
+        String name = (String)sqlRs.get(6).get(2);
+
+        assertEquals(6, id);
+        assertEquals("foo6", name);
+
+        if (executeBeforeChecks != null)
+            executeBeforeChecks.run();
+
+        assertQuery("select id, name, age, _key from PUBLIC.PERSON where _key 
= ?")
+            .withParams(useBinaryObject ? _key : _key.deserialize())
+            .matches(QueryChecker.containsIndexScan("PUBLIC", "PERSON", 
QueryUtils.PRIMARY_KEY_INDEX + "_proxy"))
+            .columnNames("ID", "NAME", "AGE", QueryUtils.KEY_FIELD_NAME)
+            .returns(id, name, 24, _key)
+            .check();
+
+        // Let's check with a smaller number of columns.
+        assertQuery("select id, age, _key from PUBLIC.PERSON where _key = ?")
+            .withParams(useBinaryObject ? _key : _key.deserialize())
+            .matches(QueryChecker.containsIndexScan("PUBLIC", "PERSON", 
QueryUtils.PRIMARY_KEY_INDEX + "_proxy"))
+            .columnNames("ID", "AGE", QueryUtils.KEY_FIELD_NAME)
+            .returns(id, 24, _key)
+            .check();
+
+        // Let's just make sure that PK search is not broken.
+        assertQuery("select id, name, age, _key from PUBLIC.PERSON where id = 
? and name = ?")
+            .withParams(id, name)
+            .matches(QueryChecker.containsIndexScan("PUBLIC", "PERSON", 
QueryUtils.PRIMARY_KEY_INDEX))
+            .columnNames("ID", "NAME", "AGE", QueryUtils.KEY_FIELD_NAME)
+            .returns(id, name, 24, _key)
+            .check();
+    }

Review Comment:
   Maybe also add tests that proxy index is not used for: min(_key), _key >= ?, 
order by _key since it doesn't provide correct ordering by _key



##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/SelectByKeyFieldTest.java:
##########
@@ -0,0 +1,250 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.processors.query.calcite.integration;
+
+import java.util.List;
+import java.util.Objects;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.calcite.CalciteQueryEngineConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.configuration.SqlConfiguration;
+import org.apache.ignite.indexing.IndexingQueryEngineConfiguration;
+import org.apache.ignite.internal.processors.query.QueryUtils;
+import org.apache.ignite.internal.processors.query.calcite.QueryChecker;
+import org.apache.ignite.internal.util.tostring.GridToStringInclude;
+import org.apache.ignite.internal.util.typedef.internal.S;
+import org.jetbrains.annotations.Nullable;
+import org.junit.Ignore;
+import org.junit.Test;
+
+/**
+ * Checks that using {@link QueryUtils#KEY_FIELD_NAME} in condition will use
+ * {@link QueryUtils#PRIMARY_KEY_INDEX pk index}.
+ */
+public class SelectByKeyFieldTest extends AbstractBasicIntegrationTest {
+    /** {@inheritDoc} */
+    @Override protected int nodeCount() {
+        return 1;
+    }
+
+    /** {@inheritDoc} */
+    @Override protected IgniteConfiguration getConfiguration(String 
igniteInstanceName) throws Exception {
+        SqlConfiguration sqlCfg = new 
SqlConfiguration().setQueryEnginesConfiguration(
+            new CalciteQueryEngineConfiguration().setDefault(true),
+            new IndexingQueryEngineConfiguration()
+        );
+
+        return super.getConfiguration(igniteInstanceName)
+            .setSqlConfiguration(sqlCfg);
+    }
+
+    /** */
+    @Test
+    public void testSimplePk() {
+        checkSimplePk(null);
+    }
+
+    /** */
+    @Test
+    public void testSimplePkAfterAddColumn() {
+        checkSimplePk(this::executeAlterTableAddColumn);
+    }
+
+    /** */
+    @Test
+    public void testSimplePkAfterDropColumn() {
+        checkSimplePk(this::executeAlterTableDropColumn);
+    }
+
+    /** */
+    @Test
+    public void testCompositePk() {
+        checkCompositePk(false, true, null);
+    }
+
+    /** */
+    @Test
+    public void testCompositePkAfterAddColumn() {
+        checkCompositePk(false, true, this::executeAlterTableAddColumn);
+    }
+
+    /** */
+    @Test
+    public void testCompositePkAfterDropColumn() {
+        checkCompositePk(false, true, this::executeAlterTableDropColumn);
+    }
+
+    /** */
+    @Test
+    public void testCompositePkWithKeyTypeAndBinaryObject() {
+        checkCompositePk(true, true, null);
+    }
+
+    /** */
+    @Test
+    @Ignore("https://issues.apache.org/jira/browse/IGNITE-28374";)
+    public void testCompositePkWithKeyTypeAndPersonCompositeKey() {
+        checkCompositePk(true, false, null);
+    }
+
+    /** */
+    private void checkSimplePk(@Nullable Runnable executeBeforeChecks) {
+        sql("create table PUBLIC.PERSON(id int primary key, name varchar, 
surname varchar, age int)");
+
+        for (int i = 0; i < 10; i++) {
+            sql(
+                "insert into PUBLIC.PERSON(id, name, surname, age) values (?, 
?, ?, ?)",
+                i, "foo" + i, "bar" + i, 18 + i
+            );
+        }
+
+        List<List<?>> sqlRs = sql("select _key, id from PUBLIC.PERSON order by 
id");
+        int _key = (Integer)sqlRs.get(7).get(0);
+        int id = (Integer)sqlRs.get(7).get(1);
+
+        assertEquals(7, _key);
+        assertEquals(7, id);
+
+        if (executeBeforeChecks != null)
+            executeBeforeChecks.run();
+
+        assertQuery("select id, name, age, _key from PUBLIC.PERSON where _key 
= ?")
+            .withParams(_key)
+            .matches(QueryChecker.containsIndexScan("PUBLIC", "PERSON", 
QueryUtils.PRIMARY_KEY_INDEX))
+            .columnNames("ID", "NAME", "AGE", QueryUtils.KEY_FIELD_NAME)
+            .returns(id, "foo7", 25, _key)
+            .check();
+
+        // Let's check with a smaller number of columns.
+        assertQuery("select id, age, _key from PUBLIC.PERSON where _key = ?")
+            .withParams(_key)
+            .matches(QueryChecker.containsIndexScan("PUBLIC", "PERSON", 
QueryUtils.PRIMARY_KEY_INDEX))
+            .columnNames("ID", "AGE", QueryUtils.KEY_FIELD_NAME)
+            .returns(id, 25, _key)
+            .check();
+
+        // Let's just make sure that PK search is not broken.
+        assertQuery("select id, name, age, _key from PUBLIC.PERSON where id = 
?")
+            .withParams(id)
+            .matches(QueryChecker.containsIndexScan("PUBLIC", "PERSON", 
QueryUtils.PRIMARY_KEY_INDEX))
+            .columnNames("ID", "NAME", "AGE", QueryUtils.KEY_FIELD_NAME)
+            .returns(id, "foo7", 25, _key)
+            .check();
+    }
+
+    /** */
+    private void checkCompositePk(
+        boolean setKeyTypeToCreateTblDdl, boolean useBinaryObject, @Nullable 
Runnable executeBeforeChecks

Review Comment:
   Codestyle: For multiline method declaration each parameter should be on it's 
own line



##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/IndexWrappedKeyScan.java:
##########
@@ -0,0 +1,101 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.processors.query.calcite.exec;
+
+import java.util.Map;
+import org.apache.calcite.util.ImmutableBitSet;
+import org.apache.calcite.util.ImmutableIntList;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.internal.cache.query.index.sorted.IndexKeyDefinition;
+import org.apache.ignite.internal.cache.query.index.sorted.IndexKeyType;
+import org.apache.ignite.internal.cache.query.index.sorted.IndexPlainRowImpl;
+import org.apache.ignite.internal.cache.query.index.sorted.IndexRow;
+import 
org.apache.ignite.internal.cache.query.index.sorted.InlineIndexRowHandler;
+import org.apache.ignite.internal.cache.query.index.sorted.inline.InlineIndex;
+import org.apache.ignite.internal.cache.query.index.sorted.keys.IndexKey;
+import 
org.apache.ignite.internal.cache.query.index.sorted.keys.IndexKeyFactory;
+import org.apache.ignite.internal.processors.query.QueryUtils;
+import 
org.apache.ignite.internal.processors.query.calcite.exec.exp.RangeIterable;
+import 
org.apache.ignite.internal.processors.query.calcite.schema.CacheTableDescriptor;
+import 
org.apache.ignite.internal.processors.query.calcite.schema.ColumnDescriptor;
+import org.apache.ignite.internal.processors.query.calcite.util.TypeUtils;
+import org.jetbrains.annotations.Nullable;
+
+/** Extension for column {@value QueryUtils#KEY_FIELD_NAME} in case of 
composite primary key. */
+public class IndexWrappedKeyScan<Row> extends IndexScan<Row> {
+    /** */
+    public IndexWrappedKeyScan(
+        ExecutionContext<Row> ectx,
+        CacheTableDescriptor desc,
+        InlineIndex idx,
+        ImmutableIntList idxFieldMapping,
+        int[] parts,
+        RangeIterable<Row> ranges,
+        @Nullable ImmutableBitSet requiredColumns
+    ) {
+        super(ectx, desc, idx, idxFieldMapping, parts, ranges, 
requiredColumns);
+    }
+
+    /** */
+    @Override protected IndexRow row2indexRow(Row bound) {
+        if (bound == null)
+            return null;
+
+        RowHandler<Row> rowHnd = ectx.rowHandler();
+
+        Object key = rowHnd.get(QueryUtils.KEY_COL, bound);
+        assert key != null : String.format("idxName=%s, bound=%s", idx.name(), 
rowHnd.toString(bound));
+
+        if (key instanceof BinaryObject)
+            return binaryObject2indexRow((BinaryObject)key);
+
+        // TODO: IGNITE-28374 Implement for POJO key class
+        return super.row2indexRow(bound);

Review Comment:
   Maybe throw an exception instead of returning wrong result?



-- 
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]

Reply via email to