]> git.ipfire.org Git - thirdparty/sqlite.git/commitdiff
Fix for ticket #73: The ORDER BY clause is significant for subqueries.
authordrh <drh@noemail.net>
Thu, 20 Jun 2002 03:38:26 +0000 (03:38 +0000)
committerdrh <drh@noemail.net>
Thu, 20 Jun 2002 03:38:26 +0000 (03:38 +0000)
This passes all regression tests, but more testing is needed to exercise
all paths through the new code. (CVS 631)

FossilOrigin-Name: 43c5aff5d078bce9292683cd40311e0dcc81ac14

manifest
manifest.uuid
src/select.c
test/select4.test

index 1afeb5d6c4218ccf4395c0c4db2a7db1fd2888b7..4de36bbeffc660ff2f58799558371b64a2584d18 100644 (file)
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C Fix\sfor\sticket\s#75:\sAutoincrement\sINTEGER\sPRIMARY\sKEY\sfields\son\san\sINSERT\neven\sif\sthe\sdata\sis\scoming\sfrom\sa\sSELECT\sstatement.\s(CVS\s630)
-D 2002-06-19T20:32:44
+C Fix\sfor\sticket\s#73:\sThe\sORDER\sBY\sclause\sis\ssignificant\sfor\ssubqueries.\nThis\spasses\sall\sregression\stests,\sbut\smore\stesting\sis\sneeded\sto\sexercise\nall\spaths\sthrough\sthe\snew\scode.\s(CVS\s631)
+D 2002-06-20T03:38:26
 F Makefile.in 6291a33b87d2a395aafd7646ee1ed562c6f2c28c
 F Makefile.template 4e11752e0b5c7a043ca50af4296ec562857ba495
 F README a4c0ba11354ef6ba0776b400d057c59da47a4cc0
@@ -37,7 +37,7 @@ F src/pager.h 6fddfddd3b73aa8abc081b973886320e3c614f0e
 F src/parse.y 2285d8967d7334d52a2188089e5a881d73ba56f6
 F src/printf.c 236ed7a79386feed4456fa728fff8be793f1547c
 F src/random.c 19e8e00fe0df32a742f115773f57651be327cabe
-F src/select.c 514b4b12f72df68e0c8b4187af302b7c22b22cf6
+F src/select.c 3eadcde4c74341d8ee7db69948cbcb16df9ae9fc
 F src/shell.c 1d22fe870ee852cfb975fd000dbe3973713d0a15
 F src/shell.tcl 27ecbd63dd88396ad16d81ab44f73e6c0ea9d20e
 F src/sqlite.h.in 0038faa6d642de06b91143ee65a131bd831d020b
@@ -90,7 +90,7 @@ F test/rowid.test 4c55943300cddf73dd0f88d40a268cab14c83274
 F test/select1.test 0d708cec567104653ec9aa49fecf3444a2e7d150
 F test/select2.test aceea74fd895b9d007512f72499db589735bd8e4
 F test/select3.test 9469c332250a75a0ef1771fb5da62dc04ec77f18
-F test/select4.test 9b5c87b7a372172d65928cf860bcd5a54694c775
+F test/select4.test 13618c1107c2b51f5ae8417f00c50f899a68056f
 F test/select5.test c2a6c4a003316ee42cbbd689eebef8fdce0db2ac
 F test/select6.test efb8d0c07a440441db87db2c4ade6904e1407e85
 F test/sort.test 3b996ce7ca385f9cd559944ac0f4027a23aa546b
@@ -137,7 +137,7 @@ F www/speed.tcl da8afcc1d3ccc5696cfb388a68982bc3d9f7f00f
 F www/sqlite.tcl 8b5884354cb615049aed83039f8dfe1552a44279
 F www/tclsqlite.tcl 1db15abeb446aad0caf0b95b8b9579720e4ea331
 F www/vdbe.tcl 2013852c27a02a091d39a766bc87cff329f21218
-P 5e8a3131aba25e22f3e25b9b1c051019381f11d1
-R 524db4044083fd865802aa3a7659d610
+P d599f75b659809a6e5eee09b0e9e6e90bde5af1e
+R 086d9eb833675513d9039b13c48320be
 U drh
