Le ven. 4 août 2017 à 02:42, Nico Williams <n...@cryptonector.com> a écrit :
> On Fri, Aug 04, 2017 at 12:35:56AM +0200, Sylvain Pointeau wrote:
> > void sp_seq_init(sqlite3_context *context, int argc, sqlite3_value
> **argv) {
> > int rc = 0;
> > sqlite3_stmt *stmt;
> > sqlite3 *db = sqlite3_context_db_handle(context);
>
> If you use sqlite3_create_function*() with nArg == -1 then you can have
> just one UDF from sp_seq_init(). You'd have to check argc here though.
>
yes this is a good idea, I will do that as well as testing the arguments as
also Petern mentioned.
>
> > const unsigned char* seq_name = sqlite3_value_text(argv[0]);
> > long seq_init_val = sqlite3_value_int64(argv[1]);
>
> seq_init_val could be optional and default to 0 or maybe 1.
>
good idea, defaukt to 1
>
> > long seq_inc_val = argc < 3 ? 1 : sqlite3_value_int64(argv[2]);
>
> I think some type checking should be done.
>
yes agree
> You could just take the argv[] values and bind them directly to the
> insert below, and use CHECK constraints on the SP_SEQUENCE table.
> Or you could use sqlite3_value_type() to check that each argument is of
> the expected value.
>
> I like the idea of using a CHECK constraint. It simplifies the C code
> by having SQLite3 do more of the work.
>
I like also the check constraint, I am just wondering if the error, if any,
would descriptive enough. I will test it at least..
> > rc = sqlite3_exec(db, "CREATE TABLE IF NOT EXISTS SP_SEQUENCE ( " \
> > " SEQ_NAME TEXT NOT NULL PRIMARY KEY, " \
> > " SEQ_VAL INTEGER NOT NULL, " \
> > " SEQ_INC INTEGER NOT NULL " \
> > " )", 0, 0, 0);
>
> You might want to add WITHOUT ROWID here (unless you want this to work
> with older versions of SQLite3). You could make this conditional on a C
> pre-processor macro.
it is a good idea, however it does not seem critical
> > if( rc != SQLITE_OK ) {
> > sqlite3_result_error(context, sqlite3_errmsg(db), -1);
> > return;
> > }
> >
> >
> > sqlite3_prepare_v2(db, "insert or replace into sp_sequence (seq_name,
> > seq_val, seq_inc) values (?, ?, ?)", -1, &stmt, 0);
>
> Should this init function re-initialize sequences if called more than
> once for the same sequence name?
>
yes I did it on purpose
> Because that's what INSERT OR REPLACE will do (since on conflict it will
> delete the old row and insert a new one). If you don't mean to
> re-initialize then use INSERT OR IGNORE.
>
> (When I want INSERT OR UPDATE I just run two statements:
>
> UPDATE .. WHERE ..;
> INSERT OR IGNORE ..;)
>
> > sqlite3_bind_text(stmt, 1, seq_name, -1, SQLITE_STATIC);
> > sqlite3_bind_int64(stmt, 2, seq_init_val);
> > sqlite3_bind_int64(stmt, 3, seq_inc_val);
> >
> > rc = sqlite3_step(stmt);
> >
> > sqlite3_finalize(stmt);
> >
> > if (rc != SQLITE_DONE) {
> > sqlite3_result_error(context, sqlite3_errmsg(db), -1);
> > return;
> > }
> >
> > sqlite3_result_int64( context, seq_init_val );
> > }
> >
> > void sp_seq_nextval(sqlite3_context *context, int argc, sqlite3_value
> > **argv) {
> > int rc = 0;
> > sqlite3_stmt *stmt;
> > sqlite3 *db = sqlite3_context_db_handle(context);
> > long seq_val = 0;
> >
> > const unsigned char* seq_name = sqlite3_value_text(argv[0]);
> >
> > sqlite3_prepare_v2(db, "update sp_sequence set seq_val = seq_val +
> > seq_inc where seq_name = ?", -1, &stmt, 0);
> >
> > sqlite3_bind_text(stmt, 1, seq_name, -1, SQLITE_STATIC);
> >
> > rc = sqlite3_step(stmt);
> >
> > sqlite3_finalize(stmt);
> >
> > if (rc != SQLITE_DONE) {
> > sqlite3_result_error(context, sqlite3_errmsg(db), -1);
> > return;
> > }
> >
> > sqlite3_prepare_v2(db, "select seq_val from sp_sequence where seq_name
> =
> > ?", -1, &stmt, 0);
> >
> > sqlite3_bind_text(stmt, 1, seq_name, -1, SQLITE_STATIC);
>
> <aside>
>
> If ever SQLite3 got higher write concurrency (it almost certainly won't,
> but derivatives might), then a simple way to ensure atomicity here might
> be to do the SELECT first then an UPDATE with a WHERE seq_val =
> :the_value_just_read, and loop as necessary.
>
> do {
> current_value = <run SELECT here>;
> <run update here using WHERE constraint to ensure that the
> current value hasn't changed>;
> } while (<updated row count == 0>);
>
> Though if one is going to have higher write concurrency then one ought
> to have SELECT .. FOR UPDATE.
>
> Anyways, no change needed here.
>
should I use the mutex lock to ensure atomicity?
> </aside>
>
> > rc = sqlite3_step(stmt);
> >
> > if( rc == SQLITE_ROW) {
> > seq_val = sqlite3_column_int64(stmt, 0);
> > }
> >
> > sqlite3_finalize(stmt);
> >
> > if (rc != SQLITE_ROW) {
> > if( rc == SQLITE_DONE ) sqlite3_result_error(context, "sequence name
> > does not exist", -1);
> > else sqlite3_result_error(context, sqlite3_errmsg(db), -1);
> > return;
> > }
> >
> > sqlite3_result_int64( context, seq_val );
> > }
> >
> >
> > int sqlite3_extension_init(
> > sqlite3 *db,
> > char **pzErrMsg,
> > const sqlite3_api_routines *pApi
> > ){
> > SQLITE_EXTENSION_INIT2(pApi)
>
> I'd create just one UDF using sp_seq_init(), make the narg -1, and check
> the argc counts in sp_seq_init().
>
yes I will do
>
> > sqlite3_create_function(db, "seq_init_inc", 3, SQLITE_UTF8, 0,
> > sp_seq_init, 0, 0);
> > sqlite3_create_function(db, "seq_init", 2, SQLITE_UTF8, 0, sp_seq_init,
> 0,
> > 0);
>
> If you made seq_next() have nArg == -1 then you could have a default
> sequence for the no-arguments case... You don't have to -- you could
> say that's just creeping featuritis!
>
:-) I will keep on specifying the sequence
>
> > sqlite3_create_function(db, "seq_nextval", 1, SQLITE_UTF8, 0,
> > sp_seq_nextval, 0, 0);
> > return 0;
> > }
>
> Nice! Did you test it?
yes! I tested it with mingw compiler (and cmake), and the sqlite3 shell to
test the seq functions.
I will do the code update and will probably create a github repository
thanks a lot for your review.
_______________________________________________
sqlite-users mailing list
sqlite-users@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
