From 48c61a16b84c2d599731177ef6a9783048011293 Mon Sep 17 00:00:00 2001 From: awrigh11 Date: Wed, 25 Jun 2025 15:51:26 +0000 Subject: [PATCH 1/5] Created FieldReplacement Visitor --- .../query/planner/DefaultQueryPlanner.java | 40 +++- .../replacement/FieldReplacementVisitor.java | 48 ++++ .../rules/DirectFieldReplacementRule.java | 42 ++++ .../rules/FieldReplacementRule.java | 9 + .../rules/RangeFieldReplacementRule.java | 101 +++++++++ .../FieldReplacementVisitorTest.java | 209 ++++++++++++++++++ 6 files changed, 448 insertions(+), 1 deletion(-) create mode 100644 warehouse/query-core/src/main/java/datawave/query/planner/replacement/FieldReplacementVisitor.java create mode 100644 warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/DirectFieldReplacementRule.java create mode 100644 warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/FieldReplacementRule.java create mode 100644 warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/RangeFieldReplacementRule.java create mode 100644 warehouse/query-core/src/test/java/datawave/query/planner/replacement/FieldReplacementVisitorTest.java diff --git a/warehouse/query-core/src/main/java/datawave/query/planner/DefaultQueryPlanner.java b/warehouse/query-core/src/main/java/datawave/query/planner/DefaultQueryPlanner.java index 04fdbeca250..c35cf7c2c68 100644 --- a/warehouse/query-core/src/main/java/datawave/query/planner/DefaultQueryPlanner.java +++ b/warehouse/query-core/src/main/java/datawave/query/planner/DefaultQueryPlanner.java @@ -178,6 +178,8 @@ import datawave.query.planner.comparator.GeoWaveQueryPlanComparator; import datawave.query.planner.pushdown.PushDownVisitor; import datawave.query.planner.pushdown.rules.PushDownRule; +import datawave.query.planner.replacement.FieldReplacementVisitor; +import datawave.query.planner.replacement.rules.FieldReplacementRule; import datawave.query.planner.rules.FieldTransformRule; import datawave.query.planner.rules.FieldTransformRuleVisitor; import datawave.query.planner.rules.NodeTransformRule; @@ -227,6 +229,13 @@ public class DefaultQueryPlanner extends QueryPlanner implements Cloneable { */ protected boolean disableTestNonExistentFields = false; + /** + * Disables Field Replacement rules + * + * @see FieldReplacementVisitor + */ + protected boolean disableFieldReplacementRules = false; + /** * Disables Whindex (value-specific) field mappings for GeoWave functions. * @@ -274,6 +283,8 @@ public class DefaultQueryPlanner extends QueryPlanner implements Cloneable { // force certain regex patterns to be pushed down to evaluation private List transformRules = Lists.newArrayList(); + private List replacementRules = Lists.newArrayList(); + protected Class> queryIteratorClazz = QueryIterator.class; protected String plannedScript = null; @@ -402,6 +413,7 @@ protected DefaultQueryPlanner(DefaultQueryPlanner other) { setPushdownThreshold(other.getPushdownThreshold()); setVisitorManager(other.getVisitorManager()); setTransformRules(other.getTransformRules() == null ? null : new ArrayList<>(other.transformRules)); + setReplacementRules(other.getReplacementRules() == null ? null : new ArrayList<>(other.replacementRules)); } public void setMetadataHelper(final MetadataHelper metadataHelper) { @@ -625,12 +637,12 @@ protected CloseableIterable process(ScannerFactory scannerFactory, Me /** * This method can be used to recreate a range stream based on plan in the configuration. The plan will be adjusted if needed for executability. * - * @see DatePartitionedQueryPlanner * @param config * @param settings * @param scannerFactory * @return a range stream * @throws DatawaveQueryException + * @see DatePartitionedQueryPlanner */ public CloseableIterable reprocess(ShardQueryConfiguration config, Query settings, ScannerFactory scannerFactory) throws DatawaveQueryException { @@ -1174,6 +1186,11 @@ protected ASTJexlScript processTree(final ASTJexlScript originalQueryTree, Shard // need to fetch field to datatype map first timedFetchDatatypes(timers, "Fetch Required Datatypes", config.getQueryTree(), config); + if (!disableFieldReplacementRules) { + // apply the configured field replacements + config.setQueryTree(timedApplyFieldReplacementRules(timers, config.getQueryTree(), replacementRules)); + } + if (!disableWhindexFieldMappings) { // apply the value-specific field mappings for GeoWave functions config.setQueryTree(timedApplyWhindexFieldMappings(timers, config.getQueryTree(), config, metadataHelper, settings)); @@ -1753,6 +1770,11 @@ protected ASTJexlScript expandRegexFunctionNodes(final ASTJexlScript script, Sha return visitorManager.validateAndVisit(() -> (RegexFunctionVisitor.expandRegex(config, metadataHelper, indexOnlyFields, script))); } + protected ASTJexlScript timedApplyFieldReplacementRules(QueryStopwatch timers, final ASTJexlScript script, List fieldReplacementRules) + throws DatawaveQueryException { + return visitorManager.timedVisit(timers, "Apply Replacement Field Mappings", () -> (FieldReplacementVisitor.apply(script, fieldReplacementRules))); + } + protected ASTJexlScript timedApplyWhindexFieldMappings(QueryStopwatch timers, final ASTJexlScript script, ShardQueryConfiguration config, MetadataHelper metadataHelper, Query settings) throws DatawaveQueryException { try { @@ -3347,6 +3369,14 @@ public void setTransformRules(List transformRules) { this.transformRules.addAll(transformRules); } + public List getReplacementRules() { + return Collections.unmodifiableList(replacementRules); + } + + public void setReplacementRules(List replacementRules) { + this.replacementRules.addAll(replacementRules); + } + /* * (non-Javadoc) * @@ -3426,6 +3456,14 @@ public boolean getDisableTestNonExistentFields() { return disableTestNonExistentFields; } + public void setDisableFieldReplacementRules(boolean disableFieldReplacementRules) { + this.disableFieldReplacementRules = disableFieldReplacementRules; + } + + public boolean getDisableFieldReplacementRules() { + return disableFieldReplacementRules; + } + public void setDisableWhindexFieldMappings(boolean disableWhindexFieldMappings) { this.disableWhindexFieldMappings = disableWhindexFieldMappings; } diff --git a/warehouse/query-core/src/main/java/datawave/query/planner/replacement/FieldReplacementVisitor.java b/warehouse/query-core/src/main/java/datawave/query/planner/replacement/FieldReplacementVisitor.java new file mode 100644 index 00000000000..073c31a77df --- /dev/null +++ b/warehouse/query-core/src/main/java/datawave/query/planner/replacement/FieldReplacementVisitor.java @@ -0,0 +1,48 @@ +package datawave.query.planner.replacement; + +import java.util.List; + +import org.apache.commons.jexl3.parser.ASTAndNode; +import org.apache.commons.jexl3.parser.ASTIdentifier; +import org.apache.commons.jexl3.parser.ASTJexlScript; +import org.apache.commons.jexl3.parser.JexlNode; +import org.apache.log4j.Logger; + +import datawave.core.common.logging.ThreadConfigurableLogger; +import datawave.query.jexl.visitors.RebuildingVisitor; +import datawave.query.planner.replacement.rules.FieldReplacementRule; + +public class FieldReplacementVisitor extends RebuildingVisitor { + private static final Logger log = ThreadConfigurableLogger.getLogger(FieldReplacementVisitor.class); + private final List rules; + + public FieldReplacementVisitor(List rules) { + this.rules = rules; + } + + public static ASTJexlScript apply(ASTJexlScript script, List rules) { + FieldReplacementVisitor visitor = new FieldReplacementVisitor(rules); + + return visitor.apply(script); + } + + @Override + public Object visit(ASTAndNode node, Object data) { + return applyRules(super.visit(node, data)); + } + + @Override + public Object visit(ASTIdentifier node, Object data) { + return applyRules(super.visit(node, data)); + } + + public JexlNode applyRules(Object node) { + JexlNode jexlNode = (JexlNode) node; + for (FieldReplacementRule rule : rules) { + if (rule.matches(jexlNode)) { + rule.apply(jexlNode); + } + } + return jexlNode; + } +} diff --git a/warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/DirectFieldReplacementRule.java b/warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/DirectFieldReplacementRule.java new file mode 100644 index 00000000000..6d31ce5e8aa --- /dev/null +++ b/warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/DirectFieldReplacementRule.java @@ -0,0 +1,42 @@ +package datawave.query.planner.replacement.rules; + +import datawave.query.jexl.JexlASTHelper; +import org.apache.commons.jexl3.parser.ASTIdentifier; +import org.apache.commons.jexl3.parser.JexlNode; + +public class DirectFieldReplacementRule implements FieldReplacementRule { + private String field = null; + private String replacement = null; + + public DirectFieldReplacementRule() { + } + + public DirectFieldReplacementRule(String field, String replacement) { + this.field = field; + this.replacement = replacement; + } + + public boolean matches(JexlNode node) { + return node instanceof ASTIdentifier && ((ASTIdentifier) node).getName().equals(field); + } + + public void apply(JexlNode node) { + JexlASTHelper.setField(node, replacement); + } + + public String getField() { + return field; + } + + public void setField(String field) { + this.field = field; + } + + public String getReplacement() { + return replacement; + } + + public void setReplacement(String replacement) { + this.replacement = replacement; + } +} diff --git a/warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/FieldReplacementRule.java b/warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/FieldReplacementRule.java new file mode 100644 index 00000000000..4eb81270f53 --- /dev/null +++ b/warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/FieldReplacementRule.java @@ -0,0 +1,9 @@ +package datawave.query.planner.replacement.rules; + +import org.apache.commons.jexl3.parser.JexlNode; + +public interface FieldReplacementRule { + boolean matches(JexlNode node); + + void apply(JexlNode node); +} diff --git a/warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/RangeFieldReplacementRule.java b/warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/RangeFieldReplacementRule.java new file mode 100644 index 00000000000..59f1b623c64 --- /dev/null +++ b/warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/RangeFieldReplacementRule.java @@ -0,0 +1,101 @@ +package datawave.query.planner.replacement.rules; + +import datawave.query.jexl.JexlASTHelper; +import datawave.query.jexl.JexlNodeFactory; +import datawave.query.jexl.LiteralRange; +import datawave.query.jexl.nodes.QueryPropertyMarker; +import org.apache.commons.jexl3.parser.ASTAndNode; +import org.apache.commons.jexl3.parser.ASTIdentifier; +import org.apache.commons.jexl3.parser.JexlNode; +import org.apache.commons.jexl3.parser.JexlNodes; +import org.apache.commons.jexl3.parser.ParserTreeConstants; + +import java.util.HashMap; +import java.util.Map; + +import static datawave.query.jexl.nodes.QueryPropertyMarker.MarkerType.EVALUATION_ONLY; + +public class RangeFieldReplacementRule implements FieldReplacementRule { + private Map fieldMap = new HashMap<>(); + + public RangeFieldReplacementRule() { + } + + public RangeFieldReplacementRule(Map fieldMap) { + this.fieldMap = fieldMap; + } + + @Override + public boolean matches(JexlNode node) { + LiteralRange range = JexlASTHelper.findRange().getRange(node); + return range != null && fieldMap.containsKey(range.getFieldName()); + } + + @Override + public void apply(JexlNode node) { + LiteralRange range = JexlASTHelper.findRange().getRange(node); + + if (range == null || !fieldMap.containsKey(range.getFieldName())) { + return; + } + + // Clone original range node and rename identifier in the clone + JexlNode clonedLower = cloneAndReplaceField(range.getLowerNode(), fieldMap.get(range.getFieldName())); + JexlNode clonedUpper = cloneAndReplaceField(range.getUpperNode(), fieldMap.get(range.getFieldName())); + + ASTAndNode newAnd = new ASTAndNode(ParserTreeConstants.JJTANDNODE); + newAnd.jjtAddChild(clonedLower, 0); + newAnd.jjtAddChild(clonedUpper, 1); + + // Mark the original node as "evaluationOnly" + QueryPropertyMarker.create(node, EVALUATION_ONLY); + + // Replace the original node in-place with a new parent AND node + ASTAndNode topLevel = new ASTAndNode(ParserTreeConstants.JJTANDNODE); + topLevel.jjtAddChild(node, 0); + topLevel.jjtAddChild(newAnd, 1); + + replaceNodeInParent(node, topLevel); + } + + private JexlNode cloneAndReplaceField(JexlNode original, String newName) { + try { + JexlNode copy = JexlNodes.newInstanceOfType(original); + + for (int i = 0; i < original.jjtGetNumChildren(); i++) { + JexlNode child = original.jjtGetChild(i); + if (child instanceof ASTIdentifier) { + ASTIdentifier newId = JexlNodeFactory.buildIdentifier(newName); + copy.jjtAddChild(newId, i); + } else { + copy.jjtAddChild(cloneAndReplaceField(child, newName), i); + } + } + return copy; + } catch (Exception e) { + throw new RuntimeException("Failed to clone node: " + original.getClass(), e); + } + } + + private void replaceNodeInParent(JexlNode oldNode, JexlNode newNode) { + JexlNode parent = oldNode.jjtGetParent(); + if (parent == null) + return; // if root node, you’ll need to replace externally todo figure out root nodes + + for (int i = 0; i < parent.jjtGetNumChildren(); i++) { + if (parent.jjtGetChild(i) == oldNode) { + parent.jjtAddChild(newNode, i); + newNode.jjtSetParent(parent); + return; + } + } + } + + public void setFieldMap(Map fieldMap) { + this.fieldMap = fieldMap; + } + + public Map getFieldMap() { + return fieldMap; + } +} diff --git a/warehouse/query-core/src/test/java/datawave/query/planner/replacement/FieldReplacementVisitorTest.java b/warehouse/query-core/src/test/java/datawave/query/planner/replacement/FieldReplacementVisitorTest.java new file mode 100644 index 00000000000..f86d0894fcb --- /dev/null +++ b/warehouse/query-core/src/test/java/datawave/query/planner/replacement/FieldReplacementVisitorTest.java @@ -0,0 +1,209 @@ +package datawave.query.planner.replacement; + +import datawave.query.jexl.JexlASTHelper; +import datawave.query.jexl.visitors.PrintingVisitor; +import datawave.query.jexl.visitors.TreeEqualityVisitor; +import datawave.query.planner.replacement.rules.DirectFieldReplacementRule; +import datawave.query.planner.replacement.rules.FieldReplacementRule; +import datawave.query.planner.replacement.rules.RangeFieldReplacementRule; +import org.apache.commons.jexl3.parser.ASTJexlScript; +import org.apache.commons.jexl3.parser.JexlNode; +import org.apache.commons.jexl3.parser.ParseException; +import org.apache.log4j.Logger; +import org.junit.Test; + +import java.util.List; +import java.util.Map; + +import static org.junit.Assert.assertTrue; + +public class FieldReplacementVisitorTest { + private static final Logger log = Logger.getLogger(FieldReplacementVisitorTest.class); + private static final DirectFieldReplacementRule dfrRule = new DirectFieldReplacementRule("HAN", "SOLO"); + private static final Map rangeMap = Map.of("R2", "D2", "STAR", "DESTROYER"); + private static final RangeFieldReplacementRule rfrRule = new RangeFieldReplacementRule(rangeMap); + + + private void testReplacement(String original, String expected, List rules) throws Exception { + // create a query tree + ASTJexlScript originalScript = JexlASTHelper.parseJexlQuery(original); + + // apply the visitor + ASTJexlScript resultScript = FieldReplacementVisitor.apply(originalScript, rules); + + // Verify the script is as expected, and has a valid lineage. + assertScriptEquality(resultScript, expected); + assertLineage(resultScript); + + // Verify the original script was not modified, and still has a valid lineage. + assertScriptEquality(originalScript, original); + assertLineage(originalScript); + + } + + private void assertScriptEquality(ASTJexlScript actualScript, String expected) throws ParseException { + ASTJexlScript expectedScript = JexlASTHelper.parseJexlQuery(expected); + TreeEqualityVisitor.Comparison comparison = TreeEqualityVisitor.checkEquality(expectedScript, actualScript); + if (!comparison.isEqual()) { + log.error("Expected " + PrintingVisitor.formattedQueryString(expectedScript)); + log.error("Actual " + PrintingVisitor.formattedQueryString(actualScript)); + } + assertTrue(comparison.getReason(), comparison.isEqual()); + } + + private void assertLineage(JexlNode node) { + assertTrue(JexlASTHelper.validateLineage(node, true)); + } + + @Test + public void regexPushdowqnTransformRuleTest() throws Exception { + // @formatter:off + String query = "R2 = 2 && ((_Bounded_ = true) && (R2 >= '2' && R2 <= '3'))"; + String expected = "R2 = 2 && ((_Bounded_ = true) && (D2 >= '2' && D2 <= '3'))" ; + // @formatter:on + testReplacement(query, expected, List.of(rfrRule)); + } + + @Test + public void regexPushdownTransformRuleTest() throws Exception { + // @formatter:off + String query = "HAN == 'x'"; + String expected = "SOLO == 'x'" ; + // @formatter:on + testReplacement(query, expected, List.of(dfrRule)); + } + +// @Test +// public void regexPushdownAnyfieldTransformRuleTest() { +// // @formatter:off +// String query = "BLA == 'x' && " + +// "BLA =~ 'ab.*' && " + +// "BLA =~ 'a.*' && " + +// "BLA =~ 'okregex' && " + +// "_ANYFIELD_ =~ '.*'"; +// String expected = "BLA == 'x' && " + +// "BLA =~ 'ab.*' && " + +// "((_Eval_ = true) && (BLA =~ 'a.*')) && " + +// "BLA =~ 'okregex' && " + +// "((_Eval_ = true) && (_ANYFIELD_ =~ '.*'))"; +// // @formatter:on +// try { +// testPushdown(query, expected); +// fail("Expected anyfield regex pushdown to fail"); +// } catch (Exception e) { +// // ok +// } +// } +// +// @Test +// public void regexSimplifierTransformRuleTest() throws Exception { +// // @formatter:off +// String query = "BLA == '.*?.*?x' && " + +// "BLA =~ 'ab.*.*' && " + +// "BLA !~ 'a.*.*.*.*?.*?' && " + +// "BLA =~ '.*?.*?.*bla.*?.*?blabla' && " + +// "_ANYFIELD_ =~ '.*.*?.*?' && " + +// "filter:excludeRegex(BLA, '.*?.*?.*bla.*?.*?blabla') && " + +// "filter:includeRegex(BLA, '.*?.*?.*bla.*?.*?blabla')"; +// String expected = "BLA == '.*?.*?x' && " + +// "BLA =~ 'ab.*?' && " + +// "BLA !~ 'a.*?' && " + +// "BLA =~ '.*?bla.*?blabla' && " + +// "_ANYFIELD_ =~ '.*?' && " + +// "filter:excludeRegex(BLA, '.*?bla.*?blabla') && " + +// "filter:includeRegex(BLA, '.*?bla.*?blabla')"; +// // @formatter:on +// testSimplify(query, expected); +// } +// +// @Test +// public void regexDotAllTransformRuleTest() throws Exception { +// // @formatter:off +// String query = "BLA == '(\\s|.)*' && " + +// "BLA !~ '(.|\\s)*' && " + +// "BLA =~ '(\\s|.)*word(.|\\s)*' &&" + +// "filter:excludeRegex(BLA, '(\\s|.)*word(.|\\s)*') && " + +// "filter:includeRegex(BLA, '(\\s|.)*word(.|\\s)*')"; +// String expected = "BLA == '(\\s|.)*' && " + +// "BLA !~ '.*' && " + +// "BLA =~ '.*word.*' &&" + +// "filter:excludeRegex(BLA, '.*word.*') && " + +// "filter:includeRegex(BLA, '.*word.*')"; +// // @formatter:on +// testDotall(query, expected); +// } +// +// @Test +// public void skipQueryMarkersTest() throws Exception { +// // @formatter:off +// String query = "BLA == 'x' && " + +// "BLA =~ 'ab.*' && (" + +// "(_Value_ = true) && (BLA =~ 'a.*')) && " + +// "((_Value_ = true) && (BLA =~ 'okregex')) && " + +// "BLA =~ '.*'"; +// String expected = "BLA == 'x' && " + +// "BLA =~ 'ab.*' && " + +// "((_Value_ = true) && (BLA =~ 'a.*')) && " + +// "((_Value_ = true) && (BLA =~ 'okregex')) && " + +// "((_Eval_ = true) && (BLA =~ '.*'))"; +// // @formatter:on +// testPushdown(query, expected); +// } +// +// @Test +// public void depthTest() throws Exception { +// // @formatter:off +// String query = "(((BLA == 'x' && " + +// "BLA =~ 'ab.*' && " + +// "BLA =~ 'a.*') && " + +// "((BLA =~ 'okregex'))) && " + +// "BLA =~ '.*')"; +// String expected = "(((_Eval_ = true) && (BLA =~ '.*')) && " + +// "(((BLA =~ 'okregex')) && " + +// "(((_Eval_ = true) && (BLA =~ 'a.*')) && " + +// "BLA =~ 'ab.*' && " + +// "BLA == 'x')))"; +// // @formatter:on +// testPushdown(query, expected, newArrayList(regexPushdownRule, reverseAndRule)); +// } +// +// @Test +// public void testANDNodeTransform() throws Exception { +// // @formatter:off +// String query = "BLA == 'x' && " + +// "BLA =~ 'ab.*' && " + +// "BLA =~ 'a.*' && " + +// "BLA =~ 'okregex' && " + +// "BLA =~ '.*'"; +// String expected = "((_Eval_ = true) && (BLA =~ '.*')) && " + +// "BLA =~ 'okregex' && " + +// "((_Eval_ = true) && (BLA =~ 'a.*')) && " + +// "BLA =~ 'ab.*' && " + +// "BLA == 'x'"; +// // @formatter:on +// testPushdown(query, expected, newArrayList(regexPushdownRule, reverseAndRule)); +// } +// +// @Test +// public void testTransformOrder() throws Exception { +// // @formatter:off +// String query = "BLA == 'x' && " + +// "BLA =~ 'ab.*' && " + +// "BLA =~ 'a.*' && " + +// "BLA =~ 'okregex' && " + +// "BLA =~ '.*'"; +// String expected1 = "BLA =~ '.*' && " + +// "BLA =~ 'okregex' && " + +// "BLA =~ 'a.*' && " + +// "BLA =~ 'ab.*' && " + +// "BLA == 'x'"; +// String expected2 = "((_Eval_ = true) && (BLA =~ '.*')) && " + +// "BLA =~ 'okregex' && " + +// "((_Eval_ = true) && (BLA =~ 'a.*')) && " + +// "BLA =~ 'ab.*' && " + +// "BLA == 'x'"; +// // @formatter:on +// testPushdown(query, expected1, newArrayList(regexPushdownRule, reverseAndRule, pullUpRule)); +// testPushdown(query, expected2, newArrayList(pullUpRule, reverseAndRule, regexPushdownRule)); +// } +} From e9283b47b354a5ba64cc978c4b88cb865d9a173b Mon Sep 17 00:00:00 2001 From: awrigh11 Date: Thu, 28 Aug 2025 20:06:39 +0000 Subject: [PATCH 2/5] Reworked rules to account for no parent being passed in --- .../replacement/FieldReplacementVisitor.java | 11 +- .../rules/DirectFieldReplacementRule.java | 3 +- .../rules/FieldReplacementRule.java | 2 +- .../rules/RangeFieldReplacementRule.java | 68 ++--- .../FieldReplacementVisitorTest.java | 253 ++++++++---------- 5 files changed, 143 insertions(+), 194 deletions(-) diff --git a/warehouse/query-core/src/main/java/datawave/query/planner/replacement/FieldReplacementVisitor.java b/warehouse/query-core/src/main/java/datawave/query/planner/replacement/FieldReplacementVisitor.java index 073c31a77df..98fa1c86668 100644 --- a/warehouse/query-core/src/main/java/datawave/query/planner/replacement/FieldReplacementVisitor.java +++ b/warehouse/query-core/src/main/java/datawave/query/planner/replacement/FieldReplacementVisitor.java @@ -1,16 +1,15 @@ package datawave.query.planner.replacement; -import java.util.List; - +import datawave.core.common.logging.ThreadConfigurableLogger; +import datawave.query.jexl.visitors.RebuildingVisitor; +import datawave.query.planner.replacement.rules.FieldReplacementRule; import org.apache.commons.jexl3.parser.ASTAndNode; import org.apache.commons.jexl3.parser.ASTIdentifier; import org.apache.commons.jexl3.parser.ASTJexlScript; import org.apache.commons.jexl3.parser.JexlNode; import org.apache.log4j.Logger; -import datawave.core.common.logging.ThreadConfigurableLogger; -import datawave.query.jexl.visitors.RebuildingVisitor; -import datawave.query.planner.replacement.rules.FieldReplacementRule; +import java.util.List; public class FieldReplacementVisitor extends RebuildingVisitor { private static final Logger log = ThreadConfigurableLogger.getLogger(FieldReplacementVisitor.class); @@ -40,7 +39,7 @@ public JexlNode applyRules(Object node) { JexlNode jexlNode = (JexlNode) node; for (FieldReplacementRule rule : rules) { if (rule.matches(jexlNode)) { - rule.apply(jexlNode); + jexlNode = rule.apply(jexlNode); } } return jexlNode; diff --git a/warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/DirectFieldReplacementRule.java b/warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/DirectFieldReplacementRule.java index 6d31ce5e8aa..fc3ec889419 100644 --- a/warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/DirectFieldReplacementRule.java +++ b/warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/DirectFieldReplacementRule.java @@ -20,8 +20,9 @@ public boolean matches(JexlNode node) { return node instanceof ASTIdentifier && ((ASTIdentifier) node).getName().equals(field); } - public void apply(JexlNode node) { + public JexlNode apply(JexlNode node) { JexlASTHelper.setField(node, replacement); + return node; } public String getField() { diff --git a/warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/FieldReplacementRule.java b/warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/FieldReplacementRule.java index 4eb81270f53..b29daa63d2a 100644 --- a/warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/FieldReplacementRule.java +++ b/warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/FieldReplacementRule.java @@ -5,5 +5,5 @@ public interface FieldReplacementRule { boolean matches(JexlNode node); - void apply(JexlNode node); + JexlNode apply(JexlNode node); } diff --git a/warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/RangeFieldReplacementRule.java b/warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/RangeFieldReplacementRule.java index 59f1b623c64..6af46756be0 100644 --- a/warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/RangeFieldReplacementRule.java +++ b/warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/RangeFieldReplacementRule.java @@ -4,10 +4,10 @@ import datawave.query.jexl.JexlNodeFactory; import datawave.query.jexl.LiteralRange; import datawave.query.jexl.nodes.QueryPropertyMarker; +import datawave.query.jexl.visitors.RebuildingVisitor; import org.apache.commons.jexl3.parser.ASTAndNode; import org.apache.commons.jexl3.parser.ASTIdentifier; import org.apache.commons.jexl3.parser.JexlNode; -import org.apache.commons.jexl3.parser.JexlNodes; import org.apache.commons.jexl3.parser.ParserTreeConstants; import java.util.HashMap; @@ -32,61 +32,41 @@ public boolean matches(JexlNode node) { } @Override - public void apply(JexlNode node) { + public JexlNode apply(JexlNode node) { LiteralRange range = JexlASTHelper.findRange().getRange(node); if (range == null || !fieldMap.containsKey(range.getFieldName())) { - return; + return node; } - // Clone original range node and rename identifier in the clone - JexlNode clonedLower = cloneAndReplaceField(range.getLowerNode(), fieldMap.get(range.getFieldName())); - JexlNode clonedUpper = cloneAndReplaceField(range.getUpperNode(), fieldMap.get(range.getFieldName())); + // Create a copy and mark it as "evaluationOnly" + JexlNode copy = RebuildingVisitor.copy(node); + JexlNode evalNode = QueryPropertyMarker.create(copy, EVALUATION_ONLY); - ASTAndNode newAnd = new ASTAndNode(ParserTreeConstants.JJTANDNODE); - newAnd.jjtAddChild(clonedLower, 0); - newAnd.jjtAddChild(clonedUpper, 1); - // Mark the original node as "evaluationOnly" - QueryPropertyMarker.create(node, EVALUATION_ONLY); + // rename the field for the range nodes + replaceField(range.getLowerNode(), fieldMap.get(range.getFieldName())); + replaceField(range.getUpperNode(), fieldMap.get(range.getFieldName())); - // Replace the original node in-place with a new parent AND node + // Create a top level AND for both the original node and the eval node ASTAndNode topLevel = new ASTAndNode(ParserTreeConstants.JJTANDNODE); - topLevel.jjtAddChild(node, 0); - topLevel.jjtAddChild(newAnd, 1); + topLevel.jjtAddChild(evalNode, 0); + topLevel.jjtAddChild(node, 1); + node.jjtSetParent(topLevel); + evalNode.jjtSetParent(topLevel); - replaceNodeInParent(node, topLevel); + return topLevel; } - private JexlNode cloneAndReplaceField(JexlNode original, String newName) { - try { - JexlNode copy = JexlNodes.newInstanceOfType(original); - - for (int i = 0; i < original.jjtGetNumChildren(); i++) { - JexlNode child = original.jjtGetChild(i); - if (child instanceof ASTIdentifier) { - ASTIdentifier newId = JexlNodeFactory.buildIdentifier(newName); - copy.jjtAddChild(newId, i); - } else { - copy.jjtAddChild(cloneAndReplaceField(child, newName), i); - } - } - return copy; - } catch (Exception e) { - throw new RuntimeException("Failed to clone node: " + original.getClass(), e); - } - } - - private void replaceNodeInParent(JexlNode oldNode, JexlNode newNode) { - JexlNode parent = oldNode.jjtGetParent(); - if (parent == null) - return; // if root node, you’ll need to replace externally todo figure out root nodes - - for (int i = 0; i < parent.jjtGetNumChildren(); i++) { - if (parent.jjtGetChild(i) == oldNode) { - parent.jjtAddChild(newNode, i); - newNode.jjtSetParent(parent); - return; + private void replaceField(JexlNode node, String newName) { + for (int i = 0; i < node.jjtGetNumChildren(); i++) { + JexlNode child = node.jjtGetChild(i); + if (child instanceof ASTIdentifier) { + ASTIdentifier newId = JexlNodeFactory.buildIdentifier(newName); + node.jjtAddChild(newId, i); + newId.jjtSetParent(node); + } else { + replaceField(child, newName); } } } diff --git a/warehouse/query-core/src/test/java/datawave/query/planner/replacement/FieldReplacementVisitorTest.java b/warehouse/query-core/src/test/java/datawave/query/planner/replacement/FieldReplacementVisitorTest.java index f86d0894fcb..5101ccebdfe 100644 --- a/warehouse/query-core/src/test/java/datawave/query/planner/replacement/FieldReplacementVisitorTest.java +++ b/warehouse/query-core/src/test/java/datawave/query/planner/replacement/FieldReplacementVisitorTest.java @@ -1,11 +1,13 @@ package datawave.query.planner.replacement; import datawave.query.jexl.JexlASTHelper; +import datawave.query.jexl.visitors.BaseVisitor; import datawave.query.jexl.visitors.PrintingVisitor; import datawave.query.jexl.visitors.TreeEqualityVisitor; import datawave.query.planner.replacement.rules.DirectFieldReplacementRule; import datawave.query.planner.replacement.rules.FieldReplacementRule; import datawave.query.planner.replacement.rules.RangeFieldReplacementRule; +import org.apache.commons.jexl3.parser.ASTAndNode; import org.apache.commons.jexl3.parser.ASTJexlScript; import org.apache.commons.jexl3.parser.JexlNode; import org.apache.commons.jexl3.parser.ParseException; @@ -20,11 +22,10 @@ public class FieldReplacementVisitorTest { private static final Logger log = Logger.getLogger(FieldReplacementVisitorTest.class); private static final DirectFieldReplacementRule dfrRule = new DirectFieldReplacementRule("HAN", "SOLO"); - private static final Map rangeMap = Map.of("R2", "D2", "STAR", "DESTROYER"); + private static final Map rangeMap = Map.of("R2", "D2", "C3", "PO"); private static final RangeFieldReplacementRule rfrRule = new RangeFieldReplacementRule(rangeMap); - - private void testReplacement(String original, String expected, List rules) throws Exception { + private void testReplacement(String original, String expected, List rules, boolean checkRange) throws Exception { // create a query tree ASTJexlScript originalScript = JexlASTHelper.parseJexlQuery(original); @@ -39,6 +40,11 @@ private void testReplacement(String original, String expected, List= '2' && D2 <= '4'))" ; + // @formatter:on + testReplacement(query, expected, List.of(rfrRule), true); + } + + @Test + public void rangeFieldReplacementWithDecimalsTest() throws Exception { + // @formatter:off + String query = "(_Bounded_ = true) && (R2 >= '2.12' && R2 <= '2.24')"; + String expected = "((_Eval_ = true) && ((_Bounded_ = true) && (R2 >= '2.12' && R2 <= '2.24'))) &&" + + "((_Bounded_ = true) && (D2 >= '2.12' && D2 <= '2.24'))" ; + // @formatter:on + testReplacement(query, expected, List.of(rfrRule), true); + } + + @Test + public void rangeFieldReplacementInLargerQueryTest() throws Exception { + // @formatter:off + String query = "(R2 == '6') || ((_Bounded_ = true) && (R2 >= '2' && R2 <= '4'))"; + String expected = "(R2 == '6') || " + + "(((_Eval_ = true) && ((_Bounded_ = true) && (R2 >= '2' && R2 <= '4'))) && " + + "((_Bounded_ = true) && (D2 >= '2' && D2 <= '4')))" ; + // @formatter:on + testReplacement(query, expected, List.of(rfrRule), true); + } + + @Test + public void rangeFieldReplacementWithMultipleRangesTest() throws Exception { // @formatter:off - String query = "R2 = 2 && ((_Bounded_ = true) && (R2 >= '2' && R2 <= '3'))"; - String expected = "R2 = 2 && ((_Bounded_ = true) && (D2 >= '2' && D2 <= '3'))" ; + String query = "((_Bounded_ = true) && (C3 >= '2' && C3 <= '4')) || ((_Bounded_ = true) && (R2 >= '2' && R2 <= '4'))"; + String expected = "(((_Eval_ = true) && ((_Bounded_ = true) && (C3 >= '2' && C3 <= '4'))) && " + + "((_Bounded_ = true) && (PO >= '2' && PO <= '4'))) || " + + "(((_Eval_ = true) && ((_Bounded_ = true) && (R2 >= '2' && R2 <= '4'))) && " + + "((_Bounded_ = true) && (D2 >= '2' && D2 <= '4')))" ; // @formatter:on - testReplacement(query, expected, List.of(rfrRule)); + testReplacement(query, expected, List.of(rfrRule), true); } @Test - public void regexPushdownTransformRuleTest() throws Exception { + public void directFieldReplacementTest() throws Exception { // @formatter:off String query = "HAN == 'x'"; String expected = "SOLO == 'x'" ; // @formatter:on - testReplacement(query, expected, List.of(dfrRule)); + testReplacement(query, expected, List.of(dfrRule), false); } -// @Test -// public void regexPushdownAnyfieldTransformRuleTest() { -// // @formatter:off -// String query = "BLA == 'x' && " + -// "BLA =~ 'ab.*' && " + -// "BLA =~ 'a.*' && " + -// "BLA =~ 'okregex' && " + -// "_ANYFIELD_ =~ '.*'"; -// String expected = "BLA == 'x' && " + -// "BLA =~ 'ab.*' && " + -// "((_Eval_ = true) && (BLA =~ 'a.*')) && " + -// "BLA =~ 'okregex' && " + -// "((_Eval_ = true) && (_ANYFIELD_ =~ '.*'))"; -// // @formatter:on -// try { -// testPushdown(query, expected); -// fail("Expected anyfield regex pushdown to fail"); -// } catch (Exception e) { -// // ok -// } -// } -// -// @Test -// public void regexSimplifierTransformRuleTest() throws Exception { -// // @formatter:off -// String query = "BLA == '.*?.*?x' && " + -// "BLA =~ 'ab.*.*' && " + -// "BLA !~ 'a.*.*.*.*?.*?' && " + -// "BLA =~ '.*?.*?.*bla.*?.*?blabla' && " + -// "_ANYFIELD_ =~ '.*.*?.*?' && " + -// "filter:excludeRegex(BLA, '.*?.*?.*bla.*?.*?blabla') && " + -// "filter:includeRegex(BLA, '.*?.*?.*bla.*?.*?blabla')"; -// String expected = "BLA == '.*?.*?x' && " + -// "BLA =~ 'ab.*?' && " + -// "BLA !~ 'a.*?' && " + -// "BLA =~ '.*?bla.*?blabla' && " + -// "_ANYFIELD_ =~ '.*?' && " + -// "filter:excludeRegex(BLA, '.*?bla.*?blabla') && " + -// "filter:includeRegex(BLA, '.*?bla.*?blabla')"; -// // @formatter:on -// testSimplify(query, expected); -// } -// -// @Test -// public void regexDotAllTransformRuleTest() throws Exception { -// // @formatter:off -// String query = "BLA == '(\\s|.)*' && " + -// "BLA !~ '(.|\\s)*' && " + -// "BLA =~ '(\\s|.)*word(.|\\s)*' &&" + -// "filter:excludeRegex(BLA, '(\\s|.)*word(.|\\s)*') && " + -// "filter:includeRegex(BLA, '(\\s|.)*word(.|\\s)*')"; -// String expected = "BLA == '(\\s|.)*' && " + -// "BLA !~ '.*' && " + -// "BLA =~ '.*word.*' &&" + -// "filter:excludeRegex(BLA, '.*word.*') && " + -// "filter:includeRegex(BLA, '.*word.*')"; -// // @formatter:on -// testDotall(query, expected); -// } -// -// @Test -// public void skipQueryMarkersTest() throws Exception { -// // @formatter:off -// String query = "BLA == 'x' && " + -// "BLA =~ 'ab.*' && (" + -// "(_Value_ = true) && (BLA =~ 'a.*')) && " + -// "((_Value_ = true) && (BLA =~ 'okregex')) && " + -// "BLA =~ '.*'"; -// String expected = "BLA == 'x' && " + -// "BLA =~ 'ab.*' && " + -// "((_Value_ = true) && (BLA =~ 'a.*')) && " + -// "((_Value_ = true) && (BLA =~ 'okregex')) && " + -// "((_Eval_ = true) && (BLA =~ '.*'))"; -// // @formatter:on -// testPushdown(query, expected); -// } -// -// @Test -// public void depthTest() throws Exception { -// // @formatter:off -// String query = "(((BLA == 'x' && " + -// "BLA =~ 'ab.*' && " + -// "BLA =~ 'a.*') && " + -// "((BLA =~ 'okregex'))) && " + -// "BLA =~ '.*')"; -// String expected = "(((_Eval_ = true) && (BLA =~ '.*')) && " + -// "(((BLA =~ 'okregex')) && " + -// "(((_Eval_ = true) && (BLA =~ 'a.*')) && " + -// "BLA =~ 'ab.*' && " + -// "BLA == 'x')))"; -// // @formatter:on -// testPushdown(query, expected, newArrayList(regexPushdownRule, reverseAndRule)); -// } -// -// @Test -// public void testANDNodeTransform() throws Exception { -// // @formatter:off -// String query = "BLA == 'x' && " + -// "BLA =~ 'ab.*' && " + -// "BLA =~ 'a.*' && " + -// "BLA =~ 'okregex' && " + -// "BLA =~ '.*'"; -// String expected = "((_Eval_ = true) && (BLA =~ '.*')) && " + -// "BLA =~ 'okregex' && " + -// "((_Eval_ = true) && (BLA =~ 'a.*')) && " + -// "BLA =~ 'ab.*' && " + -// "BLA == 'x'"; -// // @formatter:on -// testPushdown(query, expected, newArrayList(regexPushdownRule, reverseAndRule)); -// } -// -// @Test -// public void testTransformOrder() throws Exception { -// // @formatter:off -// String query = "BLA == 'x' && " + -// "BLA =~ 'ab.*' && " + -// "BLA =~ 'a.*' && " + -// "BLA =~ 'okregex' && " + -// "BLA =~ '.*'"; -// String expected1 = "BLA =~ '.*' && " + -// "BLA =~ 'okregex' && " + -// "BLA =~ 'a.*' && " + -// "BLA =~ 'ab.*' && " + -// "BLA == 'x'"; -// String expected2 = "((_Eval_ = true) && (BLA =~ '.*')) && " + -// "BLA =~ 'okregex' && " + -// "((_Eval_ = true) && (BLA =~ 'a.*')) && " + -// "BLA =~ 'ab.*' && " + -// "BLA == 'x'"; -// // @formatter:on -// testPushdown(query, expected1, newArrayList(regexPushdownRule, reverseAndRule, pullUpRule)); -// testPushdown(query, expected2, newArrayList(pullUpRule, reverseAndRule, regexPushdownRule)); -// } + @Test + public void multiRuleReplacementTest() throws Exception { + // @formatter:off + String query = "(HAN = 6) || ((_Bounded_ = true) && (R2 >= '2' && R2 <= '4'))"; + String expected = "(SOLO = 6) || " + + "(((_Eval_ = true) && ((_Bounded_ = true) && (R2 >= '2' && R2 <= '4'))) && " + + "((_Bounded_ = true) && (D2 >= '2' && D2 <= '4')))" ; + // @formatter:on + testReplacement(query, expected, List.of(rfrRule, dfrRule), true); + } + + @Test + public void onlyExactStringsAreReplacedTest() throws Exception { + // @formatter:off + String query = "HAND == 'x'"; + String expected = "HAND == 'x'" ; + // @formatter:on + testReplacement(query, expected, List.of(dfrRule), false); + + // @formatter:off + query = "THAN == 'x'"; + expected = "THAN == 'x'" ; + // @formatter:on + testReplacement(query, expected, List.of(dfrRule), false); + + // @formatter:off + query = "(_Bounded_ = true) && (R21 >= '2' && R21 <= '4')"; + expected = "(_Bounded_ = true) && (R21 >= '2' && R21 <= '4')" ; + // @formatter:on + testReplacement(query, expected, List.of(rfrRule), false); + + // @formatter:off + query = "(_Bounded_ = true) && (RR2 >= '2' && RR2 <= '4')"; + expected = "(_Bounded_ = true) && (RR2 >= '2' && RR2 <= '4')" ; + // @formatter:on + testReplacement(query, expected, List.of(rfrRule), false); + } + + @Test + public void onlyBoundedRangesAreReplacedTest() throws Exception { + // @formatter:off + String query = "(R2 >= '2' && R2 <= '4')"; + String expected = "(R2 >= '2' && R2 <= '4')" ; + // @formatter:on + testReplacement(query, expected, List.of(rfrRule), false); + } + + public class RangeTestVisitor extends BaseVisitor { + private int rangesFound = 0; + + @Override + public Object visit(ASTAndNode node, Object data) { + if (JexlASTHelper.findRange().isRange(node)) { + rangesFound++; + } + return node; + } + + public int getRangesFound() { + return rangesFound; + } + } } From de2dbd4c19a7013873d7555f38748eb6d1357542 Mon Sep 17 00:00:00 2001 From: awrigh11 Date: Thu, 18 Sep 2025 18:20:19 +0000 Subject: [PATCH 3/5] Fixed RangeFieldReplacement Rule by adding in the missing ref expression --- .../replacement/FieldReplacementVisitor.java | 9 +++++---- .../rules/DirectFieldReplacementRule.java | 6 +++--- .../rules/RangeFieldReplacementRule.java | 14 +++++++++----- .../replacement/FieldReplacementVisitorTest.java | 14 +++++++++++--- 4 files changed, 28 insertions(+), 15 deletions(-) diff --git a/warehouse/query-core/src/main/java/datawave/query/planner/replacement/FieldReplacementVisitor.java b/warehouse/query-core/src/main/java/datawave/query/planner/replacement/FieldReplacementVisitor.java index 98fa1c86668..d73b38ab638 100644 --- a/warehouse/query-core/src/main/java/datawave/query/planner/replacement/FieldReplacementVisitor.java +++ b/warehouse/query-core/src/main/java/datawave/query/planner/replacement/FieldReplacementVisitor.java @@ -1,15 +1,16 @@ package datawave.query.planner.replacement; -import datawave.core.common.logging.ThreadConfigurableLogger; -import datawave.query.jexl.visitors.RebuildingVisitor; -import datawave.query.planner.replacement.rules.FieldReplacementRule; +import java.util.List; + import org.apache.commons.jexl3.parser.ASTAndNode; import org.apache.commons.jexl3.parser.ASTIdentifier; import org.apache.commons.jexl3.parser.ASTJexlScript; import org.apache.commons.jexl3.parser.JexlNode; import org.apache.log4j.Logger; -import java.util.List; +import datawave.core.common.logging.ThreadConfigurableLogger; +import datawave.query.jexl.visitors.RebuildingVisitor; +import datawave.query.planner.replacement.rules.FieldReplacementRule; public class FieldReplacementVisitor extends RebuildingVisitor { private static final Logger log = ThreadConfigurableLogger.getLogger(FieldReplacementVisitor.class); diff --git a/warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/DirectFieldReplacementRule.java b/warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/DirectFieldReplacementRule.java index fc3ec889419..5af363e6a44 100644 --- a/warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/DirectFieldReplacementRule.java +++ b/warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/DirectFieldReplacementRule.java @@ -1,15 +1,15 @@ package datawave.query.planner.replacement.rules; -import datawave.query.jexl.JexlASTHelper; import org.apache.commons.jexl3.parser.ASTIdentifier; import org.apache.commons.jexl3.parser.JexlNode; +import datawave.query.jexl.JexlASTHelper; + public class DirectFieldReplacementRule implements FieldReplacementRule { private String field = null; private String replacement = null; - public DirectFieldReplacementRule() { - } + public DirectFieldReplacementRule() {} public DirectFieldReplacementRule(String field, String replacement) { this.field = field; diff --git a/warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/RangeFieldReplacementRule.java b/warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/RangeFieldReplacementRule.java index 6af46756be0..9f1bea5987d 100644 --- a/warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/RangeFieldReplacementRule.java +++ b/warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/RangeFieldReplacementRule.java @@ -7,7 +7,9 @@ import datawave.query.jexl.visitors.RebuildingVisitor; import org.apache.commons.jexl3.parser.ASTAndNode; import org.apache.commons.jexl3.parser.ASTIdentifier; +import org.apache.commons.jexl3.parser.ASTReferenceExpression; import org.apache.commons.jexl3.parser.JexlNode; +import org.apache.commons.jexl3.parser.JexlNodes; import org.apache.commons.jexl3.parser.ParserTreeConstants; import java.util.HashMap; @@ -43,17 +45,19 @@ public JexlNode apply(JexlNode node) { JexlNode copy = RebuildingVisitor.copy(node); JexlNode evalNode = QueryPropertyMarker.create(copy, EVALUATION_ONLY); - // rename the field for the range nodes replaceField(range.getLowerNode(), fieldMap.get(range.getFieldName())); replaceField(range.getUpperNode(), fieldMap.get(range.getFieldName())); - // Create a top level AND for both the original node and the eval node + // Create a Reference Expression for the original node and top level AND for both the ref and the eval node + ASTReferenceExpression ref = JexlNodes.makeRefExp(); ASTAndNode topLevel = new ASTAndNode(ParserTreeConstants.JJTANDNODE); - topLevel.jjtAddChild(evalNode, 0); - topLevel.jjtAddChild(node, 1); - node.jjtSetParent(topLevel); + node.jjtSetParent(ref); + ref.jjtSetParent(topLevel); evalNode.jjtSetParent(topLevel); + ref.jjtAddChild(node, 0); + topLevel.jjtAddChild(evalNode, 0); + topLevel.jjtAddChild(ref, 1); return topLevel; } diff --git a/warehouse/query-core/src/test/java/datawave/query/planner/replacement/FieldReplacementVisitorTest.java b/warehouse/query-core/src/test/java/datawave/query/planner/replacement/FieldReplacementVisitorTest.java index 5101ccebdfe..d6308616f87 100644 --- a/warehouse/query-core/src/test/java/datawave/query/planner/replacement/FieldReplacementVisitorTest.java +++ b/warehouse/query-core/src/test/java/datawave/query/planner/replacement/FieldReplacementVisitorTest.java @@ -12,6 +12,7 @@ import org.apache.commons.jexl3.parser.JexlNode; import org.apache.commons.jexl3.parser.ParseException; import org.apache.log4j.Logger; +import org.junit.Assert; import org.junit.Test; import java.util.List; @@ -41,8 +42,15 @@ private void testReplacement(String original, String expected, List 0); } } @@ -160,7 +168,7 @@ public void onlyBoundedRangesAreReplacedTest() throws Exception { testReplacement(query, expected, List.of(rfrRule), false); } - public class RangeTestVisitor extends BaseVisitor { + public class RangeVerificationVisitor extends BaseVisitor { private int rangesFound = 0; @Override From bd847644581dfce7894f5c92c26aa01ed3b48d11 Mon Sep 17 00:00:00 2001 From: awrigh11 Date: Mon, 22 Sep 2025 17:10:03 +0000 Subject: [PATCH 4/5] Fixed issue with unit test not fully evaluating ranges --- .../rules/RangeFieldReplacementRule.java | 30 +++++++++---------- .../FieldReplacementVisitorTest.java | 7 ++--- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/RangeFieldReplacementRule.java b/warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/RangeFieldReplacementRule.java index 9f1bea5987d..843b45a80c9 100644 --- a/warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/RangeFieldReplacementRule.java +++ b/warehouse/query-core/src/main/java/datawave/query/planner/replacement/rules/RangeFieldReplacementRule.java @@ -1,10 +1,10 @@ package datawave.query.planner.replacement.rules; -import datawave.query.jexl.JexlASTHelper; -import datawave.query.jexl.JexlNodeFactory; -import datawave.query.jexl.LiteralRange; -import datawave.query.jexl.nodes.QueryPropertyMarker; -import datawave.query.jexl.visitors.RebuildingVisitor; +import static datawave.query.jexl.nodes.QueryPropertyMarker.MarkerType.EVALUATION_ONLY; + +import java.util.HashMap; +import java.util.Map; + import org.apache.commons.jexl3.parser.ASTAndNode; import org.apache.commons.jexl3.parser.ASTIdentifier; import org.apache.commons.jexl3.parser.ASTReferenceExpression; @@ -12,18 +12,18 @@ import org.apache.commons.jexl3.parser.JexlNodes; import org.apache.commons.jexl3.parser.ParserTreeConstants; -import java.util.HashMap; -import java.util.Map; - -import static datawave.query.jexl.nodes.QueryPropertyMarker.MarkerType.EVALUATION_ONLY; +import datawave.query.jexl.JexlASTHelper; +import datawave.query.jexl.JexlNodeFactory; +import datawave.query.jexl.LiteralRange; +import datawave.query.jexl.nodes.QueryPropertyMarker; +import datawave.query.jexl.visitors.RebuildingVisitor; public class RangeFieldReplacementRule implements FieldReplacementRule { - private Map fieldMap = new HashMap<>(); + private Map fieldMap = new HashMap<>(); - public RangeFieldReplacementRule() { - } + public RangeFieldReplacementRule() {} - public RangeFieldReplacementRule(Map fieldMap) { + public RangeFieldReplacementRule(Map fieldMap) { this.fieldMap = fieldMap; } @@ -75,11 +75,11 @@ private void replaceField(JexlNode node, String newName) { } } - public void setFieldMap(Map fieldMap) { + public void setFieldMap(Map fieldMap) { this.fieldMap = fieldMap; } - public Map getFieldMap() { + public Map getFieldMap() { return fieldMap; } } diff --git a/warehouse/query-core/src/test/java/datawave/query/planner/replacement/FieldReplacementVisitorTest.java b/warehouse/query-core/src/test/java/datawave/query/planner/replacement/FieldReplacementVisitorTest.java index d6308616f87..794389c4639 100644 --- a/warehouse/query-core/src/test/java/datawave/query/planner/replacement/FieldReplacementVisitorTest.java +++ b/warehouse/query-core/src/test/java/datawave/query/planner/replacement/FieldReplacementVisitorTest.java @@ -1,8 +1,8 @@ package datawave.query.planner.replacement; import datawave.query.jexl.JexlASTHelper; -import datawave.query.jexl.visitors.BaseVisitor; import datawave.query.jexl.visitors.PrintingVisitor; +import datawave.query.jexl.visitors.RebuildingVisitor; import datawave.query.jexl.visitors.TreeEqualityVisitor; import datawave.query.planner.replacement.rules.DirectFieldReplacementRule; import datawave.query.planner.replacement.rules.FieldReplacementRule; @@ -50,7 +50,6 @@ private void testReplacement(String original, String expected, List 0); } } @@ -168,7 +167,7 @@ public void onlyBoundedRangesAreReplacedTest() throws Exception { testReplacement(query, expected, List.of(rfrRule), false); } - public class RangeVerificationVisitor extends BaseVisitor { + public class RangeVerificationVisitor extends RebuildingVisitor { private int rangesFound = 0; @Override @@ -176,7 +175,7 @@ public Object visit(ASTAndNode node, Object data) { if (JexlASTHelper.findRange().isRange(node)) { rangesFound++; } - return node; + return super.visit(node, data); } public int getRangesFound() { From 51e5b64688e0a369f974aa147670f5a2f90f468e Mon Sep 17 00:00:00 2001 From: awrigh11 Date: Thu, 25 Sep 2025 20:13:25 +0000 Subject: [PATCH 5/5] Updated unit tests --- .../FieldReplacementVisitorTest.java | 89 ++++++++++--------- 1 file changed, 45 insertions(+), 44 deletions(-) diff --git a/warehouse/query-core/src/test/java/datawave/query/planner/replacement/FieldReplacementVisitorTest.java b/warehouse/query-core/src/test/java/datawave/query/planner/replacement/FieldReplacementVisitorTest.java index 794389c4639..f63f742af82 100644 --- a/warehouse/query-core/src/test/java/datawave/query/planner/replacement/FieldReplacementVisitorTest.java +++ b/warehouse/query-core/src/test/java/datawave/query/planner/replacement/FieldReplacementVisitorTest.java @@ -1,12 +1,10 @@ package datawave.query.planner.replacement; -import datawave.query.jexl.JexlASTHelper; -import datawave.query.jexl.visitors.PrintingVisitor; -import datawave.query.jexl.visitors.RebuildingVisitor; -import datawave.query.jexl.visitors.TreeEqualityVisitor; -import datawave.query.planner.replacement.rules.DirectFieldReplacementRule; -import datawave.query.planner.replacement.rules.FieldReplacementRule; -import datawave.query.planner.replacement.rules.RangeFieldReplacementRule; +import static org.junit.Assert.assertTrue; + +import java.util.List; +import java.util.Map; + import org.apache.commons.jexl3.parser.ASTAndNode; import org.apache.commons.jexl3.parser.ASTJexlScript; import org.apache.commons.jexl3.parser.JexlNode; @@ -15,15 +13,18 @@ import org.junit.Assert; import org.junit.Test; -import java.util.List; -import java.util.Map; - -import static org.junit.Assert.assertTrue; +import datawave.query.jexl.JexlASTHelper; +import datawave.query.jexl.visitors.PrintingVisitor; +import datawave.query.jexl.visitors.RebuildingVisitor; +import datawave.query.jexl.visitors.TreeEqualityVisitor; +import datawave.query.planner.replacement.rules.DirectFieldReplacementRule; +import datawave.query.planner.replacement.rules.FieldReplacementRule; +import datawave.query.planner.replacement.rules.RangeFieldReplacementRule; public class FieldReplacementVisitorTest { private static final Logger log = Logger.getLogger(FieldReplacementVisitorTest.class); - private static final DirectFieldReplacementRule dfrRule = new DirectFieldReplacementRule("HAN", "SOLO"); - private static final Map rangeMap = Map.of("R2", "D2", "C3", "PO"); + private static final DirectFieldReplacementRule dfrRule = new DirectFieldReplacementRule("ABC", "XYZ"); + private static final Map rangeMap = Map.of("AA", "BB", "CC", "DD"); private static final RangeFieldReplacementRule rfrRule = new RangeFieldReplacementRule(rangeMap); private void testReplacement(String original, String expected, List rules, boolean checkRange) throws Exception { @@ -71,9 +72,9 @@ private void assertLineage(JexlNode node) { @Test public void rangeFieldReplacementTest() throws Exception { // @formatter:off - String query = "(_Bounded_ = true) && (R2 >= '2' && R2 <= '4')"; - String expected = "((_Eval_ = true) && ((_Bounded_ = true) && (R2 >= '2' && R2 <= '4'))) && " + - "((_Bounded_ = true) && (D2 >= '2' && D2 <= '4'))" ; + String query = "(_Bounded_ = true) && (AA >= '2' && AA <= '4')"; + String expected = "((_Eval_ = true) && ((_Bounded_ = true) && (AA >= '2' && AA <= '4'))) && " + + "((_Bounded_ = true) && (BB >= '2' && BB <= '4'))" ; // @formatter:on testReplacement(query, expected, List.of(rfrRule), true); } @@ -81,9 +82,9 @@ public void rangeFieldReplacementTest() throws Exception { @Test public void rangeFieldReplacementWithDecimalsTest() throws Exception { // @formatter:off - String query = "(_Bounded_ = true) && (R2 >= '2.12' && R2 <= '2.24')"; - String expected = "((_Eval_ = true) && ((_Bounded_ = true) && (R2 >= '2.12' && R2 <= '2.24'))) &&" + - "((_Bounded_ = true) && (D2 >= '2.12' && D2 <= '2.24'))" ; + String query = "(_Bounded_ = true) && (AA >= '2.12' && AA <= '2.24')"; + String expected = "((_Eval_ = true) && ((_Bounded_ = true) && (AA >= '2.12' && AA <= '2.24'))) &&" + + "((_Bounded_ = true) && (BB >= '2.12' && BB <= '2.24'))" ; // @formatter:on testReplacement(query, expected, List.of(rfrRule), true); } @@ -91,10 +92,10 @@ public void rangeFieldReplacementWithDecimalsTest() throws Exception { @Test public void rangeFieldReplacementInLargerQueryTest() throws Exception { // @formatter:off - String query = "(R2 == '6') || ((_Bounded_ = true) && (R2 >= '2' && R2 <= '4'))"; - String expected = "(R2 == '6') || " + - "(((_Eval_ = true) && ((_Bounded_ = true) && (R2 >= '2' && R2 <= '4'))) && " + - "((_Bounded_ = true) && (D2 >= '2' && D2 <= '4')))" ; + String query = "(AA == '6') || ((_Bounded_ = true) && (AA >= '2' && AA <= '4'))"; + String expected = "(AA == '6') || " + + "(((_Eval_ = true) && ((_Bounded_ = true) && (AA >= '2' && AA <= '4'))) && " + + "((_Bounded_ = true) && (BB >= '2' && BB <= '4')))" ; // @formatter:on testReplacement(query, expected, List.of(rfrRule), true); } @@ -102,11 +103,11 @@ public void rangeFieldReplacementInLargerQueryTest() throws Exception { @Test public void rangeFieldReplacementWithMultipleRangesTest() throws Exception { // @formatter:off - String query = "((_Bounded_ = true) && (C3 >= '2' && C3 <= '4')) || ((_Bounded_ = true) && (R2 >= '2' && R2 <= '4'))"; - String expected = "(((_Eval_ = true) && ((_Bounded_ = true) && (C3 >= '2' && C3 <= '4'))) && " + - "((_Bounded_ = true) && (PO >= '2' && PO <= '4'))) || " + - "(((_Eval_ = true) && ((_Bounded_ = true) && (R2 >= '2' && R2 <= '4'))) && " + - "((_Bounded_ = true) && (D2 >= '2' && D2 <= '4')))" ; + String query = "((_Bounded_ = true) && (CC >= '2' && CC <= '4')) || ((_Bounded_ = true) && (AA >= '2' && AA <= '4'))"; + String expected = "(((_Eval_ = true) && ((_Bounded_ = true) && (CC >= '2' && CC <= '4'))) && " + + "((_Bounded_ = true) && (DD >= '2' && DD <= '4'))) || " + + "(((_Eval_ = true) && ((_Bounded_ = true) && (AA >= '2' && AA <= '4'))) && " + + "((_Bounded_ = true) && (BB >= '2' && BB <= '4')))" ; // @formatter:on testReplacement(query, expected, List.of(rfrRule), true); } @@ -114,8 +115,8 @@ public void rangeFieldReplacementWithMultipleRangesTest() throws Exception { @Test public void directFieldReplacementTest() throws Exception { // @formatter:off - String query = "HAN == 'x'"; - String expected = "SOLO == 'x'" ; + String query = "ABC == 'x'"; + String expected = "XYZ == 'x'" ; // @formatter:on testReplacement(query, expected, List.of(dfrRule), false); } @@ -123,10 +124,10 @@ public void directFieldReplacementTest() throws Exception { @Test public void multiRuleReplacementTest() throws Exception { // @formatter:off - String query = "(HAN = 6) || ((_Bounded_ = true) && (R2 >= '2' && R2 <= '4'))"; - String expected = "(SOLO = 6) || " + - "(((_Eval_ = true) && ((_Bounded_ = true) && (R2 >= '2' && R2 <= '4'))) && " + - "((_Bounded_ = true) && (D2 >= '2' && D2 <= '4')))" ; + String query = "(ABC = 6) || ((_Bounded_ = true) && (AA >= '2' && AA <= '4'))"; + String expected = "(XYZ = 6) || " + + "(((_Eval_ = true) && ((_Bounded_ = true) && (AA >= '2' && AA <= '4'))) && " + + "((_Bounded_ = true) && (BB >= '2' && BB <= '4')))" ; // @formatter:on testReplacement(query, expected, List.of(rfrRule, dfrRule), true); } @@ -134,26 +135,26 @@ public void multiRuleReplacementTest() throws Exception { @Test public void onlyExactStringsAreReplacedTest() throws Exception { // @formatter:off - String query = "HAND == 'x'"; - String expected = "HAND == 'x'" ; + String query = "ABCD == 'x'"; + String expected = "ABCD == 'x'" ; // @formatter:on testReplacement(query, expected, List.of(dfrRule), false); // @formatter:off - query = "THAN == 'x'"; - expected = "THAN == 'x'" ; + query = "TABC == 'x'"; + expected = "TABC == 'x'" ; // @formatter:on testReplacement(query, expected, List.of(dfrRule), false); // @formatter:off - query = "(_Bounded_ = true) && (R21 >= '2' && R21 <= '4')"; - expected = "(_Bounded_ = true) && (R21 >= '2' && R21 <= '4')" ; + query = "(_Bounded_ = true) && (AA1 >= '2' && AA1 <= '4')"; + expected = "(_Bounded_ = true) && (AA1 >= '2' && AA1 <= '4')" ; // @formatter:on testReplacement(query, expected, List.of(rfrRule), false); // @formatter:off - query = "(_Bounded_ = true) && (RR2 >= '2' && RR2 <= '4')"; - expected = "(_Bounded_ = true) && (RR2 >= '2' && RR2 <= '4')" ; + query = "(_Bounded_ = true) && (RAA >= '2' && RAA <= '4')"; + expected = "(_Bounded_ = true) && (RAA >= '2' && RAA <= '4')" ; // @formatter:on testReplacement(query, expected, List.of(rfrRule), false); } @@ -161,8 +162,8 @@ public void onlyExactStringsAreReplacedTest() throws Exception { @Test public void onlyBoundedRangesAreReplacedTest() throws Exception { // @formatter:off - String query = "(R2 >= '2' && R2 <= '4')"; - String expected = "(R2 >= '2' && R2 <= '4')" ; + String query = "(AA >= '2' && AA <= '4')"; + String expected = "(AA >= '2' && AA <= '4')" ; // @formatter:on testReplacement(query, expected, List.of(rfrRule), false); }