Skip to content

Commit

Permalink
[#24930] YSQL: Remove superfluous field use_secondary_index from PgPr…
Browse files Browse the repository at this point in the history
…epareParameters

Summary:
The `PgPrepareParameters` has the `index_relfilenode_oid` field which can be used to detect when it is required to use secondary index. The `use_secondary_index` field is superfluous because it allows undesired data state: `use_secondary_index` is `true` and `index_relfilenode_oid` is `kInvalidOid`.
Jira: DB-14072

Test Plan: Jenkins

Reviewers: amartsinchyk, telgersma

Reviewed By: amartsinchyk

Subscribers: yql

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D40002
  • Loading branch information
d-uspenskiy committed Nov 18, 2024
1 parent 17cd464 commit 42fa945
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 39 deletions.
21 changes: 10 additions & 11 deletions src/postgres/src/backend/access/yb_access/yb_scan.c
Original file line number Diff line number Diff line change
Expand Up @@ -733,7 +733,7 @@ ybcSetupScanPlan(bool xs_want_itup, YbScanDesc ybScan, YbScanPlan scan_plan)
* types of scan.
* - "querying_colocated_table": Support optimizations for (system,
* user database and tablegroup) colocated tables
* - "index_oid, index_only_scan, use_secondary_index": Different index
* - "index_oid, index_only_scan": Different index
* scans.
* NOTE: Primary index is a special case as there isn't a primary index
* table in YugaByte.
Expand Down Expand Up @@ -763,16 +763,15 @@ ybcSetupScanPlan(bool xs_want_itup, YbScanDesc ybScan, YbScanPlan scan_plan)

if (index)
{
ybScan->prepare_params.index_relfilenode_oid =
ybScan->prepare_params.fetch_ybctids_only &&
YBIsCoveredByMainTable(index)
? YbGetRelfileNodeId(relation)
: YbGetRelfileNodeId(index);
ybScan->prepare_params.index_only_scan = xs_want_itup;
ybScan->prepare_params.use_secondary_index =
!YBIsCoveredByMainTable(index) ||
(ybScan->prepare_params.fetch_ybctids_only &&
!ybScan->prepare_params.querying_colocated_table);
YBCPgPrepareParameters *params = &ybScan->prepare_params;
params->index_relfilenode_oid = InvalidOid;
params->index_only_scan = xs_want_itup;
if (!YBIsCoveredByMainTable(index))
params->index_relfilenode_oid = YbGetRelfileNodeId(index);
else if (params->index_only_scan ||
(params->fetch_ybctids_only &&
!params->querying_colocated_table))
params->index_relfilenode_oid = YbGetRelfileNodeId(relation);
}

/* Setup descriptors for target and bind. */
Expand Down
1 change: 0 additions & 1 deletion src/postgres/src/backend/access/ybgin/ybginscan.c
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ ybginrescan(IndexScanDesc scan, ScanKey scankey, int nscankeys,
YBCPgPrepareParameters prepare_params = {
.index_relfilenode_oid = YbGetRelfileNodeId(scan->indexRelation),
.index_only_scan = scan->xs_want_itup,
.use_secondary_index = true, /* can't have ybgin primary index */
.querying_colocated_table = querying_colocated_table,
};
HandleYBStatus(YBCPgNewSelect(YBCGetDatabaseOid(scan->heapRelation),
Expand Down
12 changes: 4 additions & 8 deletions src/yb/yql/pggate/pggate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -167,14 +167,7 @@ std::optional<PgSelect::IndexQueryInfo> MakeIndexQueryInfo(
Result<std::unique_ptr<PgStatement>> MakeSelectStatement(
const PgSession::ScopedRefPtr& pg_session, const PgObjectId& table_id,
const PgObjectId& index_id, const PgPrepareParameters* prepare_params, bool is_region_local) {
// Scenarios:
// - Sequential Scan: PgSelect to read from table_id.
// - Primary Scan: PgSelect from table_id. YugaByte does not have separate table for primary key.
// - Index-Only-Scan: PgSelectIndex directly from secondary index_id.
// - IndexScan: Use PgSelectIndex to read from index_id and then PgSelect to read from table_id.
// Note that for SysTable, only one request is send for both table_id and index_id.
if (prepare_params && prepare_params->index_only_scan && prepare_params->use_secondary_index) {
RSTATUS_DCHECK(index_id.IsValid(), InvalidArgument, "Cannot run query with invalid index ID");
if (prepare_params && prepare_params->index_only_scan) {
return PgSelectIndex::Make(pg_session, index_id, is_region_local);
}

Expand Down Expand Up @@ -1463,6 +1456,9 @@ Status PgApiImpl::ExecTruncateColocated(PgStatement* handle) {
Status PgApiImpl::NewSelect(
const PgObjectId& table_id, const PgObjectId& index_id,
const PgPrepareParameters* prepare_params, bool is_region_local, PgStatement** handle) {
DCHECK(index_id.IsValid() || table_id.IsValid());
DCHECK(!(prepare_params && prepare_params->index_only_scan) || index_id.IsValid());

*handle = nullptr;
return AddToCurrentPgMemctx(
VERIFY_RESULT(MakeSelectStatement(
Expand Down
14 changes: 6 additions & 8 deletions src/yb/yql/pggate/ybc_pg_typedefs.h
Original file line number Diff line number Diff line change
Expand Up @@ -230,19 +230,18 @@ typedef struct PgSysColumns {
//
// Index-related parameters are used to describe different types of scan.
// - Sequential scan: Index parameter is not used.
// { index_relfilenode_oid, index_only_scan, use_secondary_index }
// = { kInvalidRelfileNodeOid, false, false }
// { index_relfilenode_oid, index_only_scan}
// = { kInvalidRelfileNodeOid, false}
// - IndexScan:
// { index_relfilenode_oid, index_only_scan, use_secondary_index }
// = { IndexRelfileNodeOid, false, true }
// { index_relfilenode_oid, index_only_scan}
// = { IndexRelfileNodeOid, false}
// - IndexOnlyScan:
// { index_relfilenode_oid, index_only_scan, use_secondary_index }
// = { IndexRelfileNodeOid, true, true }
// { index_relfilenode_oid, index_only_scan}
// = { IndexRelfileNodeOid, true}
// - PrimaryIndexScan: This is a special case as YugaByte doesn't have a separated
// primary-index database object from table object.
// index_relfilenode_oid = TableRelfileNodeOid
// index_only_scan = true if ROWID is wanted. Otherwise, regular rowset is wanted.
// use_secondary_index = false
//
// Attribute "querying_colocated_table"
// - If 'true', SELECT from colocated tables (of any type - database, tablegroup, system).
Expand All @@ -251,7 +250,6 @@ typedef struct PgSysColumns {
typedef struct PgPrepareParameters {
YBCPgOid index_relfilenode_oid;
bool index_only_scan;
bool use_secondary_index;
bool querying_colocated_table;
bool fetch_ybctids_only;
} YBCPgPrepareParameters;
Expand Down
17 changes: 6 additions & 11 deletions src/yb/yql/pggate/ybc_pggate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1533,18 +1533,13 @@ YBCStatus YBCPgExecTruncateColocated(YBCPgStatement handle) {
}

// SELECT Operations -------------------------------------------------------------------------------
YBCStatus YBCPgNewSelect(const YBCPgOid database_oid,
const YBCPgOid table_relfilenode_oid,
const YBCPgPrepareParameters *prepare_params,
bool is_region_local,
YBCPgStatement *handle) {
const auto index_oid = prepare_params && prepare_params->use_secondary_index
? prepare_params->index_relfilenode_oid
: kInvalidOid;
const auto index_id =
index_oid == kInvalidOid ? PgObjectId{} : PgObjectId{database_oid, index_oid};
YBCStatus YBCPgNewSelect(
YBCPgOid database_oid, YBCPgOid table_relfilenode_oid, const YBCPgPrepareParameters* params,
bool is_region_local, YBCPgStatement* handle) {
return ToYBCStatus(pgapi->NewSelect(
{database_oid, table_relfilenode_oid}, index_id, prepare_params, is_region_local, handle));
PgObjectId{database_oid, table_relfilenode_oid},
PgObjectId{database_oid, params ? params->index_relfilenode_oid : kInvalidOid},
params, is_region_local, handle));
}

YBCStatus YBCPgSetForwardScan(YBCPgStatement handle, bool is_forward_scan) {
Expand Down

0 comments on commit 42fa945

Please sign in to comment.