Cannot migrate stored procedure

[njgage]

I'm submitting a...

• Bug report
• Feature request
• Question

Current behavior

When created a stored procedure it's necessary to change the delimiter
e.g. DELIMITER $$

However, when using a SQL-file it returns ER_PARSE_ERROR:

Expected behavior

Be able to change the delimiter, create the SP and return delimiter back to standard ;

Minimal reproduction of the problem with instructions

DELIMITER //
CREATE PROCEDURE testProc()
BEGIN
SELECT 1;
END //
DELIMITER ;

What is the motivation / use case for changing the behavior?

Cannot create or migrate stored procedures

Environment

db-migrate version: 0.11.5
db-migrate-mysql with versions: 1.1.10

Additional information:
- Node version: 8.12.0  
- Platform:  Mac

Others:

---

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

[wzrdtales]

I have to push that through, to the upstream driver https://github.com/mysqljs/mysql. Can you try and verify it is possible to store a stored procedure with the raw driver?

[wzrdtales]

Quick issue dig gives this mysqljs/mysql#1683 . So upstream issue.

[wzrdtales]

Respectively, read this comment mysqljs/mysql#1683 (comment)

[wzrdtales]

So setting multipleStatements to false will allow you to do this. DELIMITER needs to removed completely since it is not SQL either.

What will be possible in some future (don't know yet when I will do it). There is a concurrency manager planned. For this I will need to spawn multiple connections and instances of the driver. On that way down I could give that freedom down to the migration, to spawn a different connection with different settings. So you could selectively disable multipleStatements. However, it would be clearly out of scope to implement a full fledged sql parsers, for a shit ton of different SQL dialects, so this problem is unfortunately unique to the MySQL driver. Not sure actually how for example the pg driver behaves on that.

[tabroughton]

hi @wzrdtales I was also testing out db-migrate and would have liked to be able to create store procedures, triggers etc all of which require adding a delimiter into the sql statement (create procedure etc). I also need multipleStatements . I understand the comments upstream that mysqljs/mysql is an implementation of the mysql spec and DELIMITER is a feature of the CLI. I suppose it won't become a feature upstream. However, I think that like the CLI implementation other interfaces could implement something and in that thread I see somebody has created what looks to be quite thorough implementation of the capability mysqljs/mysql#1683 (comment)

I was wondering if this (approach) might be a good feature to incorporate into db-migrate

[tabroughton]

After a few tests I don't think this is an issue at all as I have been able to create stored procedures with multipleStatements true in my connection config. I am using sql files and a lot of them have multiple statements, I only need separate sql files for the create procedure statements and for those files to contain a single statement but mixed in this is fine for me.

[tabroughton]

after further tests I can see that this isn't actually an issue for me at all. I have my connection config set to multipleStatements true and can indeed run multiple statements with stored procedures in the same sql file such as this one:

CREATE TABLE `foo` (`id` INT(11) NOT NULL AUTO_INCREMENT, `bar` VARCHAR(5), PRIMARY KEY (`id`));

CREATE PROCEDURE `insert_foo` (IN `@baz` VARCHAR(5))
BEGIN
  INSERT INTO `foo`(`bar`) VALUES(`@baz`);
END;

no need for a delimiter change at all

[munawarb]

@tabroughton Thanks for checking into this and experimenting with it. I had the same issue then came across your comment saying to remove the delimiter settings. With multiple statements set to true, this works when adding stored functions and procedures.

[lasergoat]

using mysql
I can confirm, with multipleStatements set to true in the database.json and no delimiters, I was able to have a migration like:

  await db.runSql(`
    CREATE PROCEDURE reprocess_merchant(IN merchant_id VARCHAR(255))
    BEGIN
      DECLARE exit handler for SQLEXCEPTION
      BEGIN
      GET DIAGNOSTICS CONDITION 1 @sqlstate = RETURNED_SQLSTATE, 
      @errno = MYSQL_ERRNO, @text = MESSAGE_TEXT;
      SET @full_error = CONCAT("ERROR ", @errno, " (", @sqlstate, "): ", @text);
      SELECT @full_error, @mid, @mid_count, @transaction_count;
      END;

      # gather numeric MID
      SELECT m.mid INTO @mid FROM merchants m
      WHERE m.id = merchant_id
      AND m.mid REGEXP '^[0-9]+$';

      -- rest of proc here

      END
  `);

[PaulVipond]

But what you can't do is:

await db.runSql(`
CREATE OR REPLACE PROCEDURE admin_report_txn_volume_last_30_days() AS
BEGIN
    echo select IF(date(created_at), count(*), 0) as txn_volume, ds.series_date as date from date_series as ds left join merchant_txn as txn on ds.series_date = date(txn.created_at) where ds.series_date >= DATE_ADD(CURDATE(), INTERVAL -30 DAY) and ds.series_date <= CURDATE() group by ds.series_date order by ds.series_date;
END ;

CREATE OR REPLACE PROCEDURE admin_report_txn_amount_last_30_days() AS
BEGIN
    echo select IF(date(created_at), sum(fiat_amount), 0) as fiat_amount, ds.series_date as date from date_series as ds left join merchant_txn as txn on ds.series_date = date(txn.created_at) where ds.series_date >= DATE_ADD(CURDATE(), INTERVAL -30 DAY) and ds.series_date <= CURDATE() group by ds.series_date order by ds.series_date;
END ;
`);

I.e. multiple procs within the same text string.

[wzrdtales]

well you shouldn't a single run sql should be a single operation

[PaulVipond]

I respect your opinion, but disagree :-) For me, migrations are there to support features that are being added to the database. If the feature requires 2, 3 or 4 stored procedures, then I would like to include them in the same migration. I understand we can't do that currently, though we worked round this by changing the generated javascript file to parse the 'up' sql file and create the stored procs in blocks.