Towards getting ORDER BY to match against the correctin columns.

This version only looks at the left-most column in a compound SELECT.
That is the correct thing to do, but not what SQLite has historically done. (CVS 4620)

FossilOrigin-Name: bbddf16ac9539c7d48adfc73c5a90eecb8df6865

manifest

@@ -1,5 +1,5 @@
-C Minor\scleanup\schanges\son\sthe\sOP_StackDepth\sopcode.\s\sAdded\sthe\nsidedelete\stest\sfor\sadditional\stesting\sof\sticket\s#2832.\s(CVS\s4619)
+C Towards\sgetting\sORDER\sBY\sto\smatch\sagainst\sthe\scorrectin\scolumns.\nThis\sversion\sonly\slooks\sat\sthe\sleft-most\scolumn\sin\sa\scompound\sSELECT.\nThat\sis\sthe\scorrect\sthing\sto\sdo,\sbut\snot\swhat\sSQLite\shas\shistorically\sdone.\s(CVS\s4620)
-D 2007-12-12T22:24:13
+D 2007-12-13T02:45:31
 F Makefile.arm-wince-mingw32ce-gcc ac5f7b2cef0cd850d6f755ba6ee4ab961b1fadf7
 F Makefile.in 0590398f62fc2c456ff4c45e9741f5a718b7e2ac
 F Makefile.linux-gcc d53183f4aa6a9192d249731c90dbdffbd2c68654
@@ -129,9 +129,9 @@ F src/pager.h f504f7ae84060fee0416a853e368d3d113c3d6fa
 F src/parse.y a780b33ef45dd7b3272319cf91e609d6f109a31c
 F src/pragma.c 0246032dbe681dded8710ac43eaf654eead1434e
 F src/prepare.c f811fdb6fd4a82cca673a6e1d5b041d6caf567f1
-F src/printf.c c94a2571a828b927c64f5e3ed3584da8a91fcaec
+F src/printf.c 5732e393c45be7c09bfca9a786daef017e0066ef
 F src/random.c 4a22746501bf36b0a088c66e38dde5daba6a35da
-F src/select.c b7408624b55f58e5aa8521edb177b26ad72f968b
+F src/select.c 63cc67c9a9cc3f32ec2205d2c769d94cd2fc6f60
 F src/server.c 087b92a39d883e3fa113cae259d64e4c7438bc96
 F src/shell.c c97be281cfc3dcb14902f45e4b16f20038eb83ff
 F src/sqlite.h.in b16a7127dad4a3e5b1b26b3d64241f3373aa12ea
@@ -389,7 +389,7 @@ F test/misc6.test 953cc693924d88e6117aeba16f46f0bf5abede91
 F test/misc7.test 3fbd0a9e3dd03331d9d76acd47bc179e1a97e15e
 F test/misuse.test 30b3a458e5a70c31e74c291937b6c82204c59f33
 F test/notnull.test 44d600f916b770def8b095a9962dbe3be5a70d82
-F test/null.test 9503e1f63e959544c006d9f01709c5b5eab67d54
+F test/null.test 93e48033673841240f137fd621317cc73b69eeb8
 F test/onefile.test b9cce375fd2a41ee3afa79a0a808954046b74458
 F test/openv2.test f5dd6b23e4dce828eb211649b600763c42a668df
 F test/pager.test 60303481b22b240c18d6dd1b64edcecc2f4b5a97
@@ -600,7 +600,7 @@ F www/tclsqlite.tcl 8be95ee6dba05eabcd27a9d91331c803f2ce2130
 F www/vdbe.tcl 87a31ace769f20d3627a64fa1fade7fed47b90d0
 F www/version3.tcl 890248cf7b70e60c383b0e84d77d5132b3ead42b
 F www/whentouse.tcl fc46eae081251c3c181bd79c5faef8195d7991a5
-P 48947e2b75d39c5be0d08fe1c7b888d9065f9116
+P c0689409320de1532be0c0cae12b4b716f6bffb9
-R bb83372b6806afd80973cd2815dddbab
+R 1ec374c4daffe98452f78ad79abdb48f
 U drh
