sqlite-statement-vtab icon indicating copy to clipboard operation
sqlite-statement-vtab copied to clipboard

Option to create scalar valued function?

Open hoehrmann opened this issue 4 years ago • 7 comments

Any thoughts on adding support for scalar valued functions? Could be something like:

CREATE VIRTUAL TABLE foo USING statement((
  SELECT expr
), foo_function);

SELECT foo_function(bar);
-- Returns the first column of SELECT (SELECT expr)

hoehrmann avatar Apr 14 '21 22:04 hoehrmann

@hoehrmann does this do what you're looking for? https://gist.github.com/0x09/53eaf9eee7da7d90cb54814dce4596a6 It allows you to write select create_function("foo_function", "expr"); to define an SQL function using an expression. The hypotenuse example from the vtab readme would look like

select create_function("hypot", "sqrt(?1 * ?1 + ?2 * ?2)");
select hypot(3,4);
5.0       

0x09 avatar Apr 15 '21 00:04 0x09

@0x09 I have basically hygiene issues with that. SELECT statements should not change the schema, and putting possibly longish SQLs into a quoted string can easily get painful due to escaping issues, and it bites with formatters and syntax highlighters.

The CREATE MODULE syntax is much closer to the CREATE FUNCTION that I would like to have, and it avoids both issues. Similarily, I could just use JOINs and sub-SELECTs to call the function with your module, but for scalar functions the syntax does feel unwieldy, hence my question.

I could probably create a patch for my approach above; would you be interested in that, or do you think there are issues with the idea in general?

hoehrmann avatar Apr 15 '21 01:04 hoehrmann

Playing around with your gist, having to finalize statements before functions are destroyed on close is a bummer. Another problem seems to be that SQLite will not let you override the created function (sqlite3_create_function_v2 fails with Error: database is locked if the function was registered before).

With the syntax as suggested above, statement_vtab_destroy could finalize the function's statement, at least I am not seeing any problem with some ad-hoc tests. In order to redefine the function, you would have to drop the virtual table, in which case the statement could be cleaned up properly aswell.

hoehrmann avatar Apr 15 '21 21:04 hoehrmann

Gluing some bits code fragments together I got this far. Unfortunately the scalar function cannot be unregistered in statement_vtab_destroy so this would at the very least need a way for scalar_call to abort early. Not sure what a good way for that would be.

Also wondering if the SQL ought to be wrapped into another SELECT (to make sqlite3_prepare report an error, instead of silently ignoring query results when the scalar function is called).

