Repository: nationalsecurityagency/datawave
Issue: GeoWavePruning visitor should not be run against legacy geo fields

The GeoWavePruning visitor throws an exception if it runs against a legacy-geo field.  This breaks the ability to query legacy geo types with the geowave functions.  We should update the geowave pruning visitor to ignore these legacy fields.

Changed files:
warehouse/query-core/src/test/java/datawave/query/jexl/visitors/GeoWavePruningVisitorTest.java
warehouse/query-core/src/main/java/datawave/query/jexl/visitors/GeoWavePruningVisitor.java
warehouse/query-core/src/test/java/datawave/query/jexl/functions/GeoWaveFunctionsDescriptorTest.java

diff --git a/warehouse/query-core/src/main/java/datawave/query/jexl/visitors/GeoWavePruningVisitor.java b/warehouse/query-core/src/main/java/datawave/query/jexl/visitors/GeoWavePruningVisitor.java
index 5bd7154bcb..695338ea40 100644
--- a/warehouse/query-core/src/main/java/datawave/query/jexl/visitors/GeoWavePruningVisitor.java
+++ b/warehouse/query-core/src/main/java/datawave/query/jexl/visitors/GeoWavePruningVisitor.java
@@ -3,12 +3,15 @@ package datawave.query.jexl.visitors;
 import com.google.common.collect.HashMultimap;
 import com.google.common.collect.Multimap;
 import datawave.data.normalizer.GeometryNormalizer;
+import datawave.data.type.AbstractGeometryType;
+import datawave.data.type.Type;
 import datawave.query.jexl.JexlASTHelper;
 import datawave.query.jexl.functions.GeoWaveFunctionsDescriptor;
 import datawave.query.jexl.functions.JexlFunctionArgumentDescriptorFactory;
 import datawave.query.jexl.functions.arguments.JexlArgumentDescriptor;
 import datawave.query.util.GeoWaveUtils;
 import datawave.query.util.MetadataHelper;
+import datawave.webservice.common.logging.ThreadConfigurableLogger;
 import org.apache.commons.jexl2.parser.ASTAndNode;
 import org.apache.commons.jexl2.parser.ASTEQNode;
 import org.apache.commons.jexl2.parser.ASTFalseNode;
@@ -18,10 +21,11 @@ import org.apache.commons.jexl2.parser.ASTReference;
 import org.apache.commons.jexl2.parser.ASTReferenceExpression;
 import org.apache.commons.jexl2.parser.JexlNode;
 import org.apache.commons.jexl2.parser.ParserTreeConstants;
+import org.apache.log4j.Logger;
 import org.locationtech.jts.geom.Geometry;
 
 import java.util.Collection;
-import java.util.Collections;
+import java.util.HashSet;
 import java.util.Set;
 
 import static org.apache.commons.jexl2.parser.JexlNodes.children;
