gpdb
gpdb copied to clipboard
Resolve a FIXME in prepare_AlterTableStmt_for_dispatch()
Need rework (see my last comment). NOT ready to review yet.
GPDB has specific ALTER TABLE commands for partition tables, which are handled differently than the other AT commands: instead of being executed on QD first and then dispatched to QEs after all the other commands are done, they are dispatched immediately in ATExecCmd() and executed on QEs at the same time as QD. This results in the possibility that AT commands could be executed in different order in QEs than in QD.
The FIXME comment suggests to make those AT partition commands same as other AT commands, but that could be very invasive, e.g.:
- The AT partition commands generate a lot of subcommands that are not in the AT family (see ATExecGPPartCmds()). We cannot put them in the AT workqueue as AT subcommands. At minimal we would need separate code to dispatch them.
- Many of the subcommands implicitly dispatch to QEs (e.g. CreateStmt/DropStmt). For those, we need to have workarounds to to execute them w/o dispatching.
- Some subcommand needs partial results from other subcommands before executing (e.g. AT_PartSplit).
In this commit we opt to take a less invasive approach which is to group the GP-specific commands together and execute them before the others. Since the other commands' order is the same on QD and QEs, we will resolve the original issue.
Here are some reminders before you submit the pull request
- [ ] Add tests for the change
- [ ] Document changes
- [ ] Communicate in the mailing list if needed
- [ ] Pass
make installcheck
- [ ] Review a PR in return to support the community
This actually would change existing behavior of some AT commands. E.g.:
CREATE TABLE foo (a int) PARTITION BY range(a) (PARTITION foopar1 START(100) END(200));
ALTER TABLE foo ADD PARTITION foopar2 START(200) END(300), ALTER COLUMN a SET NOT NULL;
Originally after the above statements the second child wil have a NOT NULL column a
, because the ADD PARTITION happens after the ALTER COLUMN:
postgres=# \d foo_1_prt_foopar2
Table "public.foo_1_prt_foopar2"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
a | integer | | not null |
Now with this PR the column would not have NOT NULL, because (1) ADD PARTITION happens before ALTER COLUMN, and (2) ALTER COLUMN doesn't recurse into the child just created because the child didn't exist during the AT prep phase.
postgres=# \d foo_1_prt_foopar2
Table "public.foo_1_prt_foopar2"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
a | integer | | |
This was found in CI test where the pg_upgrade couldn't finish because it cannot recreate a child table that doesn't have same constraint as parent. Therefore, I think this PR would create a bug (apart from behavior change). We should find something else to tackle this problem.