Renaming tables breaks if you pass a Name with a schema

[osdiab]

Describe the bug

If you do something like this:

pgm.renameTable({schema: "a", table: "foo"}, {schema: "a", table: "bar"})

It will fail because the schema is included in the new table name, which is not valid for this type of statement.

(If you try to change the schema at the same time, it will also fail for the same reason)

Then if you change it to

pgm.renameTable({schema: "a", table: "foo"}, "bar")

Then the reverse migration will fail both because the target table name has a schema, and also because the schema isn't provided in the source table name.

Steps to reproduce

No response

Logs

No response

System Info

System:
    OS: macOS 14.5
    CPU: (10) arm64 Apple M1 Max
    Memory: 14.56 GB / 64.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 18.18.2 - ~/.asdf/installs/nodejs/18.18.2/bin/node
    npm: 9.8.1 - ~/.asdf/plugins/nodejs/shims/npm
    pnpm: 8.6.2 - ~/.asdf/installs/nodejs/18.18.2/bin/pnpm
    Watchman: 2024.03.25.00 - /opt/homebrew/bin/watchman

Used Module System

cjs

[Shinigami92]

I think we could do something similar to here:

node-pg-migrate/src/operations/tables/addConstraint.ts

Lines 50 to 52 in f98e00a

	throw new Error(
	'Impossible to automatically infer down migration for addConstraint without naming constraint'
	);

What do you think?

Not sure if it is fully automatically possible to move a table to another schema, and even if, I would suggest to do it manually with another operation.
FFR: https://stackoverflow.com/a/17770152/6897682

[osdiab]

Yeah, I don't think this needs to be robust to changing schemas, in fact I think it makes more sense to actually change the type signature to not include schema in the destination name since Postgres wouldn't do that either. So basically:

function renameTable(from: Name, to: string) {
 // ...
}

// this works today:
// up: renames public.foo to public.bar
// down: renames public.bar to public.foo
renameTable("foo", "bar");

// this does not work today:
// up: renames a.foo to a.bar; this works today
// down: renames a.bar to a.foo (read the schema from the `from` parameter)
//   this fails today, since we get ALTER TABLE "bar" RENAME TO a.foo
//   problems: "bar" lacks the schema, and a.foo shouldn't have the schema in it.
renameTable({schema: "a", table: "foo"}, "bar")

If we wanted to keep backwards compatible in the type signature and still allow any Name to be provided, then

1. if from.schema !== to.schema then throw an error
2. if it does equal, then throw away the schema for to and treat it the same way as the above example.

That said any code that does this would fail today, so I doubt that the breaking change would actually break any real migrations.

If you want to call SET SCHEMA, which ALTER TABLE does support, I have no issues with that being a separate command or just part of alterTable.