When pg-minify fails, it's hard to locate the issue

[benjie]

Summary

When pg-minify fails, Graphile Migrate outputs pg-minify's error, but doesn't relate it to the file that actually caused the problem. This is particularly problematic if you have a large (e.g. initial) migration and you're using the current/ folder - you don't know which file the error occurred in.

Steps to reproduce

current/12000-insert_iso3166_countries.sql:

COPY countries (name, alpha_2, code) FROM stdin DELIMITER '|';
Kyrgyzstan|KG|417
Lao People's Democratic Republic|LA|418
Latvia|LV|428
\.

Expected results

Error on line 3 of 12000-insert_iso3166_countries.sql (we don't support COPY syntax).

Actual results

🛑 Error occurred whilst processing migration
    SQLParsingError: Error parsing SQL at {line:24153,col:11}: Unclosed text block.

Additional context

graphile-migrate 1.0.1; linux; current/ folder.

Possible Solution

The following mod to dist/commands/watch.js gives more information:

                let currentBodyMinified;
                try {
                  currentBodyMinified = pgMinify(currentBodyFromDryRun);
                } catch (e) {
                  const matches = e.message.match(/Error parsing SQL at \{line:(\d+),col:(\d+)\}/);
                  if (matches) {
                    const [_, ln, col] = matches;
                    const lines = currentBodyFromDryRun.split("\n");
                    const leadingLines = lines.slice(0, +ln);
                    const lastSplit = [...leadingLines].reverse().find(l => l.match(/^--! split/));
                    let filename = null;
                    let offset = 0;
                    if (lastSplit) {
                      filename = lastSplit.split(':')[1].trim();
                      const index = leadingLines.indexOf(lastSplit);
                      offset = index + 1;
                    }

                    const previousLine = lines[+ln - 2];
                    const line = lines[+ln - 1];
                    const nextLine = lines[+ln];
                    const details = filename ? `current/${filename} line ${+ln - offset}-ish` : `current.sql line ${ln}-ish`
                    console.error(`Error happened around here ${details}:`);
                    console.error();
                    console.error(previousLine || "");
                    console.error(line || "");
                    console.error("-".repeat(+col - 1) + "^");
                    console.error(nextLine || "");
                  }
                  throw e;
                }

produces:

Error happened around here current/12000-insert_iso3166_countries.sql line 3-ish:

Kyrgyzstan|KG|417
Lao People's Democratic Republic|LA|418
----------^
Latvia|LV|428

🛑 Error occurred whilst processing migration
    SQLParsingError: Error parsing SQL at {line:24153,col:11}: Unclosed text block.

[vitaly-t]

When pg-minify fails, it throws an extended error object of type SQLParsingError:

https://github.com/vitaly-t/pg-minify/blob/0ad663d4e66ad1d325a658be61eccf80eb5df91c/typescript/pg-minify.d.ts#L24

which contains code and position, so there is no need parsing the error text.

[benjie]

Thanks @vitaly-t; that knowledge will make addressing this much easier! ❤️

[amerryma]

Slightly related, any reason we don't support the COPY command?

[vitaly-t]

Slightly related, any reason we don't support the COPY command?

What do you mean? We do not have any such restriction.

[amerryma]

I was just reading this: Error on line 3 of 12000-insert_iso3166_countries.sql (we don't support COPY syntax). at the top.

But I guess I was using the psql method \copy which is different. I was hoping to use a .sql script with that syntax in it because I had a large external file I wanted to ingest and transform with gm run.

[benjie]

We don't currently support a way to load data files through migrations, and I don't think we support the COPY ... FROM STDIN syntax either. You could use --!include to include another SQL file if you can transform your data into SQL?

[amerryma]

Could I use .gmrc to run psql after it migrates? My process really just takes a massive jsonl file and parses it into upserts (for static data that all my databases need).

[benjie]

You could use afterAllMigrations to run a script, and have the script check to see if the data is needed, and if it is then it could run psql. afterAllMigrations will run after each time graphile-migrate migrate is called, so you can't key it to a specific migration. You could also add a graphile-worker task to your migration and then have worker load the data later.