-Z 7f1cd27c410e1c7fc4e1a09a08e84965
+Z 8d182409003e8da6cfa46f473fde17e7
index 6692c7a677dc8c353ad3726b6d4dd31a07f00bc0..fcd3b71bdf220f05e16a49209180aae724eaa250 100644 (file)
@@ -1 +1 @@
-d599f75b659809a6e5eee09b0e9e6e90bde5af1e
\ No newline at end of file
+43c5aff5d078bce9292683cd40311e0dcc81ac14
\ No newline at end of file
index ffcbe51fc95f01ad2bd336d0c746eebd81d5dea8..98ede32fc3cf9c18331dfd0b1826fae939f14c24 100644 (file)
@@ -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.94 2002/06/19 14:27:05 drh Exp $
+** $Id: select.c,v 1.95 2002/06/20 03:38:26 drh Exp $
 */
 #include "sqliteInt.h"
 
@@ -284,6 +284,26 @@ static void sqliteAggregateInfoReset(Parse *pParse){
   pParse->useAgg = 0;
 }
 
+/*
+** Insert code into "v" that will push the record on the top of the
+** stack into the sorter.
+*/
+static void pushOntoSorter(Parse *pParse, Vdbe *v, ExprList *pOrderBy){
+  char *zSortOrder;
+  int i;
+  zSortOrder = sqliteMalloc( pOrderBy->nExpr + 1 );
+  if( zSortOrder==0 ) return;
+  for(i=0; i<pOrderBy->nExpr; i++){
+    zSortOrder[i] = pOrderBy->a[i].sortOrder ? '-' : '+';
+    sqliteExprCode(pParse, pOrderBy->a[i].pExpr);
+  }
+  zSortOrder[pOrderBy->nExpr] = 0;
+  sqliteVdbeAddOp(v, OP_SortMakeKey, pOrderBy->nExpr, 0);
+  sqliteVdbeChangeP3(v, -1, zSortOrder, strlen(zSortOrder));
+  sqliteFree(zSortOrder);
+  sqliteVdbeAddOp(v, OP_SortPut, 0, 0);
+}
+
 /*
 ** This routine generates the code for the inside of the inner loop
 ** of a SELECT.
@@ -350,90 +370,97 @@ static int selectInnerLoop(
     sqliteVdbeAddOp(v, OP_PutStrKey, distinct, 0);
   }
 
-  /* If there is an ORDER BY clause, then store the results
-  ** in a sorter.
-  */
-  if( pOrderBy ){
-    char *zSortOrder;
-    sqliteVdbeAddOp(v, OP_SortMakeRec, nColumn, 0);
-    zSortOrder = sqliteMalloc( pOrderBy->nExpr + 1 );
-    if( zSortOrder==0 ) return 1;
-    for(i=0; i<pOrderBy->nExpr; i++){
-      zSortOrder[i] = pOrderBy->a[i].sortOrder ? '-' : '+';
-      sqliteExprCode(pParse, pOrderBy->a[i].pExpr);
-    }
-    zSortOrder[pOrderBy->nExpr] = 0;
-    sqliteVdbeAddOp(v, OP_SortMakeKey, pOrderBy->nExpr, 0);
-    sqliteVdbeChangeP3(v, -1, zSortOrder, strlen(zSortOrder));
-    sqliteFree(zSortOrder);
-    sqliteVdbeAddOp(v, OP_SortPut, 0, 0);
-  }else 
-
-  /* In this mode, write each query result to the key of the temporary
-  ** table iParm.
-  */
-  if( eDest==SRT_Union ){
-    sqliteVdbeAddOp(v, OP_MakeRecord, nColumn, NULL_ALWAYS_DISTINCT);
-    sqliteVdbeAddOp(v, OP_String, 0, 0);
-    sqliteVdbeAddOp(v, OP_PutStrKey, iParm, 0);
-  }else 
+  switch( eDest ){
+    /* In this mode, write each query result to the key of the temporary
+    ** table iParm.
+    */
+    case SRT_Union: {
+      sqliteVdbeAddOp(v, OP_MakeRecord, nColumn, NULL_ALWAYS_DISTINCT);
+      sqliteVdbeAddOp(v, OP_String, 0, 0);
+      sqliteVdbeAddOp(v, OP_PutStrKey, iParm, 0);
+      break;
+    }
 
-  /* Store the result as data using a unique key.
-  */
-  if( eDest==SRT_Table || eDest==SRT_TempTable ){
-    sqliteVdbeAddOp(v, OP_MakeRecord, nColumn, 0);
-    sqliteVdbeAddOp(v, OP_NewRecno, iParm, 0);
-    sqliteVdbeAddOp(v, OP_Pull, 1, 0);
-    sqliteVdbeAddOp(v, OP_PutIntKey, iParm, 0);
-  }else 
-
-  /* Construct a record from the query result, but instead of
-  ** saving that record, use it as a key to delete elements from
-  ** the temporary table iParm.
-  */
-  if( eDest==SRT_Except ){
-    int addr;
-    addr = sqliteVdbeAddOp(v, OP_MakeRecord, nColumn, NULL_ALWAYS_DISTINCT);
-    sqliteVdbeAddOp(v, OP_NotFound, iParm, addr+3);
-    sqliteVdbeAddOp(v, OP_Delete, iParm, 0);
-  }else 
-
-  /* If we are creating a set for an "expr IN (SELECT ...)" construct,
-  ** then there should be a single item on the stack.  Write this
-  ** item into the set table with bogus data.
-  */
-  if( eDest==SRT_Set ){
-    assert( nColumn==1 );
-    sqliteVdbeAddOp(v, OP_IsNull, -1, sqliteVdbeCurrentAddr(v)+3);
-    sqliteVdbeAddOp(v, OP_String, 0, 0);
-    sqliteVdbeAddOp(v, OP_PutStrKey, iParm, 0);
-  }else 
+    /* Store the result as data using a unique key.
+    */
+    case SRT_Table:
+    case SRT_TempTable: {
+      sqliteVdbeAddOp(v, OP_MakeRecord, nColumn, 0);
+      if( pOrderBy ){
+        pushOntoSorter(pParse, v, pOrderBy);
+      }else{
+        sqliteVdbeAddOp(v, OP_NewRecno, iParm, 0);
+        sqliteVdbeAddOp(v, OP_Pull, 1, 0);
+        sqliteVdbeAddOp(v, OP_PutIntKey, iParm, 0);
+      }
+      break;
+    }
 
+    /* Construct a record from the query result, but instead of
+    ** saving that record, use it as a key to delete elements from
+    ** the temporary table iParm.
+    */
+    case SRT_Except: {
+      int addr;
+      addr = sqliteVdbeAddOp(v, OP_MakeRecord, nColumn, NULL_ALWAYS_DISTINCT);
+      sqliteVdbeAddOp(v, OP_NotFound, iParm, addr+3);
+      sqliteVdbeAddOp(v, OP_Delete, iParm, 0);
+      break;
+    }
 
-  /* If this is a scalar select that is part of an expression, then
-  ** store the results in the appropriate memory cell and break out
-  ** of the scan loop.
-  */
-  if( eDest==SRT_Mem ){
-    assert( nColumn==1 );
-    sqliteVdbeAddOp(v, OP_MemStore, iParm, 1);
-    sqliteVdbeAddOp(v, OP_Goto, 0, iBreak);
-  }else
+    /* If we are creating a set for an "expr IN (SELECT ...)" construct,
+    ** then there should be a single item on the stack.  Write this
+    ** item into the set table with bogus data.
+    */
+    case SRT_Set: {
+      assert( nColumn==1 );
+      sqliteVdbeAddOp(v, OP_IsNull, -1, sqliteVdbeCurrentAddr(v)+3);
+      sqliteVdbeAddOp(v, OP_String, 0, 0);
+      if( pOrderBy ){
+        pushOntoSorter(pParse, v, pOrderBy);
+      }else{
+        sqliteVdbeAddOp(v, OP_PutStrKey, iParm, 0);
+      }
+      break;
+    }
 
-  /* Discard the results.  This is used for SELECT statements inside
-  ** the body of a TRIGGER.  The purpose of such selects is to call
-  ** user-defined functions that have side effects.  We do not care
-  ** about the actual results of the select.
-  */
-  if( eDest==SRT_Discard ){
-    sqliteVdbeAddOp(v, OP_Pop, nColumn, 0);
-  }else
+    /* If this is a scalar select that is part of an expression, then
+    ** store the results in the appropriate memory cell and break out
+    ** of the scan loop.
+    */
+    case SRT_Mem: {
+      assert( nColumn==1 );
+      if( pOrderBy ){
+        pushOntoSorter(pParse, v, pOrderBy);
+      }else{
+        sqliteVdbeAddOp(v, OP_MemStore, iParm, 1);
+        sqliteVdbeAddOp(v, OP_Goto, 0, iBreak);
+      }
+      break;
+    }
 
-  /* If none of the above, send the data to the callback function.
-  */
-  {
-    assert( eDest==SRT_Callback );
-    sqliteVdbeAddOp(v, OP_Callback, nColumn, 0);
+    /* Discard the results.  This is used for SELECT statements inside
+    ** the body of a TRIGGER.  The purpose of such selects is to call
+    ** user-defined functions that have side effects.  We do not care
+    ** about the actual results of the select.
+    */
+    case SRT_Discard: {
+      sqliteVdbeAddOp(v, OP_Pop, nColumn, 0);
+      break;
+    }
+
+    /* Send the data to the callback function.
+    */
+    default: {
+      assert( eDest==SRT_Callback );
+      if( pOrderBy ){
+        sqliteVdbeAddOp(v, OP_SortMakeRec, nColumn, 0);
+        pushOntoSorter(pParse, v, pOrderBy);
+      }else{
+        sqliteVdbeAddOp(v, OP_Callback, nColumn, 0);
+      }
+      break;
+    }
   }
   return 0;
 }
