Use cached deserialized values in SQL when possible [IMDG-125]

hazelcast-sql-core/src/test/java/com/hazelcast/sql/misc/SqlNoSerializationTest.java

@@ -0,0 +1,206 @@
+/*
+ * Copyright (c) 2008-2021, Hazelcast, Inc. All Rights Reserved.
+ *
+ * Licensed 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 com.hazelcast.sql.misc;
+
+import com.hazelcast.config.InMemoryFormat;
+import com.hazelcast.config.IndexConfig;
+import com.hazelcast.config.IndexType;
+import com.hazelcast.core.HazelcastInstance;
+import com.hazelcast.map.IMap;
+import com.hazelcast.sql.SqlResult;
+import com.hazelcast.sql.SqlRow;
+import com.hazelcast.sql.SqlTestInstanceFactory;
+import com.hazelcast.sql.impl.SqlTestSupport;
+import com.hazelcast.test.HazelcastSerialParametersRunnerFactory;
+import com.hazelcast.test.annotation.ParallelJVMTest;
+import com.hazelcast.test.annotation.QuickTest;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import java.io.Externalizable;
+import java.io.IOException;
+import java.io.ObjectInput;
+import java.io.ObjectOutput;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Test that ensures that there are no unexpected serializations when executing a query on an IMap with the
+ * default {@link InMemoryFormat#BINARY} format.
+ */
+@RunWith(Parameterized.class)
+@Parameterized.UseParametersRunnerFactory(HazelcastSerialParametersRunnerFactory.class)
+@Category({QuickTest.class, ParallelJVMTest.class})
+public class SqlNoSerializationTest extends SqlTestSupport {
+
+    private static final String MAP_NAME = "map";
+    private static final int KEY_COUNT = 100;
+
+    @Parameterized.Parameter
+    public boolean useIndex;
+
+    private final SqlTestInstanceFactory factory = SqlTestInstanceFactory.create();
+    private HazelcastInstance member;
+
+    private static volatile boolean failOnSerialization;
+
+    @Parameterized.Parameters(name = "useIndex:{0}")
+    public static Collection<Object> parameters() {
+        return Arrays.asList(true, false);
+    }
+
+    @Before
+    public void before() {
+        member = factory.newHazelcastInstance();
+        factory.newHazelcastInstance();
+
+        Map<Key, Value> localMap = new HashMap<>();
+
+        for (int i = 0; i < KEY_COUNT; i++) {
+            localMap.put(new Key(i), new Value(i));
+        }
+
+        IMap<Key, Value> map = member.getMap(MAP_NAME);
+
+        map.putAll(localMap);
+
+        // An index may change the behavior due to MapContainer.isUseCachedDeserializedValuesEnabled.
+        if (useIndex) {
+            map.addIndex(new IndexConfig().setType(IndexType.SORTED).addAttribute("val"));
+        }
+
+        failOnSerialization = true;
+    }
+
+    @After
+    public void after() {
+        failOnSerialization = false;
+
+        factory.shutdownAll();
+    }
+
+    @Test
+    public void testMapScan() {
+        check("SELECT __key, this FROM " + MAP_NAME, 100);
+    }
+
+    @Test
+    public void testIndexScan() {
+        check("SELECT __key, this FROM " + MAP_NAME + " WHERE val = 1", 1);
+    }
+
+    private void check(String sql, int expectedCount) {
+        try (SqlResult res = member.getSql().execute(sql)) {
+            int count = 0;
+
+            for (SqlRow row : res) {
+                Object key = row.getObject(0);
+                Object value = row.getObject(1);
+
+                assertTrue(key instanceof Key);
+                assertTrue(value instanceof Value);
+
+                count++;
+            }
+
+            assertEquals(expectedCount, count);
+        }
+    }
+
+    public static class Key implements Externalizable {
+
+        public int key;
+
+        public Key() {
+            // No-op
+        }
+
+        private Key(int key) {
+            this.key = key;
+        }
+
+        @Override
+        public void writeExternal(ObjectOutput out) throws IOException {
+            out.writeInt(key);
+
+            if (failOnSerialization) {
+                throw new IOException("Key serialization must not happen.");
+            }
+        }
+
+        @Override
+        public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException {
+            key = in.readInt();
+        }
+
+        @Override
+        public boolean equals(Object o) {
+            if (this == o) {
+                return true;
+            }
+
+            if (o == null || getClass() != o.getClass()) {
+                return false;
+            }
+
+            Key that = (Key) o;
+
+            return key == that.key;
+        }
+
+        @Override
+        public int hashCode() {
+            return key;
+        }
+    }
+
+    public static class Value implements Externalizable {
+
+        public int val;
+
+        public Value() {
+            // No-op
+        }
+
+        private Value(int val) {
+            this.val = val;
+        }
+
+        @Override
+        public void writeExternal(ObjectOutput out) throws IOException {
+            out.writeInt(val);
+
+            if (failOnSerialization) {
+                throw new IOException("Value serialization must not happen.");
+            }
+        }
+
+        @Override
+        public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException {
+            val = in.readInt();
+        }
+    }
+}

hazelcast/src/main/java/com/hazelcast/map/impl/MapContainer.java

@@ -469,4 +469,17 @@ public Data apply(Object input) {
             return ss.toData(input, partitioningStrategy);
         }
     }
