Author: shawkins Date: 2010-09-08 12:07:43 -0400 (Wed, 08 Sep 2010) New Revision: 2545 Modified: branches/7.1.x/engine/src/main/java/org/teiid/query/resolver/util/ResolverUtil.java branches/7.1.x/engine/src/main/java/org/teiid/query/rewriter/QueryRewriter.java branches/7.1.x/engine/src/test/java/org/teiid/query/resolver/TestResolver.java branches/7.1.x/engine/src/test/java/org/teiid/query/rewriter/TestQueryRewriter.java Log: TEIID-1250 making the order by resolving logic more uniform, allowing for the proper resolving of external symbols and improving the rewrite logic Modified: branches/7.1.x/engine/src/main/java/org/teiid/query/resolver/util/ResolverUtil.java =================================================================== --- branches/7.1.x/engine/src/main/java/org/teiid/query/resolver/util/ResolverUtil.java 2010-09-08 15:41:59 UTC (rev 2544) +++ branches/7.1.x/engine/src/main/java/org/teiid/query/resolver/util/ResolverUtil.java 2010-09-08 16:07:43 UTC (rev 2545) @@ -351,16 +351,38 @@ for (int i = 0; i < orderBy.getVariableCount(); i++) { SingleElementSymbol sortKey = orderBy.getVariable(i); if (sortKey instanceof ElementSymbol) { - int index = resolveSortKey(fromClauseGroups, knownElements, metadata, knownShortNames, - (ElementSymbol)sortKey); - if (index == -1) { - index = expressions.indexOf(SymbolMap.getExpression(sortKey)); - if (index == -1 && !isSimpleQuery) { - throw new QueryResolverException(QueryPlugin.Util.getString("ResolverUtil.invalid_unrelated", sortKey)); //$NON-NLS-1$ - } - } - orderBy.setExpressionPosition(i, index); - continue; + ElementSymbol symbol = (ElementSymbol)sortKey; + String groupPart = metadata.getGroupName(symbol.getName()); + String symbolName = symbol.getName(); + String shortName = symbol.getShortName(); + if (groupPart == null) { + int position = -1; + SingleElementSymbol matchedSymbol = null; + // walk the SELECT col short names, looking for a match on the current ORDER BY 'short name' + for(int j=0; j<knownShortNames.length; j++) { + if( !shortName.equalsIgnoreCase( knownShortNames[j] )) { + continue; + } + // if we already have a matched symbol, matching again here means it is duplicate/ambiguous + if(matchedSymbol != null) { + if (!matchedSymbol.equals(knownElements.get(j))) { + throw new QueryResolverException(ErrorMessageKeys.RESOLVER_0042, QueryPlugin.Util.getString(ErrorMessageKeys.RESOLVER_0042, symbolName)); + } + continue; + } + matchedSymbol = knownElements.get(j); + position = j; + } + if (matchedSymbol != null) { + TempMetadataID tempMetadataID = new TempMetadataID(symbol.getName(), matchedSymbol.getType()); + symbol.setMetadataID(tempMetadataID); + symbol.setType(matchedSymbol.getType()); + } + if (position != -1) { + orderBy.setExpressionPosition(i, position); + continue; + } + } } else if (sortKey instanceof ExpressionSymbol) { // check for legacy positional ExpressionSymbol es = (ExpressionSymbol)sortKey; @@ -380,77 +402,22 @@ throw new QueryResolverException(QueryPlugin.Util.getString("ResolverUtil.setquery_order_expression", sortKey)); //$NON-NLS-1$ } for (ElementSymbol symbol : ElementCollectorVisitor.getElements(sortKey, false)) { - resolveSortKey(fromClauseGroups, null, metadata, null, symbol); + try { + ResolverVisitor.resolveLanguageObject(symbol, fromClauseGroups, command.getExternalGroupContexts(), metadata); + } catch(QueryResolverException e) { + throw new QueryResolverException(e, ErrorMessageKeys.RESOLVER_0043, QueryPlugin.Util.getString(ErrorMessageKeys.RESOLVER_0043, symbol.getName()) ); + } } ResolverVisitor.resolveLanguageObject(sortKey, metadata); int index = expressions.indexOf(SymbolMap.getExpression(sortKey)); + if (index == -1 && !isSimpleQuery) { + throw new QueryResolverException(QueryPlugin.Util.getString("ResolverUtil.invalid_unrelated", sortKey)); //$NON-NLS-1$ + } orderBy.setExpressionPosition(i, index); } } - private static int resolveSortKey(List fromClauseGroups, List knownElements, - QueryMetadataInterface metadata, String[] knownShortNames, - ElementSymbol symbol) throws TeiidComponentException, - QueryMetadataException, QueryResolverException { - String symbolName = symbol.getName(); - String groupPart = metadata.getGroupName(symbolName); - String shortName = symbol.getShortName(); - - //check for union order by - if (fromClauseGroups.isEmpty() && groupPart != null && !shortName.equals(symbolName)) { - throw new QueryResolverException(ErrorMessageKeys.RESOLVER_0043, QueryPlugin.Util.getString(ErrorMessageKeys.RESOLVER_0043, symbolName)); - } - - if (knownShortNames != null) { - int position = -1; - SingleElementSymbol matchedSymbol = null; - // walk the SELECT col short names, looking for a match on the current ORDER BY 'short name' - for(int i=0; i<knownShortNames.length; i++) { - if( !shortName.equalsIgnoreCase( knownShortNames[i] )) { - continue; - } - if (groupPart != null) { - Object knownSymbol = knownElements.get(i); - if(!(knownSymbol instanceof ElementSymbol)) { - continue; - } - ElementSymbol knownElement = (ElementSymbol) knownSymbol; - GroupSymbol group = knownElement.getGroupSymbol(); - - // skip this one if the two short names are not from the same group - if (!nameMatchesGroup(groupPart.toUpperCase(), group.getCanonicalName())) { - continue; - } - } - - // if we already have a matched symbol, matching again here means it is duplicate/ambiguous - if(matchedSymbol != null) { - if (!matchedSymbol.equals(knownElements.get(i))) { - throw new QueryResolverException(ErrorMessageKeys.RESOLVER_0042, QueryPlugin.Util.getString(ErrorMessageKeys.RESOLVER_0042, symbolName)); - } - continue; - } - matchedSymbol = (SingleElementSymbol)knownElements.get(i); - position = i; - } - if (matchedSymbol != null) { - TempMetadataID tempMetadataID = new TempMetadataID(symbol.getName(), matchedSymbol.getType()); - symbol.setMetadataID(tempMetadataID); - symbol.setType(matchedSymbol.getType()); - return position; - } - } - - try { - ResolverVisitor.resolveLanguageObject(symbol, fromClauseGroups, metadata); - } catch(QueryResolverException e) { - throw new QueryResolverException(e, ErrorMessageKeys.RESOLVER_0043, QueryPlugin.Util.getString(ErrorMessageKeys.RESOLVER_0043, symbol.getName()) ); - } - return -1; - } - - /** * Get the default value for the parameter, which could be null * if the parameter is set to NULLABLE. If no default is available, Modified: branches/7.1.x/engine/src/main/java/org/teiid/query/rewriter/QueryRewriter.java =================================================================== --- branches/7.1.x/engine/src/main/java/org/teiid/query/rewriter/QueryRewriter.java 2010-09-08 15:41:59 UTC (rev 2544) +++ branches/7.1.x/engine/src/main/java/org/teiid/query/rewriter/QueryRewriter.java 2010-09-08 16:07:43 UTC (rev 2545) @@ -851,17 +851,20 @@ for (int i = 0; i < orderBy.getVariableCount(); i++) { SingleElementSymbol querySymbol = orderBy.getVariable(i); int index = orderBy.getExpressionPosition(i); + boolean isUnrelated = false; if (index == -1) { unrelatedItems.add(orderBy.getOrderByItems().get(i)); - hasUnrelatedExpression |= (querySymbol instanceof ExpressionSymbol); - continue; // must be unrelated + isUnrelated = (querySymbol instanceof ExpressionSymbol); + } else { + querySymbol = (SingleElementSymbol)projectedSymbols.get(index); } - querySymbol = (SingleElementSymbol)projectedSymbols.get(index); Expression expr = SymbolMap.getExpression(querySymbol); - if (!previousExpressions.add(expr) || (queryCommand instanceof Query && EvaluatableVisitor.isFullyEvaluatable(expr, true))) { + if (!previousExpressions.add(expr) || (queryCommand instanceof Query && EvaluatableVisitor.willBecomeConstant(expr))) { orderBy.removeOrderByItem(i--); + } else if (!isUnrelated) { + orderBy.getOrderByItems().get(i).setSymbol((SingleElementSymbol)querySymbol.clone()); } else { - orderBy.getOrderByItems().get(i).setSymbol((SingleElementSymbol)querySymbol.clone()); + hasUnrelatedExpression = true; } } if (orderBy.getVariableCount() == 0) { Modified: branches/7.1.x/engine/src/test/java/org/teiid/query/resolver/TestResolver.java =================================================================== --- branches/7.1.x/engine/src/test/java/org/teiid/query/resolver/TestResolver.java 2010-09-08 15:41:59 UTC (rev 2544) +++ branches/7.1.x/engine/src/test/java/org/teiid/query/resolver/TestResolver.java 2010-09-08 16:07:43 UTC (rev 2545) @@ -1991,16 +1991,20 @@ @Test public void testUnaliasedOrderByFails() { helpResolveException("SELECT pm1.g1.e1 e2 FROM pm1.g1 group by pm1.g1.e1 ORDER BY pm1.g1.e2"); //$NON-NLS-1$ } + + @Test public void testUnaliasedOrderByFails1() { + helpResolveException("SELECT pm1.g1.e1 e2 FROM pm1.g1 group by pm1.g1.e1 ORDER BY pm1.g1.e2 + 1"); //$NON-NLS-1$ + } /** * the group g1 is not known to the order by clause of a union */ @Test public void testUnionOrderByFail() { - helpResolveException("SELECT pm1.g1.e1 FROM pm1.g1 UNION SELECT pm1.g2.e1 FROM pm1.g2 ORDER BY g1.e1", "Error Code:ERR.015.008.0043 Message:Element 'g1.e1' in ORDER BY was not found in SELECT clause."); //$NON-NLS-1$ //$NON-NLS-2$ + helpResolveException("SELECT pm1.g1.e1 FROM pm1.g1 UNION SELECT pm1.g2.e1 FROM pm1.g2 ORDER BY g1.e1", "ORDER BY expression 'g1.e1' cannot be used with a set query."); //$NON-NLS-1$ //$NON-NLS-2$ } @Test public void testUnionOrderByFail1() { - helpResolveException("SELECT pm1.g1.e1 FROM pm1.g1 UNION SELECT pm1.g2.e1 FROM pm1.g2 ORDER BY pm1.g1.e1", "Error Code:ERR.015.008.0043 Message:Element 'pm1.g1.e1' in ORDER BY was not found in SELECT clause."); //$NON-NLS-1$ //$NON-NLS-2$ + helpResolveException("SELECT pm1.g1.e1 FROM pm1.g1 UNION SELECT pm1.g2.e1 FROM pm1.g2 ORDER BY pm1.g1.e1", "ORDER BY expression 'pm1.g1.e1' cannot be used with a set query."); //$NON-NLS-1$ //$NON-NLS-2$ } @Test public void testOrderByPartiallyQualified() { Modified: branches/7.1.x/engine/src/test/java/org/teiid/query/rewriter/TestQueryRewriter.java =================================================================== --- branches/7.1.x/engine/src/test/java/org/teiid/query/rewriter/TestQueryRewriter.java 2010-09-08 15:41:59 UTC (rev 2544) +++ branches/7.1.x/engine/src/test/java/org/teiid/query/rewriter/TestQueryRewriter.java 2010-09-08 16:07:43 UTC (rev 2545) @@ -1030,7 +1030,7 @@ procedure = procedure + "Declare String var1;\n"; //$NON-NLS-1$ procedure = procedure + "if(var1 = 'x' or var1 = 'y')\n"; //$NON-NLS-1$ procedure = procedure + "BEGIN\n"; //$NON-NLS-1$ - procedure = procedure + "Select pm1.g1.e2, Input.e2, CHANGING.e2, CHANGING.e1 from pm1.g1 order by CHANGING.e1;\n"; //$NON-NLS-1$ + procedure = procedure + "Select pm1.g1.e2, Input.e2, CHANGING.e2, CHANGING.e1 from pm1.g1 order by CHANGING.e1 + 1;\n"; //$NON-NLS-1$ procedure = procedure + "END\n"; //$NON-NLS-1$ procedure = procedure + "END\n"; //$NON-NLS-1$