Missing `;` in the sql files fails silently

[BigBoyBarney]

Hi! Thank you for this shard!

So far, my experience with Drift has been nothing short of amazing. It's so effortless to use. Fantastic work!

A minor (skill) issue I ran into earlier was that I omitted a ; from the end of the statement, which then failed silently, both at compile time and then at run time.

Example:

-- drift:migrate
create table if not exists example (
  id integer primary key
) strict

-- drift:rollback
drop table if exists example;

The above will fail silently and do nothing at run time.

Example 2:

-- drift:migrate
create table if not exists example (
  id integer primary key
) strict

insert into example (id)
values (1);

-- drift:rollback
drop table if exists example;

The above will raise a SQLite3::Exception - Syntax Error at runtime.

---

These are all PEBCAK so they're not directly Drift related. I'm not entirely sure how feasible it would be, but it would be super cool if Drift did some compile time validation of the sql files.

If not, then I suppose I'll just have to pay more attention to the queries I write :P

Thank you!

[luislavena]

Hello @BigBoyBarney, thanks for your report.

As you mention, missing semicolons will fail to identify each statement, but is beyond the role fo Drift to parse the entire SQL to determine what you wrote is correct SQL or not.

The README indicates this:

Each type (migrate, rollback) within the file can contain multiple SQL statements. Statements can span multiple lines, but all must be properly terminated using ;.

Perhaps I could improve the README for that to be clearer, but definitely not going to implement a full SQL parser to analyze all the migration files and report this.

As part of #14, I decided not to automatically consider the last statement if was missing a semicolon for the same reasons I shared in #15: implicit over explicit usage (I'm more biased over explicit usage than implicit or magic approach).

Some SQL linters can automatically detect and some can correct that for you.

Example using sqruff with the following configuration (to honor your 2 spaces usage):

[sqruff]
dialect = sqlite
rules = all

[sqruff:indentation]
indent_unit = space
tab_space_size = 2
indent_joins = true

[sqruff:rules:convention.terminator]
# require all statements to be terminated with a semicolon
require_final_semicolon = true

$ sqruff lint issue-17.sql
== [issue-17.sql] FAIL
L:   7 | P:  29 | LT01 | Expected single whitespace between "example" and ";".
                       | [layout.spacing]
L:   8 | P:   1 | CV06 | Statements must end with a semi-colon.
                       | [convention.terminator]
The linter processed 1 file(s).

Then adding the semi-colon to strict, passes:

$ sqruff lint issue-17.sql
The linter processed 1 file(s).
All Finished 📜 🎉

Cheers.

[luislavena]

(reopen as I misclicked the close button).

Thought that one option would be to raise an error if no statements are detected when processing the migration, but since the migration file contains both migrate and rollback sections, how we could determine is the valid count for a migration that is not reversible?

More things to think about it 😁

[BigBoyBarney]

Oh yeah the readme is very clear about this + I mean, it's basically just "don't write invalid SQL" haha. I was just wondering whether it was in the scope of Drift to do something like this, but an external tool is completely fine as well :D I'll check out sqruff!

Thank you for response!