@@ -33,6 +37,8 @@ import static org.apache.commons.jexl2.parser.JexlNodes.children;
  */
 public class GeoWavePruningVisitor extends RebuildingVisitor {
     
+    private static final Logger log = ThreadConfigurableLogger.getLogger(GeoWavePruningVisitor.class);
+    
     private final Multimap<String,String> prunedTerms;
     private final MetadataHelper metadataHelper;
     
@@ -61,10 +67,12 @@ public class GeoWavePruningVisitor extends RebuildingVisitor {
                 JexlArgumentDescriptor desc = JexlFunctionArgumentDescriptorFactory.F.getArgumentDescriptor((ASTFunctionNode) child);
                 if (desc instanceof GeoWaveFunctionsDescriptor.GeoWaveJexlArgumentDescriptor) {
                     GeoWaveFunctionsDescriptor.GeoWaveJexlArgumentDescriptor geoWaveDesc = (GeoWaveFunctionsDescriptor.GeoWaveJexlArgumentDescriptor) desc;
-                    Geometry geom = GeometryNormalizer.parseGeometry(geoWaveDesc.getWkt());
-                    Set<String> fields = geoWaveDesc.fields(metadataHelper, Collections.emptySet());
-                    for (String field : fields) {
-                        fieldToGeometryMap.put(field, geom);
+                    if (isPrunable(geoWaveDesc)) {
+                        Geometry geom = GeometryNormalizer.parseGeometry(geoWaveDesc.getWkt());
+                        Set<String> fields = geoWaveDesc.fields(metadataHelper, null);
+                        for (String field : fields) {
+                            fieldToGeometryMap.put(field, geom);
+                        }
                     }
                 }
             }
@@ -72,6 +80,21 @@ public class GeoWavePruningVisitor extends RebuildingVisitor {
         return super.visit(node, (fieldToGeometryMap.isEmpty() ? null : fieldToGeometryMap));
     }
     
+    private boolean isPrunable(GeoWaveFunctionsDescriptor.GeoWaveJexlArgumentDescriptor geoWaveDesc) {
+        Set<String> fields = geoWaveDesc.fields(metadataHelper, null);
+        return fields.stream().anyMatch(field -> getDatatypesForField(field).stream().anyMatch(type -> type instanceof AbstractGeometryType));
+    }
+    
+    private Set<Type<?>> getDatatypesForField(String field) {
+        Set<Type<?>> dataTypes = new HashSet<>();
+        try {
+            dataTypes.addAll(metadataHelper.getDatatypesForField(field));
+        } catch (Exception e) {
+            log.warn("Unable to determine types for field: " + field, e);
+        }
+        return dataTypes;
+    }
+    
     @Override
     public Object visit(ASTOrNode node, Object data) {
         JexlNode copiedNode = (JexlNode) super.visit(node, data);
@@ -108,13 +131,20 @@ public class GeoWavePruningVisitor extends RebuildingVisitor {
             if (queryGeometries != null && !queryGeometries.isEmpty()) {
                 String value = (String) JexlASTHelper.getLiteralValue(node);
                 if (value != null) {
-                    Geometry nodeGeometry = GeoWaveUtils.positionToGeometry(value);
-                    // if the node geometry doesn't intersect the query geometry, get rid of this node
-                    if (fieldToGeometryMap.get(field).stream().noneMatch(nodeGeometry::intersects)) {
-                        if (prunedTerms != null) {
-                            prunedTerms.put(field, value);
+                    Geometry nodeGeometry = null;
+                    try {
+                        nodeGeometry = GeoWaveUtils.positionToGeometry(value);
+                    } catch (Exception e) {
+                        log.warn("Unable to extract geometry from geo term: " + value, e);
+                    }
+                    if (nodeGeometry != null) {
+                        // if the node geometry doesn't intersect the query geometry, get rid of this node
+                        if (fieldToGeometryMap.get(field).stream().noneMatch(nodeGeometry::intersects)) {
+                            if (prunedTerms != null) {
+                                prunedTerms.put(field, value);
+                            }
+                            return new ASTFalseNode(ParserTreeConstants.JJTFALSENODE);
                         }
-                        return new ASTFalseNode(ParserTreeConstants.JJTFALSENODE);
                     }
                 }
             }
diff --git a/warehouse/query-core/src/test/java/datawave/query/jexl/functions/GeoWaveFunctionsDescriptorTest.java b/warehouse/query-core/src/test/java/datawave/query/jexl/functions/GeoWaveFunctionsDescriptorTest.java
index f1315beadc..64251f1427 100644
--- a/warehouse/query-core/src/test/java/datawave/query/jexl/functions/GeoWaveFunctionsDescriptorTest.java
+++ b/warehouse/query-core/src/test/java/datawave/query/jexl/functions/GeoWaveFunctionsDescriptorTest.java
@@ -5,6 +5,7 @@ import datawave.query.config.ShardQueryConfiguration;
 import datawave.query.jexl.JexlASTHelper;
 import datawave.query.jexl.functions.arguments.JexlArgumentDescriptor;
 import datawave.query.jexl.visitors.JexlStringBuildingVisitor;
+import datawave.query.util.MetadataHelper;
 import datawave.query.util.MockMetadataHelper;
 import org.apache.commons.jexl2.parser.ASTFunctionNode;
 import org.apache.commons.jexl2.parser.ASTJexlScript;
@@ -40,18 +41,21 @@ public class GeoWaveFunctionsDescriptorTest {
         
         ShardQueryConfiguration config = new ShardQueryConfiguration();
         
+        MockMetadataHelper helper = new MockMetadataHelper();
+        helper.addField("GEO_FIELD", "datawave.data.type.GeometryType");
+        
         // DEFAULT, maxEnvelopes = 4
-        String defaultExpandedQuery = convertFunctionToIndexQuery(query, config);
+        String defaultExpandedQuery = convertFunctionToIndexQuery(query, config, helper);
         Assert.assertEquals(expandedWkt.get(0), defaultExpandedQuery);
         
         // maxEnvelopes = 1
         config.setGeoWaveMaxEnvelopes(1);
-        String oneEnvelopeExpandedQuery = convertFunctionToIndexQuery(query, config);
+        String oneEnvelopeExpandedQuery = convertFunctionToIndexQuery(query, config, helper);
         Assert.assertEquals(expandedWkt.get(1), oneEnvelopeExpandedQuery);
         
         // maxEnvelopes = 2
         config.setGeoWaveMaxEnvelopes(2);
-        String twoEnvelopesExpandedQuery = convertFunctionToIndexQuery(query, config);
+        String twoEnvelopesExpandedQuery = convertFunctionToIndexQuery(query, config, helper);
         Assert.assertEquals(expandedWkt.get(2), twoEnvelopesExpandedQuery);
         
         // Test the the default number of envelopes produces a different expanded query than the single envelope expansion
@@ -97,14 +101,12 @@ public class GeoWaveFunctionsDescriptorTest {
         }
     }
     
-    public static String convertFunctionToIndexQuery(String queryStr, ShardQueryConfiguration config) throws ParseException {
+    public static String convertFunctionToIndexQuery(String queryStr, ShardQueryConfiguration config, MetadataHelper metadataHelper) throws ParseException {
         ASTJexlScript script = JexlASTHelper.parseJexlQuery(queryStr);
         ASTFunctionNode func = find(script);
         JexlArgumentDescriptor desc = new GeoWaveFunctionsDescriptor().getArgumentDescriptor(func);
-        MockMetadataHelper helper = new MockMetadataHelper();
-        helper.addField("GEO_FIELD", "datawave.data.type.GeometryType");
         
-        JexlNode indexQuery = desc.getIndexQuery(config, helper, null, null);
+        JexlNode indexQuery = desc.getIndexQuery(config, metadataHelper, null, null);
         return JexlStringBuildingVisitor.buildQuery(indexQuery);
     }
     
diff --git a/warehouse/query-core/src/test/java/datawave/query/jexl/visitors/GeoWavePruningVisitorTest.java b/warehouse/query-core/src/test/java/datawave/query/jexl/visitors/GeoWavePruningVisitorTest.java
index 343a794979..127df25f0c 100644
--- a/warehouse/query-core/src/test/java/datawave/query/jexl/visitors/GeoWavePruningVisitorTest.java
+++ b/warehouse/query-core/src/test/java/datawave/query/jexl/visitors/GeoWavePruningVisitorTest.java
@@ -4,12 +4,14 @@ import com.google.common.collect.HashMultimap;
 import com.google.common.collect.Multimap;
 import datawave.query.config.ShardQueryConfiguration;
 import datawave.query.jexl.JexlASTHelper;
+import datawave.query.util.MockMetadataHelper;
 import datawave.test.JexlNodeAssert;
 import org.apache.commons.jexl2.parser.ASTJexlScript;
 import org.apache.commons.jexl2.parser.ASTReference;
 import org.apache.commons.jexl2.parser.ASTReferenceExpression;
 import org.apache.commons.jexl2.parser.JexlNode;
 import org.apache.commons.jexl2.parser.ParseException;
+import org.junit.BeforeClass;
 import org.junit.Test;
 
 import static datawave.query.jexl.functions.GeoWaveFunctionsDescriptorTest.convertFunctionToIndexQuery;
@@ -18,11 +20,20 @@ import static org.junit.Assert.assertFalse;
 
 public class GeoWavePruningVisitorTest {
     
+    private static MockMetadataHelper metadataHelper;
+    
+    @BeforeClass
+    public static void setup() {
+        metadataHelper = new MockMetadataHelper();
+        metadataHelper.addField("GEO_FIELD", "datawave.data.type.GeometryType");
+        metadataHelper.addField("LEGACY_GEO_FIELD", "datawave.data.type.GeoType");
+    }
+    
     @Test
     public void testNonIntersectingTermIsPruned() throws ParseException {
         String function = "geowave:intersects(GEO_FIELD, 'POLYGON((10 10, 20 10, 20 20, 10 20, 10 10))')";
         // Get the expanded geowave terms.
-        String indexQuery = convertFunctionToIndexQuery(function, new ShardQueryConfiguration());
+        String indexQuery = convertFunctionToIndexQuery(function, new ShardQueryConfiguration(), metadataHelper);
         
         // Add a term that should be pruned.
         String query = function + " && (GEO_FIELD == '0100' || " + indexQuery + ")";
@@ -38,7 +49,7 @@ public class GeoWavePruningVisitorTest {
     public void testPrunedWrappedTermDoesNotLeaveEmptyWrappedTerm() throws ParseException {
         String function = "geowave:intersects(GEO_FIELD, 'POLYGON((10 10, 20 10, 20 20, 10 20, 10 10))')";
         // Get the expanded geowave terms.
-        String indexQuery = convertFunctionToIndexQuery(function, new ShardQueryConfiguration());
+        String indexQuery = convertFunctionToIndexQuery(function, new ShardQueryConfiguration(), metadataHelper);
         
         // Add a wrapped term that should be pruned.
         String query = function + " && ((GEO_FIELD == '0100') || " + indexQuery + ")";
@@ -50,11 +61,39 @@ public class GeoWavePruningVisitorTest {
         assertResult(query, expected, expectedPrunedTerms);
     }
     
+    @Test
+    public void testNonGeometryTermsNotPruned() throws ParseException {
+        String function = "geowave:intersects(LEGACY_GEO_FIELD, 'POLYGON((10 10, 20 10, 20 20, 10 20, 10 10))')";
+        // Get the expanded geowave terms.
+        String indexQuery = convertFunctionToIndexQuery(function, new ShardQueryConfiguration(), metadataHelper);
+        
+        // Add a term that should be pruned.
+        String query = function + " && (" + indexQuery + ")";
+        
+        Multimap<String,String> expectedPrunedTerms = HashMultimap.create();
+        
+        assertResult(query, query, expectedPrunedTerms);
+    }
+    
+    @Test
+    public void testIgnoreImproperlyFormattedTerms() throws ParseException {
+        String function = "geowave:intersects(GEO_FIELD, 'POLYGON((10 10, 20 10, 20 20, 10 20, 10 10))')";
+        // Get the expanded geowave terms.
+        String indexQuery = convertFunctionToIndexQuery(function, new ShardQueryConfiguration(), metadataHelper);
+        
+        // Add a term that should be pruned.
+        String query = function + " && (GEO_FIELD == '1f123..456' || " + indexQuery + ")";
+        
+        Multimap<String,String> expectedPrunedTerms = HashMultimap.create();
+        
+        assertResult(query, query, expectedPrunedTerms);
+    }
+    
     private void assertResult(String original, String expected, Multimap<String,String> expectedPrunedTerms) throws ParseException {
         ASTJexlScript originalScript = JexlASTHelper.parseJexlQuery(original);
         
         Multimap<String,String> prunedTerms = HashMultimap.create();
-        ASTJexlScript actualScript = GeoWavePruningVisitor.pruneTree(originalScript, prunedTerms, null);
+        ASTJexlScript actualScript = GeoWavePruningVisitor.pruneTree(originalScript, prunedTerms, metadataHelper);
         
         // Verify the result is as expected, with a valid lineage.
         JexlNodeAssert.assertThat(actualScript).isEqualTo(expected).hasValidLineage();
@@ -63,7 +102,7 @@ public class GeoWavePruningVisitorTest {
         assertNoChildlessReferences(actualScript);
         
         // Verify the correct terms were pruned.
-        assertEquals(prunedTerms, expectedPrunedTerms);
+        assertEquals(expectedPrunedTerms, prunedTerms);
         
         // Verify the original script was not modified, and has a valid lineage.
         JexlNodeAssert.assertThat(originalScript).isEqualTo(original).hasValidLineage();