zstan commented on code in PR #12452:
URL: https://github.com/apache/ignite/pull/12452#discussion_r3136925365


##########
modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/index/ComplexPrimaryKeyUnwrapSelfTest.java:
##########
@@ -210,17 +390,28 @@ public void testInlineSizeWrap2() {
      * @param expResCnt Expceted result count.
      */
     private void checkUsingIndexes(String tblName, String idVal, int 
expResCnt) {
+        checkUsingIndexes(node(), tblName, idVal, expResCnt);
+    }
+
+    /**
+     * Check using PK indexes for few cases.
+     *
+     * @param node Ignite node.
+     * @param tblName Name of table which should be checked to using PK 
indexes.
+     * @param expResCnt Expceted result count.

Review Comment:
   ```suggestion
        * @param expResCnt Expected result count.
   ```



##########
modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/index/ComplexPrimaryKeyUnwrapSelfTest.java:
##########
@@ -210,17 +390,28 @@ public void testInlineSizeWrap2() {
      * @param expResCnt Expceted result count.

Review Comment:
   ```suggestion
        * @param expResCnt Expected result count.
   ```



##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/ddl/DdlCommandHandler.java:
##########


Review Comment:
   do we need a new QueryEntityEx here ?



##########
modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/index/ComplexPrimaryKeyUnwrapSelfTest.java:
##########


Review Comment:
   ```suggestion
           assertTrue("Current plan=[" + explainPlan + ']', 
explainPlan.contains("\"_key_PK"));
   ```



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/query/schema/management/TableDescriptor.java:
##########
@@ -57,7 +57,7 @@ public class TableDescriptor {
     public TableDescriptor(GridCacheContextInfo<?, ?> cacheInfo, 
GridQueryTypeDescriptor typeDesc, boolean isSql) {
         this.cacheInfo = cacheInfo;
         this.typeDesc = typeDesc;
-        this.isSql = isSql;
+        this.isSql = isSql || typeDesc.sql();

Review Comment:
   probably it will be better to move sql flag into QueryTypeCandidate and 
reduce such spaghetti code ?



##########
modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/index/ComplexPrimaryKeyUnwrapSelfTest.java:
##########
@@ -55,19 +55,104 @@ public class ComplexPrimaryKeyUnwrapSelfTest extends 
AbstractIndexingCommonTest
      */
     @Test
     public void testComplexPk() {
+        checkComplexPk(false);
+    }
+
+    /**
+     * Test using PK indexes for complex primary key for DDL on existing cache.
+     */
+    @Test
+    public void testComplexPkExistingCache() {
+        checkComplexPk(true);
+    }
+
+    /** */
+    private void checkComplexPk(boolean existingCache) {
         String tblName = createTableName();
 
+        if (existingCache)
+            node().createCache(tblName);
+
         executeSql("CREATE TABLE " + tblName + " (id int, name varchar, age 
int, company varchar, city varchar, " +
-            "primary key (id, name, city))");
+            "primary key (id, name, city))" + (existingCache ? " WITH 
\"CACHE_NAME=" + tblName + "\"" : ""));
 
         checkUsingIndexes(tblName, "1", 2);
     }
 
+    /**
+     * Test using PK indexes for complex primary key and affinity key.
+     */
+    @Test
+    public void testComplexPkWithAffinityKey() {
+        checkComplexPkWithAffinityKey(false);
+    }
+
+    /**
+     * Test using PK indexes for complex primary key and affinity key for DDL 
on existing cache.
+     */
+    @Test
+    public void testComplexPkWithAffinityKeyExistingCache() {
+        checkComplexPkWithAffinityKey(true);
+    }
+
+    /** */
+    private void checkComplexPkWithAffinityKey(boolean existingCache) {
+        String tblName = createTableName();
+
+        if (existingCache)
+            node().createCache(tblName);
+
+        executeSql("CREATE TABLE " + tblName + " (id int, name varchar, age 
int, company varchar, city varchar, " +
+            "primary key (id, name, city)) " +
+            "WITH \"affinity_key=id" +
+            (existingCache ? ",CACHE_NAME=" + tblName : "") + "\"");
+
+        // For the new cache unwrapped key field is used as affinity key, but 
for the existing cache
+        // affinity key can't be changed, so we can't prune partitions for 
query on ID field.
+        checkUsingIndexes(tblName, "1", existingCache ? 2 : 1);
+    }
+
+    /**
+     * Test using PK indexes for complex primary key for DDL on existing cache 
on multiple nodes.
+     */
+    @Test
+    public void testComplexPkExistingCacheMultiNode() throws Exception {
+        String tblName = createTableName();
+
+        try (IgniteEx ignite1 = startGrid(1)) {
+            node().createCache(tblName);
+
+            executeSql("CREATE TABLE " + tblName + " (id int, name varchar, 
age int, company varchar, city varchar, " +
+                "primary key (id, name, city)) WITH \"CACHE_NAME=" + tblName + 
"\"");
+
+            // Check on remote node, started before table created.
+            checkUsingIndexes(ignite1, tblName, "1", 2);
+
+            try (IgniteEx ignite2 = startGrid(2)) {
+                // Check on remote node, started after table created.
+                checkUsingIndexes(ignite2, tblName, "1", 2);
+            }
+        }
+    }
+
     /**
      * Test using PK indexes for simple primary key.
      */
     @Test
     public void testSimplePk() {
+        checkSimplePk(false);
+    }
+
+    /**
+     * Test using PK indexes for simple primary key for DDL on existing cache.
+     */
+    @Test
+    public void testSimplePkExistingCache() {

Review Comment:
   Rare but flack locally and also different test from this class 
   
   <img width="1086" height="775" alt="Image" 
src="https://github.com/user-attachments/assets/2cef23ee-b5de-4160-9790-8e1bc9643a5f";
 />



##########
modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/index/ComplexPrimaryKeyUnwrapSelfTest.java:
##########


Review Comment:
   ```suggestion
        * @param expResCnt Expected result count.
   ```



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