third_party/sqlite_google/src/pragma.c - Issue 924: sqlite3 pragma get/set user_version implemented

Delta Between Two Patch Sets: third_party/sqlite_google/src/pragma.c

Issue 924: sqlite3 pragma get/set user_version implemented (Closed) SVN Base: http://google-gears.googlecode.com/svn/contrib/dimitri.glazkov/database2/gears/

Left Patch Set: Created 5 months ago

Right Patch Set: moved code to btree.c, restructured Created 4 months, 3 weeks ago

Use n/p to move between diff chunks; N/P to move between comments. Please Sign in to add in-line comments.

Jump to:

Left: Side by side diff | Download
Right: Side by side diff | Download

LEFT	RIGHT
1 /*	1 /*

2 ** 2003 April 6	2 ** 2003 April 6

3 **	3 **

4 ** The author disclaims copyright to this source code. In place of	4 ** The author disclaims copyright to this source code. In place of

5 ** a legal notice, here is a blessing:	5 ** a legal notice, here is a blessing:

6 **	6 **

7 ** May you do good and not evil.	7 ** May you do good and not evil.

8 ** May you find forgiveness for yourself and forgive others.	8 ** May you find forgiveness for yourself and forgive others.

9 ** May you share freely, never taking more than you give.	9 ** May you share freely, never taking more than you give.

10 **	10 **

11 *************************************************************************	11 *************************************************************************

12 ** This file contains code used to implement the PRAGMA command.	12 ** This file contains code used to implement the PRAGMA command.

13 **	13 **

14 ** $Id: pragma.c,v 1.139 2007/05/23 13:50:24 danielk1977 Exp $	14 ** $Id: pragma.c,v 1.139 2007/05/23 13:50:24 danielk1977 Exp $

15 */	15 */

16 #include "sqliteInt.h"	16 #include "sqliteInt.h"

17 #include "btreeInt.h"
Scott.Hess 2008/05/20 18:37:04 Maybe the new code should just live in btree.c? Maybe the new code should just live in btree.c? Dimitri 2008/05/21 13:14:31 On 2008/05/20 18:37:04, Scott.Hess wrote: > Maybe On 2008/05/20 18:37:04, Scott.Hess wrote: > Maybe the new code should just live in btree.c? Makes sense. Done.
18 #include "os.h"	17 #include "os.h"

19 #include <ctype.h>	18 #include <ctype.h>

20	19

21 /* Ignore this whole file if pragmas are disabled	20 /* Ignore this whole file if pragmas are disabled

22 */	21 */

23 #if !defined(SQLITE_OMIT_PRAGMA) && !defined(SQLITE_OMIT_PARSER)	22 #if !defined(SQLITE_OMIT_PRAGMA) && !defined(SQLITE_OMIT_PARSER)

24	23

25 #if defined(SQLITE_DEBUG) \|\| defined(SQLITE_TEST)	24 #if defined(SQLITE_DEBUG) \|\| defined(SQLITE_TEST)

26 # include "pager.h"	25 # include "pager.h"

27 # include "btree.h"	26 # include "btree.h"

(...skipping 1098 matching lines...) Show 10 above Show 10 below
1126 sqlite3BtreeSetSafetyLevel(pDb->pBt, pDb->safety_level,	1125 sqlite3BtreeSetSafetyLevel(pDb->pBt, pDb->safety_level,

1127 (db->flags&SQLITE_FullFSync)!=0);	1126 (db->flags&SQLITE_FullFSync)!=0);

1128 }	1127 }

1129 #endif	1128 #endif

1130 }	1129 }

1131 pragma_out:	1130 pragma_out:

1132 sqliteFree(zLeft);	1131 sqliteFree(zLeft);

1133 sqliteFree(zRight);	1132 sqliteFree(zRight);

1134 }	1133 }

1135	1134

1136 int sqlite3_pragma_get_user_version(sqlite3 sqlite, int user_version) {
Scott.Hess 2008/05/20 18:37:04 Suggest not naming this with "pragma", since it's Suggest not naming this with "pragma", since it's not a pragma. Also, isn't this inside the !SQLITE_OMIT_PRAGMA #ifdef anyhow? Dimitri 2008/05/21 13:14:31 On 2008/05/20 18:37:04, Scott.Hess wrote: > Sugges On 2008/05/20 18:37:04, Scott.Hess wrote: > Suggest not naming this with "pragma", since it's not a pragma. Also, isn't > this inside the !SQLITE_OMIT_PRAGMA #ifdef anyhow? Done.
1137 // grab actual (not temporary tables) database
Scott.Hess 2008/05/20 18:37:04 This ain't c++! /* ... /, baby! This ain't c++! / ... /, baby! Dimitri* 2008/05/21 13:14:31 On 2008/05/20 18:37:04, Scott.Hess wrote: > This a On 2008/05/20 18:37:04, Scott.Hess wrote: > This ain't c++! /* ... */, baby! Done.
1138 Btree *bt = sqlite->aDb[0].pBt;
Scott.Hess 2008/05/20 18:37:04 Suggest a NULL check for sqlite, mainly because th Suggest a NULL check for sqlite, mainly because the poison code (end of btree.c) does it, so I assume there was a reason. That code also checks nDb to make sure that aDb[0] is really there. Dimitri 2008/05/21 13:14:31 On 2008/05/20 18:37:04, Scott.Hess wrote: > Sugges On 2008/05/20 18:37:04, Scott.Hess wrote: > Suggest a NULL check for sqlite, mainly because the poison code (end of btree.c) > does it, so I assume there was a reason. That code also checks nDb to make sure > that aDb[0] is really there. Done. The code now returns SQLITE_ERROR (missing database) in both cases.
1139 // 6 = user_version cookie
Scott.Hess 2008/05/20 18:37:04 Suggest kUserVersionCookie or somesuch, so that yo Suggest kUserVersionCookie or somesuch, so that you can use the symbolic name in set_user_version. Dimitri 2008/05/21 13:14:31 On 2008/05/20 18:37:04, Scott.Hess wrote: > Sugges On 2008/05/20 18:37:04, Scott.Hess wrote: > Suggest kUserVersionCookie or somesuch, so that you can use the symbolic name in > set_user_version. Done.
1140 // because sqlite3BtreeGetMeta sets a table read lock, it is safe to use

1141 // without explicit read transaction. Besides, it is very unlikely we will

1142 // ever call this outside of a transaction.

1143 return sqlite3BtreeGetMeta(bt, 6, (u32 *)user_version);

1144 }

