This is an automated email from the ASF dual-hosted git repository. reshke pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/cloudberry.git
commit b8f3cdea6d47047d65e3a80d5fe54f4eaf5e6e60 Author: Huansong Fu <[email protected]> AuthorDate: Mon Aug 15 11:31:58 2022 -0700 Remove existing reloptions when AM is changed Previously, we only remove the existing reloptions when the AM is changing between Heap and AO/AOCO, since the reloptions are mutually exclusive between them. Now we do the same when changing AM between AO and AOCO too, for the following reasons: 1. Keep behaviors consistent. 2. Even between AO and AOCO there's certain reloption that's not applicable to one of them: e.g. 'compresslevel=rle_type' only applies to AOCO but not AO. 3. If customer specifies reloptions themselves within the ATSETAM statement, we then need to pick and remove those conflicting with the new reloptions. Too much complicities will be introduced. With this change, we also refactored ATExecSetRelOptions a little bit since now we don't rely on it to remove reloptions. And it becomes more readable too. --- src/backend/catalog/pg_appendonly.c | 1 + src/backend/commands/tablecmds.c | 53 ++++++-- src/test/regress/expected/alter_table_set_am.out | 10 +- src/test/regress/sql/alter_table_set_am.sql | 161 +++++++++++++++++++++++ 4 files changed, 212 insertions(+), 13 deletions(-) diff --git a/src/backend/catalog/pg_appendonly.c b/src/backend/catalog/pg_appendonly.c index c1c364cdbd..8ec58f74f3 100644 --- a/src/backend/catalog/pg_appendonly.c +++ b/src/backend/catalog/pg_appendonly.c @@ -21,6 +21,7 @@ #include "catalog/pg_am_d.h" #include "catalog/pg_appendonly.h" +#include "catalog/pg_attribute_encoding.h" #include "catalog/pg_type.h" #include "catalog/pg_proc.h" #include "catalog/gp_fastsequence.h" diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 6d196fb21a..704ee492d7 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -585,6 +585,7 @@ static bool prebuild_temp_table(Relation rel, RangeVar *tmpname, DistributedBy * static void checkATSetDistributedByStandalone(AlteredTableInfo *tab, Relation rel); static void populate_rel_col_encodings(Relation rel, List *stenc, List *withOptions); +static void remove_rel_opts(Relation rel); /* ---------------------------------------------------------------- @@ -6200,7 +6201,6 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, break; case AT_SetAccessMethod: /* SET ACCESS METHOD */ /* Set reloptions if specified any. Otherwise handled specially in Phase 3. */ - if (cmd->def) { bool aoopt_changed = false; @@ -16126,7 +16126,14 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation, if (defList == NIL && operation != AT_ReplaceRelOptions) return; /* nothing to do */ - newAM = OidIsValid(newAccessMethod) ? GetTableAmRoutineByAmId(newAccessMethod) : NULL; + + /* If we are changing access method, simply remove all the existing ones. */ + if (OidIsValid(newAccessMethod)) + { + newAM = GetTableAmRoutineByAmId(newAccessMethod); + remove_rel_opts(rel); + } else + newAM = NULL; pgclass = table_open(RelationRelationId, RowExclusiveLock); @@ -16136,14 +16143,11 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation, if (!HeapTupleIsValid(tuple)) elog(ERROR, "cache lookup failed for relation %u", relid); - if (operation == AT_ReplaceRelOptions || - (newAccessMethod != InvalidOid && IsAccessMethodAO(rel->rd_rel->relam) != IsAccessMethodAO(newAccessMethod))) + if (operation == AT_ReplaceRelOptions) { /* - * If we're supposed to replace the reloptions list, or if we're - * changing AM between heap and AO/CO so the old reloptions won't - * apply to the new table anymore, we just pretend there were - * none before. + * If we're supposed to replace the reloptions list, we just + * pretend there were none before. */ datum = (Datum) 0; isnull = true; @@ -17998,6 +18002,39 @@ get_rel_opts(Relation rel) return newOptions; } +/* + * GPDB: Convenience function to remove the pg_class.reloptions field for a given relation. + */ +static void +remove_rel_opts(Relation rel) +{ + Datum val[Natts_pg_class] = {0}; + bool null[Natts_pg_class] = {0}; + bool repl[Natts_pg_class] = {0}; + Relation classrel; + HeapTuple tup; + + classrel = table_open(RelationRelationId, RowExclusiveLock); + + tup = SearchSysCacheCopy1(RELOID, RelationGetRelid(rel)); + if (!HeapTupleIsValid(tup)) + elog(ERROR, "cache lookup failed for relation %u", RelationGetRelid(rel)); + + val[Anum_pg_class_reloptions - 1] = (Datum) 0; + null[Anum_pg_class_reloptions - 1] = true; + repl[Anum_pg_class_reloptions - 1] = true; + + tup = heap_modify_tuple(tup, RelationGetDescr(classrel), + val, null, repl); + CatalogTupleUpdate(classrel, &tup->t_self, tup); + + heap_freetuple(tup); + + table_close(classrel, RowExclusiveLock); + + CommandCounterIncrement(); +} + static RangeVar * make_temp_table_name(Relation rel, BackendId id) { diff --git a/src/test/regress/expected/alter_table_set_am.out b/src/test/regress/expected/alter_table_set_am.out index 58e0f934d4..701496a0f3 100644 --- a/src/test/regress/expected/alter_table_set_am.out +++ b/src/test/regress/expected/alter_table_set_am.out @@ -714,7 +714,7 @@ SELECT count(*) FROM gp_toolkit.__gp_aocsseg('ao2co3'); SELECT * FROM gp_toolkit.__gp_aoblkdir('ao2co3'); tupleid | segno | columngroup_no | entry_no | first_row_no | file_offset | row_count ----------+-------+----------------+----------+--------------+-------------+----------- + (0 rows) -- pg_attribute_encoding should have columns for the AOCO table @@ -735,8 +735,8 @@ SELECT c.relname, a.attnum, a.attoptions FROM pg_attribute_encoding a, pg_class SELECT c.relname, a.amname, c.reloptions FROM pg_class c JOIN pg_am a ON c.relam = a.oid WHERE c.relname LIKE 'ao2co%'; relname | amname | reloptions ---------+-----------+----------------------------------- - ao2co | ao_column | {blocksize=65536,compresslevel=5} - ao2co2 | ao_column | {blocksize=65536,compresslevel=5} + ao2co | ao_column | + ao2co2 | ao_column | ao2co3 | ao_column | {blocksize=32768,compresslevel=3} ao2co4 | ao_column | {blocksize=32768,compresslevel=3} (4 rows) @@ -746,8 +746,8 @@ SELECT c.relname,a.blocksize,a.compresslevel,a.checksum,a.compresstype,a.columns FROM pg_appendonly a, pg_class c WHERE a.relid = c.oid AND relname like ('ao2co%'); relname | blocksize | compresslevel | checksum | compresstype | columnstore ---------+-----------+---------------+----------+--------------+------------- - ao2co | 65536 | 5 | t | zlib | t - ao2co2 | 65536 | 5 | t | zlib | t + ao2co | 32768 | 0 | t | | t + ao2co2 | 32768 | 0 | t | | t ao2co3 | 32768 | 3 | t | zlib | t ao2co4 | 32768 | 3 | t | zlib | t (4 rows) diff --git a/src/test/regress/sql/alter_table_set_am.sql b/src/test/regress/sql/alter_table_set_am.sql index d2d65f65d3..11a0b90d1c 100644 --- a/src/test/regress/sql/alter_table_set_am.sql +++ b/src/test/regress/sql/alter_table_set_am.sql @@ -417,6 +417,167 @@ DROP TABLE ao2co2; DROP TABLE ao2co3; DROP TABLE ao2co4; +-- Scenario 6: AOCO to Heap +SET gp_default_storage_options = 'blocksize=65536, compresstype=zlib, compresslevel=5, checksum=true'; + +CREATE TABLE co2heap(a int, b int) WITH (appendonly=true, orientation=column); +CREATE TABLE co2heap2(a int, b int) WITH (appendonly=true, orientation=column); +CREATE TABLE co2heap3(a int, b int) WITH (appendonly=true, orientation=column); +CREATE TABLE co2heap4(a int, b int) WITH (appendonly=true, orientation=column); +CREATE INDEX aoi ON co2heap(b); + +INSERT INTO co2heap SELECT i,i FROM generate_series(1,5) i; +INSERT INTO co2heap2 SELECT i,i FROM generate_series(1,5) i; +INSERT INTO co2heap3 SELECT i,i FROM generate_series(1,5) i; +INSERT INTO co2heap4 SELECT i,i FROM generate_series(1,5) i; + +-- Prior-ATSETAM checks: +-- Check once that the AO tables have the custom reloptions +SELECT relname, reloptions FROM pg_class WHERE relname LIKE 'co2heap%'; +-- Check once that the AO tables have relfrozenxid = 0 +SELECT relname, relfrozenxid FROM pg_class WHERE relname LIKE 'co2heap%'; +-- Check once that the pg_attribute_encoding has entries for the AOCO tables. +SELECT c.relname, a.attnum, attoptions FROM pg_attribute_encoding a, pg_class c WHERE a.attrelid=c.oid AND c.relname LIKE 'co2heap%'; + +CREATE TEMP TABLE relfilebeforeco2heap AS + SELECT -1 segid, relfilenode FROM pg_class WHERE relname LIKE 'co2heap%' + UNION SELECT gp_segment_id segid, relfilenode FROM gp_dist_random('pg_class') + WHERE relname LIKE 'co2heap%' ORDER BY segid; + +-- Various cases of altering AOCO to AO: +-- 1. Basic ATSETAMs: +ALTER TABLE co2heap SET ACCESS METHOD heap; +ALTER TABLE co2heap2 SET WITH (appendoptimized=false); +-- 2. ATSETAM with reloptions: +ALTER TABLE co2heap3 SET ACCESS METHOD heap WITH (fillfactor=70); +ALTER TABLE co2heap4 SET WITH (appendoptimized=false, fillfactor=70); + +-- The tables and indexes should have been rewritten (should have different relfilenodes) +CREATE TEMP TABLE relfileafterco2heap AS + SELECT -1 segid, relfilenode FROM pg_class WHERE relname LIKE 'co2heap%' + UNION SELECT gp_segment_id segid, relfilenode FROM gp_dist_random('pg_class') + WHERE relname LIKE 'co2heap%' ORDER BY segid; + +SELECT * FROM relfilebeforeco2heap INTERSECT SELECT * FROM relfileafterco2heap; + +-- Check data is intact +SELECT count(*) FROM co2heap; +SELECT count(*) FROM co2heap2; +SELECT count(*) FROM co2heap3; +SELECT count(*) FROM co2heap4; + +-- No AO aux tables should be left. +-- Only testing 2 out of the 4 tables being created, where the tables were altered w/wo reloptions. +-- No need to test the other ones created by the alternative syntax SET WITH(). +SELECT * FROM gp_toolkit.__gp_aoseg('co2heap'); +SELECT * FROM gp_toolkit.__gp_aovisimap('co2heap'); +SELECT count(*) FROM gp_toolkit.__gp_aocsseg('co2heap'); +SELECT * FROM gp_toolkit.__gp_aoblkdir('co2heap'); +SELECT * FROM gp_toolkit.__gp_aoseg('co2heap3'); +SELECT * FROM gp_toolkit.__gp_aovisimap('co2heap3'); +SELECT count(*) FROM gp_toolkit.__gp_aocsseg('co2heap3'); +SELECT * FROM gp_toolkit.__gp_aoblkdir('co2heap3'); + +-- No pg_appendonly entries should be left too +SELECT c.relname FROM pg_class c, pg_appendonly p WHERE c.relname LIKE 'co2heap%' AND c.oid = p.relid; + +-- The altered tables should have heap AM. +SELECT c.relname, a.amname FROM pg_class c JOIN pg_am a ON c.relam = a.oid WHERE c.relname LIKE 'co2heap%'; + +-- The new heap tables shouldn't have the old AO table's reloptions +SELECT relname, reloptions FROM pg_class WHERE relname LIKE 'co2heap%'; + +-- The new heap tables should have a valid relfrozenxid +SELECT relname, relfrozenxid <> '0' FROM pg_class WHERE relname LIKE 'co2heap%'; + +-- The pg_attribute_encoding entries for the altered tables should have all gone. +SELECT c.relname, a.attnum, attoptions FROM pg_attribute_encoding a, pg_class c WHERE a.attrelid=c.oid AND c.relname LIKE 'co2heap%'; + +DROP TABLE co2heap; +DROP TABLE co2heap2; +DROP TABLE co2heap3; +DROP TABLE co2heap4; + +-- Scenario 7: AOCO to AO +CREATE TABLE co2ao(a int, b int) WITH (appendonly=true, orientation=column, compresstype=rle_type, compresslevel=3); +CREATE TABLE co2ao2(a int, b int) WITH (appendonly=true, orientation=column, compresstype=rle_type, compresslevel=3); +CREATE TABLE co2ao3(a int, b int) WITH (appendonly=true, orientation=column, compresstype=rle_type, compresslevel=3); +CREATE TABLE co2ao4(a int, b int) WITH (appendonly=true, orientation=column, compresstype=rle_type, compresslevel=3); +CREATE INDEX aoi ON co2ao(b); +CREATE INDEX aoi2 ON co2ao3(b); + +INSERT INTO co2ao SELECT i,i FROM generate_series(1,5) i; +INSERT INTO co2ao2 SELECT i,i FROM generate_series(1,5) i; +INSERT INTO co2ao3 SELECT i,i FROM generate_series(1,5) i; +INSERT INTO co2ao4 SELECT i,i FROM generate_series(1,5) i; + +-- Prior-ATSETAM checks: +-- Check once that the AOCO tables have the custom reloptions +SELECT relname, reloptions FROM pg_class WHERE relname LIKE 'co2ao%'; +-- Check once that pg_appendonly has expected entries. +SELECT c.relname, p.compresstype, p.compresslevel, p.blocksize FROM pg_class c, pg_appendonly p WHERE c.relname LIKE 'co2ao%' AND c.oid = p.relid; +-- Check once that the pg_attribute_encoding has entries for the AOCO tables. +SELECT c.relname, a.* FROM pg_attribute_encoding a, pg_class c WHERE a.attrelid=c.oid AND c.relname LIKE 'co2ao%'; + +CREATE TEMP TABLE relfilebeforeco2ao AS + SELECT -1 segid, relfilenode FROM pg_class WHERE relname LIKE 'co2ao%' + UNION SELECT gp_segment_id segid, relfilenode FROM gp_dist_random('pg_class') + WHERE relname LIKE 'co2ao%' ORDER BY segid; + +-- Various cases of altering AOCO to AO: +-- 1. Basic ATSETAMs: +ALTER TABLE co2ao SET ACCESS METHOD ao_row; +ALTER TABLE co2ao2 SET WITH (appendoptimized=true); +-- 2. ATSETAM with reloptions: +ALTER TABLE co2ao3 SET ACCESS METHOD ao_row WITH (compresstype=zlib, compresslevel=7); +ALTER TABLE co2ao4 SET WITH (appendoptimized=true, compresstype=zlib, compresslevel=7); + +-- The tables and indexes should have been rewritten (should have different relfilenodes) +CREATE TEMP TABLE relfileafterco2ao AS + SELECT -1 segid, relfilenode FROM pg_class WHERE relname LIKE 'co2ao%' + UNION SELECT gp_segment_id segid, relfilenode FROM gp_dist_random('pg_class') + WHERE relname LIKE 'co2ao%' ORDER BY segid; + +SELECT * FROM relfilebeforeco2ao INTERSECT SELECT * FROM relfileafterco2ao; + +DROP TABLE relfilebeforeco2ao; +DROP TABLE relfileafterco2ao; + +-- Check data is intact +SELECT count(*) FROM co2ao; +SELECT count(*) FROM co2ao2; +SELECT count(*) FROM co2ao3; +SELECT count(*) FROM co2ao4; + +-- AO aux tables should still be there. +-- Only testing 2 out of the 4 tables being created, where the tables were altered w/wo reloptions. +-- No need to test the other ones created by the alternative syntax SET WITH(). +SELECT * FROM gp_toolkit.__gp_aoseg('co2ao'); +SELECT * FROM gp_toolkit.__gp_aovisimap('co2ao'); +SELECT count(*) FROM gp_toolkit.__gp_aocsseg('co2ao'); +SELECT * FROM gp_toolkit.__gp_aoblkdir('co2ao'); +SELECT * FROM gp_toolkit.__gp_aoseg('co2ao3'); +SELECT * FROM gp_toolkit.__gp_aovisimap('co2ao3'); +SELECT count(*) FROM gp_toolkit.__gp_aocsseg('co2ao3'); +SELECT * FROM gp_toolkit.__gp_aoblkdir('co2ao3'); + +-- pg_appendonly entries should be still be there, but options has changed accordingly. +SELECT c.relname, p.compresstype, p.compresslevel, p.blocksize FROM pg_class c, pg_appendonly p WHERE c.relname LIKE 'co2ao%' AND c.oid = p.relid; + +-- The altered tables should show AOCO AM. +SELECT c.relname, a.amname FROM pg_class c JOIN pg_am a ON c.relam = a.oid WHERE c.relname LIKE 'co2ao%'; + +-- The new tables should have new reloptions. +SELECT relname, reloptions FROM pg_class WHERE relname LIKE 'co2ao%'; + +-- The pg_attribute_encoding entries for the altered tables should have all gone. +SELECT c.relname, a.* FROM pg_attribute_encoding a, pg_class c WHERE a.attrelid=c.oid AND c.relname LIKE 'co2ao%'; + +DROP TABLE co2ao; +DROP TABLE co2ao2; +DROP TABLE co2ao3; +DROP TABLE co2ao4; + -- Final scenario: the iterations of altering table from storage type "A" to "B" and back to "A". -- The following cases will cover all variations of such iterations: -- 1. Heap->AO->Heap->AO --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
