select c1 as x from t where ... group by x order by c1
Derby throws an exception with SQLState 42x04 complaining that it cannot resolve "c1".
The underlying problem is that the Derby parser transforms the select into a query tree for the following statement:
select * from (select c1 as x from t where ...) order by c1
The code in class OrderByColumn did not take this into account. I changed methods pullUpOrderByColumn and bindOrderByColumn to handle this case specially. pullUpOrderByColumn adds the sort key to the ResultColumnList if it cannot find it there. It is called before binding and before select list wildcards ("*") are expanded. I changed it to pull the sort key into the ResultColumnList of the subselect generated to handle GROUP BY. I also changed it to remember where it was added. This simplifies the bindOrderByColumn, which is called after pullUpOrderByColumn.
I also fixed the handling of table names in class OrderByColumn. It treated them as strings, which does not work when the schema or table name is a quoted string containing a period. I changed OrderByColumn to use class TableName to represent a table name. The ResultColumnList.getOrderByColumn methods where changed accordingly
Jack Klebanoff.
Index: java/engine/org/apache/derby/impl/sql/compile/TableName.java
===================================================================
--- java/engine/org/apache/derby/impl/sql/compile/TableName.java
(revision 168148)
+++ java/engine/org/apache/derby/impl/sql/compile/TableName.java
(working copy)
@@ -206,6 +206,9 @@
*/
public boolean equals(TableName otherTableName)
{
+ if( otherTableName == null)
+ return false;
+
String fullTableName = getFullTableName();
if (fullTableName == null)
{
Index: java/engine/org/apache/derby/impl/sql/compile/sqlgrammar.jj
===================================================================
--- java/engine/org/apache/derby/impl/sql/compile/sqlgrammar.jj (revision
168148)
+++ java/engine/org/apache/derby/impl/sql/compile/sqlgrammar.jj (working copy)
@@ -7305,6 +7305,12 @@
*
* RESOLVE - someday we should try to find matching
aggregates
* instead of just adding them.
+ *
+ * NOTE: This rewriting of the query tree makes the handling of an
ORDER BY
+ * clause difficult. See OrderByColumn.pullUpOrderByColumn. It
makes specific
+ * assumptions about the structure of the generated query tree. Do
not make
+ * any changes to this transformation without carefully
considering the
+ * OrderByColumn pullUpOrderByColumn and bindOrderByColumn methods.
*/
if (havingClause != null)
{
Index: java/engine/org/apache/derby/impl/sql/compile/OrderByColumn.java
===================================================================
--- java/engine/org/apache/derby/impl/sql/compile/OrderByColumn.java
(revision 168148)
+++ java/engine/org/apache/derby/impl/sql/compile/OrderByColumn.java
(working copy)
@@ -30,6 +30,8 @@
import org.apache.derby.iapi.sql.compile.NodeFactory;
import org.apache.derby.iapi.sql.compile.C_NodeTypes;
+import org.apache.derby.iapi.util.ReuseFactory;
+
/**
* An OrderByColumn is a column in the ORDER BY clause. An OrderByColumn
* can be ordered ascending or descending.
@@ -44,6 +46,12 @@
private ResultColumn resultCol;
private boolean ascending = true;
private ValueNode expression;
+ /**
+ * If this sort key is added to the result column list then it is at
result column position
+ * 1 + resultColumnList.size() - resultColumnList.getOrderBySelect() +
addedColumnOffset
+ * If the sort key is already in the reault colum list then
addedColumnOffset < 0.
+ */
+ private int addedColumnOffset = -1;
/**
@@ -161,31 +169,23 @@
}
}else{
- ResultColumnList targetCols = target.getResultColumns();
- ResultColumn col = null;
- int i = 1;
-
- for(i = 1;
- i <= targetCols.size();
- i ++){
-
- col = targetCols.getOrderByColumn(i);
- if(col != null &&
- col.getExpression() == expression){
-
- break;
- }
- }
-
- resultCol = col;
- columnPosition = i;
-
+ if( SanityManager.DEBUG)
+ SanityManager.ASSERT( addedColumnOffset >= 0,
+ "Order by expression was not pulled into
the result column list");
+ resolveAddedColumn(target);
}
// Verify that the column is orderable
resultCol.verifyOrderable();
}
+ private void resolveAddedColumn(ResultSetNode target)
+ {
+ ResultColumnList targetCols = target.getResultColumns();
+ columnPosition = targetCols.size() - targetCols.getOrderBySelect() +
addedColumnOffset + 1;
+ resultCol = targetCols.getResultColumn( columnPosition);
+ }
+
/**
* Pull up this orderby column if it doesn't appear in the resultset
*
@@ -195,15 +195,42 @@
public void pullUpOrderByColumn(ResultSetNode target)
throws StandardException
{
- if(expression instanceof ColumnReference){
+ ResultColumnList targetCols = target.getResultColumns();
+ // If the target is generated for a select node then we must also pull
the order by column
+ // into the select list of the subquery.
+ if((target instanceof SelectNode) && ((SelectNode)
target).getGeneratedForGroupbyClause())
+ {
+ if( SanityManager.DEBUG)
+ SanityManager.ASSERT( target.getFromList().size() == 1
+ && (target.getFromList().elementAt(0)
instanceof FromSubquery)
+ && targetCols.size() == 1
+ && targetCols.getResultColumn(1)
instanceof AllResultColumn,
+ "Unexpected structure of selectNode
generated for a group by clause");
+
+ ResultSetNode subquery = ((FromSubquery)
target.getFromList().elementAt(0)).getSubquery();
+ pullUpOrderByColumn( subquery);
+ if( resultCol == null) // The order by column is referenced by
number
+ return;
+
+ // ResultCol is in the subquery's ResultColumnList. We have to
transform this OrderByColumn
+ // so that it refers to the column added to the subquery. We
assume that the select list
+ // in the top level target is a (generated) AllResultColumn node,
so the this order by expression
+ // does not have to be pulled into the the top level
ResultColumnList. Just change this
+ // OrderByColumn to be a reference to the added column. We cannot
use an integer column
+ // number because the subquery can have a '*' in its select list,
causing the column
+ // number to change when the '*' is expanded.
+ resultCol = null;
+ targetCols.copyOrderBySelect( subquery.getResultColumns());
+ return;
+ }
+
+ if(expression instanceof ColumnReference){
+
ColumnReference cr = (ColumnReference) expression;
- ResultColumnList targetCols = target.getResultColumns();
resultCol =
targetCols.getOrderByColumn(cr.getColumnName(),
- cr.tableName !=
null ?
-
cr.tableName.getFullTableName():
- null);
+ cr.getTableNameNode());
if(resultCol == null){
resultCol = (ResultColumn)
getNodeFactory().getNode(C_NodeTypes.RESULT_COLUMN,
@@ -211,16 +238,17 @@
cr,
getContextManager());
targetCols.addResultColumn(resultCol);
+ addedColumnOffset = targetCols.getOrderBySelect();
targetCols.incOrderBySelect();
}
}else if(!isReferedColByNum(expression)){
- ResultColumnList targetCols =
target.getResultColumns();
resultCol = (ResultColumn)
getNodeFactory().getNode(C_NodeTypes.RESULT_COLUMN,
null,
expression,
getContextManager());
targetCols.addResultColumn(resultCol);
+ addedColumnOffset = targetCols.getOrderBySelect();
targetCols.incOrderBySelect();
}
}
@@ -284,7 +312,7 @@
}
- private static ResultColumn resolveColumnReference(ResultSetNode target,
+ private ResultColumn resolveColumnReference(ResultSetNode target,
ColumnReference cr)
throws StandardException{
@@ -336,8 +364,15 @@
ResultColumnList targetCols = target.getResultColumns();
resultCol = targetCols.getOrderByColumn(cr.getColumnName(),
- cr.getTableName(),
+ cr.getTableNameNode(),
sourceTableNumber);
+ /* Search targetCols before using addedColumnOffset because select
list wildcards, '*',
+ * are expanded after pullUpOrderByColumn is called. A simple column
reference in the
+ * order by clause may be found in the user specified select list now
even though it was
+ * not found when pullUpOrderByColumn was called.
+ */
+ if( resultCol == null && addedColumnOffset >= 0)
+ resolveAddedColumn(target);
if (resultCol == null || resultCol.isNameGenerated()){
String errString = cr.columnName;
Index: java/engine/org/apache/derby/impl/sql/compile/ResultColumnList.java
===================================================================
--- java/engine/org/apache/derby/impl/sql/compile/ResultColumnList.java
(revision 168148)
+++ java/engine/org/apache/derby/impl/sql/compile/ResultColumnList.java
(working copy)
@@ -354,14 +354,14 @@
* columnName and ensure that there is only one match.
*
* @param columnName The ResultColumn to get from the list
- * @param exposedName The correlation name on the OrderByColumn, if
any
+ * @param tableName The table name on the OrderByColumn, if any
* @param tableNumber The tableNumber corresponding to the FromTable
with the
- * exposed name of
exposedName, if exposedName != null.
+ * exposed name of
tableName, if tableName != null.
*
* @return the column that matches that name.
* @exception StandardException thrown on duplicate
*/
- public ResultColumn getOrderByColumn(String columnName, String
exposedName, int tableNumber)
+ public ResultColumn getOrderByColumn(String columnName, TableName
tableName, int tableNumber)
throws StandardException
{
int size = size();
@@ -378,26 +378,15 @@
* o The RC is not qualified, but its expression
is a ColumnReference
* from the same table (as determined by the
tableNumbers).
*/
- if (exposedName != null)
+ if (tableName != null)
{
- String rcTableName =
resultColumn.getTableName();
-
- if (rcTableName == null)
- {
- ValueNode rcExpr =
resultColumn.getExpression();
- if (! (rcExpr instanceof
ColumnReference))
- {
+ ValueNode rcExpr = resultColumn.getExpression();
+ if (! (rcExpr instanceof ColumnReference))
continue;
- }
- else if (tableNumber !=
((ColumnReference) rcExpr).getTableNumber())
- {
- continue;
- }
- }
- else if (!
exposedName.equals(resultColumn.getTableName()))
- {
- continue;
- }
+
+ ColumnReference cr = (ColumnReference) rcExpr;
+ if( (! tableName.equals( cr.getTableNameNode())) &&
tableNumber != cr.getTableNumber())
+ continue;
}
/* We finally got past the qualifiers, now see if the
column
@@ -430,12 +419,12 @@
* columnName and ensure that there is only one match before the bind
process.
*
* @param columnName The ResultColumn to get from the list
- * @param exposedName The correlation name on the OrderByColumn, if
any
+ * @param tableName The table name on the OrderByColumn, if any
*
* @return the column that matches that name.
* @exception StandardException thrown on duplicate
*/
- public ResultColumn getOrderByColumn(String columnName, String
exposedName)
+ public ResultColumn getOrderByColumn(String columnName, TableName
tableName)
throws StandardException
{
int size = size();
@@ -449,20 +438,16 @@
// exposedName will not be null and "*" will not have
an expression
// or tablename.
// We may be checking on "ORDER BY T.A" against "SELECT
T.B, T.A".
- if (exposedName != null)
+ if (tableName != null)
{
ValueNode rcExpr = resultColumn.getExpression();
- if (rcExpr == null ||
resultColumn.getTableName() == null)
- {
- continue;
- }
- else
- {
- if (! (rcExpr instanceof
ColumnReference) || ! exposedName.equals(resultColumn.getTableName()))
- {
- continue;
- }
- }
+ if (rcExpr == null || ! (rcExpr instanceof
ColumnReference))
+ {
+ continue;
+ }
+ ColumnReference cr = (ColumnReference) rcExpr;
+ if( ! tableName.equals( cr.getTableNameNode()))
+ continue;
}
/* We finally got past the qualifiers, now see if the
column
@@ -3925,4 +3910,9 @@
{
return orderBySelect;
}
+
+ public void copyOrderBySelect( ResultColumnList src)
+ {
+ orderBySelect = src.orderBySelect;
+ }
}
Index: java/testing/org/apache/derbyTesting/functionTests/tests/lang/orderby.sql
===================================================================
--- java/testing/org/apache/derbyTesting/functionTests/tests/lang/orderby.sql
(revision 168148)
+++ java/testing/org/apache/derbyTesting/functionTests/tests/lang/orderby.sql
(working copy)
@@ -320,6 +320,10 @@
create table bug2769(c1 int, c2 int);
insert into bug2769 values (1, 1), (1, 2), (3, 2), (3, 3);
select a.c1, sum(a.c1) from bug2769 a group by a.c1 order by a.c1;
+select bug2769.c1 as x, sum(bug2769.c1) as y from bug2769 group by bug2769.c1
order by bug2769.c1;
+select bug2769.c1 as x, sum(bug2769.c1) as y from bug2769 group by bug2769.c1
order by x;
+select c1 as x, c2 as y from bug2769 group by bug2769.c1, bug2769.c2 order by
c1 + c2;
+select c1 as x, c2 as y from bug2769 group by bug2769.c1, bug2769.c2 order by
-(c1 + c2);
rollback;
-- reset autocommit
Index: java/testing/org/apache/derbyTesting/functionTests/master/orderby.out
===================================================================
--- java/testing/org/apache/derbyTesting/functionTests/master/orderby.out
(revision 168148)
+++ java/testing/org/apache/derbyTesting/functionTests/master/orderby.out
(working copy)
@@ -759,6 +759,30 @@
-----------------------
1 |2
3 |6
+ij> select bug2769.c1 as x, sum(bug2769.c1) as y from bug2769 group by
bug2769.c1 order by bug2769.c1;
+X |Y
+-----------------------
+1 |2
+3 |6
+ij> select bug2769.c1 as x, sum(bug2769.c1) as y from bug2769 group by
bug2769.c1 order by x;
+X |Y
+-----------------------
+1 |2
+3 |6
+ij> select c1 as x, c2 as y from bug2769 group by bug2769.c1, bug2769.c2 order
by c1 + c2;
+X |Y
+-----------------------
+1 |1
+1 |2
+3 |2
+3 |3
+ij> select c1 as x, c2 as y from bug2769 group by bug2769.c1, bug2769.c2 order
by -(c1 + c2);
+X |Y
+-----------------------
+3 |3
+3 |2
+1 |2
+1 |1
ij> rollback;
ij> -- reset autocommit
autocommit on;
