From 8c08e86187ba24e74066dc0bd35c0b8dcb2503b9 Mon Sep 17 00:00:00 2001 From: drh Date: Sat, 11 Feb 2006 17:34:00 +0000 Subject: [PATCH] I give up. SUM() now throws an error on integer overflow. Those of us who think this is goofy can use TOTAL() instead. Tickets #1664, #1669, #1670, #1674. (CVS 3084) FossilOrigin-Name: 1c3e6002cd9fd5d30e197448c4d98cdd59163cac --- manifest | 16 +++++------ manifest.uuid | 2 +- src/func.c | 37 ++++++++++++++++---------- test/func.test | 72 +++++++++++++++++++++++++++++++++++++++++--------- www/lang.tcl | 15 ++++++----- 5 files changed, 100 insertions(+), 42 deletions(-) diff --git a/manifest b/manifest index d54230e73b..8aeece1d95 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Version\s3.3.4\s(CVS\s3083) -D 2006-02-11T02:12:00 +C I\sgive\sup.\s\sSUM()\snow\sthrows\san\serror\son\sinteger\soverflow.\s\sThose\sof\sus\nwho\sthink\sthis\sis\sgoofy\scan\suse\sTOTAL()\sinstead.\nTickets\s#1664,\s#1669,\s#1670,\s#1674.\s(CVS\s3084) +D 2006-02-11T17:34:00 F Makefile.in 5d8dff443383918b700e495de42ec65bc1c8865b F Makefile.linux-gcc 74ba0eadf88748a9ce3fd03d2a3ede2e6715baec F README 9c4e2d6706bdcc3efdd773ce752a8cdab4f90028 @@ -43,7 +43,7 @@ F src/date.c cd2bd5d1ebc6fa12d6312f69789ae5b0a2766f2e F src/delete.c ca404d5fd5f678e32f2f46377ad802cd0219aa99 F src/experimental.c 1b2d1a6cd62ecc39610e97670332ca073c50792b F src/expr.c 9c957fabf95ef62288151eecd5c490a629470666 -F src/func.c 93d004b453a5d9aa754e673eef75d3c9527e0f54 +F src/func.c aa62ef0f5b7d2907ea7e34a33402409f1a8806eb F src/hash.c 8747cf51d12de46512880dfcf1b68b4e24072863 F src/hash.h 1b0c445e1c89ff2aaad9b4605ba61375af001e84 F src/insert.c 67b3dc11831c58d8703eb502355ad3704ee18f66 @@ -164,7 +164,7 @@ F test/enc3.test 890508efff6677345e93bf2a8adb0489b30df030 F test/expr.test 4e65cade931e14a0194eee41e33707e7af5f397a F test/fkey1.test 153004438d51e6769fb1ce165f6313972d6263ce F test/format4.test 9f31d41d4f926cab97b2ebe6be00a6ab12dece87 -F test/func.test ba6159745ecb0b1c7cb4b082b34d5b2b09270eec +F test/func.test 1dbd4a4bb250b6e481a1b70e2d40a8836c886cfb F test/hook.test 7e7645fd9a033f79cce8fdff151e32715e7ec50a F test/in.test 40feeebc7e38576255051aad428322be1545e0f1 F test/index.test c478459611ded74745fee57f99f424da8a5f5fbd @@ -330,7 +330,7 @@ F www/fullscanb.gif f7c94cb227f060511f8909e10f570157263e9a25 F www/index-ex1-x-b.gif f9b1d85c3fa2435cf38b15970c7e3aa1edae23a3 F www/index.tcl 9c659eec487d5e9196d4125ab200cfc86e93528d F www/indirect1b1.gif adfca361d2df59e34f9c5cac52a670c2bfc303a1 -F www/lang.tcl 8761d4ce0dfd029a638cfd4c54422dd852ec6606 +F www/lang.tcl 03c852ea4e4277a0ac517b50c3ad5c237d0bc3ae F www/lockingv3.tcl f59b19d6c8920a931f096699d6faaf61c05db55f F www/mingw.tcl d96b451568c5d28545fefe0c80bee3431c73f69c F www/nulls.tcl ec35193f92485b87b90a994a01d0171b58823fcf @@ -352,7 +352,7 @@ F www/tclsqlite.tcl bb0d1357328a42b1993d78573e587c6dcbc964b9 F www/vdbe.tcl 87a31ace769f20d3627a64fa1fade7fed47b90d0 F www/version3.tcl a99cf5f6d8bd4d5537584a2b342f0fb9fa601d8b F www/whentouse.tcl 97e2b5cd296f7d8057e11f44427dea8a4c2db513 -P c0e987bcfa899c073d54fbd5a3606c704b8bc3d9 -R 74c84829966fa422ec55f82e716c67ca +P 033aaab67f3759f2f65d047ecbc027de9b37d7a7 +R cb8fe231efd5065f9640736e57eef926 U drh -Z 8d0dd4dfcf62226da288e0212a12f178 +Z 4270a5cac50f6da385f72cb1e88e2c89 diff --git a/manifest.uuid b/manifest.uuid index 6774075714..111aa0db7a 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -033aaab67f3759f2f65d047ecbc027de9b37d7a7 \ No newline at end of file +1c3e6002cd9fd5d30e197448c4d98cdd59163cac \ No newline at end of file diff --git a/src/func.c b/src/func.c index 0e050f80a6..6d1195961c 100644 --- a/src/func.c +++ b/src/func.c @@ -16,7 +16,7 @@ ** sqliteRegisterBuildinFunctions() found at the bottom of the file. ** All other code has file scope. ** -** $Id: func.c,v 1.121 2006/02/09 22:13:42 drh Exp $ +** $Id: func.c,v 1.122 2006/02/11 17:34:00 drh Exp $ */ #include "sqliteInt.h" #include @@ -817,9 +817,11 @@ static void test_error( */ typedef struct SumCtx SumCtx; struct SumCtx { - LONGDOUBLE_TYPE sum; /* Sum of terms */ - i64 cnt; /* Number of elements summed */ - u8 approx; /* True if sum is approximate */ + double rSum; /* Floating point sum */ + i64 iSum; /* Integer sum */ + i64 cnt; /* Number of elements summed */ + u8 overflow; /* True if integer overflow seen */ + u8 approx; /* True if non-integer value was input to the sum */ }; /* @@ -849,13 +851,18 @@ static void sumStep(sqlite3_context *context, int argc, sqlite3_value **argv){ if( p && type!=SQLITE_NULL ){ p->cnt++; if( type==SQLITE_INTEGER ){ - p->sum += sqlite3_value_int64(argv[0]); - if( !p->approx ){ - i64 iVal; - p->approx = p->sum!=(LONGDOUBLE_TYPE)(iVal = (i64)p->sum); + i64 v = sqlite3_value_int64(argv[0]); + p->rSum += v; + if( (p->approx|p->overflow)==0 ){ + i64 iNewSum = p->iSum + v; + int s1 = p->iSum >> (sizeof(i64)*8-1); + int s2 = v >> (sizeof(i64)*8-1); + int s3 = iNewSum >> (sizeof(i64)*8-1); + p->overflow = (s1&s2&~s3) | (~s1&~s2&s3); + p->iSum = iNewSum; } }else{ - p->sum += sqlite3_value_double(argv[0]); + p->rSum += sqlite3_value_double(argv[0]); p->approx = 1; } } @@ -864,10 +871,12 @@ static void sumFinalize(sqlite3_context *context){ SumCtx *p; p = sqlite3_aggregate_context(context, 0); if( p && p->cnt>0 ){ - if( p->approx ){ - sqlite3_result_double(context, p->sum); + if( p->overflow ){ + sqlite3_result_error(context,"integer overflow",-1); + }else if( p->approx ){ + sqlite3_result_double(context, p->rSum); }else{ - sqlite3_result_int64(context, (i64)p->sum); + sqlite3_result_int64(context, p->iSum); } } } @@ -875,13 +884,13 @@ static void avgFinalize(sqlite3_context *context){ SumCtx *p; p = sqlite3_aggregate_context(context, 0); if( p && p->cnt>0 ){ - sqlite3_result_double(context, p->sum/(double)p->cnt); + sqlite3_result_double(context, p->rSum/(double)p->cnt); } } static void totalFinalize(sqlite3_context *context){ SumCtx *p; p = sqlite3_aggregate_context(context, 0); - sqlite3_result_double(context, p ? p->sum : 0.0); + sqlite3_result_double(context, p ? p->rSum : 0.0); } /* diff --git a/test/func.test b/test/func.test index add3160fe2..95a99d0e54 100644 --- a/test/func.test +++ b/test/func.test @@ -11,7 +11,7 @@ # This file implements regression tests for SQLite library. The # focus of this file is testing built-in functions. # -# $Id: func.test,v 1.47 2006/02/09 22:13:42 drh Exp $ +# $Id: func.test,v 1.48 2006/02/11 17:34:01 drh Exp $ set testdir [file dirname $argv0] source $testdir/tester.tcl @@ -540,7 +540,9 @@ do_test func-18.6 { } } {123 123.0} -# Ticket #1664: 64-bit overflow in sum() +# Ticket #1664, #1669, #1670, #1674: An integer overflow on SUM causes +# an error. The non-standard TOTAL() function continues to give a helpful +# result. # do_test func-18.10 { execsql { @@ -550,31 +552,75 @@ do_test func-18.10 { SELECT sum(x) - ((1<<62)+1) from t6; } } 0 - -# Ticket #1669, #1670: I am told that if an integer overflow occurs -# during a sum that the result should be an error. This strikes me -# as being brittle. So I'm not doing it that way. -# making the SQL-standard version SUM() even more useless than it -# was before. -# -# The non-standard TOTAL() function continues to give a helpful result. -# do_test func-18.11 { execsql { SELECT typeof(sum(x)) FROM t6 } } integer do_test func-18.12 { - execsql { + catchsql { INSERT INTO t6 VALUES(1<<62); SELECT sum(x) - ((1<<62)*2.0+1) from t6; } -} {0.0} +} {1 {integer overflow}} do_test func-18.13 { execsql { SELECT total(x) - ((1<<62)*2.0+1) FROM t6 } } 0.0 +do_test func-18.14 { + execsql { + SELECT sum(-9223372036854775805); + } +} -9223372036854775805 +do_test func-18.15 { + catchsql { + SELECT sum(x) FROM + (SELECT 9223372036854775807 AS x UNION ALL + SELECT 10 AS x); + } +} {1 {integer overflow}} +do_test func-18.16 { + catchsql { + SELECT sum(x) FROM + (SELECT 9223372036854775807 AS x UNION ALL + SELECT -10 AS x); + } +} {0 9223372036854775797} +do_test func-18.17 { + catchsql { + SELECT sum(x) FROM + (SELECT -9223372036854775807 AS x UNION ALL + SELECT 10 AS x); + } +} {0 -9223372036854775797} +do_test func-18.18 { + catchsql { + SELECT sum(x) FROM + (SELECT -9223372036854775807 AS x UNION ALL + SELECT -10 AS x); + } +} {1 {integer overflow}} +do_test func-18.19 { + catchsql { + SELECT sum(x) FROM (SELECT 9 AS x UNION ALL SELECT -10 AS x); + } +} {0 -1} +do_test func-18.20 { + catchsql { + SELECT sum(x) FROM (SELECT -9 AS x UNION ALL SELECT 10 AS x); + } +} {0 1} +do_test func-18.21 { + catchsql { + SELECT sum(x) FROM (SELECT -10 AS x UNION ALL SELECT 9 AS x); + } +} {0 -1} +do_test func-18.22 { + catchsql { + SELECT sum(x) FROM (SELECT 10 AS x UNION ALL SELECT -9 AS x); + } +} {0 1} finish_test diff --git a/www/lang.tcl b/www/lang.tcl index 4fa79d817a..04fbbf39f1 100644 --- a/www/lang.tcl +++ b/www/lang.tcl @@ -1,7 +1,7 @@ # # Run this Tcl script to generate the lang-*.html files. # -set rcsid {$Id: lang.tcl,v 1.108 2006/02/09 22:13:42 drh Exp $} +set rcsid {$Id: lang.tcl,v 1.109 2006/02/11 17:34:02 drh Exp $} source common.tcl if {[llength $argv]>0} { @@ -1405,12 +1405,15 @@ if all values in the group are NULL. in the SQL language.

The result of total() is always a floating point value. - The result of sum() is an integer value if all non-NULL inputs are integers - and the sum is exact. If any input to sum() is neither an integer or - a NULL or if the - an integer overflow occurs at any point during the computation, + The result of sum() is an integer value if all non-NULL inputs are integers. + If any input to sum() is neither an integer or a NULL then sum() returns a floating point value - which might be an approximation to the true sum. + which might be an approximation to the true sum.

+ +

Sum() will throw an "integer overflow" exception if all inputs + are integers or NULL + and an integer overflow occurs at any point during the computation. + Total() never throws an exception.

} -- 2.47.2