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.

>   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.

>   long seq_inc_val = argc < 3 ? 1 : sqlite3_value_int64(argv[2]);

I think some type checking should be done.

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.

>   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.

>   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?

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.

</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().

>  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!

>  sqlite3_create_function(db, "seq_nextval", 1, SQLITE_UTF8, 0,
> sp_seq_nextval, 0, 0);
>  return 0;
> }

Nice!  Did you test it?
_______________________________________________
sqlite-users mailing list
sqlite-users@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users

Reply via email to