Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Errors during migration lack line numbers #102

Closed
AndreaCrotti opened this issue Nov 4, 2019 · 6 comments · Fixed by #495
Closed

Errors during migration lack line numbers #102

AndreaCrotti opened this issue Nov 4, 2019 · 6 comments · Fixed by #495
Labels

Comments

@AndreaCrotti
Copy link
Contributor

I just noticed that if I run a dbmate migrate and there is an error in one of the statements, the error being printed out is a bit useless, and doesn't even contain the line number or the actual message from Postgres.

I checked if there are any options for more verbosity but I don't see that either, so atm we simply don't get errors printed out?

@alex-tan
Copy link

@AndreaCrotti depends what the error is. If it's a syntax error in the migration file, pgsanity can help find that. As for others, I would be interested in seeing more details printed out as well. For example, if I try to drop a view that has dependent views/tables it doesn't print those out, whereas running it in psql will provide me with the list of dependent objects.

@AndreaCrotti
Copy link
Contributor Author

Yesterday we had a similar probelm, where dbmate was just failing on a given line without much information, but the actual error you could see from PSQL was a type mismatching, which made it immediately clear what the problem was, but no way to debug that just with the dbmate error.

I guess there should be a way also with the PG go bindings to get more verbose errors right?

@amacneil
Copy link
Owner

Unfortunately I haven't found a way to get more useful error messages or line numbers. The psql has some smarts here which I think are implemented client side, and don't exist in the pq go library we use.

I will keep this issue open because I agree it could be improved, and someone might have a clever idea to solve this.

@amacneil amacneil changed the title Lack of verbosity on errors during migrate Errors during migration lack line numbers Sep 27, 2020
@silbinarywolf
Copy link
Contributor

silbinarywolf commented Mar 7, 2021

I've put together a proof of concept over here:
#203

But before I continue on the effort I would like to plan things out a little first so that I don't waste time doing things that won't be accepted.

So my high-level plan would be:

  • add util function that takes the contents of an SQL file + position and returns a string with additional error reporting information.
  • avoid dependency on pkg/errors when wrapping an SQL error message with information, seems like an unnecessary dependency if it's not used elsewhere. Probably will just make a "DetailedSQLError" struct that holds an error, line number, column, position and can render that out in a way like I've done in the PR.
  • apply this utility to each SQL driver
  • add a test for the error cases, using the test "sample migration file" I've written in the PR description.
  • for the initial effort, maybe I won't bother printing out SQL near the error. So errors will be naive and just look like like this. (Unless you have opinions)
Error: line: 9, column: 35, position: 229: pq: syntax error at or near ")"

Extra effort idea if you're open to it:

  • Render out the line near the error position of the SQL for context and possibly do something like Elm error reporting wherein we show a ^ pointing to where the error is. I'm just not sure how good "position" is when it comes to different classes of SQL errors.
    image
    (Sourced from: https://elm-lang.org/news/compiler-errors-for-humans)
Error: line: 9, column: 35, position: 229: pq: syntax error at or near ")"

CREATE TABLE test_mistake (
    -- uh oh, comma here on the last field, error!
    test_mistake_id uuid NOT NULL,
                                 ^
);

@amacneil
Copy link
Owner

amacneil commented Mar 9, 2021

@silbinarywolf thanks! I will respond in the PR thread.

@bhanuc
Copy link

bhanuc commented Mar 10, 2021

Would appreciate if something of this sort is included as it improves the development experience and is a very elegant way to fix errors in an handwritten sql files.

Repository owner locked and limited conversation to collaborators Dec 19, 2022
@amacneil amacneil converted this issue into discussion #374 Dec 19, 2022

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →