Skip to content
This repository has been archived by the owner on Oct 18, 2023. It is now read-only.

query_analysis: allow CREATE FUNCTION statement #365

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

psarna
Copy link
Contributor

@psarna psarna commented Apr 26, 2023

This draft is a showcase of how little it takes to enable Wasm-powered user-defined functions in sqld (and then Turso).

It's just a draft because it shows a cheap workaround for the parser, but we can also cook a more decent solution even without changing the sqlite3-parser crate - and ideally we can just contribute to that, under a libsql feature flag.

btw: I needed to put this condition inside std::iter::from_fn because otherwise Rust isn't able to determine a single return type for this function, which isn't ideal, but if we decide to go forward with enabling Wasm udfs in sqld, we can rework that too.

/cc @MarinPostma @glommer

@psarna psarna marked this pull request as draft April 26, 2023 17:41
@psarna psarna force-pushed the create_function_analysis branch from 490d28c to 8794ea9 Compare April 26, 2023 17:44

// Workaround for CREATE FUNCTION.
// FIXME: this should be handled by the parser (or *at least* trimmed + case insensitive)
let is_create_function = s.starts_with("CREATE FUNCTION");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah this needs to be handled by the parser 100%, trimming and case insensitive won't be enough, there can be an arbitrary number of whitespace between create and function

Comment on lines +223 to +225
if is_create_function {
return maybe_create_function_stmt.take();
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not correct, the string can contain multiple statements

let mut maybe_create_function_stmt = is_create_function.then(|| {
Ok(Statement {
stmt: s.to_string(),
kind: StmtKind::Other,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is definitely a write

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants