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 2025/01/13 08:23:03 UTC

(cloudberry) branch main updated (e1168bda54 -> 325df54863)

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

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


    from e1168bda54 FIX: cherry-pick missing icw test cases
     new 87ab71c51f Fix flaky idle_gang_cleaner case (#15228)
     new 8e49f0b430 Fixed the string comparison warning
     new 2c290bc66d Error if partitioned table passed to gp_toolkit.__gp_aocsseg(oid)
     new 2786dd0c52 ASSERT relation is AO in GetAppendOnlyEntryAuxOids/GetAppendOnlyEntry
     new cc592337e0 Fix typos.
     new 6032c5878c Fix: 'unrecognized node type: 145' in transformExpr (#15184)
     new c41d675efc Enhance logging for FTS time out (#14185)
     new 0eacb9fe29 Fix IC bugs in ic_proxy_ibuf_push() (#15139)
     new 261f6bdffe Remove FIXME: no longer to pass paramExecTypes as a param (7X) (#15649)
     new de51dfcc6f Fix dangling pointer in ExecDynamicIndexScan() (#15619)
     new c0124395ea Fix an issue where we increment command counter incorrectly
     new d181ca6a0a Remove dead hash agg GUCs
     new 4e08a005fe Set next OID before restoring schema during pg_upgrade
     new a1ddb29cc1 [typo] polish parameter name of _get_remove_cmd (#15473)
     new 6e94aaf727 Change key `log` to UNRESERVED_KEYWORD (#15606)
     new ae976a6f0f Resolve GPDB_96_MERGE_FIXME in prepunion.c about recursive cte
     new 8c6a353349 Remove GPDB_96_MERGE_FIXME in subselect_gp*. (#15587)
     new cfd67240f0 Remove GPDB_95_MERGE_FIXME in groupingset*. (#15680)
     new 04d2a5a203 Fix GPDB_96_MERGE_FIXME: verify walker works on Sequence node (in nodeFuncs.c)
     new c040da238a Resolve a FIXME for gpcheckcat
     new 325df54863 Resolve two FIXMEs in extractPageInfo()

The 21 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 contrib/interconnect/proxy/ic_proxy_backend.c      |  2 +-
 contrib/interconnect/proxy/ic_proxy_client.c       |  6 +-
 contrib/interconnect/proxy/ic_proxy_iobuf.c        | 17 +++---
 contrib/interconnect/proxy/ic_proxy_peer.c         | 35 ++++++-----
 gpMgmt/bin/gpcheckcat_modules/foreign_key_check.py | 11 ----
 .../bin/gppylib/operations/buildMirrorSegments.py  |  4 +-
 gpMgmt/test/behave/mgmt_utils/gpcheckcat.feature   |  1 +
 gpMgmt/test/behave/mgmt_utils/steps/mgmt_utils.py  |  7 +++
 src/backend/access/aocs/aocssegfiles.c             |  3 +-
 src/backend/catalog/pg_appendonly.c                | 17 +++++-
 src/backend/cdb/cdbpath.c                          |  4 +-
 src/backend/cdb/cdbvars.c                          |  3 -
 src/backend/cdb/dispatcher/cdbdisp_query.c         | 25 +++-----
 src/backend/commands/cluster.c                     |  9 +++
 src/backend/executor/nodeDynamicIndexscan.c        |  8 ++-
 src/backend/fts/ftsprobe.c                         | 19 +++++-
 src/backend/nodes/nodeFuncs.c                      |  1 -
 src/backend/optimizer/prep/prepunion.c             |  5 --
 src/backend/parser/gram.y                          |  2 +-
 src/backend/parser/parse_expr.c                    | 18 ++++--
 src/backend/utils/misc/guc_gp.c                    | 37 ------------
 src/bin/pg_rewind/parsexlog.c                      | 12 ----
 src/bin/pg_rewind/pg_rewind.c                      | 15 +++++
 src/bin/pg_upgrade/pg_upgrade.c                    | 38 ++++++++----
 src/include/cdb/cdbvars.h                          |  6 --
 src/include/parser/kwlist.h                        |  2 +-
 src/include/postmaster/ftsprobe.h                  | 13 ++++
 src/include/utils/sync_guc_name.h                  |  2 -
 src/test/isolation2/input/idle_gang_cleaner.source |  3 -
 .../isolation2/output/idle_gang_cleaner.source     |  7 ---
 src/test/regress/expected/alter_table.out          |  4 ++
 src/test/regress/expected/groupingsets.out         |  6 +-
 .../regress/expected/groupingsets_optimizer.out    |  6 --
 src/test/regress/expected/ic_proxy_socket.out      | 69 ++++++++++++++++++++++
 src/test/regress/expected/subselect_gp.out         |  3 -
 .../regress/expected/subselect_gp_optimizer.out    |  3 -
 src/test/regress/greenplum_schedule                |  3 +
 src/test/regress/sql/alter_table.sql               |  5 ++
 src/test/regress/sql/groupingsets.sql              |  6 --
 src/test/regress/sql/ic_proxy_socket.sql           | 58 ++++++++++++++++++
 src/test/regress/sql/subselect_gp.sql              |  3 -
 41 files changed, 310 insertions(+), 188 deletions(-)
 create mode 100644 src/test/regress/expected/ic_proxy_socket.out
 create mode 100644 src/test/regress/sql/ic_proxy_socket.sql


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


(cloudberry) 18/21: Remove GPDB_95_MERGE_FIXME in groupingset*. (#15680)

Posted by yj...@apache.org.
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 cfd67240f0574d145d52627a6c5397c78aa61151
Author: Wenlin Zhang <zw...@vmware.com>
AuthorDate: Wed Jun 7 13:48:28 2023 +0800

    Remove GPDB_95_MERGE_FIXME in groupingset*. (#15680)
    
    When merging PG95 (commit 8ae22d15ebf), the query plan for the lateral query
    with grouping sets do not make right plans and the results are wrong.
    
    But in the current main , the plan looks right and the result is also correct.
    So remove the fixme directly.
---
 src/test/regress/expected/groupingsets.out           | 6 +-----
 src/test/regress/expected/groupingsets_optimizer.out | 6 ------
 src/test/regress/sql/groupingsets.sql                | 6 ------
 3 files changed, 1 insertion(+), 17 deletions(-)

diff --git a/src/test/regress/expected/groupingsets.out b/src/test/regress/expected/groupingsets.out
index 25b9ecbd58..a1c532f638 100644
--- a/src/test/regress/expected/groupingsets.out
+++ b/src/test/regress/expected/groupingsets.out
@@ -1039,7 +1039,6 @@ group by rollup(ten);
 (11 rows)
 
 -- More rescan tests
--- start_ignore
 select * from (values (1),(2)) v(a) left join lateral (select v.a, four, ten, count(*) from onek group by cube(four,ten)) s on true order by v.a,four,ten;
  a | a | four | ten | count 
 ---+---+------+-----+-------
@@ -1114,7 +1113,7 @@ select * from (values (1),(2)) v(a) left join lateral (select v.a, four, ten, co
  2 | 2 |      |   9 |   100
  2 | 2 |      |     |  1000
 (70 rows)
--- end_ignore
+
 select array(select row(v.a,s1.*) from (select two,four, count(*) from onek group by cube(two,four) order by two,four) s1) from (values (1),(2)) v(a);
                                                                         array                                                                         
 ------------------------------------------------------------------------------------------------------------------------------------------------------
@@ -1737,8 +1736,6 @@ SELECT a, b, count(*), max(a), max(b) FROM gstest3 GROUP BY GROUPING SETS(a, b,(
 
 COMMIT;
 -- More rescan tests
--- start_ignore
--- GPDB_95_MERGE_FIXME: the lateral query with grouping sets do not make right plans
 select * from (values (1),(2)) v(a) left join lateral (select v.a, four, ten, count(*) from onek group by cube(four,ten)) s on true order by v.a,four,ten;
  a | a | four | ten | count 
 ---+---+------+-----+-------
@@ -1814,7 +1811,6 @@ select * from (values (1),(2)) v(a) left join lateral (select v.a, four, ten, co
  2 | 2 |      |     |  1000
 (70 rows)
 
--- end_ignore
 select array(select row(v.a,s1.*) from (select two,four, count(*) from onek group by cube(two,four) order by two,four) s1) from (values (1),(2)) v(a);
                                                                         array                                                                         
 ------------------------------------------------------------------------------------------------------------------------------------------------------
diff --git a/src/test/regress/expected/groupingsets_optimizer.out b/src/test/regress/expected/groupingsets_optimizer.out
index f4990cd25b..c32318311f 100644
--- a/src/test/regress/expected/groupingsets_optimizer.out
+++ b/src/test/regress/expected/groupingsets_optimizer.out
@@ -1052,8 +1052,6 @@ group by rollup(ten);
 (11 rows)
 
 -- More rescan tests
--- start_ignore
--- GPDB_95_MERGE_FIXME: the lateral query with grouping sets do not make right plans
 select * from (values (1),(2)) v(a) left join lateral (select v.a, four, ten, count(*) from onek group by cube(four,ten)) s on true order by v.a,four,ten;
  a | a | four | ten | count 
 ---+---+------+-----+-------
@@ -1129,7 +1127,6 @@ select * from (values (1),(2)) v(a) left join lateral (select v.a, four, ten, co
  2 | 2 |      |     |  1000
 (70 rows)
 
--- end_ignore
 select array(select row(v.a,s1.*) from (select two,four, count(*) from onek group by cube(two,four) order by two,four) s1) from (values (1),(2)) v(a);
                                                                         array                                                                         
 ------------------------------------------------------------------------------------------------------------------------------------------------------
@@ -1801,8 +1798,6 @@ SELECT a, b, count(*), max(a), max(b) FROM gstest3 GROUP BY GROUPING SETS(a, b,(
 
 COMMIT;
 -- More rescan tests
--- start_ignore
--- GPDB_95_MERGE_FIXME: the lateral query with grouping sets do not make right plans
 select * from (values (1),(2)) v(a) left join lateral (select v.a, four, ten, count(*) from onek group by cube(four,ten)) s on true order by v.a,four,ten;
  a | a | four | ten | count 
 ---+---+------+-----+-------
@@ -1878,7 +1873,6 @@ select * from (values (1),(2)) v(a) left join lateral (select v.a, four, ten, co
  2 | 2 |      |     |  1000
 (70 rows)
 
--- end_ignore
 select array(select row(v.a,s1.*) from (select two,four, count(*) from onek group by cube(two,four) order by two,four) s1) from (values (1),(2)) v(a);
                                                                         array                                                                         
 ------------------------------------------------------------------------------------------------------------------------------------------------------
diff --git a/src/test/regress/sql/groupingsets.sql b/src/test/regress/sql/groupingsets.sql
index 571f108b5b..054a299b56 100644
--- a/src/test/regress/sql/groupingsets.sql
+++ b/src/test/regress/sql/groupingsets.sql
@@ -375,10 +375,7 @@ select ten, sum(distinct four) filter (where four::text ~ '123') from onek a
 group by rollup(ten);
 
 -- More rescan tests
--- start_ignore
--- GPDB_95_MERGE_FIXME: the lateral query with grouping sets do not make right plans
 select * from (values (1),(2)) v(a) left join lateral (select v.a, four, ten, count(*) from onek group by cube(four,ten)) s on true order by v.a,four,ten;
--- end_ignore
 select array(select row(v.a,s1.*) from (select two,four, count(*) from onek group by cube(two,four) order by two,four) s1) from (values (1),(2)) v(a);
 
 -- Grouping on text columns
@@ -537,10 +534,7 @@ SELECT a, b, count(*), max(a), max(b) FROM gstest3 GROUP BY GROUPING SETS(a, b,(
 COMMIT;
 
 -- More rescan tests
--- start_ignore
--- GPDB_95_MERGE_FIXME: the lateral query with grouping sets do not make right plans
 select * from (values (1),(2)) v(a) left join lateral (select v.a, four, ten, count(*) from onek group by cube(four,ten)) s on true order by v.a,four,ten;
--- end_ignore
 select array(select row(v.a,s1.*) from (select two,four, count(*) from onek group by cube(two,four) order by two,four) s1) from (values (1),(2)) v(a);
 
 -- Rescan logic changes when there are no empty grouping sets, so test


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


(cloudberry) 06/21: Fix: 'unrecognized node type: 145' in transformExpr (#15184)

Posted by yj...@apache.org.
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 6032c5878c909bb033eed4c8ab919a90fc2bb697
Author: gtygo <gt...@gmail.com>
AuthorDate: Wed Mar 29 18:48:53 2023 +0800

    Fix: 'unrecognized node type: 145' in transformExpr (#15184)
    
    In GPDB QD node, the execution path is the same as PG12:
    ```T_TypeCast -> T_TypeCast -> T_BooleanTest -> T_A_EXPr```
    However, after transformExpr(pstate, raw_default,......) in QD, we changed the path of raw_default to
    ```T_TypeCast -> T_TypeCast -> T_BooleanTest -> T_OpExpr```
    
    Then we dispatch it (alterStmt) to QE, we need to call transformExpr() in QE too. But the path of raw_default is changed. And we cannot process OpExpr in transformExpr since it's not the structure of a parse tree.
    
    So the fix is easy: we have to let transformBooleanTest not change its parameter.
    Details please see the discussion in PR threads.
---
 src/backend/parser/parse_expr.c           | 18 ++++++++++++++----
 src/test/regress/expected/alter_table.out |  4 ++++
 src/test/regress/sql/alter_table.sql      |  5 +++++
 3 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index d8704c7253..906331063b 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -2565,8 +2565,11 @@ transformXmlSerialize(ParseState *pstate, XmlSerialize *xs)
 static Node *
 transformBooleanTest(ParseState *pstate, BooleanTest *b)
 {
+	BooleanTest *newb;
 	const char *clausename;
 
+	newb = makeNode(BooleanTest);
+
 	switch (b->booltesttype)
 	{
 		case IS_TRUE:
@@ -2593,13 +2596,20 @@ transformBooleanTest(ParseState *pstate, BooleanTest *b)
 			clausename = NULL;	/* keep compiler quiet */
 	}
 
-	b->arg = (Expr *) transformExprRecurse(pstate, (Node *) b->arg);
+	/*
+	 * Define a new variable so that b->arg is not modified and this variable allows
+	 * QD to not modify the original expr.
+	 */
+
+	newb->arg = (Expr *) transformExprRecurse(pstate, (Node *) b->arg);
 
-	b->arg = (Expr *) coerce_to_boolean(pstate,
-										(Node *) b->arg,
+	newb->arg = (Expr *) coerce_to_boolean(pstate,
+										(Node *) newb->arg,
 										clausename);
+	newb->booltesttype = b->booltesttype;
+	newb->location = b->location;
 
-	return (Node *) b;
+	return (Node *) newb;
 }
 
 static Node *
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 660309425d..31a82312dd 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -4751,3 +4751,7 @@ DROP TABLE table_issue_15494;
 CREATE TABLE IF NOT EXISTS table_issue_15494(c0 boolean);
 ALTER TABLE table_issue_15494 ALTER c0 SET DEFAULT ((1.5::FLOAT) NOTNULL);
 DROP TABLE table_issue_15494;
+-- please refer to:  https://github.com/greenplum-db/gpdb/issues/15034
+create table transform_issue_15034(c DECIMAL);
+alter table transform_issue_15034 alter c SET DEFAULT (((0.1)>(0.9) IS UNKNOWN)::INT)::MONEY;
+drop table transform_issue_15034;
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 8cfd07946c..d157ad1f05 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -3120,3 +3120,8 @@ DROP TABLE table_issue_15494;
 CREATE TABLE IF NOT EXISTS table_issue_15494(c0 boolean);
 ALTER TABLE table_issue_15494 ALTER c0 SET DEFAULT ((1.5::FLOAT) NOTNULL);
 DROP TABLE table_issue_15494;
+
+-- please refer to:  https://github.com/greenplum-db/gpdb/issues/15034
+create table transform_issue_15034(c DECIMAL);
+alter table transform_issue_15034 alter c SET DEFAULT (((0.1)>(0.9) IS UNKNOWN)::INT)::MONEY;
+drop table transform_issue_15034;


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


(cloudberry) 07/21: Enhance logging for FTS time out (#14185)

Posted by yj...@apache.org.
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 c41d675efc0fc40289d8fe94307b767355c60299
Author: Yao Wang <wa...@vmware.com>
AuthorDate: Wed Mar 29 16:15:11 2023 +0800

    Enhance logging for FTS time out (#14185)
    
    Add a member fts_segment_info.timeout_count to count how many timeout
    errors reported by poll(). When we find gp_fts_probe_timeout has been
    exceeded in ftsCheckTimeout(), fts_segment_info.timeout_count will be
    logged. It should be helpful to distinguish the error is from network
    congestion (reported by poll()), or other reasons (e.g. CPU congestion
    so the process can not get CPU on time).
---
 src/backend/fts/ftsprobe.c        | 19 +++++++++++++++++--
 src/include/postmaster/ftsprobe.h | 13 +++++++++++++
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/src/backend/fts/ftsprobe.c b/src/backend/fts/ftsprobe.c
index 36c08853e2..d35f902c93 100644
--- a/src/backend/fts/ftsprobe.c
+++ b/src/backend/fts/ftsprobe.c
@@ -422,10 +422,13 @@ ftsCheckTimeout(fts_segment_info *ftsInfo, pg_time_t now)
 	{
 		elog(LOG,
 			 "FTS timeout detected for (content=%d, dbid=%d) "
-			 "state=%d, retry_count=%d,",
+			 "state=%d, retry_count=%d, timeout_count=%d ",
 			 ftsInfo->primary_cdbinfo->config->segindex,
 			 ftsInfo->primary_cdbinfo->config->dbid, ftsInfo->state,
-			 ftsInfo->retry_count);
+			 ftsInfo->retry_count, ftsInfo->timeout_count);
+
+		/* Reset timeout_count before moving to next failed state */
+		ftsInfo->timeout_count = 0;
 		ftsInfo->state = nextFailedState(ftsInfo->state);
 	}
 }
@@ -523,6 +526,17 @@ ftsPoll(fts_context *context)
 					 ftsInfo->retry_count, ftsInfo->conn->status,
 					 ftsInfo->conn->asyncStatus);
 			}
+			/*
+			 * Count time out errors reported by poll(), so we can refer it
+			 * when gp_fts_probe_timeout is exceeded in ftsCheckTimeout().
+			 * Segments for which a response is received already are not to be
+			 * counted.
+			 */
+			if (!IsFtsMessageStateSuccess(ftsInfo->state) &&
+				nready == 0)
+			{
+				ftsInfo->timeout_count++;
+			}
 			/* If poll timed-out above, check timeout */
 			ftsCheckTimeout(ftsInfo, now);
 		}
@@ -1197,6 +1211,7 @@ processResponse(fts_context *context)
 		ftsInfo->conn = NULL;
 		ftsInfo->poll_events = ftsInfo->poll_revents = 0;
 		ftsInfo->retry_count = 0;
+		ftsInfo->timeout_count = 0;
 	}
 
 	return is_updated;
diff --git a/src/include/postmaster/ftsprobe.h b/src/include/postmaster/ftsprobe.h
index 3eb76d95c0..e4e8306d59 100644
--- a/src/include/postmaster/ftsprobe.h
+++ b/src/include/postmaster/ftsprobe.h
@@ -97,6 +97,19 @@ typedef struct
 	int retry_count;
 	XLogRecPtr xlogrecptr;
 	PMRestartState restart_state;
+	/*
+	 * How many times of time error (reported by poll()) we have got
+	 * when gp_fts_probe_timeout is exceeded
+	 *
+	 * Say by default the timeout parameter passed to poll() is 50 ms
+	 * (see ftsPoll()), and gp_fts_probe_timeout is 20 sec. If we hit
+	 * gp_fts_probe_timeout:
+	 * 1. If the congestion is on network or segs, timeout_count should
+	 *    be near to 20000 / 50 = 400.
+	 * 2. If timeout_count is much less than 400, it means that FTS did
+	 *    not get enough CPU chance and the congestion must be on master.
+	 */
+	int timeout_count;
 } fts_segment_info;
 
 #ifdef USE_INTERNAL_FTS


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


(cloudberry) 13/21: Set next OID before restoring schema during pg_upgrade

Posted by yj...@apache.org.
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 4e08a005fe25ff27368f56f66ccf5a0955eb5d43
Author: Jimmy Yih <jy...@vmware.com>
AuthorDate: Tue May 30 17:35:14 2023 -0700

    Set next OID before restoring schema during pg_upgrade
    
    When creating new objects on the target cluster via pg_restore,
    objects that do not have a preassigned oid will try to get a new oid
    from the oid counter. This works in upstream Postgres but can be slow
    in GPDB because the new oid is checked against the preserved oid
    list. If the new oid is in the preserved oid list, a new oid is
    generated from the oid counter until a valid oid is found. In
    production scenarios, it would be very common to have a very, very
    large preserved oid list and starting the oid counter from
    FirstNormalObjectId (16384) would make object creation slower than
    usual near the beginning of pg_restore. To prevent pg_restore
    performance degradation from so many invalid new oids from the oid
    counter, bump the oid counter to what the source cluster has via
    pg_resetwal. If the preserved oid list logic is removed from
    pg_upgrade, revert this commit.
---
 src/bin/pg_upgrade/pg_upgrade.c | 38 +++++++++++++++++++++++++-------------
 1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index dbd22caac5..fd9c36477a 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -155,6 +155,31 @@ main(int argc, char **argv)
 
 	copy_xact_xlog_xid();
 
+	/*
+	 * GPDB: This used to be right before syncing the data directory to disk
+	 * but is needed here before create_new_objects() due to our usage of a
+	 * preserved oid list. When creating new objects on the target cluster,
+	 * objects that do not have a preassigned oid will try to get a new oid
+	 * from the oid counter. This works in upstream Postgres but can be slow
+	 * in GPDB because the new oid is checked against the preserved oid
+	 * list. If the new oid is in the preserved oid list, a new oid is
+	 * generated from the oid counter until a valid oid is found. In
+	 * production scenarios, it would be very common to have a very, very
+	 * large preserved oid list and starting the oid counter from
+	 * FirstNormalObjectId (16384) would make object creation slower than
+	 * usual near the beginning of pg_restore. To prevent pg_restore
+	 * performance degradation from so many invalid new oids from the oid
+	 * counter, bump the oid counter to what the source cluster has via
+	 * pg_resetwal. If the preserved oid list logic is removed from
+	 * pg_upgrade, move this step back to where it was before.
+	 */
+	prep_status("Setting next OID for new cluster");
+	exec_prog(UTILITY_LOG_FILE, NULL, true, true,
+			  "\"%s/pg_resetwal\" --binary-upgrade -o %u \"%s\"",
+			  new_cluster.bindir, old_cluster.controldata.chkpnt_nxtoid,
+			  new_cluster.pgdata);
+	check_ok();
+
 	/*
 	 * In upgrading from GPDB4, copy the pg_distributedlog over in vanilla.
 	 * The assumption that this works needs to be verified
@@ -208,19 +233,6 @@ main(int argc, char **argv)
 	transfer_all_new_tablespaces(&old_cluster.dbarr, &new_cluster.dbarr,
 								 old_cluster.pgdata, new_cluster.pgdata);
 
-	/*
-	 * Assuming OIDs are only used in system tables, there is no need to
-	 * restore the OID counter because we have not transferred any OIDs from
-	 * the old system, but we do it anyway just in case.  We do it late here
-	 * because there is no need to have the schema load use new oids.
-	 */
-	prep_status("Setting next OID for new cluster");
-	exec_prog(UTILITY_LOG_FILE, NULL, true, true,
-			  "\"%s/pg_resetwal\" --binary-upgrade -o %u \"%s\"",
-			  new_cluster.bindir, old_cluster.controldata.chkpnt_nxtoid,
-			  new_cluster.pgdata);
-	check_ok();
-
 	/* For non-master segments, uniquify the system identifier. */
 	if (!is_greenplum_dispatcher_mode())
 		reset_system_identifier();


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


(cloudberry) 10/21: Fix dangling pointer in ExecDynamicIndexScan() (#15619)

Posted by yj...@apache.org.
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 de51dfcc6f15b2edd0cc4929ed9b490ed5af77f8
Author: Yao Wang <wa...@vmware.com>
AuthorDate: Thu Jun 1 10:12:00 2023 +0800

    Fix dangling pointer in ExecDynamicIndexScan() (#15619)
    
    It's to fix an issue of dangling pointer in ExecDynamicIndexScan(). The
    memory slot pointed had been freed after endCurrentIndexScan(), and
    when TupIsNull() checked slot again, there is a small chance to a crash.
    The fix is to efactor the code to avoid double check by TupIsNull().
---
 src/backend/executor/nodeDynamicIndexscan.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/backend/executor/nodeDynamicIndexscan.c b/src/backend/executor/nodeDynamicIndexscan.c
index 4d7f7ff060..8586e3ffe2 100644
--- a/src/backend/executor/nodeDynamicIndexscan.c
+++ b/src/backend/executor/nodeDynamicIndexscan.c
@@ -273,9 +273,11 @@ ExecDynamicIndexScan(PlanState *pstate)
 	 * Scan index to find next tuple to return. If the current index
 	 * is exhausted, close it and open the next index for scan.
 	 */
-	while (TupIsNull(slot) &&
-		   initNextIndexToScan(node))
+	for (;;)
 	{
+		if (!initNextIndexToScan(node))
+			return NULL;
+
 		slot = ExecProcNode(&node->indexScanState->ss.ps);
 
 		if (TupIsNull(slot))
@@ -284,6 +286,8 @@ ExecDynamicIndexScan(PlanState *pstate)
 
 			node->scan_state = SCAN_INIT;
 		}
+		else
+			break;
 	}
 	return slot;
 }


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


(cloudberry) 04/21: ASSERT relation is AO in GetAppendOnlyEntryAuxOids/GetAppendOnlyEntry

Posted by yj...@apache.org.
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 2786dd0c524323ada98e6d85ec9d16029a29a185
Author: Huansong Fu <fu...@gmail.com>
AuthorDate: Fri Mar 17 12:36:21 2023 -0700

    ASSERT relation is AO in GetAppendOnlyEntryAuxOids/GetAppendOnlyEntry
    
    GetAppendOnlyEntryAuxOids / GetAppendOnlyEntry should only be called
    on a non-partitioned AO/CO table. Otherwise, there's no pg_appendonly
    entry and the relation's relcache also doesn't carry any pg_appendonly
    entry data. Putting a strong assert there.
---
 src/backend/catalog/pg_appendonly.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/src/backend/catalog/pg_appendonly.c b/src/backend/catalog/pg_appendonly.c
index dbdf0a34f3..6b439d983b 100644
--- a/src/backend/catalog/pg_appendonly.c
+++ b/src/backend/catalog/pg_appendonly.c
@@ -176,7 +176,8 @@ GetAppendOnlyEntryAttributes(Oid relid,
 
 /*
  * Get the OIDs of the auxiliary relations and their indexes for an appendonly
- * relation.
+ * relation. This should only be called on tables with pg_appendonly entries,
+ * which currently are just non-partitioned AO/CO tables.
  *
  * The OIDs will be retrieved only when the corresponding output variable is
  * not NULL.
@@ -191,6 +192,9 @@ GetAppendOnlyEntryAuxOids(Relation rel,
 {
 	Form_pg_appendonly	aoForm;
 
+	/* the relation has to be a non-partitioned AO/CO table */
+	Assert(RelationIsAppendOptimized(rel));
+
 	aoForm = rel->rd_appendonly;
 
 	if (segrelid != NULL)
@@ -209,12 +213,23 @@ GetAppendOnlyEntryAuxOids(Relation rel,
 		*visimapidxid = aoForm->visimapidxid;
 }
 
+/*
+ * Get the pg_appendonly entry for the relation. This should only be called on 
+ * tables with pg_appendonly entries, which currently are just non-partitioned
+ * AO/CO tables. The pg_appendonly data is copied into the Form_pg_appendonly
+ * pointer which should be valid.
+ */
 void
 GetAppendOnlyEntry(Relation rel, Form_pg_appendonly aoEntry)
 {
 	Form_pg_appendonly	aoForm;
 
+	/* the relation has to be a non-partitioned AO/CO table and the aoEntry is valid */
+	Assert(RelationIsAppendOptimized(rel));
+	Assert(aoEntry);
+
 	aoForm = rel->rd_appendonly;
+
 	memcpy(aoEntry, aoForm, APPENDONLY_TUPLE_SIZE);
 }
 


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


(cloudberry) 14/21: [typo] polish parameter name of _get_remove_cmd (#15473)

Posted by yj...@apache.org.
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 a1ddb29cc1233e84ba0b98fafcc875bbf3e216ba
Author: zhjwpku <zh...@gmail.com>
AuthorDate: Wed May 31 10:24:20 2023 +0800

    [typo] polish parameter name of _get_remove_cmd (#15473)
    
    The paramter `cmd` of _get_remove_cmd failed to express its meaning,
    seems better renaming it to `removeFile`.
    
    Signed-off-by: Junwang Zhao <zh...@gmail.com>
---
 gpMgmt/bin/gppylib/operations/buildMirrorSegments.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gpMgmt/bin/gppylib/operations/buildMirrorSegments.py b/gpMgmt/bin/gppylib/operations/buildMirrorSegments.py
index cfa08280b2..a071aa89e8 100644
--- a/gpMgmt/bin/gppylib/operations/buildMirrorSegments.py
+++ b/gpMgmt/bin/gppylib/operations/buildMirrorSegments.py
@@ -446,8 +446,8 @@ class GpMirrorListToBuild:
                                                        remoteHost=targetHostname)
         return None
 
-    def _get_remove_cmd(self, cmd, target_host):
-        return base.Command("remove file", "rm -f {}".format(pipes.quote(cmd)), ctxt=base.REMOTE, remoteHost=target_host)
+    def _get_remove_cmd(self, remove_file, target_host):
+        return base.Command("remove file", "rm -f {}".format(pipes.quote(remove_file)), ctxt=base.REMOTE, remoteHost=target_host)
 
     def __runWaitAndCheckWorkerPoolForErrorsAndClear(self, cmds, suppressErrorCheck=False, progressCmds=[]):
         for cmd in cmds:


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


(cloudberry) 08/21: Fix IC bugs in ic_proxy_ibuf_push() (#15139)

Posted by yj...@apache.org.
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 0eacb9fe2993791822971f432da0fc4b47d3e8e9
Author: Hongxu Ma <in...@outlook.com>
AuthorDate: Wed Mar 8 15:02:57 2023 +0800

    Fix IC bugs in ic_proxy_ibuf_push() (#15139)
    
    My previous comment 57a3140 added magic number field in the ICProxyPkt. Some bugs are found in the process of porting it to 6x: they are related to the invalid value of ICProxyPkt field, current code doesn't handle them since they don't exist in the normal package.
    
    All of them have been fixed in the 6x PR: #14998, so this PR backports the fix code and test cases into the main branch.
---
 contrib/interconnect/proxy/ic_proxy_backend.c |  2 +-
 contrib/interconnect/proxy/ic_proxy_client.c  |  6 +--
 contrib/interconnect/proxy/ic_proxy_iobuf.c   | 17 +++----
 contrib/interconnect/proxy/ic_proxy_peer.c    | 35 +++++++-------
 src/test/regress/expected/ic_proxy_socket.out | 69 +++++++++++++++++++++++++++
 src/test/regress/greenplum_schedule           |  3 ++
 src/test/regress/sql/ic_proxy_socket.sql      | 58 ++++++++++++++++++++++
 7 files changed, 162 insertions(+), 28 deletions(-)

diff --git a/contrib/interconnect/proxy/ic_proxy_backend.c b/contrib/interconnect/proxy/ic_proxy_backend.c
index b8a213832e..4ca4d45510 100644
--- a/contrib/interconnect/proxy/ic_proxy_backend.c
+++ b/contrib/interconnect/proxy/ic_proxy_backend.c
@@ -183,6 +183,7 @@ ic_proxy_backend_on_read_hello_ack(uv_stream_t *stream, ssize_t nread, const uv_
 	/* we have received a complete HELLO ACK message */
 	pkt = (ICProxyPkt *)backend->buffer;
 
+	uv_read_stop(stream);
 	/* sanity check: drop the packet with incorrect magic number */
 	if (!ic_proxy_pkt_is_valid(pkt))
 	{
@@ -193,7 +194,6 @@ ic_proxy_backend_on_read_hello_ack(uv_stream_t *stream, ssize_t nread, const uv_
 	}
 
 	Assert(ic_proxy_pkt_is(pkt, IC_PROXY_MESSAGE_HELLO_ACK));
-	uv_read_stop(stream);
 
 	/* assign connection fd after domain socket successfully connected */
 	ret = uv_fileno((uv_handle_t *)&backend->pipe, &backend->conn->sockfd);
diff --git a/contrib/interconnect/proxy/ic_proxy_client.c b/contrib/interconnect/proxy/ic_proxy_client.c
index 5c57f5bccb..8a001e1a2c 100644
--- a/contrib/interconnect/proxy/ic_proxy_client.c
+++ b/contrib/interconnect/proxy/ic_proxy_client.c
@@ -659,6 +659,9 @@ ic_proxy_client_on_hello_pkt(void *opaque, const void *data, uint16 size)
 	ICProxyKey	key;
 	ICProxyPkt *ackpkt;
 
+	/* we only expect one HELLO message */
+	uv_read_stop((uv_stream_t *) &client->pipe);
+
 	/* sanity check: drop the packet with incorrect magic number */
 	if (!ic_proxy_pkt_is_valid(pkt))
 	{
@@ -668,9 +671,6 @@ ic_proxy_client_on_hello_pkt(void *opaque, const void *data, uint16 size)
 		return;
 	}
 
-	/* we only expect one HELLO message */
-	uv_read_stop((uv_stream_t *) &client->pipe);
-
 	if (!ic_proxy_pkt_is(pkt, IC_PROXY_MESSAGE_HELLO))
 	{
 		elog(WARNING, "ic-proxy: %s: invalid %s",
diff --git a/contrib/interconnect/proxy/ic_proxy_iobuf.c b/contrib/interconnect/proxy/ic_proxy_iobuf.c
index 6fd4b73f34..3072d20f57 100644
--- a/contrib/interconnect/proxy/ic_proxy_iobuf.c
+++ b/contrib/interconnect/proxy/ic_proxy_iobuf.c
@@ -151,13 +151,14 @@ ic_proxy_ibuf_push(ICProxyIBuf *ibuf,
 			/* have a complete header now */
 
 			packet_size = ibuf->get_packet_size(ibuf->buf);
-			delta = Min(packet_size - ibuf->len, size);
-
-			memcpy(ibuf->buf + ibuf->len, data, delta);
-			ibuf->len += delta;
-			data += delta;
-			size -= delta;
-
+			if (packet_size > ibuf->len)
+			{
+				delta = Min(packet_size - ibuf->len, size);
+				memcpy(ibuf->buf + ibuf->len, data, delta);
+				ibuf->len += delta;
+				data += delta;
+				size -= delta;
+			}
 			if (ibuf->len < packet_size)
 				/* still not having a complete packet */
 				return;
@@ -175,7 +176,7 @@ ic_proxy_ibuf_push(ICProxyIBuf *ibuf,
 	{
 		packet_size = ibuf->get_packet_size(data);
 
-		if (packet_size <= size)
+		if (packet_size > 0 && packet_size <= size)
 		{
 			/* got a complete pkt */
 			callback(opaque, data, packet_size);
diff --git a/contrib/interconnect/proxy/ic_proxy_peer.c b/contrib/interconnect/proxy/ic_proxy_peer.c
index 46bca00154..0aa16e6ea2 100644
--- a/contrib/interconnect/proxy/ic_proxy_peer.c
+++ b/contrib/interconnect/proxy/ic_proxy_peer.c
@@ -154,7 +154,10 @@ ic_proxy_peer_update_name(ICProxyPeer *peer)
 static void
 ic_proxy_peer_unregister(ICProxyPeer *peer)
 {
-	Assert(peer->dbid > 0);
+	/* invalid peer */
+	if (peer->dbid == IC_PROXY_INVALID_DBID ||
+		peer->content == IC_PROXY_INVALID_CONTENT)
+		return;
 
 	if (ic_proxy_peers[peer->dbid] == peer)
 	{
@@ -538,6 +541,9 @@ ic_proxy_peer_on_hello_pkt(void *opaque, const void *data, uint16 size)
 	ICProxyPeer *peer = opaque;
 	ICProxyKey	key;
 
+	/* we only expect one hello message */
+	uv_read_stop((uv_stream_t *) &peer->tcp);
+
 	/* sanity check: drop the packet with incorrect magic number */
 	if (!ic_proxy_pkt_is_valid(pkt))
 	{
@@ -547,9 +553,6 @@ ic_proxy_peer_on_hello_pkt(void *opaque, const void *data, uint16 size)
 		return;
 	}
 
-	/* we only expect one hello message */
-	uv_read_stop((uv_stream_t *) &peer->tcp);
-
 	ic_proxy_key_from_p2c_pkt(&key, pkt);
 
 	/* TODO: verify that old dbid and content are both set or invalid */
@@ -654,15 +657,6 @@ ic_proxy_peer_on_hello_ack_pkt(void *opaque, const void *data, uint16 size)
 		elog(ERROR, "ic-proxy: %s: received incomplete HELLO ACK: size = %d",
 					 peer->name, size);
 
-	/* sanity check: drop the packet with incorrect magic number */
-	if (!ic_proxy_pkt_is_valid(pkt))
-	{
-		elogif(gp_log_interconnect >= GPVARS_VERBOSITY_DEBUG, DEBUG1,
-			"ic-proxy: %s: received %s, dropping the invalid package (magic number mismatch)",
-					peer->name, ic_proxy_pkt_to_str(pkt));
-		return;
-	}
-
 	if (peer->state & IC_PROXY_PEER_STATE_RECEIVED_HELLO_ACK)
 	{
 		/*
@@ -680,6 +674,18 @@ ic_proxy_peer_on_hello_ack_pkt(void *opaque, const void *data, uint16 size)
 		return;
 	}
 
+	/* we only expect one hello ack message */
+	uv_read_stop((uv_stream_t *) &peer->tcp);
+
+	/* sanity check: drop the packet with incorrect magic number */
+	if (!ic_proxy_pkt_is_valid(pkt))
+	{
+		elogif(gp_log_interconnect >= GPVARS_VERBOSITY_DEBUG, DEBUG1,
+			"ic-proxy: %s: received %s, dropping the invalid package (magic number mismatch)",
+					peer->name, ic_proxy_pkt_to_str(pkt));
+		return;
+	}
+
 	if (!ic_proxy_pkt_is(pkt, IC_PROXY_MESSAGE_PEER_HELLO_ACK))
 		elog(ERROR, "ic-proxy: %s: received invalid HELLO ACK: %s",
 					 peer->name, ic_proxy_pkt_to_str(pkt));
@@ -688,9 +694,6 @@ ic_proxy_peer_on_hello_ack_pkt(void *opaque, const void *data, uint16 size)
 		elog(ERROR, "ic-proxy: %s: received invalid HELLO ACK: %s",
 					 peer->name, ic_proxy_pkt_to_str(pkt));
 
-	/* we only expect one hello ack message */
-	uv_read_stop((uv_stream_t *) &peer->tcp);
-
 	elogif(gp_log_interconnect >= GPVARS_VERBOSITY_DEBUG, DEBUG1,
 		   "ic-proxy: %s: received %s", peer->name, ic_proxy_pkt_to_str(pkt));
 
diff --git a/src/test/regress/expected/ic_proxy_socket.out b/src/test/regress/expected/ic_proxy_socket.out
new file mode 100644
index 0000000000..2687745e55
--- /dev/null
+++ b/src/test/regress/expected/ic_proxy_socket.out
@@ -0,0 +1,69 @@
+-- test invalid connection request (random bytes) to ic proxy:
+-- it should be dropped due to mismatch magic number
+-- start_ignore
+CREATE OR REPLACE LANGUAGE plpython3u;
+-- end_ignore
+CREATE OR REPLACE FUNCTION send_bytes_to_icproxy()
+    RETURNS VOID as $$
+import socket, struct, random
+
+# parse host and port from gp_interconnect_proxy_addresses
+icproxy_host = ""
+icproxy_port = -1
+try:
+    res = plpy.execute("show gp_interconnect_type;", 1)
+    if res[0]["gp_interconnect_type"] == "proxy":
+        res = plpy.execute("show gp_interconnect_proxy_addresses;", 1)
+        addrs = res[0]["gp_interconnect_proxy_addresses"]
+        addr = addrs.split(",")[1]
+        icproxy_host = addr.split(":")[2]
+        icproxy_port = int(addr.split(":")[3])
+except:
+    # not working on icproxy mode, just return
+    icproxy_host = ""
+    pass
+if icproxy_host == "":
+    return
+
+def ranstr(num):
+    H = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'
+    salt = ''
+    for i in range(num):
+        salt += random.choice(H)
+    return salt
+
+for i in range(10):
+    # send the random bytes
+    s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+    s.connect((icproxy_host, icproxy_port))
+    val = struct.pack('!i', 134591) # an invalid magic number
+    s.send(val)
+
+    message_len = random.randint(100, 10000)
+    message = ranstr(message_len).encode('utf-8')
+    val = struct.pack('!i', message_len)
+    s.send(val)
+    s.sendall(message)
+    s.close()
+
+$$ LANGUAGE plpython3u;
+create table PR_14998(i int);
+NOTICE:  Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 'i' as the Greenplum Database data distribution key for this table.
+HINT:  The 'DISTRIBUTED BY' clause determines the distribution of data. Make sure column(s) chosen are the optimal data distribution key to minimize skew.
+SELECT send_bytes_to_icproxy();
+ send_bytes_to_icproxy 
+-----------------------
+ 
+(1 row)
+
+-- the query hung here before the commit adding the magic number field in the ICProxyPkt
+-- reason: random bytes cause icproxy OOM or hang forever
+SET statement_timeout = 10000;
+select count(*) from PR_14998;
+ count 
+-------
+     0
+(1 row)
+
+RESET statement_timeout;
+drop table PR_14998;
diff --git a/src/test/regress/greenplum_schedule b/src/test/regress/greenplum_schedule
index f3159b779d..d3f31f7e4e 100755
--- a/src/test/regress/greenplum_schedule
+++ b/src/test/regress/greenplum_schedule
@@ -331,6 +331,9 @@ test: directory_table
 # test if motion sockets are created with the gp_segment_configuration.address
 test: motion_socket
 
+# test invalid connection to ic proxy when gp_interconnect_type='proxy'
+test: ic_proxy_socket
+
 # subtransaction overflow test
 test: subtrx_overflow
 
diff --git a/src/test/regress/sql/ic_proxy_socket.sql b/src/test/regress/sql/ic_proxy_socket.sql
new file mode 100644
index 0000000000..2d23358985
--- /dev/null
+++ b/src/test/regress/sql/ic_proxy_socket.sql
@@ -0,0 +1,58 @@
+-- test invalid connection request (random bytes) to ic proxy:
+-- it should be dropped due to mismatch magic number
+-- start_ignore
+CREATE OR REPLACE LANGUAGE plpython3u;
+-- end_ignore
+CREATE OR REPLACE FUNCTION send_bytes_to_icproxy()
+    RETURNS VOID as $$
+import socket, struct, random
+
+# parse host and port from gp_interconnect_proxy_addresses
+icproxy_host = ""
+icproxy_port = -1
+try:
+    res = plpy.execute("show gp_interconnect_type;", 1)
+    if res[0]["gp_interconnect_type"] == "proxy":
+        res = plpy.execute("show gp_interconnect_proxy_addresses;", 1)
+        addrs = res[0]["gp_interconnect_proxy_addresses"]
+        addr = addrs.split(",")[1]
+        icproxy_host = addr.split(":")[2]
+        icproxy_port = int(addr.split(":")[3])
+except:
+    # not working on icproxy mode, just return
+    icproxy_host = ""
+    pass
+if icproxy_host == "":
+    return
+
+def ranstr(num):
+    H = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'
+    salt = ''
+    for i in range(num):
+        salt += random.choice(H)
+    return salt
+
+for i in range(10):
+    # send the random bytes
+    s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+    s.connect((icproxy_host, icproxy_port))
+    val = struct.pack('!i', 134591) # an invalid magic number
+    s.send(val)
+
+    message_len = random.randint(100, 10000)
+    message = ranstr(message_len).encode('utf-8')
+    val = struct.pack('!i', message_len)
+    s.send(val)
+    s.sendall(message)
+    s.close()
+
+$$ LANGUAGE plpython3u;
+
+create table PR_14998(i int);
+SELECT send_bytes_to_icproxy();
+-- the query hung here before the commit adding the magic number field in the ICProxyPkt
+-- reason: random bytes cause icproxy OOM or hang forever
+SET statement_timeout = 10000;
+select count(*) from PR_14998;
+RESET statement_timeout;
+drop table PR_14998;


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


(cloudberry) 12/21: Remove dead hash agg GUCs

Posted by yj...@apache.org.
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 d181ca6a0a8e7aa4e5cb612f2509607d974f8ffc
Author: Adam Lee <ad...@vmware.com>
AuthorDate: Fri Jun 2 14:41:05 2023 +0800

    Remove dead hash agg GUCs
    
    Greenplum dropped the hybrid hash aggregation after 6X and switched to
    the upstream hash agg implementation, these GUCs are not actually used
    any more, remove them.
---
 src/backend/cdb/cdbvars.c         |  3 ---
 src/backend/utils/misc/guc_gp.c   | 37 -------------------------------------
 src/include/cdb/cdbvars.h         |  6 ------
 src/include/utils/sync_guc_name.h |  2 --
 4 files changed, 48 deletions(-)

diff --git a/src/backend/cdb/cdbvars.c b/src/backend/cdb/cdbvars.c
index e21fa88e76..751e447355 100644
--- a/src/backend/cdb/cdbvars.c
+++ b/src/backend/cdb/cdbvars.c
@@ -267,8 +267,6 @@ uint32		gp_interconnect_id = 0;
 double		gp_motion_cost_per_row = 0;
 int			gp_segments_for_planner = 0;
 
-int			gp_hashagg_default_nbatches = 32;
-
 bool		gp_adjust_selectivity_for_outerjoins = true;
 bool		gp_selectivity_damping_for_scans = false;
 bool		gp_selectivity_damping_for_joins = false;
@@ -277,7 +275,6 @@ bool		gp_enable_runtime_filter = false;
 bool		gp_selectivity_damping_sigsort = true;
 
 int			gp_hashjoin_tuples_per_bucket = 5;
-int			gp_hashagg_groups_per_bucket = 5;
 
 /* Analyzing aid */
 int			gp_motion_slice_noop = 0;
diff --git a/src/backend/utils/misc/guc_gp.c b/src/backend/utils/misc/guc_gp.c
index 7b2eaf4978..f37086e59f 100644
--- a/src/backend/utils/misc/guc_gp.c
+++ b/src/backend/utils/misc/guc_gp.c
@@ -86,7 +86,6 @@
 static bool check_optimizer(bool *newval, void **extra, GucSource source);
 static bool check_verify_gpfdists_cert(bool *newval, void **extra, GucSource source);
 static bool check_dispatch_log_stats(bool *newval, void **extra, GucSource source);
-static bool check_gp_hashagg_default_nbatches(int *newval, void **extra, GucSource source);
 static bool check_gp_workfile_compression(bool *newval, void **extra, GucSource source);
 
 /* Helper function for guc setter */
@@ -3950,28 +3949,6 @@ struct config_int ConfigureNamesInt_gp[] =
 		NULL, NULL, NULL
 	},
 
-	{
-		{"gp_hashagg_groups_per_bucket", PGC_USERSET, GP_ARRAY_TUNING,
-			gettext_noop("Target density of hashtable used by Hashagg during execution"),
-			gettext_noop("A smaller value will tend to produce larger hashtables, which increases agg performance"),
-			GUC_NOT_IN_SAMPLE | GUC_NO_SHOW_ALL
-		},
-		&gp_hashagg_groups_per_bucket,
-		5, 1, 25,
-		NULL, NULL, NULL
-	},
-
-	{
-		{"gp_hashagg_default_nbatches", PGC_USERSET, QUERY_TUNING_METHOD,
-			gettext_noop("Default number of batches for hashagg's (re-)spilling phases."),
-			gettext_noop("Must be a power of two."),
-			GUC_NO_SHOW_ALL | GUC_NOT_IN_SAMPLE
-		},
-		&gp_hashagg_default_nbatches,
-		32, 4, 1048576,
-		check_gp_hashagg_default_nbatches, NULL, NULL
-	},
-
 	{
 		{"gp_motion_slice_noop", PGC_USERSET, GP_ARRAY_TUNING,
 			gettext_noop("Make motion nodes in certain slices noop"),
@@ -5273,20 +5250,6 @@ check_dispatch_log_stats(bool *newval, void **extra, GucSource source)
 	return true;
 }
 
-bool
-check_gp_hashagg_default_nbatches(int *newval, void **extra, GucSource source)
-{
-	/* Must be a power of two */
-	if (0 == (*newval & (*newval - 1)))
-	{
-		return true;
-	}
-	else
-	{
-		GUC_check_errmsg("gp_hashagg_default_nbatches must be a power of two");
-		return false;
-	}
-}
 
 bool
 check_max_running_tasks(int *newval, void **extra, GucSource source)
diff --git a/src/include/cdb/cdbvars.h b/src/include/cdb/cdbvars.h
index 292786e37c..1d9c72db61 100644
--- a/src/include/cdb/cdbvars.h
+++ b/src/include/cdb/cdbvars.h
@@ -559,7 +559,6 @@ extern bool gp_adjust_selectivity_for_outerjoins;
  * Target density for hash-node (HJ).
  */
 extern int gp_hashjoin_tuples_per_bucket;
-extern int gp_hashagg_groups_per_bucket;
 
 /*
  * Damping of selectivities of clauses which pertain to the same base
@@ -680,11 +679,6 @@ extern int gp_segworker_relative_priority;
 /*  Max size of dispatched plans; 0 if no limit */
 extern int gp_max_plan_size;
 
-/* The default number of batches to use when the hybrid hashed aggregation
- * algorithm (re-)spills in-memory groups to disk.
- */
-extern int gp_hashagg_default_nbatches;
-
 /* Get statistics for partitioned parent from a child */
 extern bool 	gp_statistics_pullup_from_child_partition;
 
diff --git a/src/include/utils/sync_guc_name.h b/src/include/utils/sync_guc_name.h
index 2d50135500..566946698d 100644
--- a/src/include/utils/sync_guc_name.h
+++ b/src/include/utils/sync_guc_name.h
@@ -64,8 +64,6 @@
 		"gp_enable_runtime_filter",
 		"gp_enable_segment_copy_checking",
 		"gp_external_enable_filter_pushdown",
-		"gp_hashagg_default_nbatches",
-		"gp_hashagg_groups_per_bucket",
 		"gp_hashjoin_tuples_per_bucket",
 		"gp_ignore_error_table",
 		"gp_indexcheck_insert",


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


(cloudberry) 11/21: Fix an issue where we increment command counter incorrectly

Posted by yj...@apache.org.
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 c0124395eac2a0437609a4eaf92351d5ae10bf73
Author: Huansong Fu <fu...@gmail.com>
AuthorDate: Thu Jun 1 21:54:32 2023 -0700

    Fix an issue where we increment command counter incorrectly
    
    In commit d3dcb9bc97d8 we added an CCI in swap_relation_files() after
    we swapped AM. But that change should really be limited to AM change and
    nothing else.
    
    This is revealed by the previous commit where we allowed TAP test to
    run vacuum successfully. So, when we are running VACUUM FULL pg_attribute,
    we could have an errorneous state where the rebuilt pg_attribute does not
    contain the entries for the intermediate table whereas we are building table
    descriptor for the intermediate table as part of the CCI which requires
    pg_attribute entries for the intermediate table... In overall I think CCI
    during swapping relation should be discouraged but if not avoidable at least
    we shouldn't do that for critical catalogs.
---
 src/backend/commands/cluster.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index bdb03c7691..ee78fd3954 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -1365,6 +1365,15 @@ swap_relation_files(Oid r1, Oid r2, bool target_is_pg_class,
 		CatalogTupleUpdateWithInfo(relRelation, &reltup2->t_self, reltup2,
 								   indstate);
 		CatalogCloseIndexes(indstate);
+
+		/*
+		 * Increment counter to reflect the AM change as the caller might soon
+		 * build the new relation descriptor which expects consistent AM and aux
+		 * tables. This shouldn't be needed for other cases as of now, especially
+		 * not for critical catalogs such as pg_attribute.
+		 */
+		if (relform1->relam != relform2->relam)
+			CommandCounterIncrement();
 	}
 	else
 	{


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


(cloudberry) 21/21: Resolve two FIXMEs in extractPageInfo()

Posted by yj...@apache.org.
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 325df54863c218a8f05726163a5051df8b4af11b
Author: Huansong Fu <fu...@gmail.com>
AuthorDate: Mon Jun 5 10:34:15 2023 -0700

    Resolve two FIXMEs in extractPageInfo()
    
    There are three points in the two FIXMEs. Let's review them one by one:
    
    1. Should we return for RM_APPEND_ONLY_ID: confirmed that RM_APPEND_ONLY_ID
    doesn't have any registered buffers in all cases. But I don't think we need
    to return there because the `for` loop will just be a no-op. There are
    other types of WALs that don't have registered buffers either, and we do
    not need to treat AO ones specially.
    
    2. Should we take care of RM_DISTRIBUTEDLOG_ID: I don't think we need to
    because it should be treated in the same way as for other SLRU related
    WALs. Also, there's no buffer registered for it either.
    
    3. Make sure RM_BITMAP_ID works after we took the upstream WAL format revamp
    (commit 2c03216d8311): I think the current code looks fine and we do not need
    to do anything. Basically, we used to handle bitmap and some other WAL types
    in extractPageInfo specially because we needed to decode the data portion in
    the WAL first and get the block number from different corresponding structs.
    With the revamp we store block number in standardized format and decoding it
    becomes the same way between different WAL types. Bitmap WALs do not look to
    be any different.
    Also one more issue that's not mentioned by the FIXME is that, for bitmap we
    used to handle a different forknum than the MAIN_FORKNUM which is INIT_FORKNUM
    in some cases (stored in xlrec->bm_fork). Now we always only handle MAIN_FORKNUM
    in extractPageInfo() like any other cases. However, this is fine because all
    forks other than the MAIN_FORKNUM is going to be ignored in process_target_wal_block_change()
    anyway because there won't be the same source data file to be found (see comments
    for isRelDataFile()). I think this was also what happened when we tried to call
    pageinfo_add() for INIT_FORKNUM before the revamp. Now we just explicitly do
    not do that for non-main forknums.
---
 src/bin/pg_rewind/parsexlog.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c
index ce2b608a02..130d120f93 100644
--- a/src/bin/pg_rewind/parsexlog.c
+++ b/src/bin/pg_rewind/parsexlog.c
@@ -456,19 +456,7 @@ extractPageInfo(XLogReaderState *record)
 			 * and will copy the missing tail from remote system.
 			 */
 		}
-
-		/*
-		 * GPDB_95_MERGE_FIXME: should we just return here? there seems be no buffer
-		 * registered when xlog is inserted.
-		 */
 	}
-	/*
-	 * GPDB_95_MERGE_FIXME:
-	 * 1. should RM_DISTRIBUTEDLOG_ID be taken care of
-	 * 2. we used to have a special treat towards RM_BITMAP_ID, now we hope the
-	 * buffers are registered properly when the xlog is inserted. We need to
-	 * revisit here to make sure RM_BITMAP_ID works.
-	 */
 
 	for (block_id = 0; block_id <= record->max_block_id; block_id++)
 	{


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


(cloudberry) 16/21: Resolve GPDB_96_MERGE_FIXME in prepunion.c about recursive cte

Posted by yj...@apache.org.
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 ae976a6f0f2065888cf3d123bb158e7b172d989a
Author: xuejing zhao <80...@users.noreply.github.com>
AuthorDate: Tue Jun 6 10:16:46 2023 +0800

    Resolve GPDB_96_MERGE_FIXME in prepunion.c about recursive cte
    
    Remove GPDB_96_MERGE_FIXME in prepunion.c:
    GPDB_96_MERGE_FIXME: On coordinator, before the merge, more complicated logic was added in
    commit ad6a6067d9 to make the loci on the WorkTableScan and the RecursiveUnion correct.
    That was largely reverted as part of the merge, and things seem to be working with this much simpler thing,
    but I'm not sure if the logic is 100% correct now.
    
    On commit ad6a6067d9 we change locus of WorkTableScan from CdbLocusType_SegmentGeneral to
    CdbLocusType_SingleQE in create_worktablescan_path.
    Then add gather motion to non_recursive_path before create_recursiveunion_path , change locus of
    non_recursive_path from CdbLocusType_SegmentGeneral to CdbLocusType_SingleQE.
    
    Current implement is:
    Before create_worktablescan_path adding gather motion, changing locus of non_recursive_path from
    CdbLocusType_SegmentGeneral to CdbLocusType_SingleQE.
    Then locus of WorkTableScan use the same locus as non_recursive_path.
    
    We moved the implementation logic before create_worktablescan_path ,And this is more simple.
---
 src/backend/optimizer/prep/prepunion.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c
index fda13f0924..62e7f40867 100644
--- a/src/backend/optimizer/prep/prepunion.c
+++ b/src/backend/optimizer/prep/prepunion.c
@@ -493,11 +493,6 @@ generate_recursion_path(SetOperationStmt *setOp, PlannerInfo *root,
 	 * SegmentGeneral, the result of the join may end up having a different
 	 * locus.
 	 *
-	 * GPDB_96_MERGE_FIXME: On master, before the merge, more complicated
-	 * logic was added in commit ad6a6067d9 to make the loci on the WorkTableScan
-	 * and the RecursiveUnion correct. That was largely reverted as part of the
-	 * merge, and things seem to be working with this much simpler thing, but
-	 * I'm not sure if the logic is 100% correct now.
 	 */
 	if (CdbPathLocus_IsSegmentGeneral(lpath->locus) || CdbPathLocus_IsSegmentGeneralWorkers(lpath->locus))
 	{


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


(cloudberry) 20/21: Resolve a FIXME for gpcheckcat

Posted by yj...@apache.org.
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 c040da238a16fc158969d823cfde3b651295f028
Author: Huansong Fu <fu...@gmail.com>
AuthorDate: Tue Jun 6 13:21:03 2023 -0700

    Resolve a FIXME for gpcheckcat
    
    The false positive case the FIXME talks about has been resolved by
    commit 7d289b5460a, which removes the foreign key reference between
    pg_rewrite.ev_class and pg_attribute.attrelid. Therefore, we do not
    need to ignore pg_rewrite anymore.
    
    In 6X, this issue has been fixed using a different approach in #11732.
    The PR discussion also mentioned the problem of foreign key reference
    between pg_rewrite.ev_class and pg_attribute.attrelid. But it was fixed
    in a different way I guess because it's not easy to change catalog in 6X.
    
    The above PR also added a behave test case for 6X. Adding the same for 7X.
---
 gpMgmt/bin/gpcheckcat_modules/foreign_key_check.py | 11 -----------
 gpMgmt/test/behave/mgmt_utils/gpcheckcat.feature   |  1 +
 gpMgmt/test/behave/mgmt_utils/steps/mgmt_utils.py  |  7 +++++++
 3 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/gpMgmt/bin/gpcheckcat_modules/foreign_key_check.py b/gpMgmt/bin/gpcheckcat_modules/foreign_key_check.py
index da9272c6c1..549f88da2d 100644
--- a/gpMgmt/bin/gpcheckcat_modules/foreign_key_check.py
+++ b/gpMgmt/bin/gpcheckcat_modules/foreign_key_check.py
@@ -47,17 +47,6 @@ class ForeignKeyCheck:
         if catname in COORDINATOR_ONLY_TABLES:
             return
 
-        # GPDB_12_MERGE_FIXME: Left outer join query generated below
-        # joins pg_rewrite and pg_attribute on ev_class == attrelid.
-        # This reports false positives (presence of null tuples) for
-        # the case when a view is defined with no columns.  The query
-        # should ideally exclude such views by adding
-        # pg_class.relnatts to the join.  But that's not possible
-        # without significantly changing the existing query generation
-        # logic.
-        if catname == 'pg_rewrite':
-            return
-
         # skip shared/non-shared tables
         if self.shared_option:
             if re.match("none", self.shared_option, re.I) and isShared:
diff --git a/gpMgmt/test/behave/mgmt_utils/gpcheckcat.feature b/gpMgmt/test/behave/mgmt_utils/gpcheckcat.feature
index d16951a2b6..6af4494482 100644
--- a/gpMgmt/test/behave/mgmt_utils/gpcheckcat.feature
+++ b/gpMgmt/test/behave/mgmt_utils/gpcheckcat.feature
@@ -253,6 +253,7 @@ Feature: gpcheckcat tests
         Given database "fkey_db" is dropped and recreated
         And the path "gpcheckcat.repair.*" is removed from current working directory
         And there is a "heap" table "gpadmin_tbl" in "fkey_db" with data
+        And there is a view without columns in "fkey_db"
         When the entry for the table "gpadmin_tbl" is removed from "pg_catalog.pg_class" with key "oid" in the database "fkey_db"
         Then the user runs "gpcheckcat -E -R missing_extraneous fkey_db"
         And gpcheckcat should print "Name of test which found this issue: missing_extraneous_pg_class" to stdout
diff --git a/gpMgmt/test/behave/mgmt_utils/steps/mgmt_utils.py b/gpMgmt/test/behave/mgmt_utils/steps/mgmt_utils.py
index deefd608a0..e836abc0f9 100644
--- a/gpMgmt/test/behave/mgmt_utils/steps/mgmt_utils.py
+++ b/gpMgmt/test/behave/mgmt_utils/steps/mgmt_utils.py
@@ -2236,6 +2236,13 @@ def impl(context, tabletype, tablename, dbname):
 def impl(context, tabletype, table_name, dbname):
     create_partition(context, tablename=table_name, storage_type=tabletype, dbname=dbname, with_data=True)
 
+@given('there is a view without columns in "{dbname}"')
+@then('there is a view without columns in "{dbname}"')
+@when('there is a view without columns in "{dbname}"')
+def impl(context, dbname):
+    with dbconn.connect(dbconn.DbURL(dbname=dbname), unsetSearchPath=False) as conn:
+        dbconn.execSQL(conn, "create view v_no_cols as select;")
+        conn.commit()
 
 @then('read pid from file "{filename}" and kill the process')
 @when('read pid from file "{filename}" and kill the process')


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


(cloudberry) 03/21: Error if partitioned table passed to gp_toolkit.__gp_aocsseg(oid)

Posted by yj...@apache.org.
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 2c290bc66d4805f9e30fc723d1633b13bef593d3
Author: Brent Doil <bd...@vmware.com>
AuthorDate: Sat Mar 18 21:48:36 2023 -0400

    Error if partitioned table passed to gp_toolkit.__gp_aocsseg(oid)
    
    The macro RelationIsAoCols(relation) only checks relam, meaning an
    AOCS partition parent, which has no backing storage, will return true.
    
    The set of GPDB specific macros defined in rel.h needs a refactor, but they are
    widely used and any changes need to be carefully considered.
    
    In the meantime, add a relkind check on the relation in gp_toolkit.__gp_aocsseg(oid)
    so we error out before trying to parse a NULL rd_appendonly entry.
    
    Authored-by: Brent Doil <bd...@vmware.com>
---
 src/backend/access/aocs/aocssegfiles.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/backend/access/aocs/aocssegfiles.c b/src/backend/access/aocs/aocssegfiles.c
index 0ab1a4ea94..ffe1fe996c 100644
--- a/src/backend/access/aocs/aocssegfiles.c
+++ b/src/backend/access/aocs/aocssegfiles.c
@@ -1242,7 +1242,8 @@ gp_aocsseg_internal(PG_FUNCTION_ARGS, Oid aocsRelOid)
 		context->aocsRelOid = aocsRelOid;
 
 		aocsRel = heap_open(aocsRelOid, AccessShareLock);
-		if (!RelationIsAoCols(aocsRel))
+		if (!RelationIsAoCols(aocsRel) ||
+				aocsRel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
 			ereport(ERROR,
 					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
 					 errmsg("'%s' is not an append-only columnar relation",


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


(cloudberry) 09/21: Remove FIXME: no longer to pass paramExecTypes as a param (7X) (#15649)

Posted by yj...@apache.org.
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 261f6bdffef46807953d095c953f8259a56eb9df
Author: Yongtao Huang <99...@users.noreply.github.com>
AuthorDate: Wed May 31 17:31:15 2023 +0800

    Remove FIXME: no longer to pass paramExecTypes as a param (7X) (#15649)
    
    Long log:
    Postgres added `paramExecTypes` in struct `PlannedStmt` at commit [e64861c7](https://github.com/postgres/postgres/commit/e64861c7).
    In gpdb 7X, we have queryDesc->plannedstmt->paramExecTypes,
    so no need to pass var `paramExecTypes` as a param.
    
    Signed-off-by: Yongtao Huang <yo...@vmware.com>
---
 src/backend/cdb/dispatcher/cdbdisp_query.c | 25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)

diff --git a/src/backend/cdb/dispatcher/cdbdisp_query.c b/src/backend/cdb/dispatcher/cdbdisp_query.c
index 82d7fbe093..626cb6465a 100644
--- a/src/backend/cdb/dispatcher/cdbdisp_query.c
+++ b/src/backend/cdb/dispatcher/cdbdisp_query.c
@@ -125,12 +125,10 @@ static List *formIdleSegmentIdList(void);
 
 static bool param_walker(Node *node, ParamWalkerContext *context);
 static Oid	findParamType(List *params, int paramid);
-static Bitmapset *getExecParamsToDispatch(PlannedStmt *stmt, ParamExecData *intPrm,
-										  List **paramExecTypes);
+static Bitmapset *getExecParamsToDispatch(PlannedStmt *stmt, ParamExecData *intPrm);
 static SerializedParams *serializeParamsForDispatch(QueryDesc *queryDesc,
 													ParamListInfo externParams,
 													ParamExecData *execParams,
-													List *paramExecTypes,
 													Bitmapset *sendParams);
 
 
@@ -145,8 +143,6 @@ static SerializedParams *serializeParamsForDispatch(QueryDesc *queryDesc,
  * 'sendParams' indicates which parameters are included and 'execParams'
  * contains their values. 'paramExecTypes' is a list indexed by paramid,
  * containing the datatype OID of each parameter.
- * GPDB_11_MERGE_FIXME: In PostgreSQL v11, we have paramExecTypes in
- * PlannedStmt, so it will no longer be necessary to pass it as a param.
  *
  * If cancelOnError is true, then any dispatching error, a cancellation
  * request from the client, or an error from any of the associated QEs,
@@ -185,7 +181,6 @@ CdbDispatchPlan(struct QueryDesc *queryDesc,
 {
 	PlannedStmt *stmt;
 	bool		is_SRI = false;
-	List	   *paramExecTypes;
 	Bitmapset  *sendParams;
 
 	Assert(Gp_role == GP_ROLE_DISPATCH);
@@ -245,11 +240,11 @@ CdbDispatchPlan(struct QueryDesc *queryDesc,
 	 * values that need to be included with the query. Then serialize them,
 	 * and also any PARAM_EXTERN parameters.
 	 */
-	sendParams = getExecParamsToDispatch(stmt, execParams, &paramExecTypes);
+	sendParams = getExecParamsToDispatch(stmt, execParams);
 	queryDesc->ddesc->paramInfo =
 		serializeParamsForDispatch(queryDesc,
 								   queryDesc->params,
-								   execParams, paramExecTypes, sendParams);
+								   execParams, sendParams);
 
 	/*
 	 * Cursor queries and bind/execute path queries don't run on the
@@ -1468,7 +1463,6 @@ static SerializedParams *
 serializeParamsForDispatch(QueryDesc *queryDesc,
 						   ParamListInfo externParams,
 						   ParamExecData *execParams,
-						   List *paramExecTypes,
 						   Bitmapset *sendParams)
 {
 	SerializedParams *result = makeNode(SerializedParams);
@@ -1517,7 +1511,7 @@ serializeParamsForDispatch(QueryDesc *queryDesc,
 	/* materialize Exec params */
 	if (!bms_is_empty(sendParams))
 	{
-		int			numExecParams = list_length(paramExecTypes);
+		int			numExecParams = list_length(queryDesc->plannedstmt->paramExecTypes);
 		int			x;
 
 		result->nExecParams = numExecParams;
@@ -1528,7 +1522,7 @@ serializeParamsForDispatch(QueryDesc *queryDesc,
 		{
 			ParamExecData *prm = &execParams[x];
 			SerializedParamExecData *sprm = &result->execParams[x];
-			Oid			ptype = list_nth_oid(paramExecTypes, x);
+			Oid			ptype = list_nth_oid(queryDesc->plannedstmt->paramExecTypes, x);
 
 			sprm->value = prm->value;
 			sprm->isnull = prm->isnull;
@@ -1563,8 +1557,7 @@ serializeParamsForDispatch(QueryDesc *queryDesc,
  * in the EState->es_param_exec_vals array.
  */
 static Bitmapset *
-getExecParamsToDispatch(PlannedStmt *stmt, ParamExecData *intPrm,
-						List **paramExecTypes)
+getExecParamsToDispatch(PlannedStmt *stmt, ParamExecData *intPrm)
 {
 	ParamWalkerContext context;
 	int			i;
@@ -1573,10 +1566,8 @@ getExecParamsToDispatch(PlannedStmt *stmt, ParamExecData *intPrm,
 	Bitmapset  *sendParams = NULL;
 
 	if (nIntPrm == 0)
-	{
-		*paramExecTypes = NIL;
 		return NULL;
-	}
+	
 	Assert(intPrm != NULL);		/* So there must be some internal parameters. */
 
 	/*
@@ -1611,12 +1602,10 @@ getExecParamsToDispatch(PlannedStmt *stmt, ParamExecData *intPrm,
 	 * Now set the bit corresponding to each init plan param. Use the datatype
 	 * info harvested above.
 	 */
-	*paramExecTypes = NIL;
 	for (i = 0; i < nIntPrm; i++)
 	{
 		Oid			paramType = findParamType(context.params, i);
 
-		*paramExecTypes = lappend_oid(*paramExecTypes, paramType);
 		if (paramType != InvalidOid)
 		{
 			sendParams = bms_add_member(sendParams, i);


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


(cloudberry) 17/21: Remove GPDB_96_MERGE_FIXME in subselect_gp*. (#15587)

Posted by yj...@apache.org.
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 8c6a3533497f16e9619740343e174453e773d4d2
Author: Wenlin Zhang <zw...@vmware.com>
AuthorDate: Wed Jun 7 10:44:15 2023 +0800

    Remove GPDB_96_MERGE_FIXME in subselect_gp*. (#15587)
    
    The commit https://github.com/greenplum-db/gpdb/commit/50239de1810692256fe2c9aaf6474f082668576f
    introduced the Fixme, the explain output will print the same subplan twice.
    
    After the commit https://github.com/greenplum-db/gpdb/commit/38d88155520790b268b1a380caa9a2d9508edc66
    merged, the fixme has been resolved, so remove the fixme directly.
---
 src/test/regress/expected/subselect_gp.out           | 3 ---
 src/test/regress/expected/subselect_gp_optimizer.out | 3 ---
 src/test/regress/sql/subselect_gp.sql                | 3 ---
 3 files changed, 9 deletions(-)

diff --git a/src/test/regress/expected/subselect_gp.out b/src/test/regress/expected/subselect_gp.out
index 9aff68f249..cd3c95daa7 100644
--- a/src/test/regress/expected/subselect_gp.out
+++ b/src/test/regress/expected/subselect_gp.out
@@ -1395,9 +1395,6 @@ SELECT bar_s.c FROM bar_s, foo_s WHERE foo_s.b = (SELECT max(i) FROM baz_s WHERE
 
 -- Same as above, but with another subquery, so it must use a SubPlan. There
 -- are two references to the same SubPlan in the plan, on different slices.
--- GPDB_96_MERGE_FIXME: this EXPLAIN output should become nicer-looking once we
--- merge upstream commit 4d042999f9, to suppress the SubPlans from being
--- printed twice.
 explain SELECT bar_s.c FROM bar_s, foo_s WHERE foo_s.b = (SELECT max(i) FROM baz_s WHERE bar_s.c = 9) AND foo_s.b = (select bar_s.d::int4);
                                                            QUERY PLAN                                                            
 ---------------------------------------------------------------------------------------------------------------------------------
diff --git a/src/test/regress/expected/subselect_gp_optimizer.out b/src/test/regress/expected/subselect_gp_optimizer.out
index c729b5e2fd..824f9e1a67 100644
--- a/src/test/regress/expected/subselect_gp_optimizer.out
+++ b/src/test/regress/expected/subselect_gp_optimizer.out
@@ -1410,9 +1410,6 @@ SELECT bar_s.c FROM bar_s, foo_s WHERE foo_s.b = (SELECT max(i) FROM baz_s WHERE
 
 -- Same as above, but with another subquery, so it must use a SubPlan. There
 -- are two references to the same SubPlan in the plan, on different slices.
--- GPDB_96_MERGE_FIXME: this EXPLAIN output should become nicer-looking once we
--- merge upstream commit 4d042999f9, to suppress the SubPlans from being
--- printed twice.
 explain SELECT bar_s.c FROM bar_s, foo_s WHERE foo_s.b = (SELECT max(i) FROM baz_s WHERE bar_s.c = 9) AND foo_s.b = (select bar_s.d::int4);
                                                       QUERY PLAN                                                       
 -----------------------------------------------------------------------------------------------------------------------
diff --git a/src/test/regress/sql/subselect_gp.sql b/src/test/regress/sql/subselect_gp.sql
index bdc4346274..b8ba35930b 100644
--- a/src/test/regress/sql/subselect_gp.sql
+++ b/src/test/regress/sql/subselect_gp.sql
@@ -673,9 +673,6 @@ SELECT bar_s.c FROM bar_s, foo_s WHERE foo_s.b = (SELECT max(i) FROM baz_s WHERE
 
 -- Same as above, but with another subquery, so it must use a SubPlan. There
 -- are two references to the same SubPlan in the plan, on different slices.
--- GPDB_96_MERGE_FIXME: this EXPLAIN output should become nicer-looking once we
--- merge upstream commit 4d042999f9, to suppress the SubPlans from being
--- printed twice.
 explain SELECT bar_s.c FROM bar_s, foo_s WHERE foo_s.b = (SELECT max(i) FROM baz_s WHERE bar_s.c = 9) AND foo_s.b = (select bar_s.d::int4);
 SELECT bar_s.c FROM bar_s, foo_s WHERE foo_s.b = (SELECT max(i) FROM baz_s WHERE bar_s.c = 9) AND foo_s.b = (select bar_s.d::int4);
 


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


(cloudberry) 01/21: Fix flaky idle_gang_cleaner case (#15228)

Posted by yj...@apache.org.
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 87ab71c51f1ce495937214e06f2a4634f3d262cd
Author: QingMa <ma...@vmware.com>
AuthorDate: Tue Mar 21 14:24:34 2023 +0800

    Fix flaky idle_gang_cleaner case (#15228)
    
    After calling `cleanupAllGangs()`, there is a chance of encountering
    the FATAL "writer segworker group shared snapshot collision" error
    when executing a query. The reason is that if the writer gang cannot
    exit before a new command starts, it will find the same session id
    in the shared snapshot and collision will happen. Additionally, since
    the session only has the writer gangs before executing the target
    query, there is no need to call `cleanupAllGangs()` before executing
    the query.
---
 src/test/isolation2/input/idle_gang_cleaner.source  | 3 ---
 src/test/isolation2/output/idle_gang_cleaner.source | 7 -------
 2 files changed, 10 deletions(-)

diff --git a/src/test/isolation2/input/idle_gang_cleaner.source b/src/test/isolation2/input/idle_gang_cleaner.source
index 1ad5365b90..0f65a3224d 100644
--- a/src/test/isolation2/input/idle_gang_cleaner.source
+++ b/src/test/isolation2/input/idle_gang_cleaner.source
@@ -4,8 +4,6 @@
 -- clean up the idle writer gangs after the timeout, 
 -- no snapshot collision error should occur.
 
-CREATE OR REPLACE FUNCTION cleanupAllGangs() RETURNS BOOL
-AS '@abs_builddir@/../regress/regress.so', 'cleanupAllGangs' LANGUAGE C;
 set gp_vmem_idle_resource_timeout to '0.5s';
 set gp_snapshotadd_timeout to 0;
 
@@ -13,7 +11,6 @@ create table target_session_id_t(target_session_id int) DISTRIBUTED REPLICATED;
 insert into target_session_id_t values(current_setting('gp_session_id')::int);
 
 create table idle_gang_cleaner_t (c1 int, c2 int);
-select cleanupAllGangs();
 
 0U: select gp_inject_fault('proc_kill', 'suspend', 2, target_session_id) from target_session_id_t;
 
diff --git a/src/test/isolation2/output/idle_gang_cleaner.source b/src/test/isolation2/output/idle_gang_cleaner.source
index 9966013142..7510ccd1b5 100644
--- a/src/test/isolation2/output/idle_gang_cleaner.source
+++ b/src/test/isolation2/output/idle_gang_cleaner.source
@@ -4,8 +4,6 @@
 -- clean up the idle writer gangs after the timeout,
 -- no snapshot collision error should occur.
 
-CREATE OR REPLACE FUNCTION cleanupAllGangs() RETURNS BOOL AS '@abs_builddir@/../regress/regress@DLSUFFIX@', 'cleanupAllGangs' LANGUAGE C;
-CREATE
 set gp_vmem_idle_resource_timeout to '0.5s';
 SET
 set gp_snapshotadd_timeout to 0;
@@ -18,11 +16,6 @@ INSERT 1
 
 create table idle_gang_cleaner_t (c1 int, c2 int);
 CREATE
-select cleanupAllGangs();
- cleanupallgangs 
------------------
- t               
-(1 row)
 
 0U: select gp_inject_fault('proc_kill', 'suspend', 2, target_session_id) from target_session_id_t;
  gp_inject_fault 


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


(cloudberry) 19/21: Fix GPDB_96_MERGE_FIXME: verify walker works on Sequence node (in nodeFuncs.c)

Posted by yj...@apache.org.
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 04d2a5a203543221bbb83918cf76c7fde4c1ab96
Author: Tao Tang <ta...@vmware.com>
AuthorDate: Wed Jun 7 11:53:51 2023 -0400

    Fix GPDB_96_MERGE_FIXME: verify walker works on Sequence node (in nodeFuncs.c)
    
    Sequence is GPDB-special node generated by ORCA. It returns tuples from the
    last child node only. The walkers in `planstate_tree_walker` to be verified are:
    1. walkers in execParallel.c:
       - no verification required as gp never touch them.
    2. ExplainPreScanNode in explain.c:
       - it works. no rels in Sequence so no switch-case entry.
    3. ExecShutdownNode in execProcnode.c:
       - it works, no special shutdown ops for Sequence
---
 src/backend/nodes/nodeFuncs.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index e0cb194fee..e1538de8b9 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -4360,7 +4360,6 @@ planstate_tree_walker(PlanState *planstate,
 									   walker, context))
 				return true;
 			break;
-		/* GPDB_96_MERGE_FIXME: verify walker works on Sequence node */
 		case T_Sequence:
 			if (planstate_walk_members(((SequenceState *) planstate)->subplans,
 									   ((SequenceState *) planstate)->numSubplans,


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


(cloudberry) 02/21: Fixed the string comparison warning

Posted by yj...@apache.org.
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 8e49f0b43014490bae4ec2659715e343db38f920
Author: Annpurna Shahani <as...@vmware.com>
AuthorDate: Sat Mar 4 12:59:21 2023 +0530

    Fixed the string comparison warning
    
    * Due to commit eb4db09544bcb8b6ea52f47effe8e30b0f736a97 string comparison
      was giving below warning, fixed the string comparison code
    
      pg_rewind.c: In function ‘main’:
      pg_rewind.c:318:75: warning: comparison with string literal results in unspecified behavior [-Waddress]
         318 |  while(getenv("SUSPEND_PG_REWIND") != NULL && getenv("SUSPEND_PG_REWIND") == "true")
             |
    
    * Now when SUSPEND_PG_REWIND is set to true, pg_rewind process will sleep
      for 120 seconds (enough for validating the skip pg_rewind warning in testcase)
    
    * Also placed the suspend pg_rewind code block under #ifdef FAULT_INJECTOR to
      avoid shipping this in production build as suggested in
      https://github.com/greenplum-db/gpdb/pull/14476#discussion_r1110459120
---
 src/bin/pg_rewind/pg_rewind.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c
index f366b0e26b..b366bf3e32 100644
--- a/src/bin/pg_rewind/pg_rewind.c
+++ b/src/bin/pg_rewind/pg_rewind.c
@@ -352,6 +352,21 @@ main(int argc, char **argv)
 
 	sanityChecks();
 
+#ifdef FAULT_INJECTOR
+	/*
+	 * SUSPEND_PG_REWIND is used for testing purposes. If set to true, the pg_rewind process will be
+	 * suspended for 120 seconds, so that it's entry can be checked in the pg_stat_activity table.
+	 * The goal being that we can run another instance of gprecoverseg and assert that it ignores
+	 * recovery for segments that already have an active pg_rewind process.
+	 */
+	char* suspend_pg_rewind = getenv("SUSPEND_PG_REWIND");
+	if(suspend_pg_rewind != NULL && strcmp(suspend_pg_rewind, "true") == 0)
+	{
+		pg_log_info("pg_rewind suspended");
+		sleep(120);
+	}
+#endif
+
 	/*
 	 * Find the common ancestor timeline between the clusters.
 	 *


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


(cloudberry) 15/21: Change key `log` to UNRESERVED_KEYWORD (#15606)

Posted by yj...@apache.org.
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 6e94aaf727f2d72854258ad762be03cc5bed492a
Author: Xiaoran Wang <wx...@vmware.com>
AuthorDate: Wed May 31 15:12:00 2023 +0800

    Change key `log` to UNRESERVED_KEYWORD (#15606)
    
    * Change key `log` to UNRESERVED_KEYWORD
    pgaudit uses log as a guc name: pguadit.log, but gpdb sets it as TYPE_FUNC_NAME_KEYWORD which means it is a reserved keyword except for function and type name. So when setting pgaudit.log, an error like "syntax error" will be reported.  So change it to UNRESERVED_KEYWORD to make pgaudit.log work.
---
 src/backend/parser/gram.y   | 2 +-
 src/include/parser/kwlist.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 3c76124fad..20e00976b2 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -19655,6 +19655,7 @@ unreserved_keyword:
 			| LOCK_P
 			| LOCKED
 			| LOCUS
+			| LOG_P
 			| LOGGED
 			| MAPPING
 			| MASTER
@@ -20262,7 +20263,6 @@ type_func_name_keyword:
 			| JOIN
 			| LEFT
 			| LIKE
-			| LOG_P
 			| NATURAL
 			| NOTNULL
 			| OUTER_P
diff --git a/src/include/parser/kwlist.h b/src/include/parser/kwlist.h
index aef5b14483..f4862691f2 100644
--- a/src/include/parser/kwlist.h
+++ b/src/include/parser/kwlist.h
@@ -285,7 +285,7 @@ PG_KEYWORD("location", LOCATION, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("lock", LOCK_P, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("locked", LOCKED, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("locus", LOCUS, UNRESERVED_KEYWORD, BARE_LABEL)
-PG_KEYWORD("log", LOG_P, TYPE_FUNC_NAME_KEYWORD, BARE_LABEL)
+PG_KEYWORD("log", LOG_P, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("logged", LOGGED, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("mapping", MAPPING, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("master", MASTER, UNRESERVED_KEYWORD, BARE_LABEL)      /* GPDB */


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


(cloudberry) 05/21: Fix typos.

Posted by yj...@apache.org.
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 cc592337e019eca6fdbe481756d51059f63a420d
Author: Zhang Mingli <av...@gmail.com>
AuthorDate: Fri Mar 24 17:20:04 2023 +0800

    Fix typos.
    
    Authored-by: Zhang Mingli avamingli@gmail.com
---
 src/backend/cdb/cdbpath.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/backend/cdb/cdbpath.c b/src/backend/cdb/cdbpath.c
index 4b832f0cbe..bcea548e96 100644
--- a/src/backend/cdb/cdbpath.c
+++ b/src/backend/cdb/cdbpath.c
@@ -2185,7 +2185,7 @@ cdbpath_motion_for_join(PlannerInfo *root,
 			CdbPathLocus_MakeReplicated(&small_rel->move_to,
 										CdbPathLocus_NumSegments(large_rel->locus), 0);
 
-		/* Replicate largeer rel if cheaper than redistributing both rels. */
+		/* Replicate larger rel if cheaper than redistributing both rels. */
 		else if (!large_rel->require_existing_order &&
 				 large_rel->ok_to_replicate &&
 				 (large_rel->bytes * CdbPathLocus_NumSegments(small_rel->locus) <
@@ -3038,7 +3038,7 @@ cdbpath_motion_for_parallel_join(PlannerInfo *root,
 	 * And in the function cdbpathlocus_join there is a rule:
 	 * <any locus type> join <Replicated> => any locus type
 	 * Proof by contradiction, it shows that when code arrives here,
-	 * is is impossible that any of the two input paths' locus
+	 * it is impossible that any of the two input paths' locus
 	 * is Replicated. So we add asserts here.
 	 */
 	Assert(!CdbPathLocus_IsReplicated(outer.locus));


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