From ad5f157791a72c1f2812d8fe76e1522fd7a6cb6b Mon Sep 17 00:00:00 2001 From: drh Date: Sat, 28 Dec 2019 00:36:51 +0000 Subject: [PATCH] Recompute the values for all generated columns after NOT NULL ON CONFLICT REPLACE constraints fire. Tickets [37823501c68a09f9] and [5fbc159eeb092130]. FossilOrigin-Name: 4cc12c18860bc4801a407cf45e88e23d3d40391f01a461fbac2cac5f102100e1 --- manifest | 16 ++--- manifest.uuid | 2 +- src/insert.c | 152 +++++++++++++++++++++++++++------------------- test/gencol1.test | 75 +++++++++++++++++++++-- 4 files changed, 169 insertions(+), 76 deletions(-) diff --git a/manifest b/manifest index 3dc5335b67..a26fc80d11 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Remove\sa\sNEVER()\sthat\sis\sno\slonger\strue.\sFix\sfor\s[36ffedcb9]. -D 2019-12-27T20:06:32.777 +C Recompute\sthe\svalues\sfor\sall\sgenerated\scolumns\safter\s\nNOT\sNULL\sON\sCONFLICT\sREPLACE\sconstraints\sfire.\nTickets\s[37823501c68a09f9]\sand\s[5fbc159eeb092130]. +D 2019-12-28T00:36:51.136 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724 @@ -489,7 +489,7 @@ F src/hash.c 8d7dda241d0ebdafb6ffdeda3149a412d7df75102cecfc1021c98d6219823b19 F src/hash.h 9d56a9079d523b648774c1784b74b89bd93fac7b365210157482e4319a468f38 F src/hwtime.h cb1d7e3e1ed94b7aa6fde95ae2c2daccc3df826be26fc9ed7fd90d1750ae6144 F src/in-operator.md 10cd8f4bcd225a32518407c2fb2484089112fd71 -F src/insert.c 831408b14a146e93a4e02ddba54dcdfd8097463b9c00ca2ed9daed790c5d452a +F src/insert.c 5d4959fd82a982669c9322a1c6ef49a99a7c59ccfea80bf79ba0d4fa34a7165a F src/legacy.c d7874bc885906868cd51e6c2156698f2754f02d9eee1bae2d687323c3ca8e5aa F src/loadext.c d74f5e7bd51f3c9d283442473eb65aef359664efd6513591c03f01881c4ae2da F src/main.c 868ae7db7a54fe859bf2ca8b7a4f24e9fa03a6134abfb7c9801d08411ef5dacb @@ -1023,7 +1023,7 @@ F test/fuzzer1.test 3d4c4b7e547aba5e5511a2991e3e3d07166cfbb8 F test/fuzzer2.test a85ef814ce071293bce1ad8dffa217cbbaad4c14 F test/fuzzerfault.test 8792cd77fd5bce765b05d0c8e01b9edcf8af8536 F test/gcfault.test dd28c228a38976d6336a3fc42d7e5f1ad060cb8c -F test/gencol1.test 66b361e39820e28b1be1a50de275d14f5a448c0cbcb71b6d38eedb683391cee4 +F test/gencol1.test f0fdeabdd7e41465f4940c6c6252188a93b0dfb287386bf5fd78a01e53af1768 F test/genesis.tcl 1e2e2e8e5cc4058549a154ff1892fe5c9de19f98 F test/having.test e4098a4b8962f9596035c3b87a8928a10648acc509f1bb8d6f96413bbf79a1b3 F test/hexlit.test 4a6a5f46e3c65c4bf1fa06f5dd5a9507a5627751 @@ -1853,7 +1853,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 e3b5fc05c00fc58be7a7c94ce1d97a5b05113f39aba03df64aab08364f85616b -R a966e346a033c0ab1a88c0d109a734aa -U dan -Z ba0d31a3cdb9b737f0aa85ce7fe964a5 +P 597896ed0ae9e2960a8f39576bd7f77a11dccc1da84b6a44ebb5c38d90ebc330 +R 63edebf0afc8152d09a090e486cf3aae +U drh +Z 208796759029520d88a49c921369035d diff --git a/manifest.uuid b/manifest.uuid index 55ef3d6a91..fd1de4217f 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -597896ed0ae9e2960a8f39576bd7f77a11dccc1da84b6a44ebb5c38d90ebc330 \ No newline at end of file +4cc12c18860bc4801a407cf45e88e23d3d40391f01a461fbac2cac5f102100e1 \ No newline at end of file diff --git a/src/insert.c b/src/insert.c index 019dec42f9..61b13bf2d5 100644 --- a/src/insert.c +++ b/src/insert.c @@ -1502,7 +1502,6 @@ void sqlite3GenerateConstraintChecks( int ix; /* Index loop counter */ int nCol; /* Number of columns */ int onError; /* Conflict resolution strategy */ - int addr1; /* Address of jump instruction */ int seenReplace = 0; /* True if REPLACE is used to resolve INT PK conflict */ int nPkField; /* Number of fields in PRIMARY KEY. 1 for ROWID tables */ Index *pUpIdx = 0; /* Index to which to apply the upsert */ @@ -1546,71 +1545,100 @@ void sqlite3GenerateConstraintChecks( /* Test all NOT NULL constraints. */ if( pTab->tabFlags & TF_HasNotNull ){ - for(i=0; iaCol[i].notNull; - if( onError==OE_None ) continue; /* No NOT NULL on this column */ - if( i==pTab->iPKey ){ - continue; /* ROWID is never NULL */ - } - if( aiChng && aiChng[i]<0 ){ - /* Don't bother checking for NOT NULL on columns that do not change */ - continue; - } - if( overrideError!=OE_Default ){ - onError = overrideError; - }else if( onError==OE_Default ){ - onError = OE_Abort; - } - if( onError==OE_Replace && pTab->aCol[i].pDflt==0 ){ - onError = OE_Abort; - } - assert( onError==OE_Rollback || onError==OE_Abort || onError==OE_Fail - || onError==OE_Ignore || onError==OE_Replace ); - addr1 = 0; - testcase( i!=sqlite3TableColumnToStorage(pTab, i) ); - testcase( pTab->aCol[i].colFlags & COLFLAG_VIRTUAL ); - testcase( pTab->aCol[i].colFlags & COLFLAG_STORED ); - iReg = sqlite3TableColumnToStorage(pTab, i) + regNewData + 1; - switch( onError ){ - case OE_Replace: { - assert( onError==OE_Replace ); - addr1 = sqlite3VdbeMakeLabel(pParse); - sqlite3VdbeAddOp2(v, OP_NotNull, iReg, addr1); - VdbeCoverage(v); - if( (pTab->aCol[i].colFlags & COLFLAG_GENERATED)==0 ){ - sqlite3ExprCode(pParse, pTab->aCol[i].pDflt, regNewData+1+i); - sqlite3VdbeAddOp2(v, OP_NotNull, iReg, addr1); - VdbeCoverage(v); - } - onError = OE_Abort; - /* Fall through into the OE_Abort case to generate code that runs - ** if both the input and the default value are NULL */ + int b2ndPass = 0; /* True if currently running 2nd pass */ + int nSeenReplace = 0; /* Number of ON CONFLICT REPLACE operations */ + int nGenerated = 0; /* Number of generated columns with NOT NULL */ + while(1){ /* Make 2 passes over columns. Exit loop via "break" */ + for(i=0; iaCol[i]; /* The column to check for NOT NULL */ + int isGenerated; /* non-zero if column is generated */ + onError = pCol->notNull; + if( onError==OE_None ) continue; /* No NOT NULL on this column */ + if( i==pTab->iPKey ){ + continue; /* ROWID is never NULL */ } - case OE_Abort: - sqlite3MayAbort(pParse); - /* Fall through */ - case OE_Rollback: - case OE_Fail: { - char *zMsg = sqlite3MPrintf(db, "%s.%s", pTab->zName, - pTab->aCol[i].zName); - sqlite3VdbeAddOp3(v, OP_HaltIfNull, SQLITE_CONSTRAINT_NOTNULL, - onError, iReg); - sqlite3VdbeAppendP4(v, zMsg, P4_DYNAMIC); - sqlite3VdbeChangeP5(v, P5_ConstraintNotNull); - VdbeCoverage(v); - if( addr1 ) sqlite3VdbeResolveLabel(v, addr1); - break; + isGenerated = pCol->colFlags & COLFLAG_GENERATED; + if( isGenerated && !b2ndPass ){ + nGenerated++; + continue; /* Generated columns processed on 2nd pass */ } - default: { - assert( onError==OE_Ignore ); - sqlite3VdbeAddOp2(v, OP_IsNull, iReg, ignoreDest); - VdbeCoverage(v); - break; + if( aiChng && aiChng[i]<0 && !isGenerated ){ + /* Do not check NOT NULL on columns that do not change */ + continue; } + if( overrideError!=OE_Default ){ + onError = overrideError; + }else if( onError==OE_Default ){ + onError = OE_Abort; + } + if( onError==OE_Replace ){ + if( b2ndPass /* REPLACE becomes ABORT on the 2nd pass */ + || pCol->pDflt==0 /* REPLACE is ABORT if no DEFAULT value */ + ){ + testcase( pCol->colFlags & COLFLAG_VIRTUAL ); + testcase( pCol->colFlags & COLFLAG_STORED ); + testcase( pCol->colFlags & COLFLAG_GENERATED ); + onError = OE_Abort; + }else{ + assert( !isGenerated ); + } + }else if( b2ndPass && !isGenerated ){ + continue; + } + assert( onError==OE_Rollback || onError==OE_Abort || onError==OE_Fail + || onError==OE_Ignore || onError==OE_Replace ); + testcase( i!=sqlite3TableColumnToStorage(pTab, i) ); + iReg = sqlite3TableColumnToStorage(pTab, i) + regNewData + 1; + switch( onError ){ + case OE_Replace: { + int addr1 = sqlite3VdbeAddOp1(v, OP_NotNull, iReg); + VdbeCoverage(v); + assert( (pCol->colFlags & COLFLAG_GENERATED)==0 ); + nSeenReplace++; + sqlite3ExprCode(pParse, pCol->pDflt, iReg); + sqlite3VdbeJumpHere(v, addr1); + break; + } + case OE_Abort: + sqlite3MayAbort(pParse); + /* Fall through */ + case OE_Rollback: + case OE_Fail: { + char *zMsg = sqlite3MPrintf(db, "%s.%s", pTab->zName, + pCol->zName); + sqlite3VdbeAddOp3(v, OP_HaltIfNull, SQLITE_CONSTRAINT_NOTNULL, + onError, iReg); + sqlite3VdbeAppendP4(v, zMsg, P4_DYNAMIC); + sqlite3VdbeChangeP5(v, P5_ConstraintNotNull); + VdbeCoverage(v); + break; + } + default: { + assert( onError==OE_Ignore ); + sqlite3VdbeAddOp2(v, OP_IsNull, iReg, ignoreDest); + VdbeCoverage(v); + break; + } + } /* end switch(onError) */ + } /* end loop i over columns */ + if( nGenerated==0 && nSeenReplace==0 ){ + /* If there are no generated columns with NOT NULL constraints + ** and no NOT NULL ON CONFLICT REPLACE constraints, then a single + ** pass is sufficient */ + break; } - } - } + if( b2ndPass ) break; /* Never need more than 2 passes */ + b2ndPass = 1; + if( nSeenReplace>0 && (pTab->tabFlags & TF_HasGenerated)!=0 ){ + /* If any NOT NULL ON CONFLICT REPLACE constraints fired on the + ** first pass, recomputed values for all generated columns, as + ** those values might depend on columns affected by the REPLACE. + */ + sqlite3ComputeGeneratedColumns(pParse, regNewData+1, pTab); + } + } /* end of 2-pass loop */ + } /* end if( has-not-null-constraints ) */ /* Test all CHECK constraints */ diff --git a/test/gencol1.test b/test/gencol1.test index d4cda59908..7d0da45821 100644 --- a/test/gencol1.test +++ b/test/gencol1.test @@ -211,16 +211,81 @@ do_catchsql_test gencol1-6.10 { REPLACE INTO t0(c1) VALUES(NULL); } {1 {NOT NULL constraint failed: t0.c0}} -# 2019-11-06 ticket b13b7dce76e9352b34e7 +# 2019-11-06 ticket https://www.sqlite.org/src/info/2399f5986134f79c +# 2019-12-27 ticket https://www.sqlite.org/src/info/5fbc159eeb092130 +# 2019-12-27 ticket https://www.sqlite.org/src/info/37823501c68a09f9 +# +# All of the above tickets deal with NOT NULL ON CONFLICT REPLACE +# constraints on tables that have generated columns. +# +reset_db do_execsql_test gencol1-7.10 { - DROP TABLE IF EXISTS t0; CREATE TABLE t0 (c0 GENERATED ALWAYS AS (1), c1 UNIQUE, c2 UNIQUE); INSERT INTO t0(c1) VALUES (1); SELECT quote(0 = t0.c2 OR t0.c1 BETWEEN t0.c2 AND 1) FROM t0; } {NULL} +do_execsql_test gencol1-7.11 { + DROP TABLE t0; + CREATE TABLE t0(c0 NOT NULL DEFAULT 'xyz', c1 AS(c0) NOT NULL); + REPLACE INTO t0(c0) VALUES(NULL); + SELECT * FROM t0; +} {xyz xyz} +do_execsql_test gencol1-7.12 { + DROP TABLE t0; + CREATE TABLE t0(c0 NOT NULL DEFAULT 'xyz', c1 AS(c0) STORED NOT NULL); + REPLACE INTO t0(c0) VALUES(NULL); + SELECT * FROM t0; +} {xyz xyz} do_execsql_test gencol1-7.20 { - SELECT 99 FROM t0 WHERE 0 = t0.c2 OR t0.c1 BETWEEN t0.c2 AND 1; -} {} + CREATE TABLE t1( + a NOT NULL DEFAULT 'aaa', + b AS(c) NOT NULL, + c NOT NULL DEFAULT 'ccc'); + REPLACE INTO t1(a,c) VALUES(NULL,NULL); + SELECT * FROM t1; +} {aaa ccc ccc} +do_execsql_test gencol1-7.21 { + DROP TABLE t1; + CREATE TABLE t1( + a NOT NULL DEFAULT 'aaa', + b AS(c) STORED NOT NULL, + c NOT NULL DEFAULT 'ccc'); + REPLACE INTO t1(a,c) VALUES(NULL,NULL); + SELECT * FROM t1; +} {aaa ccc ccc} +do_execsql_test gencol1-7.30 { + CREATE TABLE t2( + a NOT NULL DEFAULT 'aaa', + b AS(a) NOT NULL, + c NOT NULL DEFAULT 'ccc'); + REPLACE INTO t2(a,c) VALUES(NULL,NULL); + SELECT * FROM t2; +} {aaa aaa ccc} +do_execsql_test gencol1-7.31 { + DROP TABLE t2; + CREATE TABLE t2( + a NOT NULL DEFAULT 'aaa', + b AS(a) STORED NOT NULL, + c NOT NULL DEFAULT 'ccc'); + REPLACE INTO t2(a,c) VALUES(NULL,NULL); + SELECT * FROM t2; +} {aaa aaa ccc} +do_execsql_test gencol1-7.40 { + CREATE TABLE t3(a NOT NULL DEFAULT 123, b AS(a) UNIQUE); + REPLACE INTO t3 VALUES(NULL); + SELECT * FROM t3; +} {123 123} +do_execsql_test gencol1-7.41 { + SELECT * FROM t3 WHERE b=123; +} {123 123} +do_execsql_test gencol1-7.50 { + CREATE TABLE t4(a NOT NULL DEFAULT 123, b AS(a*10+4) STORED UNIQUE); + REPLACE INTO t4 VALUES(NULL); + SELECT * FROM t4; +} {123 1234} +do_execsql_test gencol1-7.51 { + SELECT * FROM t4 WHERE b=1234; +} {123 1234} # 2019-11-06 ticket 4fc08501f4e56692 do_execsql_test gencol1-8.10 { @@ -245,9 +310,9 @@ do_catchsql_test gencol1-8.20 { # 2019-11-21 Problems in the new generated column logic # reported by Yongheng Chen and Rui Zhong +reset_db do_execsql_test gencol1-9.10 { PRAGMA foreign_keys=OFF; - DROP TABLE t1; CREATE TABLE t1(aa , bb AS (17) UNIQUE); INSERT INTO t1 VALUES(17); CREATE TABLE t2(cc); -- 2.39.5