sqldelight
sqldelight copied to clipboard
SQL statements in lowercase provided confusing error messages
SQLDelight Version
1.5.3
SQLDelight Dialect
mysql
Describe the Bug
I was migrating an existing project to use SQLDelight. I had the following in the first migration schema file:
create table foo (
`id` bigint primary key not null auto_increment,
`created_at` timestamp not null default current_timestamp on update current_timestamp,
`name` varchar(32) not null unique,
key `index_on_name` (`name`)
) engine=InnoDB default charset=utf8mb4;
and I got a ream of error messages, the first of which was this:
/path/to/migration.sqm line 1:7 - '{' expected, got 'table'
1 table
^^^^^
It took a couple of minutes to figure out why this was failing since the error message wasn't too helpful but the problem turned out to be case-sensitivity. Although the SQL statement as written works fine in a MySQL 5.7 database, it needs to be converted to uppercase for SQLDelight to recognize it:
CREATE TABLE foo (
`id` bigint PRIMARY KEY NOT NULL AUTO_INCREMENT,
`created_at` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
`name` varchar(32) NOT NULL UNIQUE,
KEY `index_on_name` (`name`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;
It would be great if the SQL parsing engine in SQLDelight could detect this (probably frequent) error and emit a more useful error message. Or even better, just accept the lowercase SQL.
Stacktrace
No response
I was also very surprised by this behavior. SQL as a language is case-insensitive with regard to keywords and should be supported as such.
I got this error for SQLite dialect, but I suspect it is the same for all dialects.
its unfortunate because only treating uppercase as the keywords means you can do stuff like:
update:
UPDATE foo SET bar = ?;
and I like having sql more uniform. However I weekly hold this stance, and I think I care more about it in SQLite than I do other dialects, so maybe we can provide some way to tell the compiler not to care (that way we dont break existing code too)
We could just add %ignorecase
to the lexer.
i think that would still have the issue I posted above, if its at the lexer level than lowercase update
would get tokenized and be unusable for an identifier
It would be very helpful if it can be optionally supported.
It is very hard for me to read SQL that is all uppercase. Writing is OK because IntelliJ plugin suggests even on lowercase and change the case accordingly, but reading is so much more frequent than writing.
The uppercase is maybe little bit helpful when viewing SQL like plain text, but with IDE hightlighting it is just distracting and hard readable, with longer definitions of many consequent keywords.
Personally, the tradeoff that we cannot use SQL keywords as labels in this optional mode is worth it.
Here is a twitter poll about SQL casing from Lukas Eder, the creator of jOOQ, just for context. https://twitter.com/lukaseder/status/1052809813447593986
Are there any plans to support this feature?