Author: shawkins Date: 2010-09-08 11:30:47 -0400 (Wed, 08 Sep 2010) New Revision: 2543 Modified: branches/7.1.x/engine/src/main/java/org/teiid/query/resolver/util/ResolverUtil.java branches/7.1.x/engine/src/test/java/org/teiid/query/resolver/TestResolver.java Log: TEIID-1251 TEIID-1250 order by resolving enhancement/fix 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:26:49 UTC (rev 2542) +++ branches/7.1.x/engine/src/main/java/org/teiid/query/resolver/util/ResolverUtil.java 2010-09-08 15:30:47 UTC (rev 2543) @@ -351,10 +351,13 @@ for (int i = 0; i < orderBy.getVariableCount(); i++) { SingleElementSymbol sortKey = orderBy.getVariable(i); if (sortKey instanceof ElementSymbol) { - int index = resolveSortKey(fromClauseGroups, knownElements, metadata, isSimpleQuery, - knownShortNames, (ElementSymbol)sortKey); + 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; @@ -377,7 +380,7 @@ 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, isSimpleQuery, null, symbol); + resolveSortKey(fromClauseGroups, null, metadata, null, symbol); } ResolverVisitor.resolveLanguageObject(sortKey, metadata); @@ -387,10 +390,9 @@ } private static int resolveSortKey(List fromClauseGroups, List knownElements, - QueryMetadataInterface metadata, boolean isSimpleQuery, - String[] knownShortNames, ElementSymbol symbol) throws TeiidComponentException, + QueryMetadataInterface metadata, String[] knownShortNames, + ElementSymbol symbol) throws TeiidComponentException, QueryMetadataException, QueryResolverException { - SingleElementSymbol matchedSymbol = null; String symbolName = symbol.getName(); String groupPart = metadata.getGroupName(symbolName); String shortName = symbol.getShortName(); @@ -401,42 +403,45 @@ } 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] )) { - if (groupPart != null) { - Object knownSymbol = knownElements.get(i); - if(knownSymbol instanceof ElementSymbol) { - 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) { - throw new QueryResolverException(ErrorMessageKeys.RESOLVER_0042, QueryPlugin.Util.getString(ErrorMessageKeys.RESOLVER_0042, symbolName)); - } - matchedSymbol = (SingleElementSymbol)knownElements.get(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()); - int position = knownElements.indexOf(matchedSymbol); symbol.setMetadataID(tempMetadataID); symbol.setType(matchedSymbol.getType()); return position; } } - if (!isSimpleQuery) { - throw new QueryResolverException(QueryPlugin.Util.getString("ResolverUtil.invalid_unrelated", symbol.getName())); //$NON-NLS-1$ - } - // Didn't find it by full name or short name, so try resolving try { ResolverVisitor.resolveLanguageObject(symbol, fromClauseGroups, metadata); } catch(QueryResolverException e) { 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:26:49 UTC (rev 2542) +++ branches/7.1.x/engine/src/test/java/org/teiid/query/resolver/TestResolver.java 2010-09-08 15:30:47 UTC (rev 2543) @@ -1983,6 +1983,14 @@ @Test public void testUnaliasedOrderBySucceeds() { helpResolve("SELECT pm1.g1.e1 a, pm1.g1.e1 b FROM pm1.g1 ORDER BY pm1.g1.e1"); //$NON-NLS-1$ } + + @Test public void testUnaliasedOrderBySucceeds1() { + helpResolve("SELECT pm1.g1.e1 a FROM pm1.g1 group by pm1.g1.e1 ORDER BY pm1.g1.e1"); //$NON-NLS-1$ + } + + @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$ + } /** * the group g1 is not known to the order by clause of a union