-
Notifications
You must be signed in to change notification settings - Fork 184
feat: add UNLOGGED support for table #1405
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
Conversation
Not sure about those failed tests? I don't think those are related to this commit. |
please run format, and also use esm syntax in the test migration file (orientate on other files) |
That's what I deserve for using AI lol...every time... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…ollow the docs more explicitly
So it did pass either way, but I changed the order so it so it matches the docs. |
I don't think a TEMPORARY UNLOGGED is allowed, although that's not super clear in their docs it would make sense. |
…eateTable function
…logged and temporary options
Do we want to add support for https://www.postgresql.org/docs/current/sql-altertable.html#SQL-ALTERTABLE-DESC-SET-LOGGED-UNLOGGED ? up to you if you want to do this in a separate PR |
I don't know, I would probably just use pgm.sql to do that type of thing tbh. |
Could you explain why? |
I don't know about that @ts-expect-error directive? I didn't add that or change it and now it's failing. Anyways this is my best effort at adding the SET UNLOGGED support for ALTER |
@elucidsoft please have a last check if everything is as you want after my changes and then I can merge |
Yes those changes make it more consistent. LGTM! |
Actually, it could be argued if UNLOGGED is false that it shouldn't add anything in the ALTER as it's LOGGED by default. Up to you how you want to handle that. Both ways work. |
It's required this way, as otherwise you won't be able to unset/revert an UNLOGGED. |
This resolves #1404