Author: shawkins Date: 2011-11-02 15:48:52 -0400 (Wed, 02 Nov 2011) New Revision: 3603 Modified: trunk/build/kits/jboss-container/teiid-releasenotes.html trunk/documentation/reference/src/main/docbook/en-US/content/sql_clauses.xml trunk/engine/src/main/java/org/teiid/query/optimizer/relational/RelationalPlanner.java trunk/engine/src/main/java/org/teiid/query/optimizer/relational/plantree/NodeConstants.java trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/FrameUtil.java trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RulePushLimit.java trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RulePushSelectCriteria.java trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleRaiseAccess.java trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleRemoveOptionalJoins.java trunk/engine/src/main/java/org/teiid/query/optimizer/xml/XMLQueryPlanner.java trunk/engine/src/main/java/org/teiid/query/parser/SQLParserUtil.java trunk/engine/src/main/java/org/teiid/query/sql/lang/Limit.java trunk/engine/src/main/java/org/teiid/query/sql/visitor/SQLStringVisitor.java trunk/engine/src/main/javacc/org/teiid/query/parser/SQLParser.jj trunk/engine/src/test/java/org/teiid/query/optimizer/TestLimit.java trunk/engine/src/test/java/org/teiid/query/parser/TestOptionsAndHints.java Log: TEIID-1806 ensuring consistent handling of unordered limits Modified: trunk/build/kits/jboss-container/teiid-releasenotes.html =================================================================== --- trunk/build/kits/jboss-container/teiid-releasenotes.html 2011-11-02 16:50:24 UTC (rev 3602) +++ trunk/build/kits/jboss-container/teiid-releasenotes.html 2011-11-02 19:48:52 UTC (rev 3603) @@ -37,6 +37,7 @@ <LI><B>Native Queries</B> - added the ability to specify native query SQL for JDBC physical tables and stored procedures via extension metadata. <LI><B>View removal hint</B> - the NO_UNNEST hint now also applies to from clause views and subqueries. It will instruct the planner to not perform view flattening. <LI><B>Non-blocking statement execution</B> - Teiid JDBC extensions TeiidStatement and TeiidPreparedStatement can be used to submit queries against embedded connections with a callback to process results in a non-blocking manner. + <LI><B>NON_STRICT limit hint</B> - the NON_STRICT hint can be used with unordered limits to tell the optimizer to not inhibit push operations even if the results will not be consistent with the logical application of the limit. </UL> <h2><a name="Compatibility">Compatibility Issues</a></h2> @@ -51,6 +52,7 @@ <li>Leave was added as a reserved word. <li>Lob inlining is incompatible with clients older than 7.6. If a 7.6 server will have older clients that use lobs connect to it, then the BufferService property inlineLobs should be set to false in the teiid-jboss-beans.xml file. <li>Oracle translators assume they are using Oracle supplied drivers. If that is not the case, set the Oracle translator execution property oracleSuppliedDriver to false. + <li>Unordered limits are handled strictly by default. They are longer pushed through conditions, dup removal, or UNION not all - and cannot have conditions pushed through them. Use the NON_STRICT hint to change the behavior to Teiid 7.1 handling. </ul> <h4>from 7.4</h4> Modified: trunk/documentation/reference/src/main/docbook/en-US/content/sql_clauses.xml =================================================================== --- trunk/documentation/reference/src/main/docbook/en-US/content/sql_clauses.xml 2011-11-02 16:50:24 UTC (rev 3602) +++ trunk/documentation/reference/src/main/docbook/en-US/content/sql_clauses.xml 2011-11-02 19:48:52 UTC (rev 3603) @@ -438,6 +438,11 @@ <para>The terms FIRST/NEXT are interchangable as well as ROW/ROWS. </para> </listitem> + <listitem> + <para>The limit clause may take an optional preceeding NON_STRICT hint to indicate that push operations should not be inhibited even if the results will not be consistent with the logical application of the limit. + The hint is only needed on unordered limits, e.g. "SELECT * FROM VW /*+ NON_STRICT */ LIMIT 2". + </para> + </listitem> </itemizedlist> <itemizedlist> <para>Examples: Modified: trunk/engine/src/main/java/org/teiid/query/optimizer/relational/RelationalPlanner.java =================================================================== --- trunk/engine/src/main/java/org/teiid/query/optimizer/relational/RelationalPlanner.java 2011-11-02 16:50:24 UTC (rev 3602) +++ trunk/engine/src/main/java/org/teiid/query/optimizer/relational/RelationalPlanner.java 2011-11-02 19:48:52 UTC (rev 3603) @@ -1151,6 +1151,9 @@ if (limit.isImplicit()) { limitNode.setProperty(Info.IS_IMPLICIT_LIMIT, true); } + if (limit.isStrict()) { + limitNode.setProperty(Info.IS_STRICT, true); + } attachLast(limitNode, plan); plan = limitNode; } Modified: trunk/engine/src/main/java/org/teiid/query/optimizer/relational/plantree/NodeConstants.java =================================================================== --- trunk/engine/src/main/java/org/teiid/query/optimizer/relational/plantree/NodeConstants.java 2011-11-02 16:50:24 UTC (rev 3602) +++ trunk/engine/src/main/java/org/teiid/query/optimizer/relational/plantree/NodeConstants.java 2011-11-02 19:48:52 UTC (rev 3603) @@ -148,6 +148,7 @@ MAX_TUPLE_LIMIT, // Expression that evaluates to the max number of tuples generated OFFSET_TUPLE_COUNT, // Expression that evaluates to the tuple offset of the starting tuple IS_IMPLICIT_LIMIT, // Boolean if the limit is created by the rewriter as part of a subquery optimization + IS_STRICT, // Boolean if the unordered limit should be enforced strictly // Common AP Information ACCESS_PATTERNS, // Collection <List <Object element ID> > Modified: trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/FrameUtil.java =================================================================== --- trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/FrameUtil.java 2011-11-02 16:50:24 UTC (rev 3602) +++ trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/FrameUtil.java 2011-11-02 19:48:52 UTC (rev 3603) @@ -547,8 +547,8 @@ return null; } - public static boolean isOrderedLimit(PlanNode node) { - return node.getType() == NodeConstants.Types.TUPLE_LIMIT && NodeEditor.findNodePreOrder(node, NodeConstants.Types.SORT, NodeConstants.Types.PROJECT | NodeConstants.Types.SET_OP) != null; + public static boolean isOrderedOrStrictLimit(PlanNode node) { + return node.getType() == NodeConstants.Types.TUPLE_LIMIT && (NodeEditor.findNodePreOrder(node, NodeConstants.Types.SORT, NodeConstants.Types.PROJECT | NodeConstants.Types.SET_OP) != null || node.hasProperty(Info.IS_STRICT)); } } Modified: trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RulePushLimit.java =================================================================== --- trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RulePushLimit.java 2011-11-02 16:50:24 UTC (rev 3602) +++ trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RulePushLimit.java 2011-11-02 19:48:52 UTC (rev 3603) @@ -45,6 +45,7 @@ import org.teiid.query.optimizer.relational.plantree.NodeEditor; import org.teiid.query.optimizer.relational.plantree.NodeFactory; import org.teiid.query.optimizer.relational.plantree.PlanNode; +import org.teiid.query.optimizer.relational.plantree.NodeConstants.Info; import org.teiid.query.sql.lang.CompareCriteria; import org.teiid.query.sql.lang.Criteria; import org.teiid.query.sql.lang.SetQuery; @@ -135,7 +136,9 @@ Expression childOffset = (Expression)child.getProperty(NodeConstants.Info.OFFSET_TUPLE_COUNT); combineLimits(limitNode, metadata, parentLimit, parentOffset, childLimit, childOffset); - + if (child.hasBooleanProperty(Info.IS_STRICT)) { + limitNode.setProperty(Info.IS_STRICT, true); + } NodeEditor.removeChildNode(limitNode, child); limitNodes.remove(child); @@ -143,10 +146,12 @@ } case NodeConstants.Types.SET_OP: { - if (!SetQuery.Operation.UNION.equals(child.getProperty(NodeConstants.Info.SET_OPERATION)) - || !child.hasBooleanProperty(NodeConstants.Info.USE_ALL)) { - return false; - } + if (!SetQuery.Operation.UNION.equals(child.getProperty(NodeConstants.Info.SET_OPERATION))) { + return false; + } + if (!child.hasBooleanProperty(NodeConstants.Info.USE_ALL) && limitNode.hasBooleanProperty(Info.IS_STRICT)) { + return false; + } //distribute the limit List<PlanNode> grandChildren = new LinkedList<PlanNode>(child.getChildren()); for (PlanNode grandChild : grandChildren) { @@ -171,13 +176,16 @@ { GroupSymbol virtualGroup = child.getGroups().iterator().next(); if (virtualGroup.isProcedure()) { - return false; + return false; } if (FrameUtil.isProcedure(child.getFirstChild())) { return false; } return true; } + case NodeConstants.Types.SELECT: + case NodeConstants.Types.DUP_REMOVE: + return !limitNode.hasBooleanProperty(Info.IS_STRICT); default: { return false; Modified: trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RulePushSelectCriteria.java =================================================================== --- trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RulePushSelectCriteria.java 2011-11-02 16:50:24 UTC (rev 3602) +++ trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RulePushSelectCriteria.java 2011-11-02 19:48:52 UTC (rev 3603) @@ -343,7 +343,7 @@ } satisfyAccessPatterns(critNode, currentNode); - } else if (FrameUtil.isOrderedLimit(currentNode)) { + } else if (FrameUtil.isOrderedOrStrictLimit(currentNode)) { return currentNode; } } @@ -355,7 +355,7 @@ throws QueryPlannerException { //ensure that the criteria can be pushed further - if (sourceNode.getChildCount() == 1 && FrameUtil.isOrderedLimit(sourceNode.getFirstChild())) { + if (sourceNode.getChildCount() == 1 && FrameUtil.isOrderedOrStrictLimit(sourceNode.getFirstChild())) { return false; } Modified: trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleRaiseAccess.java =================================================================== --- trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleRaiseAccess.java 2011-11-02 16:50:24 UTC (rev 3602) +++ trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleRaiseAccess.java 2011-11-02 19:48:52 UTC (rev 3603) @@ -423,7 +423,7 @@ //don't push criteria into an invalid location above an ordered limit - shouldn't happen PlanNode limitNode = NodeEditor.findNodePreOrder(accessNode, NodeConstants.Types.TUPLE_LIMIT, NodeConstants.Types.SOURCE); - if (limitNode != null && FrameUtil.isOrderedLimit(limitNode)) { + if (limitNode != null && FrameUtil.isOrderedOrStrictLimit(limitNode)) { return false; } Modified: trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleRemoveOptionalJoins.java =================================================================== --- trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleRemoveOptionalJoins.java 2011-11-02 16:50:24 UTC (rev 3602) +++ trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RuleRemoveOptionalJoins.java 2011-11-02 19:48:52 UTC (rev 3603) @@ -44,7 +44,6 @@ import org.teiid.query.sql.lang.Criteria; import org.teiid.query.sql.lang.JoinType; import org.teiid.query.sql.symbol.AggregateSymbol; -import org.teiid.query.sql.symbol.Constant; import org.teiid.query.sql.symbol.GroupSymbol; import org.teiid.query.sql.visitor.GroupsUsedByElementsVisitor; import org.teiid.query.util.CommandContext; @@ -205,10 +204,6 @@ return NodeEditor.findAllNodes(optionalNode, NodeConstants.Types.JOIN); } - /** - * Ensure that the needed elements come only from the left hand side and - * that cardinality won't matter - */ static boolean useNonDistinctRows(PlanNode parent) { while (parent != null) { if (parent.hasBooleanProperty(NodeConstants.Info.IS_DUP_REMOVAL)) { @@ -229,16 +224,11 @@ return AggregateSymbol.areAggregatesCardinalityDependent(aggs); } case NodeConstants.Types.TUPLE_LIMIT: { - if (!(parent.getProperty(NodeConstants.Info.MAX_TUPLE_LIMIT) instanceof Constant) - || parent.getProperty(NodeConstants.Info.OFFSET_TUPLE_COUNT) != null) { + if (FrameUtil.isOrderedOrStrictLimit(parent)) { return true; } - Constant constant = (Constant)parent.getProperty(NodeConstants.Info.MAX_TUPLE_LIMIT); - if (!Integer.valueOf(1).equals(constant.getValue())) { - return true; - } } - //we assmue that projects of non-deterministic expressions do not matter + //we assume that projects of non-deterministic expressions do not matter } parent = parent.getParent(); } Modified: trunk/engine/src/main/java/org/teiid/query/optimizer/xml/XMLQueryPlanner.java =================================================================== --- trunk/engine/src/main/java/org/teiid/query/optimizer/xml/XMLQueryPlanner.java 2011-11-02 16:50:24 UTC (rev 3602) +++ trunk/engine/src/main/java/org/teiid/query/optimizer/xml/XMLQueryPlanner.java 2011-11-02 19:48:52 UTC (rev 3603) @@ -200,7 +200,7 @@ if (rsInfo.exceptionOnRowlimit()) { limit++; } - rsQuery.setLimit(new Limit(null, new Constant(new Integer(limit)))); + rsQuery.setLimit(new Limit(null, new Constant(limit))); } // this query is not eligible for staging; proceed normally. Modified: trunk/engine/src/main/java/org/teiid/query/parser/SQLParserUtil.java =================================================================== --- trunk/engine/src/main/java/org/teiid/query/parser/SQLParserUtil.java 2011-11-02 16:50:24 UTC (rev 3602) +++ trunk/engine/src/main/java/org/teiid/query/parser/SQLParserUtil.java 2011-11-02 19:48:52 UTC (rev 3603) @@ -36,6 +36,7 @@ import org.teiid.query.sql.lang.CacheHint; import org.teiid.query.sql.lang.FromClause; import org.teiid.query.sql.lang.JoinType; +import org.teiid.query.sql.lang.Limit; import org.teiid.query.sql.lang.Option; import org.teiid.query.sql.lang.QueryCommand; import org.teiid.query.sql.lang.SetQuery; @@ -213,6 +214,16 @@ return hint; } + boolean isNonStrictHint(Token t) { + String[] parts = getComment(t).split("\\s"); //$NON-NLS-1$ + for (int i = 0; i < parts.length; i++) { + if (parts[i].equalsIgnoreCase(Limit.NON_STRICT)) { + return true; + } + } + return false; + } + private static Pattern CACHE_HINT = Pattern.compile("/\\*\\+?\\s*cache(\\(\\s*(pref_mem)?\\s*(ttl:\\d{1,19})?\\s*(updatable)?\\s*(scope:(session|vdb|user))?[^\\)]*\\))?[^\\*]*\\*\\/.*", Pattern.CASE_INSENSITIVE | Pattern.DOTALL); //$NON-NLS-1$ static CacheHint getQueryCacheOption(String query) { Modified: trunk/engine/src/main/java/org/teiid/query/sql/lang/Limit.java =================================================================== --- trunk/engine/src/main/java/org/teiid/query/sql/lang/Limit.java 2011-11-02 16:50:24 UTC (rev 3602) +++ trunk/engine/src/main/java/org/teiid/query/sql/lang/Limit.java 2011-11-02 19:48:52 UTC (rev 3603) @@ -32,10 +32,13 @@ public class Limit implements LanguageObject { + + public static String NON_STRICT = "NON_STRICT"; //$NON-NLS-1$ private Expression offset; private Expression rowLimit; private boolean implicit; + private boolean strict = true; public Limit(Expression offset, Expression rowLimit) { this.offset = offset; @@ -46,6 +49,14 @@ } + public void setStrict(boolean strict) { + this.strict = strict; + } + + public boolean isStrict() { + return strict; + } + public boolean isImplicit() { return implicit; } @@ -96,6 +107,7 @@ public Limit clone() { Limit clone = new Limit(); clone.implicit = this.implicit; + clone.strict = this.strict; if (this.rowLimit != null) { clone.setRowLimit((Expression) this.rowLimit.clone()); } Modified: trunk/engine/src/main/java/org/teiid/query/sql/visitor/SQLStringVisitor.java =================================================================== --- trunk/engine/src/main/java/org/teiid/query/sql/visitor/SQLStringVisitor.java 2011-11-02 16:50:24 UTC (rev 3602) +++ trunk/engine/src/main/java/org/teiid/query/sql/visitor/SQLStringVisitor.java 2011-11-02 19:48:52 UTC (rev 3603) @@ -1777,6 +1777,14 @@ } public void visit( Limit obj ) { + if (!obj.isStrict()) { + append(BEGIN_HINT); + append(SPACE); + append(Limit.NON_STRICT); + append(SPACE); + append(END_HINT); + append(SPACE); + } if (obj.getRowLimit() == null) { append(OFFSET); append(SPACE); Modified: trunk/engine/src/main/javacc/org/teiid/query/parser/SQLParser.jj =================================================================== --- trunk/engine/src/main/javacc/org/teiid/query/parser/SQLParser.jj 2011-11-02 16:50:24 UTC (rev 3602) +++ trunk/engine/src/main/javacc/org/teiid/query/parser/SQLParser.jj 2011-11-02 19:48:52 UTC (rev 3603) @@ -3164,8 +3164,10 @@ { Expression limit = null; Expression offset = null; + Token t = null; } { + {t = getToken(1);} ((<LIMIT> offset = intParam(info) [<COMMA> limit = intParam(info)]) { @@ -3180,7 +3182,9 @@ | (limit = fetchLimit(info))) { - return new Limit(offset, limit); + Limit result = new Limit(offset, limit); + result.setStrict(!isNonStrictHint(t)); + return result; } } Modified: trunk/engine/src/test/java/org/teiid/query/optimizer/TestLimit.java =================================================================== --- trunk/engine/src/test/java/org/teiid/query/optimizer/TestLimit.java 2011-11-02 16:50:24 UTC (rev 3602) +++ trunk/engine/src/test/java/org/teiid/query/optimizer/TestLimit.java 2011-11-02 19:48:52 UTC (rev 3603) @@ -709,7 +709,7 @@ // pm3 model supports order by capFinder.addCapabilities("pm3", caps); //$NON-NLS-1$ - String sql = "SELECT * FROM (SELECT * FROM pm3.g1 limit 100) as v1 where v1.e1 = 1";//$NON-NLS-1$ + String sql = "SELECT * FROM (SELECT * FROM pm3.g1 /*+ non_strict */ limit 100) as v1 where v1.e1 = 1";//$NON-NLS-1$ String[] expectedSql = new String[] { "SELECT pm3.g1.e1, pm3.g1.e2, pm3.g1.e3, pm3.g1.e4 FROM pm3.g1 WHERE pm3.g1.e1 = '1' LIMIT 100" //$NON-NLS-1$ }; @@ -852,7 +852,7 @@ * Note here that the criteria made it to the having clause */ @Test public void testAggregateCriteriaOverUnSortedLimit() { - String sql = "select a from (SELECT MAX(e2) as a FROM pm1.g1 GROUP BY e2 LIMIT 1) x where a = 0"; //$NON-NLS-1$ + String sql = "select a from (SELECT MAX(e2) as a FROM pm1.g1 GROUP BY e2 /*+ non_strict */ LIMIT 1) x where a = 0"; //$NON-NLS-1$ FakeCapabilitiesFinder capFinder = new FakeCapabilitiesFinder(); BasicSourceCapabilities caps = new BasicSourceCapabilities(); caps.setCapabilitySupport(Capability.CRITERIA_COMPARE_EQ, true); Modified: trunk/engine/src/test/java/org/teiid/query/parser/TestOptionsAndHints.java =================================================================== --- trunk/engine/src/test/java/org/teiid/query/parser/TestOptionsAndHints.java 2011-11-02 16:50:24 UTC (rev 3602) +++ trunk/engine/src/test/java/org/teiid/query/parser/TestOptionsAndHints.java 2011-11-02 19:48:52 UTC (rev 3603) @@ -1162,4 +1162,15 @@ assertEquals("SELECT a FROM /*+ NO_UNNEST */ (SELECT a FROM db.g WHERE a2 = 5) AS x", QueryParser.getQueryParser().parseCommand(sql, new ParseInfo()).toString()); //$NON-NLS-1$ } + @Test public void testNonStrictLimit() throws QueryParserException { + String sql = "SELECT a FROM x /*+ non_strict */ limit 1"; //$NON-NLS-1$ + assertEquals("SELECT a FROM x /*+ NON_STRICT */ LIMIT 1", QueryParser.getQueryParser().parseCommand(sql, new ParseInfo()).toString()); //$NON-NLS-1$ + + sql = "SELECT a FROM x /*+ non_strict */ offset 1 row"; //$NON-NLS-1$ + assertEquals("SELECT a FROM x /*+ NON_STRICT */ OFFSET 1 ROWS", QueryParser.getQueryParser().parseCommand(sql, new ParseInfo()).toString()); //$NON-NLS-1$ + + sql = "SELECT a FROM x /*+ non_strict */ fetch first 1 rows only"; //$NON-NLS-1$ + assertEquals("SELECT a FROM x /*+ NON_STRICT */ LIMIT 1", QueryParser.getQueryParser().parseCommand(sql, new ParseInfo()).toString()); //$NON-NLS-1$ + } + }