From f35f2f92ed6ba57ddb670454a1b814f7c48946a6 Mon Sep 17 00:00:00 2001 From: drh Date: Sat, 29 Jul 2017 16:01:55 +0000 Subject: [PATCH 1/2] Move the generation of output column names earlier, to right after name resolution and before query transformations such as flattening. This prevents the names from getting mangled by query transformations, and obviates hacks in the query flattener that attempt to work around the name mangling. The resulting code is smaller and faster and gives more consistent output. This is an alternative fix to ticket [de3403bf5ae5f72ed]. FossilOrigin-Name: 09834279aeca3bda63de684a369ed64f2cbf587b5f5df1454c0a3c009a1337ad --- manifest | 17 ++++++----- manifest.uuid | 2 +- src/select.c | 83 +++++++++++++-------------------------------------- 3 files changed, 32 insertions(+), 70 deletions(-) diff --git a/manifest b/manifest index 2a591e1217..c1e088b085 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Update\sTcl\sversion\sused\sby\sthe\sTclKit\sbatch\stool\sfor\sMSVC. -D 2017-07-28T22:22:15.250 +C Move\sthe\sgeneration\sof\soutput\scolumn\snames\searlier,\sto\sright\safter\sname\nresolution\sand\sbefore\squery\stransformations\ssuch\sas\sflattening.\s\sThis\sprevents\nthe\snames\sfrom\sgetting\smangled\sby\squery\stransformations,\sand\sobviates\shacks\nin\sthe\squery\sflattener\sthat\sattempt\sto\swork\saround\sthe\sname\smangling.\nThe\sresulting\scode\sis\ssmaller\sand\sfaster\sand\sgives\smore\sconsistent\soutput.\nThis\sis\san\salternative\sfix\sto\sticket\s[de3403bf5ae5f72ed]. +D 2017-07-29T16:01:55.238 F Makefile.in d9873c9925917cca9990ee24be17eb9613a668012c85a343aef7e5536ae266e8 F Makefile.linux-gcc 7bc79876b875010e8c8f9502eb935ca92aa3c434 F Makefile.msc 02b469e9dcd5b7ee63fc1fb05babc174260ee4cfa4e0ef2e48c3c6801567a016 @@ -452,7 +452,7 @@ F src/printf.c 8757834f1b54dae512fb25eb1acc8e94a0d15dd2290b58f2563f65973265adb2 F src/random.c 80f5d666f23feb3e6665a6ce04c7197212a88384 F src/resolve.c 4324a94573b1e29286f8121e4881db59eaedc014afeb274c8d3e07ed282e0e20 F src/rowset.c 7b7e7e479212e65b723bf40128c7b36dc5afdfac -F src/select.c c6bf96a7f9d7d68f929de84738c599a30d0a725ab0b54420e70545743cd5ee7b +F src/select.c 31b35ddf55f1021f7148a01306984b057c11ebb6e3463d94677225e0a1e301a3 F src/shell.c bd6a37cbe8bf64ef6a6a74fdc50f067d3148149b4ce2b4d03154663e66ded55f F src/shell.c.in b5725acacba95ccefa57b6d068f710e29ba8239c3aa704628a1902a1f729c175 F src/sqlite.h.in 0e2603c23f0747c5660669f946e231730af000c76d1653b153dcf2c26fce0a6b @@ -1637,7 +1637,10 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0 -P 3286e1a07b0693049a07f0865bf93749c461ea8f6d1175ec2d1642886673d8ac -R fa880debd22b5643c43c3f5ac12562f1 -U mistachkin -Z 2ba3113508e5372bd91f947e270302b1 +P bcec155e0d6c6b17ae09d5a366c080723d01ff40dbc1a0ad0bb669a91db1b850 +R 1dd5a0c8909a7008cc7feb807ee35ff5 +T *branch * early-column-names +T *sym-early-column-names * +T -sym-trunk * +U drh +Z 012e37d9c831aa36f84ce05878b8d813 diff --git a/manifest.uuid b/manifest.uuid index cd8f4e8643..08ab015be8 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -bcec155e0d6c6b17ae09d5a366c080723d01ff40dbc1a0ad0bb669a91db1b850 \ No newline at end of file +09834279aeca3bda63de684a369ed64f2cbf587b5f5df1454c0a3c009a1337ad \ No newline at end of file diff --git a/src/select.c b/src/select.c index a3ada4ef79..9af93dc9ed 100644 --- a/src/select.c +++ b/src/select.c @@ -1438,13 +1438,10 @@ static const char *columnTypeImpl( ** of the SELECT statement. Return the declaration type and origin ** data for the result-set column of the sub-select. */ - if( iCol>=0 && ALWAYS(iColpEList->nExpr) ){ + if( iCol>=0 && iColpEList->nExpr ){ /* If iCol is less than zero, then the expression requests the ** rowid of the sub-select or view. This expression is legal (see ** test case misc2.2.2) - it always evaluates to NULL. - ** - ** The ALWAYS() is because iCol>=pS->pEList->nExpr will have been - ** caught already by name resolution. */ NameContext sNC; Expr *p = pS->pEList->a[iCol].pExpr; @@ -1554,18 +1551,6 @@ static void generateColumnTypes( #endif /* !defined(SQLITE_OMIT_DECLTYPE) */ } -/* -** Return the Table objecct in the SrcList that has cursor iCursor. -** Or return NULL if no such Table object exists in the SrcList. -*/ -static Table *tableWithCursor(SrcList *pList, int iCursor){ - int j; - for(j=0; jnSrc; j++){ - if( pList->a[j].iCursor==iCursor ) return pList->a[j].pTab; - } - return 0; -} - /* ** Compute the column names for a SELECT statement. @@ -1599,15 +1584,16 @@ static Table *tableWithCursor(SrcList *pList, int iCursor){ */ static void generateColumnNames( Parse *pParse, /* Parser context */ - SrcList *pTabList, /* The FROM clause of the SELECT */ - ExprList *pEList /* Expressions defining the result set */ + Select *pSelect /* Generate column names for this SELECT statement */ ){ Vdbe *v = pParse->pVdbe; int i; Table *pTab; + SrcList *pTabList; + ExprList *pEList; sqlite3 *db = pParse->db; - int fullName; /* TABLE.COLUMN if no AS clause and is a direct table ref */ - int srcName; /* COLUMN or TABLE.COLUMN if no AS clause and is direct */ + int fullName; /* TABLE.COLUMN if no AS clause and is a direct table ref */ + int srcName; /* COLUMN or TABLE.COLUMN if no AS clause and is direct */ #ifndef SQLITE_OMIT_EXPLAIN /* If this is an EXPLAIN, skip this step */ @@ -1617,6 +1603,10 @@ static void generateColumnNames( #endif if( pParse->colNamesSet || db->mallocFailed ) return; + /* Column names are determined by the left-most term of a compound select */ + while( pSelect->pPrior ) pSelect = pSelect->pPrior; + pTabList = pSelect->pSrc; + pEList = pSelect->pEList; assert( v!=0 ); assert( pTabList!=0 ); pParse->colNamesSet = 1; @@ -1631,12 +1621,11 @@ static void generateColumnNames( /* An AS clause always takes first priority */ char *zName = pEList->a[i].zName; sqlite3VdbeSetColName(v, i, COLNAME_NAME, zName, SQLITE_TRANSIENT); - }else if( srcName - && (p->op==TK_COLUMN || p->op==TK_AGG_COLUMN) - && (pTab = tableWithCursor(pTabList, p->iTable))!=0 - ){ + }else if( srcName && p->op==TK_COLUMN ){ char *zCol; int iCol = p->iColumn; + pTab = p->pTab; + assert( pTab!=0 ); if( iCol<0 ) iCol = pTab->iPKey; assert( iCol==-1 || (iCol>=0 && iColnCol) ); if( iCol<0 ){ @@ -2468,11 +2457,6 @@ static int multiSelect( if( dest.eDest!=priorOp ){ int iCont, iBreak, iStart; assert( p->pEList ); - if( dest.eDest==SRT_Output ){ - Select *pFirst = p; - while( pFirst->pPrior ) pFirst = pFirst->pPrior; - generateColumnNames(pParse, pFirst->pSrc, pFirst->pEList); - } iBreak = sqlite3VdbeMakeLabel(v); iCont = sqlite3VdbeMakeLabel(v); computeLimitRegisters(pParse, p, iBreak); @@ -2543,11 +2527,6 @@ static int multiSelect( ** tables. */ assert( p->pEList ); - if( dest.eDest==SRT_Output ){ - Select *pFirst = p; - while( pFirst->pPrior ) pFirst = pFirst->pPrior; - generateColumnNames(pParse, pFirst->pSrc, pFirst->pEList); - } iBreak = sqlite3VdbeMakeLabel(v); iCont = sqlite3VdbeMakeLabel(v); computeLimitRegisters(pParse, p, iBreak); @@ -3155,14 +3134,6 @@ static int multiSelectOrderBy( */ sqlite3VdbeResolveLabel(v, labelEnd); - /* Set the number of output columns - */ - if( pDest->eDest==SRT_Output ){ - Select *pFirst = pPrior; - while( pFirst->pPrior ) pFirst = pFirst->pPrior; - generateColumnNames(pParse, pFirst->pSrc, pFirst->pEList); - } - /* Reassembly the compound query so that it will be freed correctly ** by the calling function */ if( p->pPrior ){ @@ -3457,7 +3428,6 @@ static int flattenSubquery( Select *pSub1; /* Pointer to the rightmost select in sub-query */ SrcList *pSrc; /* The FROM clause of the outer query */ SrcList *pSubSrc; /* The FROM clause of the subquery */ - ExprList *pList; /* The result set of the outer query */ int iParent; /* VDBE cursor number of the pSub result set temp table */ int iNewParent = -1;/* Replacement table for iParent */ int isLeftJoin = 0; /* True if pSub is the right side of a LEFT JOIN */ @@ -3782,14 +3752,6 @@ static int flattenSubquery( ** We look at every expression in the outer query and every place we see ** "a" we substitute "x*3" and every place we see "b" we substitute "y+10". */ - pList = pParent->pEList; - for(i=0; inExpr; i++){ - if( pList->a[i].zName==0 ){ - char *zName = sqlite3DbStrDup(db, pList->a[i].zSpan); - sqlite3Dequote(zName); - pList->a[i].zName = zName; - } - } if( pSub->pOrderBy ){ /* At this point, any non-zero iOrderByCol values indicate that the ** ORDER BY column expression is identical to the iOrderByCol'th @@ -5219,6 +5181,14 @@ int sqlite3Select( } #endif + /* Get a pointer the VDBE under construction, allocating a new VDBE if one + ** does not already exist */ + v = sqlite3GetVdbe(pParse); + if( v==0 ) goto select_end; + if( pDest->eDest==SRT_Output ){ + generateColumnNames(pParse, p); + } + /* Try to flatten subqueries in the FROM clause up into the main query */ #if !defined(SQLITE_OMIT_SUBQUERY) || !defined(SQLITE_OMIT_VIEW) @@ -5254,11 +5224,6 @@ int sqlite3Select( } #endif - /* Get a pointer the VDBE under construction, allocating a new VDBE if one - ** does not already exist */ - v = sqlite3GetVdbe(pParse); - if( v==0 ) goto select_end; - #ifndef SQLITE_OMIT_COMPOUND_SELECT /* Handle compound SELECT statements using the separate multiSelect() ** procedure. @@ -6058,12 +6023,6 @@ int sqlite3Select( select_end: explainSetInteger(pParse->iSelectId, iRestoreSelectId); - /* Identify column names if results of the SELECT are to be output. - */ - if( rc==SQLITE_OK && pDest->eDest==SRT_Output ){ - generateColumnNames(pParse, pTabList, pEList); - } - sqlite3DbFree(db, sAggInfo.aCol); sqlite3DbFree(db, sAggInfo.aFunc); #if SELECTTRACE_ENABLED From ce00d05a7bd76437d258051fdb8ee3e561a9fd77 Mon Sep 17 00:00:00 2001 From: drh Date: Sat, 29 Jul 2017 17:02:22 +0000 Subject: [PATCH 2/2] New test cases for column name generation interacting with the query flattener. FossilOrigin-Name: 0c38dde4543d6183a6ab0b7b3b75819f56c47704756a2426d54d3f20468d78d8 --- manifest | 15 ++++++------- manifest.uuid | 2 +- test/colname.test | 55 ++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 61 insertions(+), 11 deletions(-) diff --git a/manifest b/manifest index c1e088b085..ea3d2d5936 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Move\sthe\sgeneration\sof\soutput\scolumn\snames\searlier,\sto\sright\safter\sname\nresolution\sand\sbefore\squery\stransformations\ssuch\sas\sflattening.\s\sThis\sprevents\nthe\snames\sfrom\sgetting\smangled\sby\squery\stransformations,\sand\sobviates\shacks\nin\sthe\squery\sflattener\sthat\sattempt\sto\swork\saround\sthe\sname\smangling.\nThe\sresulting\scode\sis\ssmaller\sand\sfaster\sand\sgives\smore\sconsistent\soutput.\nThis\sis\san\salternative\sfix\sto\sticket\s[de3403bf5ae5f72ed]. -D 2017-07-29T16:01:55.238 +C New\stest\scases\sfor\scolumn\sname\sgeneration\sinteracting\swith\sthe\squery\sflattener. +D 2017-07-29T17:02:22.699 F Makefile.in d9873c9925917cca9990ee24be17eb9613a668012c85a343aef7e5536ae266e8 F Makefile.linux-gcc 7bc79876b875010e8c8f9502eb935ca92aa3c434 F Makefile.msc 02b469e9dcd5b7ee63fc1fb05babc174260ee4cfa4e0ef2e48c3c6801567a016 @@ -652,7 +652,7 @@ F test/collate9.test 3adcc799229545940df2f25308dd1ad65869145a F test/collateA.test b8218ab90d1fa5c59dcf156efabb1b2599c580d6 F test/collateB.test 1e68906951b846570f29f20102ed91d29e634854ee47454d725f2151ecac0b95 F test/colmeta.test 2c765ea61ee37bc43bbe6d6047f89004e6508eb1 -F test/colname.test 08948a4809d22817e0e5de89c7c0a8bd90cb551b +F test/colname.test b111edd2a84f558567320904bb94c779d7eec47254265b5f0a3d1f3e52cc28e0 F test/conflict.test 029faa2d81a0d1cafb5f88614beb663d972c01db F test/conflict2.test bb0b94cf7196c64a3cbd815c66d3ee98c2fecd9c F test/conflict3.test a83db76a6c3503b2fa057c7bfb08c318d8a422202d8bc5b86226e078e5b49ff9 @@ -1637,10 +1637,7 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0 -P bcec155e0d6c6b17ae09d5a366c080723d01ff40dbc1a0ad0bb669a91db1b850 -R 1dd5a0c8909a7008cc7feb807ee35ff5 -T *branch * early-column-names -T *sym-early-column-names * -T -sym-trunk * +P 09834279aeca3bda63de684a369ed64f2cbf587b5f5df1454c0a3c009a1337ad +R 67883e010f09f35ff4da8532a7b615d9 U drh -Z 012e37d9c831aa36f84ce05878b8d813 +Z d8a6008eca97bd8ffc0a711cdb6dfcd3 diff --git a/manifest.uuid b/manifest.uuid index 08ab015be8..9e8657dbe9 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -09834279aeca3bda63de684a369ed64f2cbf587b5f5df1454c0a3c009a1337ad \ No newline at end of file +0c38dde4543d6183a6ab0b7b3b75819f56c47704756a2426d54d3f20468d78d8 \ No newline at end of file diff --git a/test/colname.test b/test/colname.test index e16304d4a9..1497fc275c 100644 --- a/test/colname.test +++ b/test/colname.test @@ -13,7 +13,6 @@ # The focus of this file is testing how SQLite generates the names # of columns in a result set. # -# $Id: colname.test,v 1.7 2009/06/02 15:47:38 drh Exp $ set testdir [file dirname $argv0] source $testdir/tester.tcl @@ -326,4 +325,58 @@ do_test colname-8.1 { } } {123} +# 2017-07-29: Interaction between column naming and query flattening. +# For years now, the query flattener has inserted AS clauses on the +# outer query that were the original SQL text of the column. This caused +# column-name shifts when the query flattener was enhanced, breaking +# legacy applications. See https://sqlite.org/src/info/41c27bc0ff1d3135 +# for details. +# +# To fix this, the column naming logic was moved ahead of the query +# flattener so that column names are assigned before the query flattener +# runs. +# +db close +sqlite3 db :memory: +do_test colname-9.100 { + db eval { + CREATE TABLE t1(a,b); + INSERT INTO t1 VALUES(1,2); + CREATE VIEW v1(x,y) AS SELECT a,b FROM t1; + } + execsql2 {SELECT v1.x, (Y) FROM v1} + # Prior to the fix, this would return: "v1.x 1 (Y) 2" +} {x 1 y 2} +do_test colname-9.110 { + execsql2 {SELECT * FROM v1} +} {x 1 y 2} +do_test colname-9.120 { + db eval { + CREATE VIEW v2(x,y) AS SELECT a,b FROM t1 LIMIT 10; + } + execsql2 {SELECT * FROM v2 WHERE 1} +} {x 1 y 2} +do_test colname-9.130 { + execsql2 {SELECT v2.x, [v2].[y] FROM v2 WHERE 1} +} {x 1 y 2} +do_test colname-9.140 { + execsql2 {SELECT +x, +y FROM v2 WHERE 1} +} {+x 1 +y 2} + +do_test colname-9.200 { + db eval { + CREATE TABLE t2(c,d); + INSERT INTO t2 VALUES(3,4); + CREATE VIEW v3 AS SELECT c AS a, d AS b FROM t2; + } + execsql2 {SELECT t1.a, v3.a AS n FROM t1 LEFT JOIN v3} +} {a 1 n 3} +do_test colname-9.211 { + execsql2 {SELECT t1.a AS n, v3.a FROM t1 JOIN v3} +} {n 1 a 3} +do_test colname-9.210 { + execsql2 {SELECT t1.a, v3.a AS n FROM t1 JOIN v3} +} {a 1 n 3} + + finish_test