Backport [IGNITE 10645] SQL properties ownership flag should be set at configuration parsing, not query execution to 2.5-master2

modules/core/src/main/java/org/apache/ignite/internal/processors/query/QueryUtils.java

@@ -548,10 +548,6 @@ public static void processBinaryMeta(GridKernalContext ctx, QueryEntity qryEntit
         Set<String> notNulls = qryEntity.getNotNullFields();
         Map<String, Object> dlftVals = qryEntity.getDefaultFieldValues();
 
-        // We have to distinguish between empty and null keyFields when the key is not of SQL type -
-        // when a key is not of SQL type, absence of a field in nonnull keyFields tell us that this field
-        // is a value field, and null keyFields tells us that current configuration
-        // does not tell us anything about this field's ownership.
         boolean hasKeyFields = (keyFields != null);
 
         boolean isKeyClsSqlType = isSqlType(d.keyClass());
@@ -565,20 +561,28 @@ public static void processBinaryMeta(GridKernalContext ctx, QueryEntity qryEntit
             }
         }
 
+        // We are creating binary properties for all the fields, even if field is of sql type (keyFieldName or
+        // valueFieldName). In that case we rely on the fact, that binary property's methods value() and
+        // setValue() will never get called, because there is no value to extract, key/val object itself is a
+        // value.
         for (Map.Entry<String, String> entry : qryEntity.getFields().entrySet()) {
-            Boolean isKeyField;
+            String fieldName = entry.getKey();
+            String fieldType = entry.getValue();
 
-            if (isKeyClsSqlType) // We don't care about keyFields in this case - it might be null, or empty, or anything
+            boolean isKeyField;
+
+            if (isKeyClsSqlType)
+                // Entire key is not field of itself, even if it is set in "keyFields".
                 isKeyField = false;
             else
-                isKeyField = (hasKeyFields ? keyFields.contains(entry.getKey()) : null);
+                isKeyField = hasKeyFields && keyFields.contains(fieldName);
 
-            boolean notNull = notNulls != null && notNulls.contains(entry.getKey());
+            boolean notNull = notNulls != null && notNulls.contains(fieldName);
 
-            Object dfltVal = dlftVals != null ? dlftVals.get(entry.getKey()) : null;
+            Object dfltVal = dlftVals != null ? dlftVals.get(fieldName) : null;
 
             QueryBinaryProperty prop = buildBinaryProperty(ctx, entry.getKey(),
-                U.classForName(entry.getValue(), Object.class, true),
+                U.classForName(fieldType, Object.class, true),
                 d.aliases(), isKeyField, notNull, dfltVal);
 
             d.addProperty(prop, false);
@@ -727,7 +731,7 @@ else if (idxTyp != null)
      * @throws IgniteCheckedException On error.
      */
     public static QueryBinaryProperty buildBinaryProperty(GridKernalContext ctx, String pathStr, Class<?> resType,
-        Map<String, String> aliases, @Nullable Boolean isKeyField, boolean notNull, Object dlftVal) throws IgniteCheckedException {
+        Map<String, String> aliases, boolean isKeyField, boolean notNull, Object dlftVal) throws IgniteCheckedException {
         String[] path = pathStr.split("\\.");
 
         QueryBinaryProperty res = null;

modules/core/src/main/java/org/apache/ignite/internal/processors/query/property/QueryBinaryProperty.java

@@ -53,18 +53,15 @@ public class QueryBinaryProperty implements GridQueryProperty {
     /** Result class. */
     private Class<?> type;
 
-    /** */
-    private volatile int isKeyProp;
+    /** Defines where value should be extracted from : cache entry's key or value. */
+    private final boolean isKeyProp;
 
     /** Binary field to speed-up deserialization. */
     private volatile BinaryField field;
 
     /** Flag indicating that we already tried to take a field. */
     private volatile boolean fieldTaken;
 
-    /** Whether user was warned about missing property. */
-    private volatile boolean warned;
-
     /** */
     private final boolean notNull;
 
@@ -78,13 +75,13 @@ public class QueryBinaryProperty implements GridQueryProperty {
      * @param propName Property name.
      * @param parent Parent property.
      * @param type Result type.
-     * @param key {@code true} if key property, {@code false} otherwise, {@code null}  if unknown.
+     * @param key {@code true} if key property, {@code false} otherwise.
      * @param alias Field alias.
      * @param notNull {@code true} if null value is not allowed.
      * @param defaultValue Default value.
      */
     public QueryBinaryProperty(GridKernalContext ctx, String propName, QueryBinaryProperty parent,
-        Class<?> type, @Nullable Boolean key, String alias, boolean notNull, Object defaultValue) {
+        Class<?> type, boolean key, String alias, boolean notNull, Object defaultValue) {
         this.ctx = ctx;
 
         log = ctx.log(QueryBinaryProperty.class);
@@ -94,10 +91,7 @@ public QueryBinaryProperty(GridKernalContext ctx, String propName, QueryBinaryPr
         this.parent = parent;
         this.type = type;
         this.notNull = notNull;
-
-        if (key != null)
-            this.isKeyProp = key ? 1 : -1;
-
+        this.isKeyProp = key;
         this.defaultValue = defaultValue;
     }
 
@@ -115,33 +109,12 @@ public QueryBinaryProperty(GridKernalContext ctx, String propName, QueryBinaryPr
                 throw new IgniteCheckedException("Non-binary object received as a result of property extraction " +
                     "[parent=" + parent + ", propName=" + propName + ", obj=" + obj + ']');
         }
-        else {
-            int isKeyProp0 = isKeyProp;
-
-            if (isKeyProp0 == 0) {
-                // Key is allowed to be a non-binary object here.
-                // We check key before value consistently with ClassProperty.
-                if (key instanceof BinaryObject && ((BinaryObject)key).hasField(propName))
-                    isKeyProp = isKeyProp0 = 1;
-                else if (val instanceof BinaryObject && ((BinaryObject)val).hasField(propName))
-                    isKeyProp = isKeyProp0 = -1;
-                else {
-                    if (!warned) {
-                        U.warn(log, "Neither key nor value have property \"" + propName + "\" " +
-                            "(is cache indexing configured correctly?)");
-
-                        warned = true;
-                    }
-
-                    return null;
-                }
-            }
-
-            obj = isKeyProp0 == 1 ? key : val;
-        }
+        else
+            obj = isKeyProp ? key : val;
 
         if (obj instanceof BinaryObject) {
             BinaryObject obj0 = (BinaryObject) obj;
+
             return fieldValue(obj0);
         }
         else if (obj instanceof BinaryObjectBuilder) {
@@ -263,13 +236,7 @@ private Object fieldValue(BinaryObject obj) {
 
     /** {@inheritDoc} */
     @Override public boolean key() {
-        int isKeyProp0 = isKeyProp;
-
-        if (isKeyProp0 == 0)
-            throw new IllegalStateException("Ownership flag not set for binary property. Have you set 'keyFields'" +
-                " property of QueryEntity in programmatic or XML configuration?");
-
-        return isKeyProp0 == 1;
+        return isKeyProp;
     }
 
     /** {@inheritDoc} */

modules/core/src/test/java/org/apache/ignite/client/FunctionalTest.java

@@ -162,7 +162,10 @@ public void testCacheConfiguration() throws Exception {
                             SimpleEntry::getKey, SimpleEntry::getValue, (a, b) -> a, LinkedHashMap::new
                         ))
                     )
-                    .setKeyFields(Collections.singleton("id"))
+                    // During query normalization null keyFields become empty set.
+                    // Set empty collection for comparator.
+                    .setKeyFields(Collections.emptySet())
+                    .setKeyFieldName("id")
                     .setNotNullFields(Collections.singleton("id"))
                     .setDefaultFieldValues(Collections.singletonMap("id", 0))
                     .setIndexes(Collections.singletonList(new QueryIndex("id", true, "IDX_EMPLOYEE_ID")))

modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/CacheBinaryKeyConcurrentQueryTest.java

@@ -17,6 +17,7 @@
 
 package org.apache.ignite.internal.processors.cache;
 
+import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 import java.util.concurrent.Callable;
@@ -240,6 +241,8 @@ private CacheConfiguration cacheConfiguration(String name, CacheAtomicityMode at
         qryEntity.addQueryField("id", Integer.class.getName(), null);
         qryEntity.addQueryField("val", Integer.class.getName(), null);
 
+        qryEntity.setKeyFields(Collections.singleton("id"));
+
         qryEntity.setIndexes(F.asList(new QueryIndex("id"), new QueryIndex("val")));
 
         ccfg.setQueryEntities(F.asList(qryEntity));

modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/IgniteCacheDistributedJoinCollocatedAndNotTest.java

@@ -19,6 +19,8 @@
 
 import java.io.Serializable;
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashSet;
 import java.util.List;
 import java.util.concurrent.atomic.AtomicInteger;
 import org.apache.ignite.Ignite;
@@ -87,6 +89,8 @@ public class IgniteCacheDistributedJoinCollocatedAndNotTest extends AbstractInde
             entity.addQueryField("affKey", Integer.class.getName(), null);
             entity.addQueryField("name", String.class.getName(), null);
 
+            entity.setKeyFields(new HashSet<>(Arrays.asList("id", "affKey")));
+
             ccfg.setQueryEntities(F.asList(entity));
 
             ccfgs.add(ccfg);

modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/IgniteCacheJoinQueryWithAffinityKeyTest.java

@@ -18,6 +18,7 @@
 package org.apache.ignite.internal.processors.cache;
 
 import java.io.Serializable;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -365,6 +366,7 @@ private CacheConfiguration cacheConfiguration(CacheMode cacheMode,
         QueryEntity person = new QueryEntity();
         person.setKeyType(personKeyType);
         person.setValueType(Person.class.getName());
+        person.setKeyFields(Collections.singleton("id"));
         person.addQueryField("orgId", Integer.class.getName(), null);
         person.addQueryField("id", Integer.class.getName(), null);
         person.addQueryField("name", String.class.getName(), null);