Skip to content

Commit 60abc22

Browse files
jakiavoidik
authored andcommitted
[#29256] YSQL: replace hacky varno logic
Summary: Commit de8c392 fixes a problem by modifying varnos during Explain output formatting, which is hacky. Commit 141703a fixes a different problem facing a similar situation, but it applies the fix in the right place, set_plan_refs, except that it disables the fix in case there are no index quals to pushdown. That doesn't seem to be a valid condition to do this fix, so bring it out of that condition. Then, remove the hacky varno modification code in explain.c as it is no longer needed. Also consider commit a1a5160 yb_copy_replace_varnos_mutator also doing varno modifications. That can be investigated separately. Jira: DB-19033 Test Plan: On Almalinux 8: ./yb_build.sh fastdebug --gcc13 daemons initdb --skip-pg-parquet \ --java-test TestPgRegressDistinctPushdown Close: #29256 Reviewers: amartsinchyk, patnaik.balivada Reviewed By: amartsinchyk Subscribers: yql Differential Revision: https://phorge.dev.yugabyte.com/D48031
1 parent c6a3ee1 commit 60abc22

File tree

2 files changed

+9
-37
lines changed

2 files changed

+9
-37
lines changed

src/postgres/src/backend/commands/explain.c

Lines changed: 2 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,6 @@ static void YbExplainDistinctPrefixLen(PlanState *planstate, List *indextlist,
183183
ExplainState *es, List *ancestors);
184184
static void show_ybtidbitmap_info(YbBitmapTableScanState *planstate,
185185
ExplainState *es);
186-
static Node *yb_fix_indexpr_mutator(Node *node, int *newvarno);
187186

188187
typedef enum YbStatLabel
189188
{
@@ -6620,7 +6619,6 @@ YbExplainDistinctPrefixLen(PlanState *planstate, List *indextlist,
66206619
bool useprefix;
66216620
int keyno;
66226621
ListCell *tlelc;
6623-
Index scanrelid;
66246622

66256623
initStringInfo(&distinct_prefix_key_buf);
66266624

@@ -6629,25 +6627,21 @@ YbExplainDistinctPrefixLen(PlanState *planstate, List *indextlist,
66296627
planstate->plan,
66306628
ancestors);
66316629
useprefix = (list_length(es->rtable) > 1 || es->verbose);
6632-
scanrelid = ((Scan *) planstate->plan)->scanrelid;
66336630

66346631
keyno = 0;
66356632
foreach(tlelc, indextlist)
66366633
{
66376634
TargetEntry *indextle;
6638-
Node *indexpr;
66396635
char *exprstr;
66406636

66416637
if (keyno >= yb_distinct_prefixlen)
66426638
break;
66436639

66446640
indextle = (TargetEntry *) lfirst(tlelc);
66456641

6646-
/* Fix the varno of prefix to scanrelid after making a copy. */
6647-
indexpr = yb_fix_indexpr_mutator((Node *) indextle->expr,
6648-
(void *) &scanrelid);
66496642
/* Deparse the expression, showing any top-level cast */
6650-
exprstr = deparse_expression(indexpr, context, useprefix, true);
6643+
exprstr = deparse_expression((Node *) indextle->expr, context,
6644+
useprefix, true);
66516645
resetStringInfo(&distinct_prefix_key_buf);
66526646
appendStringInfoString(&distinct_prefix_key_buf, exprstr);
66536647
/* Emit one property-list item per key */
@@ -6659,27 +6653,3 @@ YbExplainDistinctPrefixLen(PlanState *planstate, List *indextlist,
66596653
ExplainPropertyList("Distinct Keys", result, es);
66606654
}
66616655
}
6662-
6663-
static Node *
6664-
yb_fix_indexpr_mutator(Node *node, int *newvarno)
6665-
{
6666-
if (node == NULL)
6667-
return NULL;
6668-
6669-
if (nodeTag(node) == T_Var)
6670-
{
6671-
Var *var = palloc(sizeof(Var));
6672-
6673-
/* Copy old var into a new one and adjust varno */
6674-
*var = *((Var *) node);
6675-
if (!IS_SPECIAL_VARNO(var->varno))
6676-
var->varno = *newvarno;
6677-
if (var->varnosyn > 0)
6678-
var->varnosyn = *newvarno;
6679-
6680-
return (Node *) var;
6681-
}
6682-
6683-
return expression_tree_mutator(node, yb_fix_indexpr_mutator,
6684-
(void *) newvarno);
6685-
}

src/postgres/src/backend/optimizer/plan/setrefs.c

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -612,8 +612,6 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
612612
/*
613613
* YB: Index quals has to be fixed to refer to index columns,
614614
* not main table columns, so we need to index the indextlist.
615-
* Also, indextlist has to be converted, as ANALYZE may use it.
616-
* Skip that if we don't have index pushdown quals.
617615
*/
618616
if (splan->yb_idx_pushdown.quals)
619617
{
@@ -634,11 +632,15 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
634632
INDEX_VAR,
635633
rtoffset,
636634
NUM_EXEC_TLIST(plan));
637-
splan->indextlist =
638-
fix_scan_list(root, splan->indextlist, rtoffset,
639-
NUM_EXEC_TLIST(plan));
640635
pfree(index_itlist);
641636
}
637+
/*
638+
* YB: Also, indextlist has to be converted as ANALYZE may use
639+
* it.
640+
*/
641+
splan->indextlist =
642+
fix_scan_list(root, splan->indextlist, rtoffset,
643+
NUM_EXEC_TLIST(plan));
642644
splan->indexqual =
643645
fix_scan_list(root, splan->indexqual,
644646
rtoffset, 1);

0 commit comments

Comments
 (0)