Skip to content

Commit

Permalink
TEIID-2633 ensuring that copy criteria with transitive joins only
Browse files Browse the repository at this point in the history 
retains necessary join criteria
  • Loading branch information
@shawkins
shawkins committed Aug 21, 2013
1 parent 173a767 commit e9b2388
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 33 deletions.
9 changes: 6 additions & 3 deletions engine/src/main/java/org/teiid/query/optimizer/relational/RelationalPlanner.java
View file
Expand Up @@ -524,9 +524,13 @@ public RuleStack buildRules() {
rules.push(RuleConstants.PUSH_SELECT_CRITERIA);
rules.push(RuleConstants.PLAN_JOINS);
}
if(hints.hasJoin) {
rules.push(RuleConstants.CLEAN_CRITERIA);
rules.push(RuleConstants.COPY_CRITERIA);
}
rules.push(RuleConstants.RAISE_ACCESS);
if (hints.hasFunctionBasedColumns) {
rules.push(RuleConstants.SUBSTITUE_EXPRESSIONS);
rules.push(RuleConstants.SUBSTITUTE_EXPRESSIONS);
}
if (hints.hasSetQuery) {
rules.push(RuleConstants.PLAN_UNIONS);
Expand All @@ -537,8 +541,7 @@ public RuleStack buildRules() {
rules.push(RuleConstants.CLEAN_CRITERIA);
}
if(hints.hasJoin) {
rules.push(RuleConstants.COPY_CRITERIA);
rules.push(RuleConstants.PUSH_NON_JOIN_CRITERIA);
rules.push(RuleConstants.PUSH_NON_JOIN_CRITERIA);
}
if(hints.hasVirtualGroups) {
rules.push(RuleConstants.MERGE_VIRTUAL);
Expand Down
17 changes: 9 additions & 8 deletions engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleCleanCriteria.java
View file
Expand Up @@ -55,7 +55,7 @@ public PlanNode execute(PlanNode plan, QueryMetadataInterface metadata, Capabili

boolean pushRaiseNull = false;

pushRaiseNull = clean(plan);
pushRaiseNull = clean(plan, !rules.contains(RuleConstants.COPY_CRITERIA));

if (pushRaiseNull) {
rules.push(RuleConstants.RAISE_NULL);
Expand All @@ -64,23 +64,24 @@ public PlanNode execute(PlanNode plan, QueryMetadataInterface metadata, Capabili
return plan;
}

private boolean clean(PlanNode plan)
private boolean clean(PlanNode plan, boolean removeAllPhantom)
throws TeiidComponentException {
boolean pushRaiseNull = false;
plan.setProperty(Info.OUTPUT_COLS, null);
for (PlanNode node : plan.getChildren()) {
pushRaiseNull |= clean(node);
pushRaiseNull |= clean(node, removeAllPhantom);
}
if (plan.getType() == NodeConstants.Types.SELECT) {
pushRaiseNull |= cleanCriteria(plan);
pushRaiseNull |= cleanCriteria(plan, removeAllPhantom);
}
return pushRaiseNull;
}

boolean cleanCriteria(PlanNode critNode) throws TeiidComponentException {
if (critNode.hasBooleanProperty(NodeConstants.Info.IS_PHANTOM)) {
NodeEditor.removeChildNode(critNode.getParent(), critNode);
return false;
boolean cleanCriteria(PlanNode critNode, boolean removeAllPhantom) throws TeiidComponentException {
if (critNode.hasBooleanProperty(NodeConstants.Info.IS_PHANTOM)
&& (removeAllPhantom || critNode.hasBooleanProperty(Info.IS_COPIED))) {
NodeEditor.removeChildNode(critNode.getParent(), critNode);
return false;
}

//TODO: remove dependent set criteria that has not been meaningfully pushed from its parent join
Expand Down
2 changes: 1 addition & 1 deletion engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleConstants.java
View file
Expand Up @@ -50,5 +50,5 @@ private RuleConstants() { }
public static final OptimizerRule CALCULATE_COST = new RuleCalculateCost();
public static final OptimizerRule PLAN_SORTS = new RulePlanSorts();
public static final OptimizerRule DECOMPOSE_JOIN = new RuleDecomposeJoin();
public static final OptimizerRule SUBSTITUE_EXPRESSIONS = new RuleSubstituteExpressions();
public static final OptimizerRule SUBSTITUTE_EXPRESSIONS = new RuleSubstituteExpressions();
}
51 changes: 37 additions & 14 deletions engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleCopyCriteria.java
View file
Expand Up @@ -96,9 +96,10 @@ public PlanNode execute(PlanNode plan, QueryMetadataInterface metadata, Capabili
return plan;
}

if (tryToCopy(plan, new Set[2], metadata)) {
if (tryToCopy(plan, new Set[2], metadata, false)) {
//Push any newly created criteria nodes and try to copy them afterwards
rules.push(RuleConstants.COPY_CRITERIA);
rules.push(RuleConstants.RAISE_ACCESS);
rules.push(RuleConstants.PUSH_NON_JOIN_CRITERIA);
}

Expand Down Expand Up @@ -127,7 +128,8 @@ private boolean copyCriteria(Criteria crit,
List<Criteria> joinCriteria,
Set<Criteria> combinedCriteria,
boolean checkForGroupReduction,
QueryMetadataInterface metadata) {
QueryMetadataInterface metadata,
boolean underAccess) {
int startGroups = GroupsUsedByElementsVisitor.getGroups(crit).size();

Criteria tgtCrit = (Criteria) crit.clone();
Expand All @@ -153,8 +155,28 @@ private boolean copyCriteria(Criteria crit,
return false;
}

boolean isNew = combinedCriteria.add(tgtCrit);

if (underAccess) {
boolean use = false;
if (isNew && !checkForGroupReduction) {
if (endGroups == 1) {
use = true;
} else if (tgtCrit instanceof CompareCriteria) {
CompareCriteria cc = (CompareCriteria)tgtCrit;
int leftGroups = GroupsUsedByElementsVisitor.getGroups(cc.getLeftExpression()).size();
if (leftGroups == endGroups || leftGroups == 0) {
use = true;
}
}
}
if (!use) {
return false;
}
}

//if this is unique or it a duplicate but reduced a current join conjunct, return true
if (combinedCriteria.add(tgtCrit)) {
if (isNew) {
joinCriteria.add(tgtCrit);
return true;
} else if (checkForGroupReduction) {
Expand All @@ -172,7 +194,7 @@ private boolean copyCriteria(Criteria crit,
* @param node
* @return true if criteria has been created
*/
private boolean tryToCopy(PlanNode node, Set<Criteria>[] criteriaInfo, QueryMetadataInterface metadata) {
private boolean tryToCopy(PlanNode node, Set<Criteria>[] criteriaInfo, QueryMetadataInterface metadata, boolean underAccess) {
boolean changedTree = false;

if (node == null) {
Expand All @@ -184,14 +206,14 @@ private boolean tryToCopy(PlanNode node, Set<Criteria>[] criteriaInfo, QueryMeta
JoinType jt = (JoinType)node.getProperty(NodeConstants.Info.JOIN_TYPE);

if (jt == JoinType.JOIN_FULL_OUTER) {
return visitChildern(node, criteriaInfo, changedTree, metadata);
return visitChildern(node, criteriaInfo, changedTree, metadata, underAccess);
}

Set<Criteria>[] leftChildCriteria = new Set[2];
Set<Criteria>[] rightChildCriteria = new Set[2];

changedTree |= tryToCopy(node.getFirstChild(), leftChildCriteria, metadata);
changedTree |= tryToCopy(node.getLastChild(), rightChildCriteria, metadata);
changedTree |= tryToCopy(node.getFirstChild(), leftChildCriteria, metadata, underAccess);
changedTree |= tryToCopy(node.getLastChild(), rightChildCriteria, metadata, underAccess);

List<Criteria> joinCrits = (List<Criteria>) node.getProperty(NodeConstants.Info.JOIN_CRITERIA);
Set<Criteria> combinedCriteria = null;
Expand Down Expand Up @@ -227,11 +249,11 @@ private boolean tryToCopy(PlanNode node, Set<Criteria>[] criteriaInfo, QueryMeta

if (!toCopy.isEmpty()) {

changedTree |= createCriteria(false, toCopy, combinedCriteria, srcToTgt, newJoinCrits, metadata);
changedTree |= createCriteria(false, toCopy, combinedCriteria, srcToTgt, newJoinCrits, metadata, underAccess);

srcToTgt = buildElementMap(allCriteria, null, null, metadata);

changedTree |= createCriteria(true, joinCrits, combinedCriteria, srcToTgt, newJoinCrits, metadata);
changedTree |= createCriteria(true, joinCrits, combinedCriteria, srcToTgt, newJoinCrits, metadata, underAccess);
}

joinCrits.addAll(newJoinCrits);
Expand All @@ -254,7 +276,7 @@ private boolean tryToCopy(PlanNode node, Set<Criteria>[] criteriaInfo, QueryMeta
return changedTree;
}

changedTree = visitChildern(node, criteriaInfo, changedTree, metadata);
changedTree = visitChildern(node, criteriaInfo, changedTree, metadata, underAccess);

//visit select nodes on the way back up
switch (node.getType()) {
Expand Down Expand Up @@ -292,7 +314,7 @@ private boolean createCriteria(boolean copyingJoinCriteria, Collection<Criteria>
Set<Criteria> combinedCriteria,
Map<Expression, Expression> srcToTgt,
List<Criteria> newJoinCrits,
QueryMetadataInterface metadata) {
QueryMetadataInterface metadata, boolean underAccess) {
boolean changedTree = false;
if (srcToTgt.size() == 0) {
return changedTree;
Expand All @@ -301,7 +323,7 @@ private boolean createCriteria(boolean copyingJoinCriteria, Collection<Criteria>
while (i.hasNext()) {
Criteria crit = i.next();

if (copyCriteria(crit, srcToTgt, newJoinCrits, combinedCriteria, copyingJoinCriteria, metadata)) {
if (copyCriteria(crit, srcToTgt, newJoinCrits, combinedCriteria, copyingJoinCriteria, metadata, underAccess)) {
changedTree = true;
if (!copyingJoinCriteria) {
crit = newJoinCrits.get(newJoinCrits.size() - 1);
Expand Down Expand Up @@ -342,12 +364,13 @@ private void visitSelectNode(PlanNode node,
private boolean visitChildern(PlanNode node,
Set<Criteria>[] criteriaInfo,
boolean changedTree,
QueryMetadataInterface metadata) {
QueryMetadataInterface metadata, boolean underAccess) {
if (node.getChildCount() > 0) {
underAccess |= node.getType() == NodeConstants.Types.ACCESS;
List<PlanNode> children = node.getChildren();
for (int i = 0; i < children.size(); i++) {
PlanNode childNode = children.get(i);
changedTree |= tryToCopy(childNode, i==0?criteriaInfo:new Set[2], metadata);
changedTree |= tryToCopy(childNode, i==0?criteriaInfo:new Set[2], metadata, underAccess);
}
}
return changedTree;
Expand Down
11 changes: 10 additions & 1 deletion engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleRaiseNull.java
View file
Expand Up @@ -36,6 +36,7 @@
import org.teiid.query.optimizer.relational.OptimizerRule;
import org.teiid.query.optimizer.relational.RuleStack;
import org.teiid.query.optimizer.relational.plantree.NodeConstants;
import org.teiid.query.optimizer.relational.plantree.NodeConstants.Info;
import org.teiid.query.optimizer.relational.plantree.NodeEditor;
import org.teiid.query.optimizer.relational.plantree.NodeFactory;
import org.teiid.query.optimizer.relational.plantree.PlanNode;
Expand Down Expand Up @@ -219,14 +220,22 @@ PlanNode raiseNullNode(PlanNode rootNode, List<PlanNode> nodes, PlanNode nullNod
case NodeConstants.Types.PROJECT:
{
// check for project into
PlanNode upperProject = NodeEditor.findParent(parentNode.getParent(), NodeConstants.Types.PROJECT);
PlanNode upperProject = NodeEditor.findParent(parentNode.getParent(), NodeConstants.Types.PROJECT, NodeConstants.Types.SOURCE);

if (upperProject == null
|| upperProject.getProperty(NodeConstants.Info.INTO_GROUP) == null) {
return raiseNullNode(rootNode, parentNode, nullNode, nodes);
}
break;
}
case NodeConstants.Types.SOURCE:
{
PlanNode upperProject = parentNode.getParent();
if (upperProject != null && upperProject.getType() == NodeConstants.Types.PROJECT && upperProject.hasProperty(Info.INTO_GROUP)) {
break; //an insert plan
}
return raiseNullNode(rootNode, parentNode, nullNode, nodes);
}
default:
{
return raiseNullNode(rootNode, parentNode, nullNode, nodes);
Expand Down
21 changes: 15 additions & 6 deletions engine/src/test/java/org/teiid/query/optimizer/TestOptimizer.java
View file
Expand Up @@ -2655,11 +2655,8 @@ public static TransformationMetadata example1() {
* See also case 2912.
*/
@Test public void testCopyCriteriaWithOuterJoin5_defect10050(){

ProcessorPlan plan = helpPlan(
"select pm2.g1.e1, pm2.g2.e1, pm2.g3.e1 from ( (pm2.g1 right outer join pm2.g2 on pm2.g1.e1=pm2.g2.e1) right outer join pm2.g3 on pm2.g2.e1=pm2.g3.e1) where pm2.g3.e1 = 'a'", example1(), //$NON-NLS-1$
new String[] { "SELECT g_2.e1, g_1.e1, g_0.e1 FROM pm2.g3 AS g_0 LEFT OUTER JOIN (pm2.g2 AS g_1 LEFT OUTER JOIN pm2.g1 AS g_2 ON g_2.e1 = g_1.e1 AND g_2.e1 = 'a') ON g_1.e1 = g_0.e1 AND g_1.e1 = 'a' WHERE g_0.e1 = 'a'" }); //$NON-NLS-1$
checkNodeTypes(plan, FULL_PUSHDOWN);
helpPlan("select pm2.g1.e1, pm1.g2.e1, pm2.g3.e1 from ( (pm2.g1 right outer join pm1.g2 on pm2.g1.e1=pm1.g2.e1) right outer join pm2.g3 on pm1.g2.e1=pm2.g3.e1) where pm2.g3.e1 = 'a'", example1(), //$NON-NLS-1$
new String[] { "SELECT g_0.e1 FROM pm1.g2 AS g_0 WHERE g_0.e1 = 'a'", "SELECT g_0.e1 FROM pm2.g1 AS g_0 WHERE g_0.e1 = 'a'", "SELECT g_0.e1 FROM pm2.g3 AS g_0 WHERE g_0.e1 = 'a'" }); //$NON-NLS-1$
}

/**
Expand All @@ -2680,7 +2677,19 @@ public static TransformationMetadata example1() {
ProcessorPlan plan = helpPlan("select pm2.g1.e1, pm2.g2.e1 from pm2.g1 right outer join pm2.g2 on pm2.g2.e1=pm2.g1.e1 where pm2.g2.e1 IN ('a', 'b')", example1(), //$NON-NLS-1$
new String[] { "SELECT pm2.g1.e1, pm2.g2.e1 FROM pm2.g2 LEFT OUTER JOIN pm2.g1 ON pm2.g2.e1 = pm2.g1.e1 AND pm2.g1.e1 IN ('a', 'b') WHERE pm2.g2.e1 IN ('a', 'b')" }); //$NON-NLS-1$
checkNodeTypes(plan, FULL_PUSHDOWN);
}
}

@Test public void testCopyCriteriaWithTransitivePushdown(){
ProcessorPlan plan = helpPlan("select pm2.g1.e1, pm2.g2.e1 from pm2.g1, pm2.g2, pm2.g3 where pm2.g1.e1 = pm2.g2.e1 and pm2.g2.e1 = pm2.g3.e1", example1(), //$NON-NLS-1$
new String[] { "SELECT g_0.e1, g_1.e1 FROM pm2.g1 AS g_0, pm2.g2 AS g_1, pm2.g3 AS g_2 WHERE (g_0.e1 = g_1.e1) AND (g_1.e1 = g_2.e1)" }); //$NON-NLS-1$
checkNodeTypes(plan, FULL_PUSHDOWN);
}

@Test public void testCopyCriteriaWithTransitivePushdown1(){
ProcessorPlan plan = helpPlan("select pm2.g1.e1, pm2.g2.e1 from pm2.g1, pm2.g2, pm2.g3 where pm2.g1.e1 = pm2.g2.e1 and pm2.g2.e1 = pm2.g3.e1 and pm2.g1.e1 = 'a'", example1(), //$NON-NLS-1$
new String[] { "SELECT g_0.e1, g_1.e1 FROM pm2.g1 AS g_0, pm2.g2 AS g_1, pm2.g3 AS g_2 WHERE (g_0.e1 = g_1.e1) AND (g_1.e1 = g_2.e1) AND (g_0.e1 = 'a') AND (g_1.e1 = 'a') AND (g_2.e1 = 'a')" }); //$NON-NLS-1$
checkNodeTypes(plan, FULL_PUSHDOWN);
}

@Test public void testCleanCriteria(){

Expand Down

0 comments on commit e9b2388

Please sign in to comment.