-Z ed8f32215ce7ea227bff236ab22d0da6
+Z 92df68b2adbb4d9afb7d6635a4dfe3d8

manifest.uuid

@@ -1 +1 @@
-c0689409320de1532be0c0cae12b4b716f6bffb9
+bbddf16ac9539c7d48adfc73c5a90eecb8df6865

src/printf.c

@@ -74,6 +74,7 @@
 #define etSRCLIST    14 /* a pointer to a SrcList */
 #define etPOINTER    15 /* The %p conversion */
 #define etSQLESCAPE3 16 /* %w -> Strings with '\"' doubled */
+#define etORDINAL    17 /* %r -> 1st, 2nd, 3rd, 4th, etc.  English only */
 
 
 /*
@@ -133,6 +134,7 @@ static const et_info fmtinfo[] = {
   {  'p', 16, 0, etPOINTER,    0,  1 },
   {  'T',  0, 2, etTOKEN,      0,  0 },
   {  'S',  0, 2, etSRCLIST,    0,  0 },
+  {  'r', 10, 3, etORDINAL,    0,  0 },
 };
 #define etNINFO  (sizeof(fmtinfo)/sizeof(fmtinfo[0]))
 
@@ -384,6 +386,7 @@ static void vxprintf(
         flag_longlong = sizeof(char*)==sizeof(i64);
         flag_long = sizeof(char*)==sizeof(long int);
         /* Fall through into the next case */
