You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudberry.apache.org by ma...@apache.org on 2025/02/21 09:45:16 UTC

(cloudberry) 01/30: Remove unused index strategy and subtype information from Orca (#16868)

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

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

commit 38d8088792319ce214daccd50a17e9d435c53dc8
Author: Chris Hajas <ch...@vmware.com>
AuthorDate: Tue Dec 19 09:18:35 2023 -0800

    Remove unused index strategy and subtype information from Orca (#16868)
    
    Since the GPDB 5 merge, this logic is now done during execution and is no longer populated in the plan node. Thus Orca doesn't need to retrieve this information and we can simplify some code paths.
    
    Additionally, the previous code was wrong and wouldn't work: we were populating the index strategy information with the subtype oid:
    
    ```
    *index_strategy_list = gpdb::LAppendInt(*index_strategy_list, index_qual_info->m_index_subtype_oid);
    ```
---
 .../gpopt/translate/CTranslatorDXLToPlStmt.cpp     | 65 ++++------------------
 src/backend/gpopt/translate/CTranslatorUtils.cpp   | 51 -----------------
 src/include/gpopt/translate/CIndexQualInfo.h       | 15 +----
 .../gpopt/translate/CTranslatorDXLToPlStmt.h       |  3 +-
 src/include/gpopt/translate/CTranslatorUtils.h     |  6 --
 5 files changed, 13 insertions(+), 127 deletions(-)

diff --git a/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp b/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp
index b6b8231ae2..b4e06c53c3 100644
--- a/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp
+++ b/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp
@@ -865,8 +865,6 @@ CTranslatorDXLToPlStmt::TranslateDXLIndexScan(
 	// translate index condition list
 	List *index_cond = NIL;
 	List *index_orig_cond = NIL;
-	List *index_strategy_list = NIL;
-	List *index_subtype_list = NIL;
 
 	// Translate Index Conditions if Index isn't used for order by.
 	if (!IsIndexForOrderBy(&base_table_context, ctxt_translation_prev_siblings,
@@ -878,8 +876,7 @@ CTranslatorDXLToPlStmt::TranslateDXLIndexScan(
 			physical_idx_scan_dxlop->GetDXLTableDescr(),
 			false,	// is_bitmap_index_probe
 			md_index, md_rel, output_context, &base_table_context,
-			ctxt_translation_prev_siblings, &index_cond, &index_orig_cond,
-			&index_strategy_list, &index_subtype_list);
+			ctxt_translation_prev_siblings, &index_cond, &index_orig_cond);
 	}
 
 	index_scan->indexqual = index_cond;
@@ -1019,8 +1016,6 @@ CTranslatorDXLToPlStmt::TranslateDXLIndexOnlyScan(
 	// translate index condition list
 	List *index_cond = NIL;
 	List *index_orig_cond = NIL;
-	List *index_strategy_list = NIL;
-	List *index_subtype_list = NIL;
 
 	// Translate Index Conditions if Index isn't used for order by.
 	if (!IsIndexForOrderBy(&base_table_context, ctxt_translation_prev_siblings,
@@ -1032,8 +1027,7 @@ CTranslatorDXLToPlStmt::TranslateDXLIndexOnlyScan(
 			physical_idx_scan_dxlop->GetDXLTableDescr(),
 			false,	// is_bitmap_index_probe
 			md_index, md_rel, output_context, &base_table_context,
-			ctxt_translation_prev_siblings, &index_cond, &index_orig_cond,
-			&index_strategy_list, &index_subtype_list);
+			ctxt_translation_prev_siblings, &index_cond, &index_orig_cond);
 	}
 
 	index_scan->indexqual = index_cond;
@@ -1092,8 +1086,7 @@ CTranslatorDXLToPlStmt::TranslateIndexConditions(
 	const IMDRelation *md_rel, CDXLTranslateContext *output_context,
 	CDXLTranslateContextBaseTable *base_table_context,
 	CDXLTranslationContextArray *ctxt_translation_prev_siblings,
-	List **index_cond, List **index_orig_cond, List **index_strategy_list,
-	List **index_subtype_list)
+	List **index_cond, List **index_orig_cond)
 {
 	// array of index qual info
 	CIndexQualInfoArray *index_qual_info_array =
@@ -1180,12 +1173,10 @@ CTranslatorDXLToPlStmt::TranslateIndexConditions(
 
 		Node *left_arg;
 		Node *right_arg;
-		bool is_null_test_type = false;
 		if (IsA(index_cond_expr, NullTest))
 		{
 			// NullTest only has one arg
 			left_arg = (Node *) (((NullTest *) index_cond_expr)->arg);
-			is_null_test_type = true;
 			right_arg = nullptr;
 		}
 		else
@@ -1247,33 +1238,10 @@ CTranslatorDXLToPlStmt::TranslateIndexConditions(
 			attno = ((Var *) right_arg)->varattno;
 		}
 
-		// NullTest indexqual doesn't need strategy or subtype
-		if (is_null_test_type)
-		{
-			index_qual_info_array->Append(GPOS_NEW(m_mp) CIndexQualInfo(
-				attno, index_cond_expr, original_index_cond_expr,
-				InvalidStrategy, InvalidOid));
-		}
-		else
-		{
-			// retrieve index strategy and subtype
-			StrategyNumber strategy_num;
-			OID index_subtype_oid = InvalidOid;
-
-			OID cmp_operator_oid =
-				CTranslatorUtils::OidCmpOperator(index_cond_expr);
-			GPOS_ASSERT(InvalidOid != cmp_operator_oid);
-			OID op_family_oid = CTranslatorUtils::GetOpFamilyForIndexQual(
-				attno, CMDIdGPDB::CastMdid(index->MDId())->Oid());
-			GPOS_ASSERT(InvalidOid != op_family_oid);
-			gpdb::IndexOpProperties(cmp_operator_oid, op_family_oid,
-									&strategy_num, &index_subtype_oid);
-
-			// create index qual
-			index_qual_info_array->Append(GPOS_NEW(m_mp) CIndexQualInfo(
-				attno, index_cond_expr, original_index_cond_expr, strategy_num,
-				index_subtype_oid));
-		}
+		// create index qual
+		index_qual_info_array->Append(GPOS_NEW(m_mp) CIndexQualInfo(
+			attno, index_cond_expr, original_index_cond_expr));
+
 		if (modified_null_test_cond_dxlnode != nullptr)
 		{
 			modified_null_test_cond_dxlnode->Release();
@@ -1290,10 +1258,6 @@ CTranslatorDXLToPlStmt::TranslateIndexConditions(
 		*index_cond = gpdb::LAppend(*index_cond, index_qual_info->m_expr);
 		*index_orig_cond =
 			gpdb::LAppend(*index_orig_cond, index_qual_info->m_original_expr);
-		*index_strategy_list = gpdb::LAppendInt(
-			*index_strategy_list, index_qual_info->m_index_subtype_oid);
-		*index_subtype_list = gpdb::LAppendOid(
-			*index_subtype_list, index_qual_info->m_index_subtype_oid);
 	}
 
 	// clean up
@@ -4507,8 +4471,6 @@ CTranslatorDXLToPlStmt::TranslateDXLDynIdxOnlyScan(
 	// translate index condition list
 	List *index_cond = NIL;
 	List *index_orig_cond = NIL;
-	List *index_strategy_list = NIL;
-	List *index_subtype_list = NIL;
 
 	TranslateIndexConditions(
 		(*dyn_idx_only_scan_dxlnode)
@@ -4516,8 +4478,7 @@ CTranslatorDXLToPlStmt::TranslateDXLDynIdxOnlyScan(
 		dyn_index_only_scan_dxlop->GetDXLTableDescr(),
 		false,	// is_bitmap_index_probe
 		md_index, md_rel, output_context, &base_table_context,
-		ctxt_translation_prev_siblings, &index_cond, &index_orig_cond,
-		&index_strategy_list, &index_subtype_list);
+		ctxt_translation_prev_siblings, &index_cond, &index_orig_cond);
 
 
 	dyn_idx_only_scan->indexscan.indexqual = index_cond;
@@ -4590,8 +4551,6 @@ CTranslatorDXLToPlStmt::TranslateDXLDynIdxScan(
 	// translate index condition list
 	List *index_cond = NIL;
 	List *index_orig_cond = NIL;
-	List *index_strategy_list = NIL;
-	List *index_subtype_list = NIL;
 
 	TranslateIndexConditions(
 		(*dyn_idx_only_scan_dxlnode)
@@ -4599,8 +4558,7 @@ CTranslatorDXLToPlStmt::TranslateDXLDynIdxScan(
 		dyn_index_scan_dxlop->GetDXLTableDescr(),
 		false,	// is_bitmap_index_probe
 		md_index, md_rel, output_context, &base_table_context,
-		ctxt_translation_prev_siblings, &index_cond, &index_orig_cond,
-		&index_strategy_list, &index_subtype_list);
+		ctxt_translation_prev_siblings, &index_cond, &index_orig_cond);
 
 
 	dyn_idx_only_scan->indexscan.indexqual = index_cond;
@@ -6688,14 +6646,11 @@ CTranslatorDXLToPlStmt::TranslateDXLBitmapIndexProbe(
 	CDXLNode *index_cond_list_dxlnode = (*bitmap_index_probe_dxlnode)[0];
 	List *index_cond = NIL;
 	List *index_orig_cond = NIL;
-	List *index_strategy_list = NIL;
-	List *index_subtype_list = NIL;
 
 	TranslateIndexConditions(
 		index_cond_list_dxlnode, table_descr, true /*is_bitmap_index_probe*/,
 		index, md_rel, output_context, base_table_context,
-		ctxt_translation_prev_siblings, &index_cond, &index_orig_cond,
-		&index_strategy_list, &index_subtype_list);
+		ctxt_translation_prev_siblings, &index_cond, &index_orig_cond);
 
 	bitmap_idx_scan->indexqual = index_cond;
 	bitmap_idx_scan->indexqualorig = index_orig_cond;
diff --git a/src/backend/gpopt/translate/CTranslatorUtils.cpp b/src/backend/gpopt/translate/CTranslatorUtils.cpp
index 904c455844..0887c72725 100644
--- a/src/backend/gpopt/translate/CTranslatorUtils.cpp
+++ b/src/backend/gpopt/translate/CTranslatorUtils.cpp
@@ -792,57 +792,6 @@ CTranslatorUtils::GetScanDirection(EdxlIndexScanDirection idx_scan_direction)
 	return NoMovementScanDirection;
 }
 
-//---------------------------------------------------------------------------
-//	@function:
-//		CTranslatorUtils::OidCmpOperator
-//
-//	@doc:
-//		Extract comparison operator from an OpExpr, ScalarArrayOpExpr or RowCompareExpr
-//
-//---------------------------------------------------------------------------
-OID
-CTranslatorUtils::OidCmpOperator(Expr *expr)
-{
-	GPOS_ASSERT(IsA(expr, OpExpr) || IsA(expr, ScalarArrayOpExpr) ||
-				IsA(expr, RowCompareExpr));
-
-	switch (expr->type)
-	{
-		case T_OpExpr:
-			return ((OpExpr *) expr)->opno;
-
-		case T_ScalarArrayOpExpr:
-			return ((ScalarArrayOpExpr *) expr)->opno;
-
-		case T_RowCompareExpr:
-			return LInitialOID(((RowCompareExpr *) expr)->opnos);
-
-		default:
-			GPOS_RAISE(gpdxl::ExmaDXL, gpdxl::ExmiDXL2PlStmtConversion,
-					   GPOS_WSZ_LIT("Unsupported comparison"));
-			return InvalidOid;
-	}
-}
-
-//---------------------------------------------------------------------------
-//	@function:
-//		CTranslatorUtils::GetOpFamilyForIndexQual
-//
-//	@doc:
-//		Extract comparison operator family for the given index column
-//
-//---------------------------------------------------------------------------
-OID
-CTranslatorUtils::GetOpFamilyForIndexQual(INT attno, OID index_oid)
-{
-	gpdb::RelationWrapper rel = gpdb::GetRelation(index_oid);
-	GPOS_ASSERT(rel);
-	GPOS_ASSERT(attno <= rel->rd_index->indnatts);
-
-	OID op_family_oid = rel->rd_opfamily[attno - 1];
-
-	return op_family_oid;
-}
 
 //---------------------------------------------------------------------------
 //	@function:
diff --git a/src/include/gpopt/translate/CIndexQualInfo.h b/src/include/gpopt/translate/CIndexQualInfo.h
index b2c45d080c..52c9853d94 100644
--- a/src/include/gpopt/translate/CIndexQualInfo.h
+++ b/src/include/gpopt/translate/CIndexQualInfo.h
@@ -48,20 +48,9 @@ public:
 	// original index qual expression
 	Expr *m_original_expr;
 
-	// index strategy information
-	StrategyNumber m_strategy_num;
-
-	// index subtype
-	OID m_index_subtype_oid;
-
 	// ctor
-	CIndexQualInfo(AttrNumber attno, Expr *expr, Expr *original_expr,
-				   StrategyNumber strategy_number, OID index_subtype_oid)
-		: m_attno(attno),
-		  m_expr(expr),
-		  m_original_expr(original_expr),
-		  m_strategy_num(strategy_number),
-		  m_index_subtype_oid(index_subtype_oid)
+	CIndexQualInfo(AttrNumber attno, Expr *expr, Expr *original_expr)
+		: m_attno(attno), m_expr(expr), m_original_expr(original_expr)
 	{
 		GPOS_ASSERT((IsA(m_expr, OpExpr) && IsA(m_original_expr, OpExpr)) ||
 					(IsA(m_expr, ScalarArrayOpExpr) &&
diff --git a/src/include/gpopt/translate/CTranslatorDXLToPlStmt.h b/src/include/gpopt/translate/CTranslatorDXLToPlStmt.h
index d37da06dba..50a250edca 100644
--- a/src/include/gpopt/translate/CTranslatorDXLToPlStmt.h
+++ b/src/include/gpopt/translate/CTranslatorDXLToPlStmt.h
@@ -562,8 +562,7 @@ private:
 		const IMDRelation *md_rel, CDXLTranslateContext *output_context,
 		CDXLTranslateContextBaseTable *base_table_context,
 		CDXLTranslationContextArray *ctxt_translation_prev_siblings,
-		List **index_cond, List **index_orig_cond, List **index_strategy_list,
-		List **index_subtype_list);
+		List **index_cond, List **index_orig_cond);
 
 	// translate the index filters
 	List *TranslateDXLIndexFilter(
diff --git a/src/include/gpopt/translate/CTranslatorUtils.h b/src/include/gpopt/translate/CTranslatorUtils.h
index a7add28929..b6187ad099 100644
--- a/src/include/gpopt/translate/CTranslatorUtils.h
+++ b/src/include/gpopt/translate/CTranslatorUtils.h
@@ -134,12 +134,6 @@ public:
 	static ScanDirection GetScanDirection(
 		EdxlIndexScanDirection idx_scan_direction);
 
-	// get the oid of comparison operator
-	static OID OidCmpOperator(Expr *expr);
-
-	// get the opfamily for index key
-	static OID GetOpFamilyForIndexQual(INT attno, OID oid_index);
-
 	// find the n-th column descriptor in the table descriptor
 	static const CDXLColDescr *GetColumnDescrAt(
 		const CDXLTableDescr *table_descr, ULONG pos);


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