Skip to content

Conversation

@drew-u410
Copy link

@drew-u410 drew-u410 commented Aug 21, 2025

  • In postgres: 17.6 it looks like a \restrict header was added to pg_dump output.
  • In using dbmate, it seems to be quite confused trying to parse these files with this \restrict header in them.
  • The TrimLeadingSQLComments looks like maybe the best place to update and also trim these out.
  • Confusingly, this change to pg_dump also ends a file with \unrestrict causing an issue parsing at the end too.

--

  • The mysql test failure seems unrelated?

@drew-u410 drew-u410 force-pushed the drew/fix_restrict_headers branch from 635dafc to 7cd56b2 Compare August 21, 2025 15:05
@drew-u410 drew-u410 force-pushed the drew/fix_restrict_headers branch from 7cd56b2 to ccc14b0 Compare August 21, 2025 15:11
@drew-u410 drew-u410 changed the title [pg] \restrict header fix [pg] \restrict header + footer fix Aug 21, 2025
@drew-u410 drew-u410 marked this pull request as ready for review August 21, 2025 15:22
@dossy
Copy link
Collaborator

dossy commented Aug 21, 2025

The MySQL test failure is due to this:

#673 (comment)

@dossy
Copy link
Collaborator

dossy commented Aug 21, 2025

Disappointed that they didn't implement this with \set restrict <key> and \unset restrict which would have been 100% backwards compatible.

I'm not sure I like the idea of filtering all possible (future) meta-commands from pg_dump output vs. only removing \restrict and \unrestrict` because who knows what else will break in the future.

I also don't like the idea of filtering them out at all: if you're using a version of Postgres 17.6+ and it emits these, you are responsible for loading these into a version of Postgres that understands them, or removing them yourself. I don't think dbmate should be interfering with the output of the dump tool the user is using.

I feel like the right move here is to make dbmate's SQL parser smarter enough to not choke on what Postgres deems valid input to its own tools, and not filter them out.

Maybe I'm not quite understanding the issue, so if anyone else has thoughts, please chime in.

@drew-u410
Copy link
Author

Yea- I truly wasn't sure. Feel free to discard / approach differently.

It just totally broke and I felt compelled to at least propose some solution for discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants