Skip to content

Commit

Permalink
bugfix: reset modifications of sqlite3_vtab (#1027)
Browse files Browse the repository at this point in the history
* Remove `libsql_module` field from `sqlite3_vtab`

* remove `libsql_module` struct

Also removed the associated functions `libsql_create_module_v2`, `libsql_create_module` functions'.

The `libsql_module` had a function `xPreparedSql` which is now moved to `sqlite_module`. The `sqlite_module` might get changed in the upstream, so added some padding space for our custom functions

* generate ffi bindings

* Add a github workflow to test crsqlite

* fix crsqlite: remove `pLibsqlModule` references

* Add tests for sqlite-vss extension
  • Loading branch information
avinassh authored Feb 15, 2024
1 parent 111b783 commit d178de8
Show file tree
Hide file tree
Showing 14 changed files with 134 additions and 116 deletions.
79 changes: 79 additions & 0 deletions .github/workflows/extensions-test.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
name: Extensions Tests

on:
push:
branches: [ "main" ]
pull_request:
merge_group:
branches: [ "main" ]

env:
CARGO_TERM_COLOR: always
PROTOC_VERSION: 3.23.4

jobs:
c-tests:
runs-on: ubuntu-latest
defaults:
run:
working-directory: libsql-sqlite3
name: CR SQLite C Tests

steps:
- uses: hecrj/setup-rust-action@v1
- name: Checkout repository
uses: actions/checkout@v3
- name: build libsql
run: |
./configure
make libsql
- name: build
run: |
cd ext/crr
make loadable
- name: test
run: |
cd ext/crr
make test
rs-tests:
runs-on: ubuntu-latest
defaults:
run:
working-directory: libsql-sqlite3
name: CR SQLite Rust Tests

steps:
- uses: hecrj/setup-rust-action@v1
- name: Checkout repository
uses: actions/checkout@v3
- name: build libsql
run: |
./configure
make libsql
- name: test
run: |
cd ext/crr/rs/core
cargo test --features=loadable_extension
extensions-tests:
runs-on: ubuntu-latest
name: Extensions Tests

steps:
- uses: hecrj/setup-rust-action@v1
- name: Checkout repository
uses: actions/checkout@v3
- name: build libsql and ffi bindings
run: |
cargo xtask build-bundled
- name: download extensions
run: |
cd libsql-sqlite3/test/rust_suite
export VSS_VERSION="v0.1.2"
wget https://github.com/asg017/sqlite-vss/releases/download/$VSS_VERSION/sqlite-vss-$VSS_VERSION-loadable-linux-x86_64.tar.gz
tar -xvf sqlite-vss-$VSS_VERSION-loadable-linux-x86_64.tar.gz -C src
- name: test
run: |
cd libsql-sqlite3/test/rust_suite
cargo test --features extensions
62 changes: 13 additions & 49 deletions libsql-ffi/bundled/src/sqlite3.c
Original file line number Diff line number Diff line change
Expand Up @@ -7658,7 +7658,6 @@ typedef struct sqlite3_vtab sqlite3_vtab;
typedef struct sqlite3_index_info sqlite3_index_info;
typedef struct sqlite3_vtab_cursor sqlite3_vtab_cursor;
typedef struct sqlite3_module sqlite3_module;
typedef struct libsql_module libsql_module;

/*
** CAPI3REF: Virtual Table Object
Expand Down Expand Up @@ -7716,11 +7715,10 @@ struct sqlite3_module {
** Those below are for version 4 and greater. */
int (*xIntegrity)(sqlite3_vtab *pVTab, const char *zSchema,
const char *zTabName, int mFlags, char **pzErr);
};

/* libSQL extensions for modules */
struct libsql_module {
int iVersion;
// reserved for sqlite extension
void (*reserved[5])();
// following methods are added by libsql
int (*xPreparedSql)(sqlite3_vtab_cursor*, const char*);
};

Expand Down Expand Up @@ -7968,14 +7966,6 @@ SQLITE_API int sqlite3_create_module_v2(
void *pClientData, /* Client data for xCreate/xConnect */
void(*xDestroy)(void*) /* Module destructor function */
);
SQLITE_API int libsql_create_module(
sqlite3 *db, /* SQLite connection to register module with */
const char *zName, /* Name of the module */
const sqlite3_module *p, /* Methods for the module */
const libsql_module *pLibsql, /* Methods for the module */
void *pClientData, /* Client data for xCreate/xConnect */
void(*xDestroy)(void*) /* Module destructor function */
);

