Support query parameters in SQL generation
DuckDB requires that you run a prepare statement to use query parameters, but most other databases allow them to be used in a single step. For most systems, this is the primary security for protecting against SQL injection. Since the mosaic server accepts raw SQL, other measures have to be taken, likely via database user permissions. The primary use case here would be to simplify escaping, or allow for performance improvements as suggestion in #483.
Considerations:
- With the new visitor pattern to generate SQL, there would need to be some state to collect parameters as the SQL is generated
- There probably also needs to be some accessible state to make an auto-incremented id or other unique id available, to avoid naming collisions
- Positional parameters could be tricky to align, so I'd prefer to see named parameter support, but I think both could be supported by specific visitors
This isn't particularly urgent. I've been trained to always use params, so my knee jerk reaction when working on a BigQuery dialect was to use them, but as I typed out this issue, the whole mosaic query pathway is a giant SQL injection :)
Related issue:
- https://github.com/uwdata/mosaic/issues/483
I had a draft pull request in https://github.com/uwdata/mosaic/pull/486 for prepared statements/query params. It wasn't helping with perf and also surfaced a bug in ducked node. But I think with the visitor pattern from recently, redoing this feature should be a lot easier. I think it makes sense to move to query params as it should allow us to use native numbers as well rather than having to escape + stringify them.