+      case etORDINAL:
       case etRADIX:
         if( infop->flags & FLAG_SIGNED ){
           i64 v;
@@ -410,6 +413,9 @@ static void vxprintf(
           precision = width-(prefix!=0);
         }
         bufpt = &buf[etBUFSIZE-1];
+        if( xtype==etORDINAL ){
+          bufpt -= 2;
+        }
         {
           register const char *cset;      /* Use registers for speed */
           register int base;
@@ -420,6 +426,13 @@ static void vxprintf(
             longvalue = longvalue/base;
           }while( longvalue>0 );
         }
+        if( xtype==etORDINAL ){
+          static const char zOrd[] = "thstndrd";
+          int x = buf[etBUFSIZE-4] - '0';
+          if( x>=4 ) x = 0;
+          buf[etBUFSIZE-3] = zOrd[x*2];
+          buf[etBUFSIZE-2] = zOrd[x*2+1];
+        }
         length = &buf[etBUFSIZE-1]-bufpt;
         for(idx=precision-length; idx>0; idx--){
           *(--bufpt) = '0';                             /* Zero pad */

src/select.c

@@ -12,7 +12,7 @@
 ** This file contains C code routines that are called by the parser
 ** to handle SELECT statements in SQLite.
 **
-** $Id: select.c,v 1.365 2007/12/10 18:51:48 danielk1977 Exp $
+** $Id: select.c,v 1.366 2007/12/13 02:45:31 drh Exp $
 */
 #include "sqliteInt.h"
 
@@ -1412,100 +1412,222 @@ static int prepSelectStmt(Parse *pParse, Select *p){
   return rc;
 }
 
-#ifndef SQLITE_OMIT_COMPOUND_SELECT
 /*
-** This routine associates entries in an ORDER BY expression list with
+** pE is a pointer to an expression which is a single term in
-** columns in a result.  For each ORDER BY expression, the opcode of
+** ORDER BY or GROUP BY clause.
-** the top-level node is changed to TK_COLUMN and the iColumn value of
-** the top-level node is filled in with column number and the iTable
-** value of the top-level node is filled with iTable parameter.
 **
-** Any entry that does not match is flagged as an error.  The number
+** If pE evaluates to an integer constant i, then return i.
-** of errors is returned.
+** This is an indication to the caller that it should sort
+** by the i-th column of the result set.
+**
+** If pE is a well-formed expression and the SELECT statement
+** is not compound, then return 0.  This indicates to the
+** caller that it should sort by the value of the ORDER BY
+** expression.
+**
+** If the SELECT is compound, then attempt to match pE against
+** result set columns in the left-most SELECT statement.  Return
+** the index i of the matching column, as an indication to the 
+** caller that it should sort by the i-th column.  If there is
+** no match, return -1 and leave an error message in pParse.
 */
-static int matchOrderbyToColumn(
+static int matchOrderByTermToExprList(
-  Parse *pParse,          /* A place to leave error messages */
+  Parse *pParse,     /* Parsing context for error messages */
-  Select *pSelect,        /* Match to result columns of this SELECT */
+  Select *pSelect,   /* The SELECT statement with the ORDER BY clause */
-  ExprList *pOrderBy,     /* The ORDER BY values to match against columns */
+  Expr *pE,          /* The specific ORDER BY term */
-  int iTable              /* Insert this value in iTable */
+  int idx,           /* When ORDER BY term is this */
+  int isCompound,    /* True if this is a compound SELECT */
+  u8 *pHasAgg        /* True if expression contains aggregate functions */
 ){
-  int nErr = 0;
+  int i;             /* Loop counter */
-  int i, j;
+  ExprList *pEList;  /* The columns of the result set */
-  sqlite3 *db = pParse->db;
+  NameContext nc;    /* Name context for resolving pE */
-  int nExpr;
 
-  if( pSelect==0 || pOrderBy==0 ) return 1;
+
-  if( sqlite3SelectResolve(pParse, pSelect, 0) ){
+  /* If the term is an integer constant, return the value of that
+  ** constant */
+  pEList = pSelect->pEList;
+  if( sqlite3ExprIsInteger(pE, &i) ){
+    if( i<=0 ){
+      /* If i is too small, make it too big.  That way the calling
+      ** function still sees a value that is out of range, but does
+      ** not confuse the column number with 0 or -1 result code.
+      */
+      i = pEList->nExpr+1;
+    }
+    return i;
+  }
+
+  /* If the term is a simple identifier that try to match that identifier
+  ** against a column name in the result set.
+  */
+  if( pE->op==TK_ID || (pE->op==TK_STRING && pE->token.z[0]!='\'') ){
+    sqlite3 *db = pParse->db;
+    char *zCol = sqlite3NameFromToken(db, &pE->token);
+    if( db->mallocFailed ){
+      return -1;
+    }
+    for(i=0; i<pEList->nExpr; i++){
+      char *zAs = pEList->a[i].zName;
+      if( zAs!=0 && sqlite3StrICmp(zAs, zCol)==0 ){
+        sqlite3_free(zCol);
+        return i+1;
+      }
+    }
+    sqlite3_free(zCol);
+  }
+
+  /* Resolve all names in the ORDER BY term expression
+  */
+  memset(&nc, 0, sizeof(nc));
+  nc.pParse = pParse;
+  nc.pSrcList = pSelect->pSrc;
+  nc.pEList = pEList;
+  nc.allowAgg = 1;
+  nc.nErr = 0;
+  if( sqlite3ExprResolveNames(&nc, pE) ){
+    return -1;
+  }
+  if( nc.hasAgg && pHasAgg ){
+    *pHasAgg = 1;
+  }
+
+  /* For a compound SELECT, we need to try to match the ORDER BY
+  ** expression against an expression in the result set
+  */
+  if( isCompound ){
+    for(i=0; i<pEList->nExpr; i++){
+      if( sqlite3ExprCompare(pEList->a[i].pExpr, pE) ){
+        return i+1;
+      }
+    }
+    sqlite3ErrorMsg(pParse, "%r ORDER BY term does not match any "
+       "column in the result set of the left-most SELECT", idx);
+    return -1;
+  }else{
+    return 0;
+  }
+}
+
+
+/*
+** Analyze and ORDER BY or GROUP BY clause in a simple SELECT statement.
+** Return the number of errors seen.
+**
+** Every term of the ORDER BY or GROUP BY clause needs to be an
+** expression.  If any expression is an integer constant, then
+** that expression is replaced by the corresponding 
+** expression from the result set.
+*/
+static int processOrderGroupBy(
+  Parse *pParse,        /* Parsing context.  Leave error messages here */
+  Select *pSelect,      /* The SELECT statement containing the clause */
+  ExprList *pOrderBy,   /* The ORDER BY or GROUP BY clause to be processed */
+  int isOrder,          /* 1 for ORDER BY.  0 for GROUP BY */
+  u8 *pHasAgg           /* Set to TRUE if any term contains an aggregate */
+){
+  int i;
+  sqlite3 *db = pParse->db;
+  ExprList *pEList;
+
+  if( pOrderBy==0 ) return 0;
+  if( pOrderBy->nExpr>SQLITE_MAX_COLUMN ){
+    const char *zType = isOrder ? "ORDER" : "GROUP";
+    sqlite3ErrorMsg(pParse, "too many terms in %s BY clause", zType);
     return 1;
   }
-
+  pEList = pSelect->pEList;
-  nExpr = pSelect->pEList->nExpr;
+  if( pEList==0 ){
-  for(i=0; nErr==0 && i<pOrderBy->nExpr; i++){
+    return 0;
+  }
+  for(i=0; i<pOrderBy->nExpr; i++){
+    int iCol;
     Expr *pE = pOrderBy->a[i].pExpr;
-    int iCol = -1;
+    iCol = matchOrderByTermToExprList(pParse, pSelect, pE, i+1, 0, pHasAgg);
-
-    if( sqlite3ExprIsInteger(pE, &iCol) ){
-      if( iCol<=0 || iCol>nExpr ){
-        sqlite3ErrorMsg(pParse,
-          "ORDER BY position %d should be between 1 and %d",
-          iCol, nExpr);
-        nErr++;
-        break;
-      }
-      iCol--;
-    }else{
-      Select *p;
-      for(p=pSelect; p; p=p->pPrior){
-        ExprList *pEList = p->pEList;
-        Expr *pDup = sqlite3ExprDup(db, pE);
-
-        NameContext nc;
-
-        memset(&nc, 0, sizeof(nc));
-        nc.pParse = pParse;
-        nc.pSrcList = p->pSrc;
-        nc.pEList = pEList;
-        nc.allowAgg = 1;
-        nc.nErr = 0;
-        if( sqlite3ExprResolveNames(&nc, pDup) ){
-          sqlite3ErrorClear(pParse);
-        }else{
-          struct ExprList_item *pItem;
-          for(j=0, pItem=pEList->a; j<pEList->nExpr; j++, pItem++){
-            if( sqlite3ExprCompare(pItem->pExpr, pDup) ){
-              if( iCol>=0 && iCol!=j ){
-                sqlite3ErrorMsg(
-                    pParse, "ORDER BY term number %d is ambiguous", i+1
-                );
-              }else{
-                iCol = j;
-              }
-            }
-          }
-        }
-        sqlite3ExprDelete(pDup);
-      }
-    }
-
     if( iCol<0 ){
-      sqlite3ErrorMsg(pParse,
+      return 1;
-        "ORDER BY term number %d does not match any result column", i+1);
-    }else{
-      pE->op = TK_COLUMN;
-      pE->iTable = iTable;
-      pE->iAgg = -1;
-      pE->iColumn = iCol;
-      pOrderBy->a[i].done = 1;
     }
-
+    if( iCol>pEList->nExpr ){
-    if( pParse->nErr ){
+      const char *zType = isOrder ? "ORDER" : "GROUP";
-      return pParse->nErr;
+      sqlite3ErrorMsg(pParse, 
+         "%r %s BY term out of range - should be "
+         "between 1 and %d", i+1, zType, pEList->nExpr);
+      return 1;
+    }
+    if( iCol>0 ){
+      CollSeq *pColl = pE->pColl;
+      int flags = pE->flags & EP_ExpCollate;
+      sqlite3ExprDelete(pE);
+      pE = sqlite3ExprDup(db, pEList->a[iCol-1].pExpr);
+      pOrderBy->a[i].pExpr = pE;
+      if( pColl && flags ){
+        pE->pColl = pColl;
+        pE->flags |= flags;
+      }
     }
   }
-
+  return 0;
-  return SQLITE_OK;
+}
+
+/*
+** Analyze and ORDER BY or GROUP BY clause in a SELECT statement.  Return
+** the number of errors seen.
+**
+** The processing depends on whether the SELECT is simple or compound.
+** For a simple SELECT statement, evry term of the ORDER BY or GROUP BY
+** clause needs to be an expression.  If any expression is an integer
+** constant, then that expression is replaced by the corresponding 
+** expression from the result set.
+**
+** For compound SELECT statements, every expression needs to be of
+** type TK_COLUMN with a iTable value as given in the 4th parameter.
+** If any expression is an integer, that becomes the column number.
+** Otherwise, match the expression against result set columns from
+** the left-most SELECT.
+*/
+static int processCompoundOrderBy(
+  Parse *pParse,        /* Parsing context.  Leave error messages here */
+  Select *pSelect,      /* The SELECT statement containing the ORDER BY */
+  int iTable            /* Output table for compound SELECT statements */
+){
+  int i;
+  ExprList *pOrderBy;
+  ExprList *pEList;
+
+  pOrderBy = pSelect->pOrderBy;
+  if( pOrderBy==0 ) return 0;
+  if( pOrderBy->nExpr>SQLITE_MAX_COLUMN ){
+    sqlite3ErrorMsg(pParse, "too many terms in ORDER BY clause");
+    return 1;
+  }
+  while( pSelect->pPrior ){
+    pSelect = pSelect->pPrior;
+  }
+  pEList = pSelect->pEList;
+  if( pEList==0 ){
+    return 1;
+  }
+  for(i=0; i<pOrderBy->nExpr; i++){
+    int iCol;
+    Expr *pE = pOrderBy->a[i].pExpr;
+    iCol = matchOrderByTermToExprList(pParse, pSelect, pE, i+1, 1, 0);
+    if( iCol<0 ){
+      return 1;
+    }
+    if( iCol>pEList->nExpr ){
+      sqlite3ErrorMsg(pParse, 
+         "%r ORDER BY term out of range - should be "
+         "between 1 and %d", i+1, pEList->nExpr);
+      return 1;
+    }
+    pE->op = TK_COLUMN;
+    pE->iTable = iTable;
+    pE->iAgg = -1;
+    pE->iColumn = iCol-1;
+    pE->pTab = 0;
+  }
+  return 0;
 }
-#endif /* #ifndef SQLITE_OMIT_COMPOUND_SELECT */
 
 /*
 ** Get a VDBE for the given parser context.  Create a new one if necessary.
@@ -1768,7 +1890,7 @@ static int multiSelect(
         ** intermediate results.
         */
         unionTab = pParse->nTab++;
-        if( pOrderBy && matchOrderbyToColumn(pParse, p, pOrderBy, unionTab) ){
+        if( processCompoundOrderBy(pParse, p, unionTab) ){
           rc = 1;
           goto multi_select_end;
         }
@@ -1865,7 +1987,7 @@ static int multiSelect(
       */
       tab1 = pParse->nTab++;
       tab2 = pParse->nTab++;
-      if( pOrderBy && matchOrderbyToColumn(pParse,p,pOrderBy,tab1) ){
+      if( processCompoundOrderBy(pParse, p, tab1) ){
         rc = 1;
         goto multi_select_end;
       }
@@ -2579,57 +2701,6 @@ static int simpleMinMaxQuery(Parse *pParse, Select *p, int eDest, int iParm){
   return 1;
 }
 
-/*
-** Analyze and ORDER BY or GROUP BY clause in a SELECT statement.  Return
-** the number of errors seen.
-**
-** An ORDER BY or GROUP BY is a list of expressions.  If any expression
-** is an integer constant, then that expression is replaced by the
-** corresponding entry in the result set.
-*/
-static int processOrderGroupBy(
-  NameContext *pNC,     /* Name context of the SELECT statement. */
-  ExprList *pOrderBy,   /* The ORDER BY or GROUP BY clause to be processed */
-  const char *zType     /* Either "ORDER" or "GROUP", as appropriate */
-){
-  int i;
-  ExprList *pEList = pNC->pEList;     /* The result set of the SELECT */
-  Parse *pParse = pNC->pParse;     /* The result set of the SELECT */
-  assert( pEList );
-
-  if( pOrderBy==0 ) return 0;
-  if( pOrderBy->nExpr>SQLITE_MAX_COLUMN ){
-    sqlite3ErrorMsg(pParse, "too many terms in %s BY clause", zType);
-    return 1;
-  }
-  for(i=0; i<pOrderBy->nExpr; i++){
-    int iCol;
-    Expr *pE = pOrderBy->a[i].pExpr;
-    if( sqlite3ExprIsInteger(pE, &iCol) ){
-      if( iCol>0 && iCol<=pEList->nExpr ){
-        CollSeq *pColl = pE->pColl;
-        int flags = pE->flags & EP_ExpCollate;
-        sqlite3ExprDelete(pE);
-        pE = sqlite3ExprDup(pParse->db, pEList->a[iCol-1].pExpr);
-        pOrderBy->a[i].pExpr = pE;
-        if( pColl && flags ){
-          pE->pColl = pColl;
-          pE->flags |= flags;
-        }
-      }else{
-        sqlite3ErrorMsg(pParse, 
-           "%s BY column number %d out of range - should be "
-           "between 1 and %d", zType, iCol, pEList->nExpr);
-        return 1;
-      }
-    }
-    if( sqlite3ExprResolveNames(pNC, pE) ){
-      return 1;
-    }
-  }
-  return 0;
-}
-
 /*
 ** This routine resolves any names used in the result set of the
 ** supplied SELECT statement. If the SELECT statement being resolved
@@ -2723,10 +2794,12 @@ int sqlite3SelectResolve(
      sqlite3ExprResolveNames(&sNC, p->pHaving) ){
     return SQLITE_ERROR;
   }
-  if( p->pPrior==0 && processOrderGroupBy(&sNC, p->pOrderBy, "ORDER") ){
+  if( p->pPrior==0 ){
-    return SQLITE_ERROR;
+    if( processOrderGroupBy(pParse, p, p->pOrderBy, 0, &sNC.hasAgg) ){
+      return SQLITE_ERROR;
+    }
   }
-  if( processOrderGroupBy(&sNC, pGroupBy, "GROUP") ){
+  if( processOrderGroupBy(pParse, p, pGroupBy, 0, &sNC.hasAgg) ){
     return SQLITE_ERROR;
   }
 
@@ -2946,6 +3019,15 @@ int sqlite3Select(
   if( sqlite3AuthCheck(pParse, SQLITE_SELECT, 0, 0, 0) ) return 1;
   memset(&sAggInfo, 0, sizeof(sAggInfo));
 
+  pOrderBy = p->pOrderBy;
+  if( IgnorableOrderby(eDest) ){
+    p->pOrderBy = 0;
+  }
+  if( sqlite3SelectResolve(pParse, p, 0) ){
+    goto select_end;
+  }
+  p->pOrderBy = pOrderBy;
+
 #ifndef SQLITE_OMIT_COMPOUND_SELECT
   /* If there is are a sequence of queries, do the earlier ones first.
   */
@@ -2965,15 +3047,6 @@ int sqlite3Select(
   }
 #endif
 
-  pOrderBy = p->pOrderBy;
-  if( IgnorableOrderby(eDest) ){
-    p->pOrderBy = 0;
-  }
-  if( sqlite3SelectResolve(pParse, p, 0) ){
-    goto select_end;
-  }
-  p->pOrderBy = pOrderBy;
-
   /* Make local copies of the parameters for this query.
   */
   pTabList = p->pSrc;

test/null.test

@@ -153,11 +153,26 @@ do_test null-5.1 {
 # as distinct
 #
 ifcapable compound {
-do_test null-6.1 {
+  do_test null-6.1 {
-  execsql {
+    execsql {
-    select b from t1 union select c from t1 order by c;
+      select b from t1 union select c from t1 order by b;
-  }
+    }
-} {{} 0 1}
+  } {{} 0 1}
+  do_test null-6.2 {
+    execsql {
+      select b from t1 union select c from t1 order by 1;
+    }
+  } {{} 0 1}
+  do_test null-6.3 {
+    execsql {
+      select b from t1 union select c from t1 order by t1.b;
+    }
+  } {{} 0 1}
+  do_test null-6.4 {
+    execsql {
+      select b from t1 union select c from t1 order by main.t1.b;
+    }
+  } {{} 0 1}
 } ;# ifcapable compound
 
 # The UNIQUE constraint only applies to non-null values