jhuynh1 commented on a change in pull request #6028:
URL: https://github.com/apache/geode/pull/6028#discussion_r578786729



##########
File path: 
geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledIndexOperation.java
##########
@@ -104,7 +105,13 @@ public Object evaluate(ExecutionContext context) throws 
TypeMismatchException,
     }
 
     if (rcvr instanceof Map) {
-      return ((Map) rcvr).get(index);
+      if (context instanceof QueryExecutionContext) {

Review comment:
       So when we execute a query and the index is a map index... if the key is 
not present in the map, we could get an actual "null" back.  When we were 
adding the NULL key as a collection into the map, we were possibly getting a 
valid collection?
   
   I'm surprised a bit that the != queries work but I haven't walked through 
this code in a while.  SInce the tests are passing, that's great.  I would 
assume the != queries will now be doing a full region crawl... if that's the 
case, they will be a lot slower.. not sure if that's the case, but if it is, it 
might not be acceptable => might need to convince the dev list on this one...

##########
File path: 
geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledIndexOperation.java
##########
@@ -104,7 +105,13 @@ public Object evaluate(ExecutionContext context) throws 
TypeMismatchException,
     }
 
     if (rcvr instanceof Map) {
-      return ((Map) rcvr).get(index);
+      if (context instanceof QueryExecutionContext) {
+        return ((Map) rcvr).get(index);
+      }
+      if (((Map<?, ?>) rcvr).containsKey(index)) {

Review comment:
       Both if statements do the same thing, maybe add an || to the previous if 
call, but I am ok with keeping them separate.
   
   

##########
File path: 
geode-core/src/main/java/org/apache/geode/cache/query/internal/index/CompactRangeIndex.java
##########
@@ -1555,6 +1555,9 @@ private void applyProjection(boolean add, 
ExecutionContext context)
         throws FunctionDomainException, TypeMismatchException, 
NameResolutionException,
         QueryInvocationTargetException, IMQException {
       Object indexKey = indexedExpr.evaluate(context);
+      if (IndexManager.NULL.equals(indexKey)) {
+        return;
+      }
       if (indexKey == null) {

Review comment:
       Should this still be present?  wouldn't this case be "iimpossible now 
that we added the three lines above?"
   




----------------------------------------------------------------
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:
[email protected]


Reply via email to