Skip to content

Re-used index after ALTER COLUMN TYPE shouldn't change relfilenode #474

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.

Sign up for GitHub

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

Merged
my-ship-it merged 1 commit into from
Jun 20, 2024
Merged

Re-used index after ALTER COLUMN TYPE shouldn't change relfilenode #474

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions src/backend/commands/tablecmds.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14753,9 +14753,68 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd,
* If ATController() is called by internal utility command, not driven
* by QD, we still need to add items to the work queue.
* See details at ATController
*
* However, we need to check if we are to reuse the relfilenode of an
* existing index. This information is unique to QD and each QE. If so,
* we need to replace that with QE's own relfilenode. Note that this is
* not a problem for foreign key operator oids (see TryReuseForeignKey)
* since those are the same among QD/QEs.
*/
if (Gp_role == GP_ROLE_EXECUTE && context != NULL)
{
ListCell *lc;
Relation rel;
Relation irel = NULL;
AlteredTableInfo *tab;

/* Caller should already have acquired whatever lock we need. */
rel = relation_open(oldRelId, NoLock);
tab = ATGetQueueEntry(wqueue, rel);

/* Check the commands I got from QD for any re-used index. */
foreach (lc, tab->subcmds[AT_PASS_OLD_INDEX])
{
AlterTableCmd *cmd = castNode(AlterTableCmd, lfirst(lc));
IndexStmt *stmt;

Assert(cmd->subtype == AT_ReAddIndex);

stmt = (IndexStmt *) cmd->def;

/* if we are not reusing this index, continue */
if (!OidIsValid(stmt->oldNode))
continue;

if (irel == NULL)
{
Oid indoid = InvalidOid;
HeapTuple tup;

tup = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(oldId));
if (HeapTupleIsValid(tup))
indoid = oldId;
else
/* not an index, them it must be a constraint */
indoid = get_constraint_index(oldId);

if (HeapTupleIsValid(tup))
ReleaseSysCache(tup);

Assert(OidIsValid(indoid));

/* We might not open index on QE, so acquire a valid lock */
irel = index_open(indoid, AccessShareLock);
}

/* replace it with my own */
stmt->oldNode = irel->rd_node.relNode;
}

if (irel != NULL)
index_close(irel, AccessShareLock);
relation_close(rel, NoLock);
return;
}

