Fix: fix ORCA bugs and tests

Author: liushengsong

.github/workflows/pg16-merge-validation.yml

@@ -423,7 +423,7 @@ jobs:
             if ! su - gpadmin -c "
               cd ${SRC_DIR}
               export LD_LIBRARY_PATH=${INSTALL_PREFIX}/lib:${LD_LIBRARY_PATH}
-              CFLAGS='-O0 -g3' ./configure \
+              CFLAGS='-g3' ./configure \
                 --prefix=${INSTALL_PREFIX} \
                 --with-pgport=5432 \
                 --enable-cassert \

src/Makefile

@@ -16,23 +16,6 @@ include Makefile.mock
 # In PostgreSQL, libpqwalreceiver is linked into a separate shared library.
 # In GPDB, it's linked as a normal object, check src/backend/replication/Makefile.
 # Remove backend/replication/libpqwalreceiver from SUBDIRS here.
-#SUBDIRS = \
-#	common \
-#	port \
-#	timezone \
-#	backend \
-#	backend/utils/mb/conversion_procs \
-#	backend/snowball \
-#	include \
-#	interfaces \
-#	backend/replication/pgoutput \
-#	fe_utils \
-#	bin
-#	pl \
-#	makefiles \
-#	test/regress \
-#	test/isolation \
-#	test/perl
 
 SUBDIRS = \
 	common \
@@ -47,7 +30,10 @@ SUBDIRS = \
 	fe_utils \
 	bin \
 	pl \
-	test/regress
+	makefiles \
+	test/regress \
+	test/isolation \
+	test/perl
 
 ifeq ($(with_llvm), yes)
 SUBDIRS += backend/jit/llvm

src/backend/commands/matview.c

