workerd icon indicating copy to clipboard operation
workerd copied to clipboard

Feature request: add offset to SQLite's error mesages

Open rozenmd opened this issue 1 year ago • 3 comments

The issue

While looking into https://github.com/cloudflare/workers-sdk/issues/4343 I noticed a difference between how workerd/d1 reports errors, and how SQLite shell reports errors:

wrangler d1 execute DBNAME --local --command='CREATE TABLE accounts (
  id tinytext NOT NULL,
  username varchar(32) NOT NULL,
  disabled tinyint(1) NOT NULL DEFAULT 0,
  email varchar(255) NOT NULL,
  password tinytext NOT NULL,
  oplvl tinyint(4) NOT NULL DEFAULT 0,
  created timestamp NOT NULL DEFAULT current_timestamp(),
  lastchange timestamp NOT NULL DEFAULT current_timestamp(),
  token tinytext NOT NULL DEFAULT '',
  session tinytext NOT NULL DEFAULT ''
);'

X [ERROR] near "(": syntax error

vs

sqlite> CREATE TABLE accounts (
  id tinytext NOT NULL,
  username varchar(32) NOT NULL,
  disabled tinyint(1) NOT NULL DEFAULT 0,
  email varchar(255) NOT NULL,
  password tinytext NOT NULL,
  oplvl tinyint(4) NOT NULL DEFAULT 0,
  created timestamp NOT NULL DEFAULT current_timestamp(),
  lastchange timestamp NOT NULL DEFAULT current_timestamp(),
  token tinytext NOT NULL DEFAULT '',
  session tinytext NOT NULL DEFAULT ''
);
Parse error: near "(": syntax error
  eated timestamp NOT NULL DEFAULT current_timestamp(),   lastchange timestamp N
                                      error here ---^

Some context

It turns out this is because the sqlite shell implements nice errors here: https://github.com/sqlite/sqlite/blob/master/src/shell.c.in#L3097-L3101, while regular sqlite doesn't return the exact location of the error.

Potential fix

Back in SQLite Release 3.38.4, we got access to the sqlite3_error_offset() interface:

Added the sqlite3_error_offset() interface, which can sometimes help to localize an SQL error to a specific character in the input SQL text, so that applications can provide better error messages.

Using it, we could make the error message returned from workerd more like:

X [ERROR] near "(" at offset 17: syntax error

rather than:

X [ERROR] near "(": syntax error

rozenmd avatar Nov 06 '23 15:11 rozenmd

I had a quick try at implementing sqlite3_error_offset() into workerd's SQLite errors and had success with it - in the event that the error was not related to a specific token then sqlite3_error_offset() would return -1.

image

(when provided an input of SELECT ;)

If it's possible to further emulate the shells behaviour, given we have access to the input SQL, and do a 'error here' arrow then that'd be awesome.

https://github.com/sqlite/sqlite/blob/ff9ff548018bbe276dbd7bf5418af8bdfd659a24/src/shell.c.in#L3028-L3032

KianNH avatar Nov 06 '23 15:11 KianNH

@rozenmd @KianNH

Thank you both for taking my feedback in stride!

james-pre avatar Nov 06 '23 21:11 james-pre

No problem. We should also be able to implement the input & arrow indicator that SQLite's shell has too.

image

KianNH avatar Nov 06 '23 21:11 KianNH