Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql.file does not support multiple statements for sqlite #90

Open
petehunt opened this issue Jul 26, 2020 · 1 comment
Open

sql.file does not support multiple statements for sqlite #90

petehunt opened this issue Jul 26, 2020 · 1 comment
Labels
bug Something isn't working SQLite

Comments

@petehunt
Copy link

hey - thanks for a great library. i haven't tested this with any other db besides sqlite, but it doesn't seem to support multiple statements via sql.file: https://gist.github.com/petehunt/6a889fc8d01e14fb1667a1e7f9cb0ffd

this is pretty important as most of the time i have a separate .sql file to set up the schema. right now i work around it by reading the file directly and splitting on semicolons -- not the most optimal way to do it i think :)

@ForbesLindesay
Copy link
Owner

I think this is specific to SQLite, because we use Database.all to run the underlying query and that API only supports passing a single query.

Overall this is pretty inconsistent between databases though. I'm starting to think the best option would be to have an API in @databases/sql that you can call to split multiple statements into an Array (i.e. SQLQuery[]). We could then make a breaking change to @databases/pg and possibly @databases/mysql so that the interface becomes overloaded like:

interface Queryable {
  /**
   * Accepts an SQLQuery with a single statement, and errors if there are multiple statements in one query
   */
  query(q: SQLQuery): Promise<any[]>

  /**
   * Accepts an array of SQLQueries with one statement each, and returns an array where each
   * element represents the results of one statement.
   *
   * If you pass it an empty array it returns an empty array without any call to the database.
   * If you pass it an array with only one statement, it returns an array of length 1 containing
   * an array of the results to the statement.
   */
  query(q: SQLQuery[]): Promise<any[][]>
}

We would then extend SQLQuery with:

interface SQLQuery {
  // Split this query into an array where each item in the array is one statement.
  // If this query is an empty string, or only contains comments, a string of length 0 is returned.
  split(): SQLQuery[]
  // ...existing methods & properties...
}

The breaking change here is that currently we allow you to pass multiple statements in one go to query in Postgres. If you do that, we run all statements, and return the results of the last statement.

Decisions / Questions

  1. Is it a good idea to require the caller of the API to be explicit about when a query has multiple statements? The problem with supporting this implicitly, is that it can be easy to have code that breaks when there is only one query or where there are 0 queries. We could keep the postgres behaviour for query (i.e. if there are multiple queries, just return the value from the last query).
  2. What happens when a query fails? If this API is used outside of a transaction, we could implicitly create a transaction for just these queries, this might be the safest thing to do. SQLite's .exec function just runs them one at a time and stops at the first error, but this feels pretty dangerous.

@ForbesLindesay ForbesLindesay added bug Something isn't working SQLite labels Feb 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working SQLite
Projects
None yet
Development

No branches or pull requests

2 participants