TEIID-5045 fixing modification of true criteria

teiid · Aug 30, 2017 · 5e8f4ad · 5e8f4ad
1 parent abd6751
commit 5e8f4ad
Show file tree

Hide file tree

Showing 3 changed files with 83 additions and 21 deletions.
diff --git a/engine/src/main/java/org/teiid/query/rewriter/QueryRewriter.java b/engine/src/main/java/org/teiid/query/rewriter/QueryRewriter.java
@@ -106,13 +106,9 @@ public class QueryRewriter {
     private static final String WRITE_THROUGH = "write-through"; //$NON-NLS-1$
 
     private static final Constant ZERO_CONSTANT = new Constant(0, DataTypeManager.DefaultDataClasses.INTEGER);
-	public static final CompareCriteria TRUE_CRITERIA = new CompareCriteria(new Constant(1, DataTypeManager.DefaultDataClasses.INTEGER), CompareCriteria.EQ, new Constant(1, DataTypeManager.DefaultDataClasses.INTEGER));
-    public static final CompareCriteria FALSE_CRITERIA = new CompareCriteria(new Constant(1, DataTypeManager.DefaultDataClasses.INTEGER), CompareCriteria.EQ, ZERO_CONSTANT) {
-    	public void setOptional(Boolean isOptional) {};
-    };
-    public static final CompareCriteria UNKNOWN_CRITERIA = new CompareCriteria(new Constant(null, DataTypeManager.DefaultDataClasses.STRING), CompareCriteria.NE, new Constant(null, DataTypeManager.DefaultDataClasses.STRING)) {
-    	public void setOptional(Boolean isOptional) {};
-    };
+	public static final CompareCriteria TRUE_CRITERIA = new ImmutableCompareCriteria(new Constant(1, DataTypeManager.DefaultDataClasses.INTEGER), CompareCriteria.EQ, new Constant(1, DataTypeManager.DefaultDataClasses.INTEGER));
+    public static final CompareCriteria FALSE_CRITERIA = new ImmutableCompareCriteria(new Constant(1, DataTypeManager.DefaultDataClasses.INTEGER), CompareCriteria.EQ, ZERO_CONSTANT);
+    public static final CompareCriteria UNKNOWN_CRITERIA = new ImmutableCompareCriteria(new Constant(null, DataTypeManager.DefaultDataClasses.STRING), CompareCriteria.NE, new Constant(null, DataTypeManager.DefaultDataClasses.STRING));
 
     private static final Map<String, String> ALIASED_FUNCTIONS = new TreeMap<String, String>(String.CASE_INSENSITIVE_ORDER);
     private static final Set<String> PARSE_FORMAT_TYPES = new TreeSet<String>(String.CASE_INSENSITIVE_ORDER);
@@ -1358,29 +1354,25 @@ private Criteria rewriteCriteria(NotCriteria criteria) throws TeiidComponentExce
         	//reduce to only negation of predicates, so that the null/unknown handling criteria is applied appropriately
     		return rewriteCriteria(Criteria.applyDemorgan(innerCrit));
         } 