/*
* We expect that we will get only ALTER TABLE and CREATE INDEX
Expand Down
85 changes: 85 additions & 0 deletions src/test/regress/expected/alter_table_gp.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,3 +279,88 @@ SELECT * FROM dropped_col_v;
----
(0 rows)

-- alter indexed column to the same type shouldn't change the index' relfilenode on QD and QEs.
-- helper utilities to check compare relfilenodes
drop table if exists relfilenodecheck;
create table relfilenodecheck(segid int, relname text, relfilenodebefore int, relfilenodeafter int, casename text);
prepare capturerelfilenodebefore as
insert into relfilenodecheck select -1 segid, relname, pg_relation_filenode(relname::text) as relfilenode, null::int, $1 as casename from pg_class where relname like $2
union select gp_segment_id segid, relname, pg_relation_filenode(relname::text) as relfilenode, null::int, $1 as casename from gp_dist_random('pg_class')
where relname like $2 order by segid;
prepare checkrelfilenodediff as
select a.segid, b.casename, b.relname, (relfilenodebefore != a.relfilenode) rewritten
from
(
select -1 segid, relname, pg_relation_filenode(relname::text) as relfilenode
from pg_class
where relname like $2
union
select gp_segment_id segid, relname, pg_relation_filenode(relname::text) as relfilenode
from gp_dist_random('pg_class')
where relname like $2 order by segid
)a, relfilenodecheck b
where b.casename like $1 and b.relname like $2 and a.segid = b.segid;
create table attype_indexed(a int, b int);
create index attype_indexed_i on attype_indexed(b);
insert into attype_indexed select i,i from generate_series(1, 100)i;
-- alter to same type.
-- check relfilenode before AT
execute capturerelfilenodebefore('alter column type same', 'attype_indexed_i');
alter table attype_indexed alter column b type int;
-- relfilenode stay same as before
execute checkrelfilenodediff('alter column type same', 'attype_indexed_i');
segid | casename | relname | rewritten
-------+------------------------+------------------+-----------
0 | alter column type same | attype_indexed_i | f
1 | alter column type same | attype_indexed_i | f
2 | alter column type same | attype_indexed_i | f
-1 | alter column type same | attype_indexed_i | f
(4 rows)

-- insert works fine
insert into attype_indexed select i,i from generate_series(1, 100)i;
select count(*) from attype_indexed;
count
-------
200
(1 row)

-- alter to different type, relfilenode should change
execute capturerelfilenodebefore('alter column diff type', 'attype_indexed_i');
alter table attype_indexed alter column b type text;
execute checkrelfilenodediff('alter column diff type', 'attype_indexed_i');
segid | casename | relname | rewritten
-------+------------------------+------------------+-----------
0 | alter column diff type | attype_indexed_i | t
1 | alter column diff type | attype_indexed_i | t
2 | alter column diff type | attype_indexed_i | t
-1 | alter column diff type | attype_indexed_i | t
(4 rows)

--insert works fine
insert into attype_indexed select i, 'abc'::text from generate_series(1, 100) i;
select count(*) from attype_indexed;
count
-------
300
(1 row)

-- alter column with exclusion constraint
create table attype_indexed_constr(
c circle,
dkey inet,
exclude using gist (dkey inet_ops with =, c with &&)
);
-- not change
execute capturerelfilenodebefore('alter column diff type', 'attype_indexed_constr_dkey_c_excl');
alter table attype_indexed_constr alter column c type circle;
execute checkrelfilenodediff('alter column diff type', 'attype_indexed_constr_dkey_c_excl');
segid | casename | relname | rewritten
-------+------------------------+-----------------------------------+-----------
0 | alter column diff type | attype_indexed_constr_dkey_c_excl | f
1 | alter column diff type | attype_indexed_constr_dkey_c_excl | f
2 | alter column diff type | attype_indexed_constr_dkey_c_excl | f
-1 | alter column diff type | attype_indexed_constr_dkey_c_excl | f
(4 rows)

drop table relfilenodecheck;
65 changes: 65 additions & 0 deletions src/test/regress/sql/alter_table_gp.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,3 +238,68 @@ CREATE TABLE dropped_col_t2(i1 int, i2 int);
CREATE VIEW dropped_col_v AS SELECT dropped_col_t1.i1 FROM dropped_col_t1 JOIN dropped_col_t2 ON dropped_col_t1.i1=dropped_col_t2.i1;
ALTER TABLE dropped_col_t1 DROP COLUMN i2;
SELECT * FROM dropped_col_v;

-- alter indexed column to the same type shouldn't change the index' relfilenode on QD and QEs.

-- helper utilities to check compare relfilenodes
drop table if exists relfilenodecheck;
create table relfilenodecheck(segid int, relname text, relfilenodebefore int, relfilenodeafter int, casename text);

prepare capturerelfilenodebefore as
insert into relfilenodecheck select -1 segid, relname, pg_relation_filenode(relname::text) as relfilenode, null::int, $1 as casename from pg_class where relname like $2
union select gp_segment_id segid, relname, pg_relation_filenode(relname::text) as relfilenode, null::int, $1 as casename from gp_dist_random('pg_class')
where relname like $2 order by segid;

prepare checkrelfilenodediff as
select a.segid, b.casename, b.relname, (relfilenodebefore != a.relfilenode) rewritten
from
(
select -1 segid, relname, pg_relation_filenode(relname::text) as relfilenode
from pg_class
where relname like $2
union
select gp_segment_id segid, relname, pg_relation_filenode(relname::text) as relfilenode
from gp_dist_random('pg_class')
where relname like $2 order by segid
)a, relfilenodecheck b
where b.casename like $1 and b.relname like $2 and a.segid = b.segid;

create table attype_indexed(a int, b int);
create index attype_indexed_i on attype_indexed(b);

insert into attype_indexed select i,i from generate_series(1, 100)i;

-- alter to same type.
-- check relfilenode before AT
execute capturerelfilenodebefore('alter column type same', 'attype_indexed_i');
alter table attype_indexed alter column b type int;
-- relfilenode stay same as before
execute checkrelfilenodediff('alter column type same', 'attype_indexed_i');

-- insert works fine
insert into attype_indexed select i,i from generate_series(1, 100)i;
select count(*) from attype_indexed;

-- alter to different type, relfilenode should change
execute capturerelfilenodebefore('alter column diff type', 'attype_indexed_i');
alter table attype_indexed alter column b type text;
execute checkrelfilenodediff('alter column diff type', 'attype_indexed_i');

--insert works fine
insert into attype_indexed select i, 'abc'::text from generate_series(1, 100) i;
select count(*) from attype_indexed;

-- alter column with exclusion constraint
create table attype_indexed_constr(
c circle,
dkey inet,
exclude using gist (dkey inet_ops with =, c with &&)
);

-- not change
execute capturerelfilenodebefore('alter column diff type', 'attype_indexed_constr_dkey_c_excl');
alter table attype_indexed_constr alter column c type circle;
execute checkrelfilenodediff('alter column diff type', 'attype_indexed_constr_dkey_c_excl');

drop table relfilenodecheck;