weavejester / ragtime

Database-independent migration library
Eclipse Public License 1.0
610 stars 85 forks source link

Consider changing the SQL separator to support MySQL users for the better #159

Closed dharrigan closed 11 months ago

dharrigan commented 11 months ago

Hi,

Presently, ragtime looks for --;; within a sql file to split up statements (as defined here and here.)

However, for MySQL, the comment --;; is invalid and flags an error in the SQL when attempting to execute a perfectly valid SQL file, as MySQL requires that a -- is followed either by a space or a control character. This is detailed in the MySQL reference manual here. This does not apply to other databases, such as PostgreSQL or Oracle which both treat a comment as a line starting with -- all the way until the end of the line.

Thus, when loading a SQL file into tools such as DataGrip, the file is littered with errors as the parser (following MySQL parsing rules) correctly flags --;; as being in error.

Given that MySQL is (still) hugely popular, I would hazzard a guess that others also encounter this issue when using Ragtime for migrations on MySQL.

I propose that the check be made, either for --;; or -- ;;, therefore changing the regex from this:

(defn- read-sql [file]
  (str/split (slurp file) #"(?m)\n\s*--;;\s*\n"))

to this:

(defn- read-sql [file]
  (str/split (slurp file) #"(?m)\n\s*--\s?;;\s*\n"))

This would ensure continued behaviour for existing users (and those not using MySQL), but permit those using MySQL to use -- ;; (with a single space) as the separator (and therefore keep the MySQL parser happy...)

I would be happy to submit a PR if this is consideration is accepted.

Thank you.

-=david=-

weavejester commented 11 months ago

This seems a very sensible change, and I'd welcome a PR.

dharrigan commented 11 months ago

PR ready :-)