/*
** CAPI3REF: Remove Unnecessary Virtual Table Implementations
Expand Down Expand Up @@ -8014,7 +8004,6 @@ SQLITE_API int sqlite3_drop_modules(
*/
struct sqlite3_vtab {
const sqlite3_module *pModule; /* The module for this virtual table */
const libsql_module *pLibsqlModule; /* The libSQL module for this virtual table */
int nRef; /* Number of open cursors */
char *zErrMsg; /* Error message from sqlite3_mprintf() */
/* Virtual table implementations will typically add additional fields */
Expand Down Expand Up @@ -18561,7 +18550,6 @@ struct Savepoint {
*/
struct Module {
const sqlite3_module *pModule; /* Callback pointers */
const libsql_module *pLibsqlModule; /* Callback pointers for libSQL */
const char *zName; /* Name passed to create_module() */
int nRefModule; /* Number of pointers to this object */
void *pAux; /* pAux passed to create_module() */
Expand Down Expand Up @@ -21836,7 +21824,6 @@ SQLITE_PRIVATE Module *sqlite3VtabCreateModule(
sqlite3*,
const char*,
const sqlite3_module*,
const libsql_module*,
void*,
void(*)(void*)
);
Expand Down Expand Up @@ -101258,7 +101245,6 @@ case OP_VInitIn: { /* out2, ncycle */
*/
case OP_VPreparedSql: {
const sqlite3_module *pModule;
const libsql_module *pLibsqlModule;
sqlite3_vtab_cursor *pVCur;
sqlite3_vtab *pVtab;
VdbeCursor *pCur;
Expand All @@ -101269,14 +101255,13 @@ case OP_VPreparedSql: {
pVCur = pCur->uc.pVCur;
pVtab = pVCur->pVtab;
pModule = pVtab->pModule;
pLibsqlModule = pVtab->pLibsqlModule;

/* Invoke the xPreparedSql method */
if (pLibsqlModule) {
if( pLibsqlModule->xPreparedSql && p->zSql ){
rc = pLibsqlModule->xPreparedSql(pVCur, p->zSql);
if( rc ) goto abort_due_to_error;
}
if( pModule->iVersion>=700 ) {
if (pModule->xPreparedSql && p->zSql) {
rc = pModule->xPreparedSql(pVCur, p->zSql);
if (rc) goto abort_due_to_error;
}
}

break;
Expand Down Expand Up @@ -141651,7 +141636,7 @@ SQLITE_PRIVATE Module *sqlite3PragmaVtabRegister(sqlite3 *db, const char *zName)
if( pName==0 ) return 0;
if( (pName->mPragFlg & (PragFlg_Result0|PragFlg_Result1))==0 ) return 0;
assert( sqlite3HashFind(&db->aModule, zName)==0 );
return sqlite3VtabCreateModule(db, zName, &pragmaVtabModule, NULL, (void*)pName, 0);
return sqlite3VtabCreateModule(db, zName, &pragmaVtabModule, (void*)pName, 0);
}

#endif /* SQLITE_OMIT_VIRTUALTABLE */
Expand Down Expand Up @@ -155133,7 +155118,6 @@ SQLITE_PRIVATE Module *sqlite3VtabCreateModule(
sqlite3 *db, /* Database in which module is registered */
const char *zName, /* Name assigned to this module */
const sqlite3_module *pModule, /* The definition of the module */
const libsql_module *pLibsqlModule, /* The definition of the libSQL module */
void *pAux, /* Context pointer for xCreate/xConnect */
void (*xDestroy)(void *) /* Module destructor function */
){
Expand All @@ -155154,7 +155138,6 @@ SQLITE_PRIVATE Module *sqlite3VtabCreateModule(
memcpy(zCopy, zName, nName+1);
pMod->zName = zCopy;
pMod->pModule = pModule;
pMod->pLibsqlModule = pLibsqlModule;
pMod->pAux = pAux;
pMod->xDestroy = xDestroy;
pMod->pEpoTab = 0;
Expand Down Expand Up @@ -155183,14 +155166,13 @@ static int createModule(
sqlite3 *db, /* Database in which module is registered */
const char *zName, /* Name assigned to this module */
const sqlite3_module *pModule, /* The definition of the module */
const libsql_module *pLibsqlModule, /* The definition of the libSQL module */
void *pAux, /* Context pointer for xCreate/xConnect */
void (*xDestroy)(void *) /* Module destructor function */
){
int rc = SQLITE_OK;

sqlite3_mutex_enter(db->mutex);
(void)sqlite3VtabCreateModule(db, zName, pModule, pLibsqlModule, pAux, xDestroy);
(void)sqlite3VtabCreateModule(db, zName, pModule, pAux, xDestroy);
rc = sqlite3ApiExit(db, rc);
if( rc!=SQLITE_OK && xDestroy ) xDestroy(pAux);
sqlite3_mutex_leave(db->mutex);
Expand All @@ -155210,7 +155192,7 @@ SQLITE_API int sqlite3_create_module(
#ifdef SQLITE_ENABLE_API_ARMOR
if( !sqlite3SafetyCheckOk(db) || zName==0 ) return SQLITE_MISUSE_BKPT;
#endif
return createModule(db, zName, pModule, NULL, pAux, 0);
return createModule(db, zName, pModule, pAux, 0);
}

/*
Expand All @@ -155226,24 +155208,7 @@ SQLITE_API int sqlite3_create_module_v2(
#ifdef SQLITE_ENABLE_API_ARMOR
if( !sqlite3SafetyCheckOk(db) || zName==0 ) return SQLITE_MISUSE_BKPT;
#endif
return createModule(db, zName, pModule, NULL, pAux, xDestroy);
}

/*
** External API function used to create a new virtual-table module.
*/
int libsql_create_module_v2(
sqlite3 *db, /* Database in which module is registered */
const char *zName, /* Name assigned to this module */
const sqlite3_module *pModule, /* The definition of the module */
const libsql_module *pLibsqlModule, /* The definition of the module */
void *pAux, /* Context pointer for xCreate/xConnect */
void (*xDestroy)(void *) /* Module destructor function */
){
#ifdef SQLITE_ENABLE_API_ARMOR
if( !sqlite3SafetyCheckOk(db) || zName==0 ) return SQLITE_MISUSE_BKPT;
#endif
return createModule(db, zName, pModule, pLibsqlModule, pAux, xDestroy);
return createModule(db, zName, pModule, pAux, xDestroy);
}

/*
Expand All @@ -155263,7 +155228,7 @@ SQLITE_API int sqlite3_drop_modules(sqlite3 *db, const char** azNames){
for(ii=0; azNames[ii]!=0 && strcmp(azNames[ii],pMod->zName)!=0; ii++){}
if( azNames[ii]!=0 ) continue;
}
createModule(db, pMod->zName, 0, 0, 0, 0);
createModule(db, pMod->zName, 0, 0, 0);
}
return SQLITE_OK;
}
Expand Down Expand Up @@ -155743,7 +155708,6 @@ static int vtabCallConstructor(
** the sqlite3_vtab object if successful. */
memset(pVTable->pVtab, 0, sizeof(pVTable->pVtab[0]));
pVTable->pVtab->pModule = pMod->pModule;
pVTable->pVtab->pLibsqlModule = pMod->pLibsqlModule;
pMod->nRefModule++;
pVTable->nRef = 1;
if( sCtx.bDeclared==0 ){
Expand Down
17 changes: 3 additions & 14 deletions libsql-ffi/bundled/src/sqlite3.h
Original file line number Diff line number Diff line change
Expand Up @@ -7284,7 +7284,6 @@ typedef struct sqlite3_vtab sqlite3_vtab;
typedef struct sqlite3_index_info sqlite3_index_info;
typedef struct sqlite3_vtab_cursor sqlite3_vtab_cursor;
typedef struct sqlite3_module sqlite3_module;
typedef struct libsql_module libsql_module;

/*
** CAPI3REF: Virtual Table Object
Expand Down Expand Up @@ -7342,11 +7341,10 @@ struct sqlite3_module {
** Those below are for version 4 and greater. */
int (*xIntegrity)(sqlite3_vtab *pVTab, const char *zSchema,
const char *zTabName, int mFlags, char **pzErr);
};

/* libSQL extensions for modules */
struct libsql_module {
int iVersion;
// reserved for sqlite extension
void (*reserved[5])();
// following methods are added by libsql
int (*xPreparedSql)(sqlite3_vtab_cursor*, const char*);
};

Expand Down Expand Up @@ -7594,14 +7592,6 @@ SQLITE_API int sqlite3_create_module_v2(
void *pClientData, /* Client data for xCreate/xConnect */
void(*xDestroy)(void*) /* Module destructor function */
);
SQLITE_API int libsql_create_module(
sqlite3 *db, /* SQLite connection to register module with */
const char *zName, /* Name of the module */
const sqlite3_module *p, /* Methods for the module */
const libsql_module *pLibsql, /* Methods for the module */
void *pClientData, /* Client data for xCreate/xConnect */
void(*xDestroy)(void*) /* Module destructor function */
);

/*
** CAPI3REF: Remove Unnecessary Virtual Table Implementations
Expand Down Expand Up @@ -7640,7 +7630,6 @@ SQLITE_API int sqlite3_drop_modules(
*/
struct sqlite3_vtab {
const sqlite3_module *pModule; /* The module for this virtual table */
const libsql_module *pLibsqlModule; /* The libSQL module for this virtual table */
int nRef; /* Number of open cursors */
char *zErrMsg; /* Error message from sqlite3_mprintf() */
/* Virtual table implementations will typically add additional fields */
Expand Down
4 changes: 2 additions & 2 deletions libsql-sqlite3/ext/crr/rs/core/src/create_cl_set_vtab.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,6 @@ fn connect_create_shared(
nRef: 0,
pModule: core::ptr::null(),
zErrMsg: core::ptr::null_mut(),
#[cfg(feature = "libsql")]
pLibsqlModule: core::ptr::null_mut(),
},
base_table_name: base_name_from_virtual_name(args.table_name).to_owned(),
db_name: args.database_name.to_owned(),
Expand Down Expand Up @@ -259,6 +257,8 @@ static MODULE: sqlite_nostd::module = sqlite_nostd::module {
xRollbackTo: None,
xShadowName: None,
xIntegrity: None,
reserved: [None, None, None, None, None],
xPreparedSql: None,
};

pub fn create_module(db: *mut sqlite::sqlite3) -> Result<ResultCode, ResultCode> {
Expand Down
4 changes: 2 additions & 2 deletions libsql-sqlite3/ext/crr/rs/core/src/unpack_columns_vtab.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ extern "C" fn connect(
nRef: 0,
pModule: core::ptr::null(),
zErrMsg: core::ptr::null_mut(),
#[cfg(feature = "libsql")]
pLibsqlModule: core::ptr::null_mut(),
}));
let _ = sqlite::vtab_config(db, sqlite::INNOCUOUS);
}
Expand Down Expand Up @@ -256,6 +254,8 @@ static MODULE: sqlite_nostd::module = sqlite_nostd::module {
xRollbackTo: None,
xShadowName: None,
xIntegrity: None,
reserved: [None, None, None, None, None],
xPreparedSql: None,
};

/**
Expand Down
1 change: 1 addition & 0 deletions libsql-sqlite3/ext/crr/src/changes-vtab.c
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ sqlite3_module crsql_changesModule = {
/* xShadowName */ 0
#ifdef LIBSQL
,
/* reserved */ NULL,
/* xPreparedSql */ 0
#endif
};
2 changes: 1 addition & 1 deletion libsql-sqlite3/src/pragma.c
Original file line number Diff line number Diff line change
Expand Up @@ -2936,7 +2936,7 @@ Module *sqlite3PragmaVtabRegister(sqlite3 *db, const char *zName){
if( pName==0 ) return 0;
if( (pName->mPragFlg & (PragFlg_Result0|PragFlg_Result1))==0 ) return 0;
assert( sqlite3HashFind(&db->aModule, zName)==0 );
return sqlite3VtabCreateModule(db, zName, &pragmaVtabModule, NULL, (void*)pName, 0);
return sqlite3VtabCreateModule(db, zName, &pragmaVtabModule, (void*)pName, 0);
}

#endif /* SQLITE_OMIT_VIRTUALTABLE */
Expand Down
Loading

0 comments on commit d178de8

Please sign in to comment.