@@ -444,7 +471,13 @@ static int selectInnerLoop(
 ** we need to run the sorter and output the results.  The following
 ** routine generates the code needed to do that.
 */
-static void generateSortTail(Select *p, Vdbe *v, int nColumn){
+static void generateSortTail(
+  Select *p,       /* The SELECT statement */
+  Vdbe *v,         /* Generate code into this VDBE */
+  int nColumn,     /* Number of columns of data */
+  int eDest,       /* Write the sorted results here */
+  int iParm        /* Optional parameter associated with eDest */
+){
   int end = sqliteVdbeMakeLabel(v);
   int addr;
   sqliteVdbeAddOp(v, OP_Sort, 0, 0);
@@ -455,7 +488,36 @@ static void generateSortTail(Select *p, Vdbe *v, int nColumn){
   if( p->nLimit>0 ){
     sqliteVdbeAddOp(v, OP_LimitCk, 0, end);
   }
-  sqliteVdbeAddOp(v, OP_SortCallback, nColumn, 0);
+  switch( eDest ){
+    case SRT_Callback: {
+      sqliteVdbeAddOp(v, OP_SortCallback, nColumn, 0);
+      break;
+    }
+    case SRT_Table:
+    case SRT_TempTable: {
+      sqliteVdbeAddOp(v, OP_NewRecno, iParm, 0);
+      sqliteVdbeAddOp(v, OP_Pull, 1, 0);
+      sqliteVdbeAddOp(v, OP_PutIntKey, iParm, 0);
+      break;
+    }
+    case SRT_Set: {
+      assert( nColumn==1 );
+      sqliteVdbeAddOp(v, OP_IsNull, -1, sqliteVdbeCurrentAddr(v)+3);
+      sqliteVdbeAddOp(v, OP_String, 0, 0);
+      sqliteVdbeAddOp(v, OP_PutStrKey, iParm, 0);
+      break;
+    }
+    case SRT_Mem: {
+      assert( nColumn==1 );
+      sqliteVdbeAddOp(v, OP_MemStore, iParm, 1);
+      sqliteVdbeAddOp(v, OP_Goto, 0, end);
+      break;
+    }
+    default: {
+      assert( end==0 ); /* Cannot happen */
+      break;
+    }
+  }
   sqliteVdbeAddOp(v, OP_Goto, 0, addr);
   sqliteVdbeResolveLabel(v, end);
   sqliteVdbeAddOp(v, OP_SortReset, 0, 0);
@@ -898,6 +960,9 @@ Vdbe *sqliteGetVdbe(Parse *pParse){
 /*
 ** This routine is called to process a query that is really the union
 ** or intersection of two or more separate queries.
+**
+** "p" points to the right-most of the two queries.  The results should
+** be stored in eDest with parameter iParm.
 */
 static int multiSelect(Parse *pParse, Select *p, int eDest, int iParm){
   int rc;             /* Success code from a subroutine */
@@ -939,15 +1004,14 @@ static int multiSelect(Parse *pParse, Select *p, int eDest, int iParm){
       int unionTab;    /* Cursor number of the temporary table holding result */
       int op;          /* One of the SRT_ operations to apply to self */
       int priorOp;     /* The SRT_ operation to apply to prior selects */
+      ExprList *pOrderBy;  /* The ORDER BY clause for the right SELECT */
 
       priorOp = p->op==TK_ALL ? SRT_Table : SRT_Union;
-      if( eDest==priorOp ){
+      if( eDest==priorOp && p->pOrderBy==0 ){
         /* We can reuse a temporary table generated by a SELECT to our
-        ** right.  This also means we are not the right-most select and so
-        ** we cannot have an ORDER BY clause
+        ** right.
         */
         unionTab = iParm;
-        assert( p->pOrderBy==0 );
       }else{
         /* We will need to create our own temporary table to hold the
         ** intermediate results.
@@ -978,14 +1042,17 @@ static int multiSelect(Parse *pParse, Select *p, int eDest, int iParm){
          case TK_ALL:     op = SRT_Table;    break;
       }
       p->pPrior = 0;
+      pOrderBy = p->pOrderBy;
+      p->pOrderBy = 0;
       rc = sqliteSelect(pParse, p, op, unionTab, 0, 0, 0);
       p->pPrior = pPrior;
+      p->pOrderBy = pOrderBy;
       if( rc ) return rc;
 
       /* Convert the data in the temporary table into whatever form
       ** it is that we currently need.
       */      
-      if( eDest!=priorOp ){
+      if( eDest!=priorOp || unionTab!=iParm ){
         int iCont, iBreak, iStart;
         assert( p->pEList );
         if( eDest==SRT_Callback ){
@@ -1004,7 +1071,7 @@ static int multiSelect(Parse *pParse, Select *p, int eDest, int iParm){
         sqliteVdbeResolveLabel(v, iBreak);
         sqliteVdbeAddOp(v, OP_Close, unionTab, 0);
         if( p->pOrderBy ){
-          generateSortTail(p, v, p->pEList->nExpr);
+          generateSortTail(p, v, p->pEList->nExpr, eDest, iParm);
         }
       }
       break;
@@ -1061,7 +1128,7 @@ static int multiSelect(Parse *pParse, Select *p, int eDest, int iParm){
       sqliteVdbeAddOp(v, OP_Close, tab2, 0);
       sqliteVdbeAddOp(v, OP_Close, tab1, 0);
       if( p->pOrderBy ){
-        generateSortTail(p, v, p->pEList->nExpr);
+        generateSortTail(p, v, p->pEList->nExpr, eDest, iParm);
       }
       break;
     }
@@ -1538,10 +1605,16 @@ int sqliteSelect(
     goto select_end;
   }
 
-  /* ORDER BY is ignored if we are not sending the result to a callback.
+  /* ORDER BY is ignored for some destinations.
   */
-  if( eDest!=SRT_Callback ){
-    pOrderBy = 0;
+  switch( eDest ){
+    case SRT_Union:
+    case SRT_Except:
+    case SRT_Discard:
+      pOrderBy = 0;
+      break;
+    default:
+      break;
   }
 
   /* At this point, we should have allocated all the cursors that we
@@ -1830,7 +1903,7 @@ int sqliteSelect(
   ** and send them to the callback one by one.
   */
   if( pOrderBy ){
-    generateSortTail(p, v, pEList->nExpr);
+    generateSortTail(p, v, pEList->nExpr, eDest, iParm);
   }
 
 
index c586641476ae7bd5454764fa3012e63f93db24e9..2f3d2ef336e30b5d4673bea717ca7c6418ddca4a 100644 (file)
@@ -12,7 +12,7 @@
 # focus of this file is testing UNION, INTERSECT and EXCEPT operators
 # in SELECT statements.
 #
-# $Id: select4.test,v 1.10 2002/06/02 16:09:03 drh Exp $
+# $Id: select4.test,v 1.11 2002/06/20 03:38:26 drh Exp $
 
 set testdir [file dirname $argv0]
 source $testdir/tester.tcl
@@ -51,6 +51,28 @@ do_test select4-1.1c {
     ORDER BY log;
   }
 } {0 1 2 3 4 5 5 6 7 8}
+do_test select4-1.1d {
+  execsql {
+    CREATE TABLE t2 AS
+      SELECT DISTINCT log FROM t1
+      UNION ALL
+      SELECT n FROM t1 WHERE log=3
+      ORDER BY log;
+    SELECT * FROM t2;
+  }
+} {0 1 2 3 4 5 5 6 7 8}
+execsql {DROP TABLE t2}
+do_test select4-1.1e {
+  execsql {
+    CREATE TABLE t2 AS
+      SELECT DISTINCT log FROM t1
+      UNION ALL
+      SELECT n FROM t1 WHERE log=3
+      ORDER BY log DESC;
+    SELECT * FROM t2;
+  }
+} {8 7 6 5 5 4 3 2 1 0}
+execsql {DROP TABLE t2}
 do_test select4-1.2 {
   execsql {
     SELECT log FROM t1 WHERE n IN 
@@ -99,7 +121,7 @@ do_test select4-2.3 {
 
 # Except operator
 #
-do_test select4-3.1 {
+do_test select4-3.1.1 {
   execsql {
     SELECT DISTINCT log FROM t1
     EXCEPT
@@ -107,6 +129,28 @@ do_test select4-3.1 {
     ORDER BY log;
   }
 } {0 1 2 3 4}
+do_test select4-3.1.2 {
+  execsql {
+    CREATE TABLE t2 AS 
+      SELECT DISTINCT log FROM t1
+      EXCEPT
+      SELECT n FROM t1 WHERE log=3
+      ORDER BY log;
+    SELECT * FROM t2;
+  }
+} {0 1 2 3 4}
+execsql {DROP TABLE t2}
+do_test select4-3.1.3 {
+  execsql {
+    CREATE TABLE t2 AS 
+      SELECT DISTINCT log FROM t1
+      EXCEPT
+      SELECT n FROM t1 WHERE log=3
+      ORDER BY log DESC;
+    SELECT * FROM t2;
+  }
+} {4 3 2 1 0}
+execsql {DROP TABLE t2}
 do_test select4-3.2 {
   execsql {
     SELECT log FROM t1 WHERE n IN 
@@ -127,7 +171,7 @@ do_test select4-3.3 {
 
 # Intersect operator
 #
-do_test select4-4.1 {
+do_test select4-4.1.1 {
   execsql {
     SELECT DISTINCT log FROM t1
     INTERSECT
@@ -135,6 +179,36 @@ do_test select4-4.1 {
     ORDER BY log;
   }
 } {5}
+do_test select4-4.1.2 {
+  execsql {
+    SELECT DISTINCT log FROM t1 UNION ALL SELECT 6
+    INTERSECT
+    SELECT n FROM t1 WHERE log=3
+    ORDER BY log;
+  }
+} {5 6}
+do_test select4-4.1.3 {
+  execsql {
+    CREATE TABLE t2 AS
+      SELECT DISTINCT log FROM t1 UNION ALL SELECT 6
+      INTERSECT
+      SELECT n FROM t1 WHERE log=3
+      ORDER BY log;
+    SELECT * FROM t2;
+  }
+} {5 6}
+execsql {DROP TABLE t2}
+do_test select4-4.1.4 {
+  execsql {
+    CREATE TABLE t2 AS
+      SELECT DISTINCT log FROM t1 UNION ALL SELECT 6
+      INTERSECT
+      SELECT n FROM t1 WHERE log=3
+      ORDER BY log DESC;
+    SELECT * FROM t2;
+  }
+} {6 5}
+execsql {DROP TABLE t2}
 do_test select4-4.2 {
   execsql {
     SELECT log FROM t1 WHERE n IN