@@ -2340,6 +2340,7 @@ get_prestate_rte(RangeTblEntry *rte, MV_TriggerTable *table,
 	rte->relkind = 0;
 	rte->rellockmode = 0;
 	rte->tablesample = NULL;
+	rte->perminfoindex = 0;		/* subquery RTE does not need permission check */
 	rte->inh = false;			/* must not be set for a subquery */
 
 	return rte;
@@ -2403,6 +2404,7 @@ replace_rte_with_delta(RangeTblEntry *rte, MV_TriggerTable *table, bool is_new,
 	rte->relkind = 0;
 	rte->rellockmode = 0;
 	rte->tablesample = NULL;
+	rte->perminfoindex = 0;		/* subquery RTE does not need permission check */
 	rte->inh = false;			/* must not be set for a subquery */
 
 	return rte;

src/backend/executor/nodeSplitMerge.c

@@ -146,7 +146,10 @@ MergeTupleTableSlot(TupleTableSlot *slot, SplitMerge *plannode, SplitMergeState
 		/* Compute segment ID for the new row */
 		int32		target_seg;
 
-		target_seg = evalHashKey(node, newslot->tts_values, newslot->tts_isnull);
+		if (node->cdbhash)
+			target_seg = evalHashKey(node, newslot->tts_values, newslot->tts_isnull);
+		else
+			target_seg = cdbhashrandomseg(plannode->numHashSegments);
 
 		slot->tts_values[node->segid_attno - 1] = Int32GetDatum(target_seg);
 		slot->tts_isnull[node->segid_attno - 1] = false;

src/backend/gpopt/gpdbwrappers.cpp

@@ -34,6 +34,7 @@
 #include "catalog/pg_collation.h"
 extern "C" {
 #include "access/amapi.h"
+#include "commands/defrem.h"
 #include "access/external.h"
 #include "access/genam.h"
 #include "catalog/pg_inherits.h"
@@ -1960,8 +1961,8 @@ gpdb::GetMVNDistinct(Oid stat_oid)
 {
 	GP_WRAP_START;
 	{
-		/* CBDB_16_MERGE: xxx: do we need ihn = true in any case? */
-		return statext_ndistinct_load(stat_oid, false);
+		bool inh = has_subclass(StatisticsGetRelation(stat_oid, false));
+		return statext_ndistinct_load(stat_oid, inh);
 	}
 	GP_WRAP_END;
 }
@@ -1971,7 +1972,8 @@ gpdb::GetMVDependencies(Oid stat_oid)
 {
 	GP_WRAP_START;
 	{
-		return statext_dependencies_load(stat_oid, false, true);
+		bool inh = has_subclass(StatisticsGetRelation(stat_oid, false));
+		return statext_dependencies_load(stat_oid, inh, true);
 	}
 	GP_WRAP_END;
 }
@@ -2818,11 +2820,12 @@ gpdb::TestexprIsHashable(Node *testexpr, List *param_ids)
 }
 
 RTEPermissionInfo *
-gpdb::GetRTEPermissionInfo(List *rteperminfos,
-											   const RangeTblEntry *rte)
+gpdb::GetRTEPermissionInfo(List *rteperminfos, const RangeTblEntry *rte)
 {
 	GP_WRAP_START;
 	{
+		// Cast away const: upstream getRTEPermissionInfo() only reads
+		// rte->perminfoindex and rte->relid but its signature lacks const.
 		return getRTEPermissionInfo(rteperminfos, (RangeTblEntry *) rte);
 	}
 	GP_WRAP_END;

src/backend/gpopt/translate/CContextDXLToPlStmt.cpp

@@ -594,16 +594,16 @@ CContextDXLToPlStmt::GetRTEIndexByAssignedQueryId(
 
 //---------------------------------------------------------------------------
 //	@function:
-//		CContextDXLToPlStmt::AddPerfmInfo
+//		CContextDXLToPlStmt::AddPermInfo
 //
 //	@doc:
-//		Add a Perfission Info list entry
+//		Add a Permission Info list entry
 //
 //---------------------------------------------------------------------------
 void
-CContextDXLToPlStmt::AddPerfmInfo(RTEPermissionInfo *pi)
+CContextDXLToPlStmt::AddPermInfo(RTEPermissionInfo *pi)
 {
-	// add rte to rtable entries list
+	// add permission info to list
 	m_perminfo_list = gpdb::LAppend(m_perminfo_list, pi);
 }
 

src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp

@@ -659,9 +659,24 @@ CTranslatorDXLToPlStmt::TranslateDXLTblScan(
 
 		// The postgres_fdw wrapper does not support row level security. So
 		// passing only the query_quals while creating the foreign scan node.
+		//
+		// BuildForeignScan internally calls build_simple_rel which looks up
+		// RTEPermissionInfo via root->parse->rteperminfos.  The RTE here was
+		// newly created by ORCA with its own perminfoindex numbering, which
+		// may not match m_orig_query->rteperminfos (e.g. after the rewriter
+		// expands external-table ON SELECT rules into subqueries the outer
+		// query's rteperminfos shrinks).  Temporarily swap in ORCA's own
+		// perminfos list so the indices are consistent.
+		Query *orig_query = m_dxl_to_plstmt_context->m_orig_query;
+		List *saved_perminfos = orig_query->rteperminfos;
+		orig_query->rteperminfos =
+			m_dxl_to_plstmt_context->GetPermInfosList();
+
 		ForeignScan *foreign_scan =
 			gpdb::CreateForeignScan(oidRel, index, query_quals, targetlist,
-									m_dxl_to_plstmt_context->m_orig_query, rte);
+									orig_query, rte);
+
+		orig_query->rteperminfos = saved_perminfos;
 		foreign_scan->scan.scanrelid = index;
 		plan = &(foreign_scan->scan.plan);
 		plan_return = (Plan *) foreign_scan;
@@ -4611,9 +4626,15 @@ CTranslatorDXLToPlStmt::TranslateDXLDynForeignScan(
 											   RelationGetDescr(childRel),
 											   index, qual, targetlist);
 
+	// Same perminfos swap as in the non-dynamic foreign scan path above.
+	Query *orig_query = m_dxl_to_plstmt_context->m_orig_query;
+	List *saved_perminfos = orig_query->rteperminfos;
+	orig_query->rteperminfos =
+		m_dxl_to_plstmt_context->GetPermInfosList();
+
 	ForeignScan *foreign_scan_first_part =
 		gpdb::CreateForeignScan(oid_first_child, index, qual, targetlist,
-								m_dxl_to_plstmt_context->m_orig_query, rte);
+								orig_query, rte);
 
 	// Set the plan fields to the first partition. We still want the plan type to be
 	// a dynamic foreign scan
@@ -4645,11 +4666,14 @@ CTranslatorDXLToPlStmt::TranslateDXLDynForeignScan(
 
 		ForeignScan *foreign_scan =
 			gpdb::CreateForeignScan(rte->relid, index, qual, targetlist,
-									m_dxl_to_plstmt_context->m_orig_query, rte);
+									orig_query, rte);
 
 		dyn_foreign_scan->fdw_private_list = gpdb::LAppend(
 			dyn_foreign_scan->fdw_private_list, foreign_scan->fdw_private);
 	}
+
+	orig_query->rteperminfos = saved_perminfos;
+
 	// convert qual and targetlist back to root relation. This is used by the
 	// executor node to remap to the children
 	gpdb::RelationWrapper prevRel = gpdb::GetRelation(rte->relid);
@@ -5336,7 +5360,7 @@ CTranslatorDXLToPlStmt::ProcessDXLTblDescr(
 	rte->eref = alias;
 	rte->alias = alias;
 
-	m_dxl_to_plstmt_context->AddPerfmInfo(pi);
+	m_dxl_to_plstmt_context->AddPermInfo(pi);
 
 	// set up rte <> perm info link.
 	rte->perminfoindex = gpdb::ListLength(

src/backend/optimizer/plan/orca.c

@@ -518,7 +518,6 @@ transformGroupedWindows(Node *node, void *context)
 
 		Query	   *subq;
 		RangeTblEntry *rte;
-		RTEPermissionInfo *perminfo;
 		RangeTblRef *ref;
 		Alias	   *alias;
 		bool		hadSubLinks = qry->hasSubLinks;
@@ -545,6 +544,7 @@ transformGroupedWindows(Node *node, void *context)
 
 		/* Core of subquery input table expression: */
 		subq->rtable = qry->rtable; /* before windowing */
+		subq->rteperminfos = qry->rteperminfos; /* before windowing */
 		subq->jointree = qry->jointree; /* before windowing */
 		subq->targetList = NIL;		/* fill in later */
 
@@ -578,11 +578,9 @@ transformGroupedWindows(Node *node, void *context)
 		rte->eref = NULL;			/* fill in later */
 		rte->inFromCl = true;
 
-		perminfo = makeNode(RTEPermissionInfo);
-		perminfo->requiredPerms = ACL_SELECT;
-
 		/*
-		 * Default? rte->inh = 0; rte->checkAsUser = 0;
+		 * Subquery RTEs do not need RTEPermissionInfo.  Permission checks
+		 * are performed on the base tables within the subquery itself.
 		 */
 
 		/*
@@ -605,7 +603,7 @@ transformGroupedWindows(Node *node, void *context)
 
 		/* Core of outer query input table expression: */
 		qry->rtable = list_make1(rte);
-		qry->rteperminfos = list_make1(perminfo);
+		qry->rteperminfos = NIL;
 		qry->jointree = (FromExpr *) makeNode(FromExpr);
 		qry->jointree->fromlist = list_make1(ref);
 		qry->jointree->quals = NULL;

src/include/gpopt/translate/CContextDXLToPlStmt.h

@@ -109,7 +109,7 @@ class CContextDXLToPlStmt
 	// list of all rtable entries
 	List *m_rtable_entries_list;
 
-	// list of all rtable entries
+	// list of all RTEPermissionInfo entries
 	List *m_perminfo_list;
 
 	// list of all subplan entries
@@ -249,8 +249,8 @@ class CContextDXLToPlStmt
 	Index GetRTEIndexByAssignedQueryId(ULONG assigned_query_id_for_target_rel,
 									   BOOL *is_rte_exists);
 
-	// add a perm info.
-	void AddPerfmInfo(RTEPermissionInfo *pi);
+	// add a permission info entry
+	void AddPermInfo(RTEPermissionInfo *pi);
 
 	// get perm info from m_perminfo_list by given index
 	RTEPermissionInfo *GetPermInfoByIndex(Index index);

src/test/isolation2/expected/concurrent_drop_truncate_tablespace.out

@@ -131,4 +131,4 @@ SELECT gp_inject_fault('drop_tablespace_after_acquire_lock', 'reset', dbid) FROM
 DROP
 -- fail
 2<:  <... completed>
-ERROR:  tablespace "concurrent_tblspace" does not exist  (seg0 127.0.1.1:7002 pid=1865353)
+ERROR:  could not create directory "pg_tblspc/XXXX":  No such file or directory