[GitHub] [ignite] javaronok commented on a change in pull request #9139: IGNITE-13723 Fixed ComplexSecondaryKeyUnwrapSelfTest test

[GitBox <gi...@apache.org>]

javaronok commented on a change in pull request #9139:
URL: https://github.com/apache/ignite/pull/9139#discussion_r645472470



##########
File path: modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/index/ComplexSecondaryKeyUnwrapSelfTest.java
##########
@@ -55,15 +52,15 @@ public void testSecondaryIndexWithIntersectColumnsComplexPk() {
 
         executeSql("CREATE INDEX ON " + tblName + "(id, name, city)");
 
-        checkUsingIndexes(tblName, "'1'");
+        checkUsingIndexes(tblName, "'1'", 2, "Query should be splitted");
     }
 
     /**
      * Test using secondary index with simple PK.
      */
     @Test
     public void testSecondaryIndexSimplePk() {
-        HashMap<String, String> types = new HashMap() {
+        Map<String, String> types = new LinkedHashMap<String, String>() {

Review comment:
       The order of types from the declaration is cosily for debugging. Just for this. But I can revert it back.

##########
File path: modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/index/ComplexSecondaryKeyUnwrapSelfTest.java
##########
@@ -97,7 +94,7 @@ public void testSecondaryIndexSimplePk() {
 
             executeSql("CREATE INDEX ON " + tblName + "(id, name, city)");
 
-            checkUsingIndexes(tblName, val);
+            checkUsingIndexes(tblName, val, 1, "Query with type column: " + type);

Review comment:
       The parameter expResCnt in method should determine number items in the plan. 
   I'll separate method for the two-phase plan only. 

##########
File path: modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/index/ComplexSecondaryKeyUnwrapSelfTest.java
##########
@@ -106,30 +103,34 @@ public void testSecondaryIndexSimplePk() {
      *
      * @param tblName name of table which should be checked to using secondary indexes.
      * @param nameVal Value for name param.
+     * @param expResCnt Expceted result count.
+     * @param assertLabel Assert label.
      */
-    private void checkUsingIndexes(String tblName, String nameVal) {
+    private void checkUsingIndexes(String tblName, String nameVal, int expResCnt, String assertLabel) {

Review comment:
       Thanks for the link, I'll fix it

##########
File path: modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/index/ComplexSecondaryKeyUnwrapSelfTest.java
##########
@@ -106,30 +103,34 @@ public void testSecondaryIndexSimplePk() {
      *
      * @param tblName name of table which should be checked to using secondary indexes.
      * @param nameVal Value for name param.
+     * @param expResCnt Expceted result count.
+     * @param assertLabel Assert label.
      */
-    private void checkUsingIndexes(String tblName, String nameVal) {
+    private void checkUsingIndexes(String tblName, String nameVal, int expResCnt, String assertLabel) {
         String explainSQL = "explain SELECT * FROM " + tblName + " WHERE ";
 
         List<List<?>> results = executeSql(explainSQL + "id=1");
 
-        assertUsingSecondaryIndex(results);
+        assertUsingSecondaryIndex(results, 2, assertLabel); // always merge_scan for non key (or affinity) fields condition

Review comment:
       Thanks for the link, I'll fix 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org