You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudberry.apache.org by yj...@apache.org on 2024/12/25 23:39:03 UTC

(cloudberry) 14/24: Removing AOCO add column fixme

This is an automated email from the ASF dual-hosted git repository.

yjhjstz pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudberry.git

commit 9916d77c7c05482bb46774a3b8756621cc93c569
Author: Divyesh Vanjare <va...@vmware.com>
AuthorDate: Wed Aug 10 16:37:34 2022 -0700

    Removing AOCO add column fixme
    
    We won't need to execute it on QE.
    QD/utility can benefit from this optimization.
---
 src/backend/commands/tablecmds.c                | 10 ++++-----
 src/test/regress/expected/alter_table_aocs2.out | 30 +++++++++++++++++++++++++
 src/test/regress/sql/alter_table_aocs2.sql      | 22 ++++++++++++++++++
 3 files changed, 56 insertions(+), 6 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3af452d491..ba85d6324d 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -8644,13 +8644,11 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
 	 * We have to do it while processing the root partition because that's the
 	 * only level where the `ADD COLUMN` subcommands are populated.
 	 *
-	 * GPDB_12_MERGE_FIXME: Given now wqueue gets dispatched from QD to QE, no
-	 * need to perform this step on QE. Only need to execute this block of
-	 * code on QD and QE will get the information to perform optimized rewrite
-	 * for CO or not. Leaving fixme here as CO code is not working currently,
-	 * hence hard to validate if works correctly or not.
+	 * QD will dispatch wqueue and the QE will get all the info
+	 * to perform the column optimized rewrite.
+	 * So, we only need to execute this block on QD.
 	 */
-	if (!recursing && (tab->relkind == RELKIND_PARTITIONED_TABLE || tab->relkind == RELKIND_RELATION))
+	if (!recursing && (tab->relkind == RELKIND_PARTITIONED_TABLE || tab->relkind == RELKIND_RELATION) && Gp_role != GP_ROLE_EXECUTE)
 	{
 		bool	aocs_write_new_columns_only;
 		/*
diff --git a/src/test/regress/expected/alter_table_aocs2.out b/src/test/regress/expected/alter_table_aocs2.out
index 29f1c482da..e0622f8a92 100644
--- a/src/test/regress/expected/alter_table_aocs2.out
+++ b/src/test/regress/expected/alter_table_aocs2.out
@@ -969,3 +969,33 @@ SELECT * FROM subpartition_aoco_leaf;
      0 | subpartition_aoco_leaf_1_prt_intermediate_2_prt_leaf | ao_column | full table rewritten
 (6 rows)
 
+-- Check if add column doesn't rewrite the table
+CREATE TABLE addcol(i int) WITH (appendonly=true, orientation=column);
+INSERT INTO addcol SELECT generate_series(1, 5);
+CREATE TEMP TABLE relfilebefore AS
+    SELECT -1 segid, relname, relfilenode FROM pg_class WHERE relname LIKE 'addcol%'
+    UNION SELECT gp_segment_id segid, relname, relfilenode FROM gp_dist_random('pg_class')
+    WHERE relname LIKE 'addcol%' ORDER BY segid;
+ALTER TABLE addcol ADD COLUMN j int DEFAULT 5;
+CREATE TEMP TABLE relfileafter AS
+    SELECT -1 segid, relname, relfilenode FROM pg_class WHERE relname LIKE 'addcol%'
+    UNION SELECT gp_segment_id segid, relname, relfilenode FROM gp_dist_random('pg_class')
+    WHERE relname LIKE 'addcol%' ORDER BY segid;
+-- table shouldn't be rewritten
+SELECT count(*) FROM (SELECT * FROM relfilebefore UNION SELECT * FROM relfileafter)a;
+ count 
+-------
+     4
+(1 row)
+
+-- data is intact
+SELECT * FROM addcol;
+ i | j 
+---+---
+ 2 | 5
+ 3 | 5
+ 4 | 5
+ 5 | 5
+ 1 | 5
+(5 rows)
+
diff --git a/src/test/regress/sql/alter_table_aocs2.sql b/src/test/regress/sql/alter_table_aocs2.sql
index e96d9f0376..1312abf68e 100644
--- a/src/test/regress/sql/alter_table_aocs2.sql
+++ b/src/test/regress/sql/alter_table_aocs2.sql
@@ -611,3 +611,25 @@ ALTER TABLE subpartition_aoco_leaf ADD COLUMN new_col2 int DEFAULT 1, ALTER COLU
 SELECT * FROM subpartition_aoco_leaf;
 
 :chk_co_opt_qry;
+
+-- Check if add column doesn't rewrite the table
+CREATE TABLE addcol(i int) WITH (appendonly=true, orientation=column);
+INSERT INTO addcol SELECT generate_series(1, 5);
+
+CREATE TEMP TABLE relfilebefore AS
+    SELECT -1 segid, relname, relfilenode FROM pg_class WHERE relname LIKE 'addcol%'
+    UNION SELECT gp_segment_id segid, relname, relfilenode FROM gp_dist_random('pg_class')
+    WHERE relname LIKE 'addcol%' ORDER BY segid;
+
+ALTER TABLE addcol ADD COLUMN j int DEFAULT 5;
+
+CREATE TEMP TABLE relfileafter AS
+    SELECT -1 segid, relname, relfilenode FROM pg_class WHERE relname LIKE 'addcol%'
+    UNION SELECT gp_segment_id segid, relname, relfilenode FROM gp_dist_random('pg_class')
+    WHERE relname LIKE 'addcol%' ORDER BY segid;
+
+-- table shouldn't be rewritten
+SELECT count(*) FROM (SELECT * FROM relfilebefore UNION SELECT * FROM relfileafter)a;
+
+-- data is intact
+SELECT * FROM addcol;


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org