feat: add UNLOGGED support for table#1405
Conversation
elucidsoft
changed the title
|
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... |
Shinigami92
left a comment
Shinigami92
left a comment
There was a problem hiding this comment.
Please add a test with combination of temporary = true and I think unloggedStr and temporaryStr need to be swapped (as far as I see in the psgl examples)
Let me check that, good catch. |
…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
Shinigami92
changed the title
|
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. |
2 participants
This resolves #1404