Skip to content

Commit

Permalink
fixed the fact that ORCA did not check the availability of the index …
Browse files Browse the repository at this point in the history
…in the

current transaction as the postgres optimizer does
  • Loading branch information
KnightMurloc committed Sep 22, 2023
1 parent d0a5bc0 commit a7a900c
Show file tree
Hide file tree
Showing 8 changed files with 80 additions and 5 deletions.
31 changes: 29 additions & 2 deletions src/backend/gpopt/translate/CTranslatorRelcacheToDXL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ extern "C" {
#include "utils/lsyscache.h"
#include "utils/rel.h"
#include "utils/relcache.h"
#include "utils/snapmgr.h"
#include "utils/syscache.h"
#include "utils/typcache.h"
}
Expand All @@ -41,6 +42,7 @@ extern "C" {
#include "gpopt/base/CUtils.h"
#include "gpopt/gpdbwrappers.h"
#include "gpopt/mdcache/CMDAccessor.h"
#include "gpopt/mdcache/CMDCache.h"
#include "gpopt/translate/CTranslatorRelcacheToDXL.h"
#include "gpopt/translate/CTranslatorScalarToDXL.h"
#include "gpopt/translate/CTranslatorUtils.h"
Expand Down Expand Up @@ -308,7 +310,7 @@ CTranslatorRelcacheToDXL::RetrieveRelIndexInfoForPartTable(CMemoryPool *mp,

GPOS_TRY
{
if (IsIndexSupported(index_rel))
if (IsIndexSupported(index_rel) && IsIndexVisible(index_rel))
{
CMDIdGPDB *mdid_index =
GPOS_NEW(mp) CMDIdGPDB(IMDId::EmdidInd, index_oid);
Expand All @@ -318,6 +320,10 @@ CTranslatorRelcacheToDXL::RetrieveRelIndexInfoForPartTable(CMemoryPool *mp,
GPOS_NEW(mp) CMDIndexInfo(mdid_index, is_partial);
md_index_info_array->Append(md_index_info);
}
else if (!IsIndexVisible(index_rel))
{
CMDCache::SetTransient(TransactionXmin);
}

gpdb::CloseRelation(index_rel);
}
Expand Down Expand Up @@ -364,7 +370,7 @@ CTranslatorRelcacheToDXL::RetrieveRelIndexInfoForNonPartTable(CMemoryPool *mp,

GPOS_TRY
{
if (IsIndexSupported(index_rel))
if (IsIndexSupported(index_rel) && IsIndexVisible(index_rel))
{
CMDIdGPDB *mdid_index =
GPOS_NEW(mp) CMDIdGPDB(IMDId::EmdidInd, index_oid);
Expand All @@ -373,6 +379,10 @@ CTranslatorRelcacheToDXL::RetrieveRelIndexInfoForNonPartTable(CMemoryPool *mp,
CMDIndexInfo(mdid_index, false /* is_partial */);
md_index_info_array->Append(md_index_info);
}
else if (!IsIndexVisible(index_rel))
{
CMDCache::SetTransient(TransactionXmin);
}

gpdb::CloseRelation(index_rel);
}
Expand Down Expand Up @@ -3329,6 +3339,23 @@ CTranslatorRelcacheToDXL::IsIndexSupported(Relation index_rel)
GIN_AM_OID == index_rel->rd_rel->relam);
}

//---------------------------------------------------------------------------
// @function:
// CTranslatorRelcacheToDXL::IsIndexVisible
//
// @doc:
// Check if index is visible in current transaction
//
//---------------------------------------------------------------------------
BOOL
CTranslatorRelcacheToDXL::IsIndexVisible(Relation index_rel)
{
return !(index_rel->rd_index->indcheckxmin &&
!TransactionIdPrecedes(
HeapTupleHeaderGetXmin(index_rel->rd_indextuple->t_data),
TransactionXmin));
}

//---------------------------------------------------------------------------
// @function:
// CTranslatorRelcacheToDXL::RetrievePartConstraintForIndex
Expand Down
7 changes: 6 additions & 1 deletion src/backend/gpopt/utils/COptTasks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#include "cdb/cdbvars.h"
#include "utils/fmgroids.h"
#include "utils/guc.h"
#include "utils/snapmgr.h"
#undef setstate

#include "gpos/_api.h"
Expand Down Expand Up @@ -498,7 +499,10 @@ COptTasks::OptimizeTask(void *ptr)
CMDCache::Init();
CMDCache::SetCacheQuota(optimizer_mdcache_size * 1024L);
}
else if (reset_mdcache)
else if (reset_mdcache ||
(CMDCache::IsTransient() &&
!TransactionIdEquals(TransactionXmin,
CMDCache::GetTransientXmin())))
{
CMDCache::Reset();
CMDCache::SetCacheQuota(optimizer_mdcache_size * 1024L);
Expand Down Expand Up @@ -605,6 +609,7 @@ COptTasks::OptimizeTask(void *ptr)
(PlannedStmt *) gpdb::CopyObject(ConvertToPlanStmtFromDXL(
mp, &mda, plan_dxl, opt_ctxt->m_query->canSetTag,
query_to_dxl_translator->GetDistributionHashOpsKind()));
opt_ctxt->m_plan_stmt->transientPlan = CMDCache::IsTransient();
}

CStatisticsConfig *stats_conf = optimizer_config->GetStatsConf();
Expand Down
28 changes: 28 additions & 0 deletions src/backend/gporca/libgpopt/include/gpopt/mdcache/CMDCache.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ class CMDCache
// the maximum size of the cache
static ULLONG m_ullCacheQuota;

// is cached data transient
static BOOL m_transient;

// in which transaction did it become transient
static uint32_t m_transientXmin;

// private ctor
CMDCache(){};

Expand Down Expand Up @@ -86,6 +92,28 @@ class CMDCache
return m_pcache;
}

// mark cache as transient
static void
SetTransient(uint32_t transientXmin)
{
m_transient = true;
m_transientXmin = transientXmin;
}

// get transaction id in which did it become transient
static uint32_t
GetTransientXmin()
{
return m_transientXmin;
}

// get is cached data transient
static BOOL
IsTransient()
{
return m_transient;
}

}; // class CMDCache

} // namespace gpopt
Expand Down
9 changes: 9 additions & 0 deletions src/backend/gporca/libgpopt/src/mdcache/CMDCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ CMDAccessor::MDCache *CMDCache::m_pcache = NULL;
// maximum size of the cache
ULLONG CMDCache::m_ullCacheQuota = UNLIMITED_CACHE_QUOTA;