--- a/statement_vtab.c
+++ b/statement_vtab.c
@@ -18,6 +18,8 @@ struct statement_vtab {
 	size_t sql_len;
 	int num_inputs;
 	int num_outputs;
+	char* scalar_name;
+	sqlite3_stmt* scalar_stmt;
 };
 
 struct statement_cursor {
@@ -28,6 +30,35 @@ struct statement_cursor {
 	sqlite3_value** param_argv;
 };
 
+static void scalar_call(sqlite3_context *ctx, int argc, sqlite3_value **argv) {
+  
+	int ret = SQLITE_OK;
+
+	struct statement_vtab const* const vtab = sqlite3_user_data(ctx);
+
+	sqlite3_stmt* const stmt = vtab->scalar_stmt;
+
+  for (int i = 0; i < argc; i++)
+    if ((ret = sqlite3_bind_value(stmt, i + 1, argv[i])) != SQLITE_OK)
+      goto end;
+  if ((ret = sqlite3_step(stmt)) != SQLITE_ROW) {
+    if (ret == SQLITE_DONE)
+      ret = SQLITE_MISUSE;
+    goto end;
+  }
+  sqlite3_result_value(ctx, sqlite3_column_value(stmt, 0));
+	sqlite3_reset(stmt);
+
+end:
+  if (ret != SQLITE_ROW)
+    sqlite3_result_error_code(ctx, ret);
+
+}
+
 static char* build_create_statement(sqlite3_stmt* stmt) {
 	sqlite3_str* sql = sqlite3_str_new(NULL);
 	sqlite3_str_appendall(sql,"CREATE TABLE x( ");
@@ -53,8 +84,22 @@ static char* build_create_statement(sqlite3_stmt* stmt) {
 }
 
 static int statement_vtab_destroy(sqlite3_vtab* pVTab){
-	sqlite3_free(((struct statement_vtab*)pVTab)->sql);
+
+	struct statement_vtab* vtab = (struct statement_vtab*)pVTab;
+
+	int ret = SQLITE_OK;
+
+	// FIXME(bh): What if either fails?
+	ret = sqlite3_finalize(vtab->scalar_stmt);
+
+	// NOTE(bh): Sadly this results in SQLITE_BUSY
+	ret = sqlite3_create_function_v2(vtab->db, vtab->scalar_name,
+		vtab->num_inputs, SQLITE_UTF8, NULL, NULL, NULL, NULL, NULL);
+
+	sqlite3_free(vtab->scalar_name);
+	sqlite3_free(vtab->sql);
 	sqlite3_free(pVTab);
+
 	return SQLITE_OK;
 }
 
@@ -114,6 +159,28 @@ static int statement_vtab_create(sqlite3* db, void* pAux, int argc, const char*
 
 	sqlite3_free(create);
 	sqlite3_finalize(stmt);
+
+	if (argc >= 4) {
+
+		sqlite3_mutex_enter(mutex);
+		if((ret = sqlite3_prepare_v2(db,vtab->sql,vtab->sql_len,&vtab->scalar_stmt,NULL)) != SQLITE_OK)
+			goto sqlite_error;
+		sqlite3_mutex_leave(mutex);
+
+		sqlite3_mutex_enter(mutex);
+		if ((ret = sqlite3_create_function_v2(
+						db, argv[4], vtab->num_inputs, SQLITE_UTF8,
+						vtab, scalar_call, NULL, NULL, NULL)) != SQLITE_OK)
+			goto sqlite_error;
+		sqlite3_mutex_leave(mutex);
+
+		if(!(vtab->scalar_name = sqlite3_mprintf("%s",argv[4]))) {
+			ret = SQLITE_NOMEM;
+			goto error;
+		}
+
+	}
+
 	return SQLITE_OK;
 
 sqlite_error:

hoehrmann avatar Apr 15 '21 23:04 hoehrmann

Round about this should work https://github.com/pullhub/sqlite-statement-vtab/commit/b919ea8542f62007dc33f280fa0b4e1c5a7c5a33

hoehrmann avatar Apr 16 '21 00:04 hoehrmann

@hoehrmann that makes sense. I think that function creation is sort of out of scope for what I'd like the module to do (especially since the behavior/treatment of function calls vs vtab queries is so different) but I also would consider it more or less complete and don't expect the module to need much additional development, so if you'd like to start a fork for this feature there shouldn't be much/any maintenance burden.

To be honest I would have preferred some function syntax for producing multiple columns rather than using virtual tables for this in the first place (e.g. select multi(args) as [x, y]) -- vtabs were just the closest thing and as a bonus they are a bit more powerful.

SELECT statements should not change the schema

It's weird but this is technically already the case in sqlite due to https://www.sqlite.org/lang_corefunc.html#load_extension

0x09 avatar Apr 17 '21 21:04 0x09

SELECT statements should not change the schema

Well, they do. When you select load_extension('Foo'), which is the official way to dynamically load an extension. Extensions register functions (and vtab modules) typically, which is not visible in sqlite_master per-se, but they sure can, if they want to. So there are precedents.

(Oh, just noticed you already replied that just above. Silly me)

putting possibly longish SQLs into a quoted string can easily get painful

That I agree on, OTOH.

ddevienne avatar Jan 25 '23 08:01 ddevienne