1145

1146 int sqlite3_pragma_set_user_version(sqlite3 *sqlite, int user_version) {
Scott.Hess 2008/05/20 18:37:04 Same checks down here, of course. Same checks down here, of course. Dimitri 2008/05/21 13:14:31 On 2008/05/20 18:37:04, Scott.Hess wrote: > Same c On 2008/05/20 18:37:04, Scott.Hess wrote: > Same checks down here, of course. Done.
1147 Btree *bt = sqlite->aDb[0].pBt;

1148 int rc;

1149 // have to enclose this in a write-transaction, if transaction isn't already

1150 // in progress

1151 int needs_transaction = bt->inTrans != TRANS_WRITE;

1152 if (needs_transaction) {
Scott.Hess 2008/05/20 18:37:04 SQLite code does if( needs_transaction ){. Indeed SQLite code does if( needs_transaction ){. Indeed, that is exactly opposite of our conventions! Dimitri 2008/05/21 13:14:31 On 2008/05/20 18:37:04, Scott.Hess wrote: > SQLite On 2008/05/20 18:37:04, Scott.Hess wrote: > SQLite code does if( needs_transaction ){. Indeed, that is exactly opposite of > our conventions! Done.
1153 // begin write transaction, if not already started

1154 rc = sqlite3BtreeBeginTrans(bt, 1);

1155 if (rc != SQLITE_OK) {
Scott.Hess 2008/05/20 18:37:04 SQLite convention would be if( rc!=SQLITE_OK ) ret SQLite convention would be if( rc!=SQLITE_OK ) return rc; Dimitri 2008/05/21 13:14:31 On 2008/05/20 18:37:04, Scott.Hess wrote: > SQLite On 2008/05/20 18:37:04, Scott.Hess wrote: > SQLite convention would be if( rc!=SQLITE_OK ) return rc; Done.
1156 return rc;

1157 }

1158 }

1159 rc = sqlite3BtreeUpdateMeta(bt, 6, (u32)user_version);

1160 if (needs_transaction) {

1161 if (rc != SQLITE_OK) {

1162 sqlite3BtreeRollback(bt);

1163 return rc;

1164 }

1165 return sqlite3BtreeCommit(bt);

1166 }

1167 return rc;
Scott.Hess 2008/05/20 18:37:04 Have you tried a version of this which looked like Have you tried a version of this which looked like: if( bt->inTrans==TRANS_WRITE ){ return sqlite3BtreeUpdateMeta(...); }else{ int rc = sqlite3BTreeBeginTrans(bt, 1); if( rc!=SQLITE_OK ) return rc; rc = sqlite3BtreeUpdateMeta(...); if( rc==SQLITE_OK ) return sqlite3BtreeCommit(bt); sqlite3BtreeRollback(bt); return rc; } This field is too small for me to say if that couldn't be further improved by some re-ordering. Dimitri 2008/05/21 13:14:31 On 2008/05/20 18:37:04, Scott.Hess wrote: > Have y On 2008/05/20 18:37:04, Scott.Hess wrote: > Have you tried a version of this which looked like: > > if( bt->inTrans==TRANS_WRITE ){ > return sqlite3BtreeUpdateMeta(...); > }else{ > int rc = sqlite3BTreeBeginTrans(bt, 1); > if( rc!=SQLITE_OK ) return rc; > rc = sqlite3BtreeUpdateMeta(...); > if( rc==SQLITE_OK ) return sqlite3BtreeCommit(bt); > sqlite3BtreeRollback(bt); > return rc; > } > > This field is too small for me to say if that couldn't be further improved by > some re-ordering. > Restructured as you suggested. Done.
1168 }

1169

1170 #endif /* SQLITE_OMIT_PRAGMA \|\| SQLITE_OMIT_PARSER */	1135 #endif /* SQLITE_OMIT_PRAGMA \|\| SQLITE_OMIT_PARSER */

LEFT	RIGHT