🐛 Handle queries starting with a semicolon

CHANGELOG.md

@@ -1,5 +1,11 @@
 # CHANGELOG
 
+## Unreleased
+
+- Fixed a bug where a query starting with a semicolon would result in a crash
+  instead of a proper syntax error.
+  ([Giacomo Cavalieri](https://github.com/giacomocavalieri))
+
 ## v1.7.0 - 2024-09-09
 
 - Fixed a bug where an authentication error would result in a failure with a

birdie_snapshots/query_starting_with_a_semicolon_produces_syntax_error_instead_of_crashing.accepted

@@ -0,0 +1,14 @@
+---
+version: 1.2.1
+title: query starting with a semicolon produces syntax error instead of crashing
+file: ./test/squirrel_test.gleam
+test_name: query_starting_with_a_semicolon_produces_syntax_error_instead_of_crashing_test
+---
+Error: Invalid query [42601]
+
+    ╭─ query.sql
+    │
+  1 │ ;select 1
+      ┬
+      ╰─ syntax error at or near ";"
+    ┆

src/squirrel/internal/database/postgres.gleam

@@ -43,7 +43,15 @@ import squirrel/internal/query.{
 }
 import squirrel/internal/scram
 
-const find_postgres_type_query = "
+fn find_postgres_type_query() -> UntypedQuery {
+  let assert Ok(name) = gleam.identifier("find_postgres_type_query")
+
+  query.UntypedQuery(
+    file: "",
+    starting_line: 1,
+    name:,
+    comment: [],
+    content: "
 select
   -- The name of the type or, if the type is an array, the name of its
   -- elements' type.
@@ -63,9 +71,19 @@ from
   left join pg_type as elem on type.typelem = elem.oid
 where
   type.oid = $1
-"
+    ",
+  )
+}
 
-const find_column_nullability_query = "
+fn find_column_nullability_query() -> UntypedQuery {
+  let assert Ok(name) = gleam.identifier("find_column_nullability_query")
+
+  query.UntypedQuery(
+    file: "",
+    starting_line: 1,
+    name:,
+    comment: [],
+    content: "
 select
   -- Whether the column has a not-null constraint.
   attnotnull
@@ -76,7 +94,16 @@ where
   attrelid = $1
   -- The index of the column we're looking for.
   and attnum = $2
-"
+    ",
+  )
+}
+
+fn explain_query(query: UntypedQuery) -> UntypedQuery {
+  query.UntypedQuery(
+    ..query,
+    content: "explain (format json, verbose) " <> query.content,
+  )
+}
 
 // --- TYPES -------------------------------------------------------------------
 
@@ -596,11 +623,7 @@ fn find_gleam_type(query: UntypedQuery, oid: Int) -> Db(gleam.Type) {
   // The only parameter to this query is the oid of the type to lookup:
   // that's a 32bit integer (its oid needed to prepare the query is 23).
   let params = [pg.Parameter(<<oid:32>>)]
-  let run_query =
-    find_postgres_type_query
-    |> run_query(query.file, params, [23])
-
-  use res <- eval.try(run_query)
+  use res <- eval.try(run_query(find_postgres_type_query(), params, [23]))
 
   // We know the output must only contain two values: the name and a boolean to
   // check wether it is an array or not.
@@ -630,10 +653,13 @@ fn query_plan(query: UntypedQuery, parameters: Int) -> Db(Plan) {
   // all the possible holes in the user supplied query with null values;
   // otherwise, the server would complain that it has arguments that are not
   // bound.
-  let explain_query = "explain (format json, verbose) " <> query.content
   let params = list.repeat(pg.Null, parameters)
-  let run_query = run_query(explain_query, query.file, params, [])
-  use res <- eval.try(run_query)
+  let res =
+    explain_query(query)
+    |> run_query(params, [])
+    |> eval.map_error(adjust_parse_error_for_explain)
+
+  use res <- eval.try(res)
 
   // We know the output will only contain a single row that is the json string
   // containing the query plan.
@@ -760,8 +786,7 @@ fn column_nullability(table table: Int, column column: Int) -> Db(Nullability) {
   // - the oid of the table (a 32bit integer, oid is 23)
   // - the index of the column (a 32 bit integer, oid is 23)
   let params = [pg.Parameter(<<table:32>>), pg.Parameter(<<column:32>>)]
-  let run_query =
-    find_column_nullability_query |> run_query("", params, [23, 23])
+  let run_query = run_query(find_column_nullability_query(), params, [23, 23])
   use res <- eval.try(run_query)
 
   // We know the output will only have only one column, that is the boolean
@@ -791,8 +816,7 @@ fn column_nullability(table table: Int, column column: Int) -> Db(Nullability) {
 /// > specific hard coded queries that are guaranteed to return a single row.
 ///
 fn run_query(
-  query: String,
-  query_file: String,
+  query: UntypedQuery,
   parameters: List(pg.ParameterValue),
   parameters_object_ids: List(Int),
 ) -> Db(List(BitArray)) {
@@ -818,7 +842,7 @@ fn run_query(
   //   making new requests
   use _ <- eval.try(
     send_all([
-      pg.FeParse("", query, parameters_object_ids),
+      pg.FeParse("", query.content, parameters_object_ids),
       pg.FeBind(
         portal: "",
         statement_name: "",
@@ -838,7 +862,7 @@ fn run_query(
   use msg <- eval.try(receive())
   let assert pg.BeBindComplete = msg
   use msg <- eval.try(receive())
-  use res <- eval.try(expect_data_row(msg, query_file))
+  use res <- eval.try(expect_data_row(msg, query.file))
   use msg <- eval.try(receive())
   let assert pg.BeCommandComplete(_, _) = msg
   use msg <- eval.try(receive())
@@ -1061,6 +1085,63 @@ fn invalid_column_error(
   )
 }
 
+/// Some queries might be parsed correctly at first but result in a parse error
+/// later when we try to `explain` those. For example:
+///
+/// ```sql
+/// ; select 1;
+/// ```
+///
+/// Will parse fine at first but when we try to explain it we get a parse error
+/// since the query would be:
+///
+/// ```sql
+/// explain (format json, verbose) ; select 1;
+/// --                             ^ parse error here
+/// ```
+///
+/// So in case we get a parse error during the explain step we want to slightly
+/// change it: we drop the explain part from the query so it doesn't show up in
+/// the public facing error.
+///
+fn adjust_parse_error_for_explain(error: Error) -> Error {
+  case error {
+    CannotParseQuery(
+      file:,
+      name:,
+      content:,
+      starting_line:,
+      error_code:,
+      pointer:,
+      additional_error_message:,
+      hint:,
+    ) -> {
+      let pointer = case pointer {
+        Some(Pointer(ByteIndex(index), message)) ->
+          // We also need to update any pointer to make sure it's pointing to
+          // the right place since we've dropped the first 31 bytes of the
+          // query's content.
+          Some(Pointer(ByteIndex(index - 31), message))
+
+        _ -> pointer
+      }
+
+      CannotParseQuery(
+        file:,
+        name:,
+        content: string.drop_left(content, 31),
+        starting_line:,
+        error_code:,
+        pointer:,
+        additional_error_message:,
+        hint:,
+      )
+    }
+
+    _ -> error
+  }
+}
+
 // --- DECODERS ----------------------------------------------------------------
 
 fn json_plans_decoder(data: Dynamic) -> Result(List(Plan), DecodeErrors) {

test/squirrel_test.gleam

@@ -488,3 +488,11 @@ on conflict on constraint wobble do nothing;
     title: "when a constraint doesn't exist there should be an error message",
   )
 }
+
+// https://github.com/giacomocavalieri/squirrel/issues/24
+pub fn query_starting_with_a_semicolon_produces_syntax_error_instead_of_crashing_test() {
+  should_error(";select 1")
+  |> birdie.snap(
+    title: "query starting with a semicolon produces syntax error instead of crashing",
+  )
+}