+
+    public boolean isUseCachedDeserializedValuesEnabled(int partitionId) {
+        CacheDeserializedValues cacheDeserializedValues = getMapConfig().getCacheDeserializedValues();
+        switch (cacheDeserializedValues) {
+            case NEVER:
+                return false;
+            case ALWAYS:
+                return true;
+            default:
+                //if index exists then cached value is already set -> let's use it
+                return getIndexes(partitionId).haveAtLeastOneIndex();
+        }
+    }
 }

hazelcast/src/main/java/com/hazelcast/map/impl/query/PartitionScanRunner.java

@@ -16,7 +16,6 @@
 
 package com.hazelcast.map.impl.query;
 
-import com.hazelcast.config.CacheDeserializedValues;
 import com.hazelcast.config.InMemoryFormat;
 import com.hazelcast.internal.cluster.ClusterService;
 import com.hazelcast.internal.iteration.IterationPointer;
@@ -82,7 +81,7 @@ public void run(String mapName, Predicate predicate, int partitionId, Result res
         MapContainer mapContainer = mapServiceContext.getMapContainer(mapName);
         RecordStore<Record> recordStore = partitionContainer.getRecordStore(mapName);
         boolean nativeMemory = recordStore.getInMemoryFormat() == InMemoryFormat.NATIVE;
-        boolean useCachedValues = isUseCachedDeserializedValuesEnabled(mapContainer, partitionId);
+        boolean useCachedValues = mapContainer.isUseCachedDeserializedValuesEnabled(partitionId);
         Extractors extractors = mapServiceContext.getExtractors(mapName);
         Map.Entry<Integer, Map.Entry> nearestAnchorEntry =
                 pagingPredicate == null ? null : pagingPredicate.getNearestAnchorEntry();
@@ -165,17 +164,4 @@ public QueryableEntriesSegment run(String mapName, Predicate predicate, int part
         }
         return new QueryableEntriesSegment(resultList, pointers);
     }
-
-    protected boolean isUseCachedDeserializedValuesEnabled(MapContainer mapContainer, int partitionId) {
-        CacheDeserializedValues cacheDeserializedValues = mapContainer.getMapConfig().getCacheDeserializedValues();
-        switch (cacheDeserializedValues) {
-            case NEVER:
-                return false;
-            case ALWAYS:
-                return true;
-            default:
-                //if index exists then cached value is already set -> let's use it
-                return mapContainer.getIndexes(partitionId).haveAtLeastOneIndex();
-        }
-    }
 }

hazelcast/src/main/java/com/hazelcast/query/impl/CachedQueryEntry.java

@@ -72,6 +72,11 @@ public K getKey() {
         return keyObject;
     }
 
+    @Override
+    public Data getKeyData() {
+        return keyData;
+    }
+
     @Override
     public V getValue() {
         if (valueObject == null) {
@@ -80,11 +85,6 @@ public V getValue() {
         return valueObject;
     }
 
-    @Override
-    public Data getKeyData() {
-        return keyData;
-    }
-
     @Override
     public Data getValueData() {
         if (valueData == null) {
@@ -93,6 +93,39 @@ public Data getValueData() {
         return valueData;
     }
 
+    @Override
+    public K getKeyIfPresent() {
+        return keyObject != null ? keyObject : null;
+    }
+
+    @Override
+    public Data getKeyDataIfPresent() {
+        return keyData;
+    }
+
+    @SuppressWarnings("unchecked")
+    @Override
+    public V getValueIfPresent() {
+        if (valueObject != null) {
+            return valueObject;
+        }
+
+        Object possiblyNotData = record.getValue();
+
+        return possiblyNotData instanceof Data ? null : (V) possiblyNotData;
+    }
+
+    @Override
+    public Data getValueDataIfPresent() {
+        if (valueData != null) {
+            return valueData;
+        }
+
+        Object possiblyData = record.getValue();
+
+        return possiblyData instanceof Data ? (Data) possiblyData : null;
+    }
+
     public Object getByPrioritizingDataValue() {
         if (valueData != null) {
             return valueData;

hazelcast/src/main/java/com/hazelcast/query/impl/QueryEntry.java

@@ -16,8 +16,8 @@
 
 package com.hazelcast.query.impl;
 
-import com.hazelcast.internal.serialization.InternalSerializationService;
 import com.hazelcast.internal.serialization.Data;
+import com.hazelcast.internal.serialization.InternalSerializationService;
 import com.hazelcast.query.impl.getters.Extractors;
 
 /**
@@ -69,19 +69,51 @@ public Object getKey() {
         return serializationService.toObject(key);
     }
 
+    @Override
+    public Data getKeyData() {
+        return key;
+    }
+
     @Override
     public Object getValue() {
         return serializationService.toObject(value);
     }
 
     @Override
-    public Data getKeyData() {
+    public Data getValueData() {
+        return serializationService.toData(value);
+    }
+
+    @Override
+    public Object getKeyIfPresent() {
+        return null;
+    }
+
+    @Override
+    public Data getKeyDataIfPresent() {
         return key;
     }
 
     @Override
-    public Data getValueData() {
-        return serializationService.toData(value);
+    public Object getValueIfPresent() {
+        if (!(value instanceof Data)) {
+            return value;
+        }
+
+        Object possiblyNotData = record.getValue();
+
+        return possiblyNotData instanceof Data ? null : possiblyNotData;
+    }
+
+    @Override
+    public Data getValueDataIfPresent() {
+        if (value instanceof Data) {
+            return (Data) value;
+        }
+
+        Object possiblyData = record.getValue();
+
+        return possiblyData instanceof Data ? (Data) possiblyData : null;
     }
 
     @Override