Skip to content
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

feat: add --pre-plan-file flag #168

Closed
wants to merge 4 commits into from

Conversation

aleclarson
Copy link

@aleclarson aleclarson commented Sep 2, 2024

Description

Add --pre-plan-file argument. It should point to a SQL file. Its statements are executed before the migration plan statements.

Motivation

This is particularly useful for settings (e.g. SET statements) that need to be applied before the migration plan. I'm especially interested in running SET check_function_bodies = off; as suggested in #129 (comment).

Testing

None yet.

Notes

You have my permission to take this PR and run with it! :)

@CLAassistant
Copy link

CLAassistant commented Sep 2, 2024

CLA assistant check
All committers have signed the CLA.

@aleclarson
Copy link
Author

aleclarson commented Sep 2, 2024

I've tested this locally and it's working as expected :)

edit: Eh, hold on a sec...

@bplunkett-stripe
Copy link
Collaborator

In order to get this to pass plan validation, you will need to wire it through the plan generator as well (into the library and not just the CLI). I can also try to pick this up later if you can wait a bit.

@aleclarson
Copy link
Author

aleclarson commented Sep 2, 2024

I have it running the pre-plan file on the temp DB now as well. For some reason, even after SET check_function_bodies = off; is executed, the function is still being checked (and therefore failing with type "foo" does not exist (SQLSTATE 42704) as it did before.

@aleclarson

This comment was marked as outdated.

@aleclarson

This comment was marked as outdated.

@aleclarson
Copy link
Author

I won't have time to finish this one

@aleclarson aleclarson closed this Sep 9, 2024
@bplunkett-stripe
Copy link
Collaborator

Just wanted to double check this isn't blocking you from using the tooling. In in the interim, skipping plan validation should work?

@aleclarson
Copy link
Author

@bplunkett-stripe Can't say for sure. I ended up taking a totally different approach that involves parsing the SQL myself, doing a topological sort, creating any objects that don't yet exist, and only then do I call pg-schema-diff (with plan validation disabled). This is working well for me, which means the pre-plan file is no longer needed --in my case--.

@bplunkett-stripe
Copy link
Collaborator

@bplunkett-stripe Can't say for sure. I ended up taking a totally different approach that involves parsing the SQL myself, doing a topological sort, creating any objects that don't yet exist, and only then do I call pg-schema-diff (with plan validation disabled). This is working well for me, which means the pre-plan file is no longer needed --in my case--.

Ahh okay. I'm sorry you had to create that kind of work around!

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.

3 participants