You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudberry.apache.org by ma...@apache.org on 2024/12/31 06:11:05 UTC
(cloudberry) branch main updated: Fix crash that partition table has no encoding attributes for new columns (#820)
This is an automated email from the ASF dual-hosted git repository.
maxyang pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudberry.git
The following commit(s) were added to refs/heads/main by this push:
new 976e4c8289 Fix crash that partition table has no encoding attributes for new columns (#820)
976e4c8289 is described below
commit 976e4c82897d351a5a97f632eec6fe57e9ab6a80
Author: Hao Wu <gf...@gmail.com>
AuthorDate: Tue Dec 31 14:11:00 2024 +0800
Fix crash that partition table has no encoding attributes for new columns (#820)
The encoding attributes are not added for new columns added by alter
table. The parent hasn't any information on the new columns. Its
encoding is determined by the storage type, or default encoding.
---
src/backend/access/aocs/aocsam_handler.c | 13 +++++-
src/backend/access/common/reloptions_gp.c | 6 +--
src/backend/commands/tablecmds.c | 2 +-
.../gpopt/translate/CTranslatorDXLToPlStmt.cpp | 2 +-
.../uao_ddl/gp_partition_tables_column.out | 53 ++++++++++++++++++++++
src/test/regress/greenplum_schedule | 2 +-
.../sql/uao_ddl/gp_partition_tables_column.sql | 18 ++++++++
7 files changed, 88 insertions(+), 8 deletions(-)
diff --git a/src/backend/access/aocs/aocsam_handler.c b/src/backend/access/aocs/aocsam_handler.c
index f8466269ef..6ebe7402b3 100644
--- a/src/backend/access/aocs/aocsam_handler.c
+++ b/src/backend/access/aocs/aocsam_handler.c
@@ -2342,6 +2342,7 @@ aoco_transform_column_encoding_clauses(Relation rel, List *aocoColumnEncoding,
bool foundCompressTypeNone = false;
char *cmplevel = NULL;
bool foundBlockSize = false;
+ bool hasAttrs;
char *arg;
List *retList = list_copy(aocoColumnEncoding);
DefElem *el;
@@ -2352,6 +2353,14 @@ aoco_transform_column_encoding_clauses(Relation rel, List *aocoColumnEncoding,
char *compresstype = NULL;
NameData compresstype_nd;
+ /*
+ * The relam of partition table may be ao table, but partition table
+ * has no entry in pg_appendonly. It shouldn't fetch encoding options
+ * from here for partition tables. See details in function
+ * transformColumnEncoding.
+ */
+ hasAttrs = rel && rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE;
+
foreach(lc, aocoColumnEncoding)
{
dl = (DefElem *) lfirst(lc);
@@ -2388,7 +2397,7 @@ aoco_transform_column_encoding_clauses(Relation rel, List *aocoColumnEncoding,
* table setting in pg_appendonly is preferred over default
* options in GUC gp_default_storage_option.
*/
- if (rel)
+ if (hasAttrs)
{
GetAppendOnlyEntryAttributes(RelationGetRelid(rel),
&blocksize,
@@ -2399,7 +2408,7 @@ aoco_transform_column_encoding_clauses(Relation rel, List *aocoColumnEncoding,
compresstype = NameStr(compresstype_nd);
}
- if (!foundCompressType && rel && compresstype[0])
+ if (!foundCompressType && hasAttrs && compresstype[0])
{
el = makeDefElem("compresstype", (Node *) makeString(pstrdup(compresstype)), -1);
retList = lappend(retList, el);
diff --git a/src/backend/access/common/reloptions_gp.c b/src/backend/access/common/reloptions_gp.c
index 96a30a7c7d..65efb1c689 100644
--- a/src/backend/access/common/reloptions_gp.c
+++ b/src/backend/access/common/reloptions_gp.c
@@ -1689,9 +1689,10 @@ transformColumnEncoding(const TableAmRoutine *tam, Relation rel, List *colDefs,
encoding = copyObject(deflt->encoding);
else if (!explicitOnly)
{
- if (parentenc)
+ ColumnReferenceStorageDirective *parent_col_encoding;
+ parent_col_encoding = find_crsd(d->colname, parentenc);
+ if (parent_col_encoding)
{
- ColumnReferenceStorageDirective *parent_col_encoding = find_crsd(d->colname, parentenc);
encoding = transformStorageEncodingClause(parent_col_encoding->encoding, true);
}
else if (d->typeName) {
@@ -1701,7 +1702,6 @@ transformColumnEncoding(const TableAmRoutine *tam, Relation rel, List *colDefs,
encoding = tam->transform_column_encoding_clauses(rel, encoding, true, true);
}
if (!encoding && createDefaultOne) {
- Assert(tam == GetTableAmRoutineByAmId(rel->rd_rel->relam));
encoding = default_column_encoding_clause(rel);
}
}
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index a6706848eb..1fcf89b618 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -8579,7 +8579,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
add_column_datatype_dependency(myrelid, newattnum, attribute.atttypid);
add_column_collation_dependency(myrelid, newattnum, attribute.attcollation);
- if (OidIsValid(rel->rd_rel->relam) && rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE)
+ if (OidIsValid(rel->rd_rel->relam))
{
List *enc;
const TableAmRoutine *tam = GetTableAmRoutineByAmId(rel->rd_rel->relam);
diff --git a/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp b/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp
index cd2e04aa1b..9918dfd31c 100644
--- a/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp
+++ b/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp
@@ -4042,7 +4042,7 @@ CTranslatorDXLToPlStmt::TranslateDXLSequence(
Plan *
CTranslatorDXLToPlStmt::TranslateDXLDynTblScan(
const CDXLNode *dyn_tbl_scan_dxlnode, CDXLTranslateContext *output_context,
- CDXLTranslationContextArray *ctxt_translation_prev_siblings)
+ CDXLTranslationContextArray * /*ctxt_translation_prev_siblings*/)
{
// translate table descriptor into a range table entry
CDXLPhysicalDynamicTableScan *dyn_tbl_scan_dxlop =
diff --git a/src/test/regress/expected/uao_ddl/gp_partition_tables_column.out b/src/test/regress/expected/uao_ddl/gp_partition_tables_column.out
index ced1945907..b031ad64dd 100644
--- a/src/test/regress/expected/uao_ddl/gp_partition_tables_column.out
+++ b/src/test/regress/expected/uao_ddl/gp_partition_tables_column.out
@@ -177,5 +177,58 @@ select relname, relkind, amname, reloptions from pg_class c left join pg_am am o
pt2_ao_column_1_prt_11 | r | ao_column |
(1 row)
+create table gppt_ao_column.alter_part_tab1 (id SERIAL, a1 int, a2 char(5), a3 text)
+ WITH (appendonly=true, orientation=column, compresstype=zlib)
+ partition by list(a2) subpartition by range(a1)
+ subpartition template (
+ default subpartition subothers,
+ subpartition sp1 start(1) end(9) with(appendonly=true,orientation=column,compresstype=rle_type),
+ subpartition sp2 start(11) end(20) with(appendonly=true,orientation=column,compresstype=zstd))
+ (partition p1 values('val1') , partition p2 values('val2'));
+create index on gppt_ao_column.alter_part_tab1(a1);
+create index on gppt_ao_column.alter_part_tab1 using bitmap(a3);
+alter table gppt_ao_column.alter_part_tab1 add column a4 numeric default 5.5;
+update gppt_ao_column.alter_part_tab1 set a4 = a1 % 2;
+ALTER TABLE gppt_ao_column.alter_part_tab1 ADD partition p31 values(1);
+\d+ gppt_ao_column.alter_part_tab1
+ Partitioned table "gppt_ao_column.alter_part_tab1"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Compression Type | Compression Level | Block Size | Description
+--------+--------------+-----------+----------+---------------------------------------------+----------+--------------+------------------+-------------------+------------+-------------
+ id | integer | | not null | nextval('alter_part_tab1_id_seq'::regclass) | plain | | zlib | 1 | 32768 |
+ a1 | integer | | | | plain | | zlib | 1 | 32768 |
+ a2 | character(5) | | | | extended | | zlib | 1 | 32768 |
+ a3 | text | | | | extended | | zlib | 1 | 32768 |
+ a4 | numeric | | | 5.5 | main | | none | 0 | 32768 |
+Partition key: LIST (a2)
+Indexes:
+ "alter_part_tab1_a1_idx" btree (a1)
+ "alter_part_tab1_a3_idx" bitmap (a3)
+Partitions: alter_part_tab1_1_prt_p1 FOR VALUES IN ('val1 '), PARTITIONED,
+ alter_part_tab1_1_prt_p2 FOR VALUES IN ('val2 '), PARTITIONED,
+ alter_part_tab1_1_prt_p31 FOR VALUES IN ('1 '), PARTITIONED
+Distributed by: (id)
+Options: compresstype=zlib
+
+\d+ gppt_ao_column.alter_part_tab1_1_prt_p31
+ Partitioned table "gppt_ao_column.alter_part_tab1_1_prt_p31"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Compression Type | Compression Level | Block Size | Description
+--------+--------------+-----------+----------+---------------------------------------------+----------+--------------+------------------+-------------------+------------+-------------
+ id | integer | | not null | nextval('alter_part_tab1_id_seq'::regclass) | plain | | zlib | 1 | 32768 |
+ a1 | integer | | | | plain | | zlib | 1 | 32768 |
+ a2 | character(5) | | | | extended | | zlib | 1 | 32768 |
+ a3 | text | | | | extended | | zlib | 1 | 32768 |
+ a4 | numeric | | | 5.5 | main | | none | 0 | 32768 |
+Partition of: alter_part_tab1 FOR VALUES IN ('1 ')
+Partition constraint: ((a2 IS NOT NULL) AND (a2 = '1 '::character(5)))
+Partition key: RANGE (a1)
+Indexes:
+ "alter_part_tab1_1_prt_p31_a1_idx" btree (a1)
+ "alter_part_tab1_1_prt_p31_a3_idx" bitmap (a3)
+Partitions: alter_part_tab1_1_prt_p31_2_prt_sp1 FOR VALUES FROM (1) TO (9),
+ alter_part_tab1_1_prt_p31_2_prt_sp2 FOR VALUES FROM (11) TO (20),
+ alter_part_tab1_1_prt_p31_2_prt_subothers DEFAULT
+Distributed by: (id)
+Options: compresstype=zlib
+
reset default_table_access_method;
reset search_path;
diff --git a/src/test/regress/greenplum_schedule b/src/test/regress/greenplum_schedule
index 5979e8f9ff..8f83c38787 100755
--- a/src/test/regress/greenplum_schedule
+++ b/src/test/regress/greenplum_schedule
@@ -230,7 +230,7 @@ test: uao_ddl/alter_ao_table_setstorage_column uao_ddl/alter_ao_table_col_ddl_ro
test: uao_ddl/alter_ao_part_exch_column uao_ddl/alter_ao_part_tables_row uao_ddl/create_ao_table_500cols_column uao_ddl/alter_ao_part_tables_column
test: uao_ddl/alter_drop_allcol_row uao_ddl/alter_drop_allcol_column uao_ddl/alter_rollback_row uao_ddl/alter_rollback_column uao_ddl/uao_allalter_row uao_ddl/uao_allalter_column
-
+test: uao_ddl/gp_partition_tables_column uao_ddl/gp_partition_tables_row
# These tests use gp_select_invisible and VACUUM, and will get confused if there are
# concurrent transactions holding back the global xmin.
diff --git a/src/test/regress/sql/uao_ddl/gp_partition_tables_column.sql b/src/test/regress/sql/uao_ddl/gp_partition_tables_column.sql
index 4e6325b0c3..3eb4f533dd 100644
--- a/src/test/regress/sql/uao_ddl/gp_partition_tables_column.sql
+++ b/src/test/regress/sql/uao_ddl/gp_partition_tables_column.sql
@@ -58,5 +58,23 @@ ALTER TABLE gppt_ao_column.pt2_ao_column ADD PARTITION START ('2027-01-01') INCL
\d+ gppt_ao_column.pt2_ao_column_1_prt_11
select relname, relkind, amname, reloptions from pg_class c left join pg_am am on c.relam=am.oid where relname='pt2_ao_column_1_prt_11';
+create table gppt_ao_column.alter_part_tab1 (id SERIAL, a1 int, a2 char(5), a3 text)
+ WITH (appendonly=true, orientation=column, compresstype=zlib)
+ partition by list(a2) subpartition by range(a1)
+ subpartition template (
+ default subpartition subothers,
+ subpartition sp1 start(1) end(9) with(appendonly=true,orientation=column,compresstype=rle_type),
+ subpartition sp2 start(11) end(20) with(appendonly=true,orientation=column,compresstype=zstd))
+ (partition p1 values('val1') , partition p2 values('val2'));
+
+create index on gppt_ao_column.alter_part_tab1(a1);
+create index on gppt_ao_column.alter_part_tab1 using bitmap(a3);
+alter table gppt_ao_column.alter_part_tab1 add column a4 numeric default 5.5;
+update gppt_ao_column.alter_part_tab1 set a4 = a1 % 2;
+ALTER TABLE gppt_ao_column.alter_part_tab1 ADD partition p31 values(1);
+\d+ gppt_ao_column.alter_part_tab1
+\d+ gppt_ao_column.alter_part_tab1_1_prt_p31
+
+
reset default_table_access_method;
reset search_path;
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org