// is cached data is transient
BOOL CMDCache::m_transient = false;

// in which transaction did it become transient
uint32_t CMDCache::m_transientXmin = 0;

//---------------------------------------------------------------------------
// @function:
// CMDCache::Init
Expand Down Expand Up @@ -127,6 +133,9 @@ CMDCache::Reset()

Shutdown();
Init();

m_transient = false;
m_transientXmin = 0;
}

// EOF
1 change: 0 additions & 1 deletion src/backend/optimizer/plan/orca.c
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,6 @@ optimize_query(Query *parse, ParamListInfo boundParams)
result->relationOids = glob->relationOids;
result->invalItems = glob->invalItems;
result->oneoffPlan = glob->oneoffPlan;
result->transientPlan = glob->transientPlan;

return result;
}
3 changes: 3 additions & 0 deletions src/include/gpopt/translate/CTranslatorRelcacheToDXL.h
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,9 @@ class CTranslatorRelcacheToDXL
// check if index is supported
static BOOL IsIndexSupported(Relation index_rel);

// check if index is visible
static BOOL IsIndexVisible(Relation index_rel);

// retrieve index info list of partitioned table
static List *RetrievePartTableIndexInfo(Relation rel);

Expand Down
3 changes: 2 additions & 1 deletion src/test/isolation/expected/create_index_hot.out
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
Parsed test spec with 2 sessions

starting permutation: s2begin s2select s1update s1createindexonc s2select s2forceindexscan s2select
starting permutation: s2begin s2select s1optimizeroff s1update s1createindexonc s2select s2forceindexscan s2select
step s2begin: BEGIN ISOLATION LEVEL SERIALIZABLE;
step s2select: select '#' as expected, c from hot where c = '#'
union all
select '$', c from hot where c = '$';
expected c

# #
step s1optimizeroff: set optimizer = off;
step s1update: update hot set c = '$' where c = '#';
step s1createindexonc: create index idx_c on hot (c);
step s2select: select '#' as expected, c from hot where c = '#'
Expand Down
3 changes: 3 additions & 0 deletions src/test/isolation/specs/create_index_hot.spec
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ teardown

# Update a row, and create an index on the updated column. This produces
# a broken HOT chain.
#FIXME do not turn off the optimizer when ORCA stops always using Split Update.
session "s1"
step "s1optimizeroff" {set optimizer = off;}
step "s1update" { update hot set c = '$' where c = '#'; }
step "s1createindexonc" { create index idx_c on hot (c); }

Expand All @@ -39,6 +41,7 @@ permutation
"s2begin"
"s2select"

"s1optimizeroff"
"s1update"
"s1createindexonc"

Expand Down

0 comments on commit a7a900c

Please sign in to comment.