-        if (innerCrit instanceof Negatable) {
-        	((Negatable) innerCrit).negate();
-        	return rewriteCriteria(innerCrit);
-        }
-        if (innerCrit instanceof NotCriteria) {
-        	return rewriteCriteria(((NotCriteria)innerCrit).getCriteria());
+        if(innerCrit == TRUE_CRITERIA) {
+            return FALSE_CRITERIA;
+        } else if(innerCrit == FALSE_CRITERIA) {
+            return TRUE_CRITERIA;
+        } else if (innerCrit == UNKNOWN_CRITERIA) {
+            return UNKNOWN_CRITERIA;
         }
-        innerCrit = rewriteCriteria(innerCrit);
         if (innerCrit instanceof Negatable) {
         	((Negatable) innerCrit).negate();
         	return rewriteCriteria(innerCrit);
         }
         if (innerCrit instanceof NotCriteria) {
         	return rewriteCriteria(((NotCriteria)innerCrit).getCriteria());
         }
-        if(innerCrit == TRUE_CRITERIA) {
-            return FALSE_CRITERIA;
-        } else if(innerCrit == FALSE_CRITERIA) {
-            return TRUE_CRITERIA;
-        } else if (innerCrit == UNKNOWN_CRITERIA) {
-            return UNKNOWN_CRITERIA;
+        Criteria newInnerCrit = rewriteCriteria(innerCrit);
+        if (!newInnerCrit.equals(innerCrit)) {
+            criteria.setCriteria(newInnerCrit);
+            return rewriteCriteria(criteria);
         }
-        criteria.setCriteria(innerCrit);
         return criteria;
 	}
 
@@ -1405,6 +1397,9 @@ private Criteria rewriteCriteria(BetweenCriteria criteria) throws TeiidComponent
     }
 
 	private Criteria rewriteCriteria(CompareCriteria criteria) throws TeiidComponentException, TeiidProcessingException{
+	    if (criteria == TRUE_CRITERIA || criteria == UNKNOWN_CRITERIA || criteria == FALSE_CRITERIA) {
+	        return criteria;
+	    }
 		Expression leftExpr = rewriteExpressionDirect(criteria.getLeftExpression());
 		Expression rightExpr = rewriteExpressionDirect(criteria.getRightExpression());
 		criteria.setLeftExpression(leftExpr);

diff --git a/engine/src/main/java/org/teiid/query/sql/lang/ImmutableCompareCriteria.java b/engine/src/main/java/org/teiid/query/sql/lang/ImmutableCompareCriteria.java
@@ -0,0 +1,58 @@
+/*
+ * Copyright Red Hat, Inc. and/or its affiliates
+ * and other contributors as indicated by the @author tags and
+ * the COPYRIGHT.txt file distributed with this work.
+ *
+ * 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 org.teiid.query.sql.lang;
+
+import org.teiid.core.TeiidRuntimeException;
+import org.teiid.core.util.EquivalenceUtil;
+import org.teiid.query.sql.symbol.Expression;
+
+public class ImmutableCompareCriteria extends CompareCriteria {
+
+    public ImmutableCompareCriteria(Expression leftExpression, int operator, Expression rightExpression) {
+        super.setLeftExpression(leftExpression);
+        super.setOperator(operator);
+        super.setRightExpression(rightExpression);
+    }
+
+    @Override
+    public void setLeftExpression(Expression expression) {
+        if (!EquivalenceUtil.areEqual(expression, getLeftExpression())) {
+            throw new TeiidRuntimeException("Attempt to modify immutable object"); //$NON-NLS-1$
+        }
+    }
+
+    @Override
+    public void setRightExpression(Expression expression) {
+        if (!EquivalenceUtil.areEqual(expression, getRightExpression())) {
+            throw new TeiidRuntimeException("Attempt to modify immutable object"); //$NON-NLS-1$
+        }
+    }
+
+    public void setOperator(int operator) {
+        if (!EquivalenceUtil.areEqual(operator, getOperator())) {
+            throw new TeiidRuntimeException("Attempt to modify immutable object"); //$NON-NLS-1$
+        }
+    };
+
+    @Override
+    public void setOptional(Boolean isOptional) {
+
+    }
+
+}
diff --git a/engine/src/test/java/org/teiid/query/rewriter/TestQueryRewriter.java b/engine/src/test/java/org/teiid/query/rewriter/TestQueryRewriter.java
@@ -1661,6 +1661,15 @@ private void addTestData() {
     	helpTestRewriteCriteria("pm1.g1.e3", "pm1.g1.e3 = true");
     }
 
+    @Test public void testRewriteExpressionCriteriaBooleanLiterals() {
+        helpTestRewriteCriteria("not(true)", "1 = 0"); //$NON-NLS-1$ //$NON-NLS-2$
+        helpTestRewriteCriteria("not(true)", "1 = 0"); //$NON-NLS-1$ //$NON-NLS-2$
+        helpTestRewriteCriteria("not(false)", "1 = 1"); //$NON-NLS-1$ //$NON-NLS-2$
+        helpTestRewriteCriteria("not(false)", "1 = 1"); //$NON-NLS-1$ //$NON-NLS-2$
+        helpTestRewriteCriteria("not(unknown)", "null <> null"); //$NON-NLS-1$ //$NON-NLS-2$
+        helpTestRewriteCriteria("not(unknown)", "null <> null"); //$NON-NLS-1$ //$NON-NLS-2$
+    }
+
     @Test public void testRewritePredicateOptimization() throws Exception {
     	helpTestRewriteCriteria("pm1.g1.e2 in (1, 2, 3) and pm1.g1.e2 in (2, 3, 4)", "pm1.g1.e2 in (2, 3)");
     }