Don't treate IndexStmt like AlterTable when DefineIndex is called from ProcessUtilitySlow.
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t46951psql -h localhost -U postgresBuilt from patchset v1 (message #1), July 28, 2026 at 01:40 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t46951_1 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t46951_1 && git checkout t46951_1Patchset v1 (message #1) is on t46951_1
Hi,
Recently read the code, I find that when calling DefineIndex
from ProcessUtilitySlow, is_alter_table will be set true if
this statement is came from expandTableLikeClause.
I check the code of DefineIndex, there are only two places use
is_alter_table:
1. the function index_check_primary_key
2. print a debug log on what the statement is
For 1, since we are doing create table xxx (like yyy including
indexes), we are sure that the check relationHasPrimaryKey in the
function index_check_primary_key will be satisfied because we are just
create the new table.
For 2, I do not think it will mislead the user if we print it as
CreateStmt.
Based on the above, I think we can always a false value
for is_alter_table when DefineIndex is called from
ProcessUtilitySlow.
Here I attach a patch. Any ideas?
Thanks a lot.
Best,
Zhenghua Lyu
=?UTF-8?B?5q2j5Y2O5ZCV?= <kainwen@gmail.com> writes:
Recently read the code, I find that when calling DefineIndex
from ProcessUtilitySlow, is_alter_table will be set true if
this statement is came from expandTableLikeClause.
Yeah.
Based on the above, I think we can always a false value
for is_alter_table when DefineIndex is called from
ProcessUtilitySlow.
Why do you think this is an improvement? Even if it's correct,
the code savings is so negligible that I'm not sure I want to
expend brain cells on figuring out whether it's correct. The
comment you want to remove does not suggest that it's optional
which value we should pass, so I think the burden of proof
is to show that this patch is okay not that somebody else
has to demonstrate that it isn't.
regards, tom lane
Hi, thanks a lot for your reply!
Why do you think this is an improvement?
I hit the issue in Greenplum Database (Massively Parallel Postgres),
the MPP architecture is that coordinator dispatch statement to segments.
The dispatch logic is quit different for AlterTable and CreateTableLike:
* alter table: for each sub command, it will not dispatch; later it will
dispatch
the alter table statement as a whole.
* for create table like statement, like `create table t (like t1 including
indexes);`
this statement's 2nd stmt, has to dispatch to segments, but now it is
treated
as alter table, the dispatch logic is broken for this case in Greenplum.
I look into the issue and Greenplum Database wants to keep align with
Upstream
as possible. That is why I ask if we can force it to false.
Best,
Zhenghua Lyu
Tom Lane <tgl@sss.pgh.pa.us> 于2022年11月18日周五 06:26写道：
Show quoted text
=?UTF-8?B?5q2j5Y2O5ZCV?= <kainwen@gmail.com> writes:
Recently read the code, I find that when calling DefineIndex
from ProcessUtilitySlow, is_alter_table will be set true if
this statement is came from expandTableLikeClause.Yeah.
Based on the above, I think we can always a false value
for is_alter_table when DefineIndex is called from
ProcessUtilitySlow.Why do you think this is an improvement? Even if it's correct,
the code savings is so negligible that I'm not sure I want to
expend brain cells on figuring out whether it's correct. The
comment you want to remove does not suggest that it's optional
which value we should pass, so I think the burden of proof
is to show that this patch is okay not that somebody else
has to demonstrate that it isn't.regards, tom lane