Repository: incubator-geode Updated Branches: refs/heads/develop fd4df9aac -> 45299f3fa
GEODE-1676: Multiple Not Equals in query with Compact Range Index returns incorrect results * Copy the not equals parameters instead of modifying the original set * PdxString and String comparison is now added to the comparison logic for Compact Range Index Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/45299f3f Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/45299f3f Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/45299f3f Branch: refs/heads/develop Commit: 45299f3fa0e62872d35e416409bac965cf377db6 Parents: fd4df9a Author: Jason Huynh <huyn...@gmail.com> Authored: Wed Jul 27 15:26:09 2016 -0700 Committer: Jason Huynh <huyn...@gmail.com> Committed: Fri Aug 5 09:29:12 2016 -0700 ---------------------------------------------------------------------- .../query/internal/index/MemoryIndexStore.java | 3 +- .../cache/query/internal/types/TypeUtils.java | 20 +++-- .../dunit/CompactRangeIndexQueryDUnitTest.java | 91 ++++++++++++++++++++ .../query/internal/types/TypeUtilTest.java | 46 ++++++++++ 4 files changed, 152 insertions(+), 8 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/45299f3f/geode-core/src/main/java/com/gemstone/gemfire/cache/query/internal/index/MemoryIndexStore.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/com/gemstone/gemfire/cache/query/internal/index/MemoryIndexStore.java b/geode-core/src/main/java/com/gemstone/gemfire/cache/query/internal/index/MemoryIndexStore.java index 83f8c02..59e7d21 100644 --- a/geode-core/src/main/java/com/gemstone/gemfire/cache/query/internal/index/MemoryIndexStore.java +++ b/geode-core/src/main/java/com/gemstone/gemfire/cache/query/internal/index/MemoryIndexStore.java @@ -18,6 +18,7 @@ package com.gemstone.gemfire.cache.query.internal.index; import java.util.AbstractMap.SimpleImmutableEntry; import java.util.Collection; +import java.util.HashSet; import java.util.Iterator; import java.util.Map; import java.util.NoSuchElementException; @@ -614,7 +615,7 @@ private Object convertToIndexKey(Object key, RegionEntry entry) Object indexKey, Collection keysToRemove, long iteratorStartTime) { this.map = submap; this.indexKey = indexKey; - this.keysToRemove = keysToRemove; + this.keysToRemove = keysToRemove == null? null : new HashSet(keysToRemove); this.iteratorStartTime = iteratorStartTime; currentEntry = new MemoryIndexStoreEntry(iteratorStartTime); } http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/45299f3f/geode-core/src/main/java/com/gemstone/gemfire/cache/query/internal/types/TypeUtils.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/com/gemstone/gemfire/cache/query/internal/types/TypeUtils.java b/geode-core/src/main/java/com/gemstone/gemfire/cache/query/internal/types/TypeUtils.java index 14c798f..d244798 100644 --- a/geode-core/src/main/java/com/gemstone/gemfire/cache/query/internal/types/TypeUtils.java +++ b/geode-core/src/main/java/com/gemstone/gemfire/cache/query/internal/types/TypeUtils.java @@ -36,6 +36,7 @@ import com.gemstone.gemfire.cache.query.types.ObjectType; import com.gemstone.gemfire.internal.i18n.LocalizedStrings; import com.gemstone.gemfire.pdx.internal.EnumInfo.PdxInstanceEnumInfo; import com.gemstone.gemfire.pdx.internal.PdxInstanceEnum; +import com.gemstone.gemfire.pdx.internal.PdxString; /** @@ -166,13 +167,18 @@ public class TypeUtils implements OQLLexerTokenTypes return QueryService.UNDEFINED; } } - - if (obj1 instanceof PdxInstanceEnumInfo && obj2 instanceof Enum) { - obj2 = new PdxInstanceEnum((Enum<?>) obj2); - } else if (obj1 instanceof Enum && obj2 instanceof PdxInstanceEnumInfo) { - obj1 = new PdxInstanceEnum((Enum<?>) obj1); - } - + + if (obj1 instanceof PdxInstanceEnumInfo && obj2 instanceof Enum) { + obj2 = new PdxInstanceEnum((Enum<?>) obj2); + } else if (obj1 instanceof Enum && obj2 instanceof PdxInstanceEnumInfo) { + obj1 = new PdxInstanceEnum((Enum<?>) obj1); + } + + if (obj1 instanceof PdxString && obj2 instanceof String) { + obj2 = new PdxString((String) obj2); + } else if (obj1 instanceof String && obj2 instanceof PdxString) { + obj1 = new PdxString((String) obj1); + } try { int r; http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/45299f3f/geode-core/src/test/java/com/gemstone/gemfire/cache/query/dunit/CompactRangeIndexQueryDUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/com/gemstone/gemfire/cache/query/dunit/CompactRangeIndexQueryDUnitTest.java b/geode-core/src/test/java/com/gemstone/gemfire/cache/query/dunit/CompactRangeIndexQueryDUnitTest.java new file mode 100644 index 0000000..8b27309 --- /dev/null +++ b/geode-core/src/test/java/com/gemstone/gemfire/cache/query/dunit/CompactRangeIndexQueryDUnitTest.java @@ -0,0 +1,91 @@ +/* + * 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 com.gemstone.gemfire.cache.query.dunit; + +import static com.gemstone.gemfire.internal.cache.execute.DistributedRegionFunctionExecutionDUnitTest.region; +import static java.rmi.activation.ActivationGroup.getSystem; +import static org.junit.Assert.*; + +import java.util.Properties; + +import org.junit.Test; +import org.junit.experimental.categories.Category; + +import com.gemstone.gemfire.cache.Cache; +import com.gemstone.gemfire.cache.CacheException; +import com.gemstone.gemfire.cache.Region; +import com.gemstone.gemfire.cache.RegionShortcut; +import com.gemstone.gemfire.cache.query.Query; +import com.gemstone.gemfire.cache.query.QueryService; +import com.gemstone.gemfire.cache.query.QueryTestUtils; +import com.gemstone.gemfire.cache.query.SelectResults; +import com.gemstone.gemfire.cache.query.data.Portfolio; +import com.gemstone.gemfire.cache.query.data.PortfolioPdx; +import com.gemstone.gemfire.cache.query.internal.ResultsSet; +import com.gemstone.gemfire.cache.query.internal.index.IndexManager; +import com.gemstone.gemfire.cache.query.internal.index.IndexManager.TestHook; +import com.gemstone.gemfire.cache30.CacheSerializableRunnable; +import com.gemstone.gemfire.test.dunit.Assert; +import com.gemstone.gemfire.test.dunit.AsyncInvocation; +import com.gemstone.gemfire.test.dunit.DistributedTestUtils; +import com.gemstone.gemfire.test.dunit.Host; +import com.gemstone.gemfire.test.dunit.Invoke; +import com.gemstone.gemfire.test.dunit.SerializableRunnable; +import com.gemstone.gemfire.test.dunit.VM; +import com.gemstone.gemfire.test.dunit.Wait; +import com.gemstone.gemfire.test.dunit.cache.internal.JUnit4CacheTestCase; +import com.gemstone.gemfire.test.dunit.internal.JUnit4DistributedTestCase; +import com.gemstone.gemfire.test.junit.categories.DistributedTest; + +@Category(DistributedTest.class) +public class CompactRangeIndexQueryDUnitTest extends JUnit4CacheTestCase { + + VM vm0; + + @Override + public final void postSetUp() throws Exception { + getSystem(); + Invoke.invokeInEveryVM(new SerializableRunnable("getSystem") { + public void run() { + getSystem(); + } + }); + Host host = Host.getHost(0); + vm0 = host.getVM(0); + } + + @Test + public void multipleNotEqualsClausesOnAPartitionedRegionShouldReturnCorrectResults() throws Exception { + Cache cache = getCache(); + Region region = cache.createRegionFactory(RegionShortcut.PARTITION).create("portfolios"); + int numMatching = 10; + QueryService qs = cache.getQueryService(); + qs.createIndex("statusIndex", "p.status", "/portfolios p"); + for (int i = 0; i < numMatching * 2; i++) { + PortfolioPdx p = new PortfolioPdx(i); + if (i < numMatching) { + p.status = "1"; + } + region.put("KEY-"+ i, p); + } + + Query q = qs.newQuery("select * from /portfolios p where p.pk <> '0' and p.status <> '0' and p.status <> '1' and p.status <> '2'"); + SelectResults rs = (SelectResults) q.execute(); + assertEquals( numMatching, rs.size()); + } + +} http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/45299f3f/geode-core/src/test/java/com/gemstone/gemfire/cache/query/internal/types/TypeUtilTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/com/gemstone/gemfire/cache/query/internal/types/TypeUtilTest.java b/geode-core/src/test/java/com/gemstone/gemfire/cache/query/internal/types/TypeUtilTest.java new file mode 100644 index 0000000..bb1cede --- /dev/null +++ b/geode-core/src/test/java/com/gemstone/gemfire/cache/query/internal/types/TypeUtilTest.java @@ -0,0 +1,46 @@ +/* + * 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 com.gemstone.gemfire.cache.query.internal.types; + +import static org.junit.Assert.*; + +import org.junit.Test; +import org.junit.experimental.categories.Category; + +import com.gemstone.gemfire.cache.query.internal.parse.OQLLexerTokenTypes; +import com.gemstone.gemfire.pdx.internal.PdxString; +import com.gemstone.gemfire.test.junit.categories.UnitTest; + +@Category(UnitTest.class) +public class TypeUtilTest { + + @Test + public void comparingEquivalentPdxStringToStringShouldMatchCorrectly() throws Exception { + String theString = "MyString"; + PdxString pdxString = new PdxString(theString); + assertTrue((Boolean)TypeUtils.compare(pdxString, theString, OQLLexerTokenTypes.TOK_EQ)); + assertTrue((Boolean)TypeUtils.compare(theString, pdxString, OQLLexerTokenTypes.TOK_EQ)); + } + + @Test + public void comparingUnequivalentPdxStringToStringShouldNotMatch() throws Exception { + String theString = "MyString"; + PdxString pdxString = new PdxString("AnotherString"); + assertFalse((Boolean)TypeUtils.compare(pdxString, theString, OQLLexerTokenTypes.TOK_EQ)); + assertFalse((Boolean)TypeUtils.compare(theString, pdxString, OQLLexerTokenTypes.TOK_EQ)); + } +}
