Thread: Re: Increase value of OUTER_VAR
On 06.03.21 15:59, Tom Lane wrote: > Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes: >> On 04.03.21 20:01, Tom Lane wrote: >>> (2) Does that datatype change need to propagate anywhere besides >>> what I touched here? I did not make any effort to search for >>> other places. > >> I think > >> Var.varnosyn >> CurrentOfExpr.cvarno > >> should also have their type changed. > > Agreed as to CurrentOfExpr.cvarno. But I think the entire point of > varnosyn is that it saves the original rangetable reference and > *doesn't* get overwritten with OUTER_VAR etc. So that one is a > different animal, and I'm inclined to leave it as Index. Can we move forward with this? I suppose there was still some uncertainty about whether all the places that need changing have been identified, but do we have a better idea how to find them?
Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
> Can we move forward with this?
> I suppose there was still some uncertainty about whether all the places
> that need changing have been identified, but do we have a better idea
> how to find them?
We could just push the change and see what happens. But I was thinking
more in terms of doing that early in the v15 cycle. I remain skeptical
that we need a near-term fix.
regards, tom lane
I wrote:
> Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
>> Can we move forward with this?
> We could just push the change and see what happens. But I was thinking
> more in terms of doing that early in the v15 cycle. I remain skeptical
> that we need a near-term fix.
To make sure we don't forget, I added an entry to the next CF for this.
regards, tom lane
On 4/8/21 8:13 AM, Tom Lane wrote:
> I wrote:
>> Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
>>> Can we move forward with this?
>
>> We could just push the change and see what happens. But I was thinking
>> more in terms of doing that early in the v15 cycle. I remain skeptical
>> that we need a near-term fix.
>
> To make sure we don't forget, I added an entry to the next CF for this.
Thanks for your efforts.
I tried to dive deeper: replace ROWID_VAR with -4 and explicitly change
types of varnos in the description of functions that can only work with
special varnos.
Use cases of OUTER_VAR looks simple (i guess). Use cases of INNER_VAR is
more complex because of the map_variable_attnos(). It is needed to
analyze how negative value of INNER_VAR can affect on this function.
INDEX_VAR causes potential problem:
in ExecInitForeignScan() and ExecInitForeignScan() we do
tlistvarno = INDEX_VAR;
here tlistvarno has non-negative type.
ROWID_VAR caused two problems in the check-world tests:
set_pathtarget_cost_width():
if (var->varno < root->simple_rel_array_size)
{
RelOptInfo *rel = root->simple_rel_array[var->varno];
...
and
replace_nestloop_params_mutator():
if (!bms_is_member(var->varno, root->curOuterRels))
I skipped this problems to see other weak points, but check-world
couldn't find another.
--
regards,
Andrey Lepikhov
Postgres Professional
Attachment
Here's a more fleshed-out version of this patch. I ran around and
fixed all the places where INNER_VAR etc. were being assigned directly to
a variable or parameter of type Index, and also grepped for 'Index.*varno'
to find suspicious declarations. (I didn't change every last instance
of the latter though; just places that could possibly be looking at
post-setrefs.c Vars.)
I concluded that we don't really need to change the type of
CurrentOfExpr.cvarno, because that's never set to a special value.
The main thing I remain concerned about is whether there are more
places like set_pathtarget_cost_width(), where we could be making
an inequality comparison on "varno" that would now be wrong.
I tried to catch this by enabling -Wsign-compare and -Wsign-conversion,
but that produced so many thousands of uninteresting warnings that
I soon gave up. I'm not sure there's any good way to catch remaining
places like that except to commit the patch and wait for trouble
reports.
So I'm inclined to propose pushing this and seeing what happens.
regards, tom lane
diff --git a/src/backend/executor/execScan.c b/src/backend/executor/execScan.c
index 69ab34573e..9f1d8b6d1e 100644
--- a/src/backend/executor/execScan.c
+++ b/src/backend/executor/execScan.c
@@ -282,7 +282,7 @@ ExecAssignScanProjectionInfo(ScanState *node)
* As above, but caller can specify varno expected in Vars in the tlist.
*/
void
-ExecAssignScanProjectionInfoWithVarno(ScanState *node, Index varno)
+ExecAssignScanProjectionInfoWithVarno(ScanState *node, int varno)
{
TupleDesc tupdesc = node->ss_ScanTupleSlot->tts_tupleDescriptor;
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index ad11392b99..4ab1302313 100644
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -65,7 +65,7 @@
#include "utils/typcache.h"
-static bool tlist_matches_tupdesc(PlanState *ps, List *tlist, Index varno, TupleDesc tupdesc);
+static bool tlist_matches_tupdesc(PlanState *ps, List *tlist, int varno, TupleDesc tupdesc);
static void ShutdownExprContext(ExprContext *econtext, bool isCommit);
@@ -553,7 +553,7 @@ ExecAssignProjectionInfo(PlanState *planstate,
*/
void
ExecConditionalAssignProjectionInfo(PlanState *planstate, TupleDesc inputDesc,
- Index varno)
+ int varno)
{
if (tlist_matches_tupdesc(planstate,
planstate->plan->targetlist,
@@ -579,7 +579,7 @@ ExecConditionalAssignProjectionInfo(PlanState *planstate, TupleDesc inputDesc,
}
static bool
-tlist_matches_tupdesc(PlanState *ps, List *tlist, Index varno, TupleDesc tupdesc)
+tlist_matches_tupdesc(PlanState *ps, List *tlist, int varno, TupleDesc tupdesc)
{
int numattrs = tupdesc->natts;
int attrno;
diff --git a/src/backend/executor/nodeCustom.c b/src/backend/executor/nodeCustom.c
index c82060e6d1..1dfa53c381 100644
--- a/src/backend/executor/nodeCustom.c
+++ b/src/backend/executor/nodeCustom.c
@@ -31,7 +31,7 @@ ExecInitCustomScan(CustomScan *cscan, EState *estate, int eflags)
CustomScanState *css;
Relation scan_rel = NULL;
Index scanrelid = cscan->scan.scanrelid;
- Index tlistvarno;
+ int tlistvarno;
/*
* Allocate the CustomScanState object. We let the custom scan provider
diff --git a/src/backend/executor/nodeForeignscan.c b/src/backend/executor/nodeForeignscan.c
index 9dc38d47ea..cd93d1d768 100644
--- a/src/backend/executor/nodeForeignscan.c
+++ b/src/backend/executor/nodeForeignscan.c
@@ -128,7 +128,7 @@ ExecInitForeignScan(ForeignScan *node, EState *estate, int eflags)
ForeignScanState *scanstate;
Relation currentRelation = NULL;
Index scanrelid = node->scan.scanrelid;
- Index tlistvarno;
+ int tlistvarno;
FdwRoutine *fdwroutine;
/* check for unsupported flags */
diff --git a/src/backend/nodes/makefuncs.c b/src/backend/nodes/makefuncs.c
index 01c110cd2f..7d1a01d1ed 100644
--- a/src/backend/nodes/makefuncs.c
+++ b/src/backend/nodes/makefuncs.c
@@ -63,7 +63,7 @@ makeSimpleA_Expr(A_Expr_Kind kind, char *name,
* creates a Var node
*/
Var *
-makeVar(Index varno,
+makeVar(int varno,
AttrNumber varattno,
Oid vartype,
int32 vartypmod,
@@ -85,7 +85,7 @@ makeVar(Index varno,
* them, but just initialize them to the given varno/varattno. This
* reduces code clutter and chance of error for most callers.
*/
- var->varnosyn = varno;
+ var->varnosyn = (Index) varno;
var->varattnosyn = varattno;
/* Likewise, we just set location to "unknown" here */
@@ -100,7 +100,7 @@ makeVar(Index varno,
* TargetEntry
*/
Var *
-makeVarFromTargetEntry(Index varno,
+makeVarFromTargetEntry(int varno,
TargetEntry *tle)
{
return makeVar(varno,
@@ -131,7 +131,7 @@ makeVarFromTargetEntry(Index varno,
*/
Var *
makeWholeRowVar(RangeTblEntry *rte,
- Index varno,
+ int varno,
Index varlevelsup,
bool allowScalar)
{
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index e32b92e299..bcb116ae8d 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -1116,7 +1116,7 @@ _outVar(StringInfo str, const Var *node)
{
WRITE_NODE_TYPE("VAR");
- WRITE_UINT_FIELD(varno);
+ WRITE_INT_FIELD(varno);
WRITE_INT_FIELD(varattno);
WRITE_OID_FIELD(vartype);
WRITE_INT_FIELD(vartypmod);
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index f0b34ecfac..4c3ffa482c 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -577,7 +577,7 @@ _readVar(void)
{
READ_LOCALS(Var);
- READ_UINT_FIELD(varno);
+ READ_INT_FIELD(varno);
READ_INT_FIELD(varattno);
READ_OID_FIELD(vartype);
READ_INT_FIELD(vartypmod);
diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index 8577c7b138..fd9566bcc7 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -5963,7 +5963,8 @@ set_pathtarget_cost_width(PlannerInfo *root, PathTarget *target)
Assert(var->varlevelsup == 0);
/* Try to get data from RelOptInfo cache */
- if (var->varno < root->simple_rel_array_size)
+ if (!IS_SPECIAL_VARNO(var->varno) &&
+ var->varno < root->simple_rel_array_size)
{
RelOptInfo *rel = root->simple_rel_array[var->varno];
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 439e6b6426..1244723ffe 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -4808,7 +4808,8 @@ replace_nestloop_params_mutator(Node *node, PlannerInfo *root)
/* Upper-level Vars should be long gone at this point */
Assert(var->varlevelsup == 0);
/* If not to be replaced, we can just return the Var unmodified */
- if (!bms_is_member(var->varno, root->curOuterRels))
+ if (IS_SPECIAL_VARNO(var->varno) ||
+ !bms_is_member(var->varno, root->curOuterRels))
return node;
/* Replace the Var with a nestloop Param */
return (Node *) replace_nestloop_param_var(root, var);
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 61ccfd300b..5e4a1a856b 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -31,7 +31,7 @@
typedef struct
{
- Index varno; /* RT index of Var */
+ int varno; /* RT index of Var */
AttrNumber varattno; /* attr number of Var */
AttrNumber resno; /* TLE position of Var */
} tlist_vinfo;
@@ -66,7 +66,7 @@ typedef struct
{
PlannerInfo *root;
indexed_tlist *subplan_itlist;
- Index newvarno;
+ int newvarno;
int rtoffset;
double num_exec;
} fix_upper_expr_context;
@@ -143,15 +143,15 @@ static void set_dummy_tlist_references(Plan *plan, int rtoffset);
static indexed_tlist *build_tlist_index(List *tlist);
static Var *search_indexed_tlist_for_var(Var *var,
indexed_tlist *itlist,
- Index newvarno,
+ int newvarno,
int rtoffset);
static Var *search_indexed_tlist_for_non_var(Expr *node,
indexed_tlist *itlist,
- Index newvarno);
+ int newvarno);
static Var *search_indexed_tlist_for_sortgroupref(Expr *node,
Index sortgroupref,
indexed_tlist *itlist,
- Index newvarno);
+ int newvarno);
static List *fix_join_expr(PlannerInfo *root,
List *clauses,
indexed_tlist *outer_itlist,
@@ -163,7 +163,7 @@ static Node *fix_join_expr_mutator(Node *node,
static Node *fix_upper_expr(PlannerInfo *root,
Node *node,
indexed_tlist *subplan_itlist,
- Index newvarno,
+ int newvarno,
int rtoffset, double num_exec);
static Node *fix_upper_expr_mutator(Node *node,
fix_upper_expr_context *context);
@@ -468,16 +468,6 @@ add_rte_to_flat_rtable(PlannerGlobal *glob, RangeTblEntry *rte)
glob->finalrtable = lappend(glob->finalrtable, newrte);
- /*
- * Check for RT index overflow; it's very unlikely, but if it did happen,
- * the executor would get confused by varnos that match the special varno
- * values.
- */
- if (IS_SPECIAL_VARNO(list_length(glob->finalrtable)))
- ereport(ERROR,
- (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
- errmsg("too many range table entries")));
-
/*
* If it's a plain relation RTE, add the table to relationOids.
*
@@ -1921,10 +1911,8 @@ fix_scan_expr_mutator(Node *node, fix_scan_expr_context *context)
{
CurrentOfExpr *cexpr = (CurrentOfExpr *) copyObject(node);
- Assert(cexpr->cvarno != INNER_VAR);
- Assert(cexpr->cvarno != OUTER_VAR);
- if (!IS_SPECIAL_VARNO(cexpr->cvarno))
- cexpr->cvarno += context->rtoffset;
+ Assert(!IS_SPECIAL_VARNO((int) cexpr->cvarno));
+ cexpr->cvarno += context->rtoffset;
return (Node *) cexpr;
}
if (IsA(node, PlaceHolderVar))
@@ -2421,7 +2409,7 @@ build_tlist_index(List *tlist)
* (so nothing other than Vars and PlaceHolderVars can be matched).
*/
static indexed_tlist *
-build_tlist_index_other_vars(List *tlist, Index ignore_rel)
+build_tlist_index_other_vars(List *tlist, int ignore_rel)
{
indexed_tlist *itlist;
tlist_vinfo *vinfo;
@@ -2473,9 +2461,9 @@ build_tlist_index_other_vars(List *tlist, Index ignore_rel)
*/
static Var *
search_indexed_tlist_for_var(Var *var, indexed_tlist *itlist,
- Index newvarno, int rtoffset)
+ int newvarno, int rtoffset)
{
- Index varno = var->varno;
+ int varno = var->varno;
AttrNumber varattno = var->varattno;
tlist_vinfo *vinfo;
int i;
@@ -2513,7 +2501,7 @@ search_indexed_tlist_for_var(Var *var, indexed_tlist *itlist,
*/
static Var *
search_indexed_tlist_for_non_var(Expr *node,
- indexed_tlist *itlist, Index newvarno)
+ indexed_tlist *itlist, int newvarno)
{
TargetEntry *tle;
@@ -2555,7 +2543,7 @@ static Var *
search_indexed_tlist_for_sortgroupref(Expr *node,
Index sortgroupref,
indexed_tlist *itlist,
- Index newvarno)
+ int newvarno)
{
ListCell *lc;
@@ -2773,7 +2761,7 @@ static Node *
fix_upper_expr(PlannerInfo *root,
Node *node,
indexed_tlist *subplan_itlist,
- Index newvarno,
+ int newvarno,
int rtoffset,
double num_exec)
{
diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index 62a1668796..2bf2459395 100644
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -80,7 +80,7 @@ static Node *pull_up_simple_union_all(PlannerInfo *root, Node *jtnode,
static void pull_up_union_leaf_queries(Node *setOp, PlannerInfo *root,
int parentRTindex, Query *setOpQuery,
int childRToffset);
-static void make_setop_translation_list(Query *query, Index newvarno,
+static void make_setop_translation_list(Query *query, int newvarno,
AppendRelInfo *appinfo);
static bool is_simple_subquery(PlannerInfo *root, Query *subquery,
RangeTblEntry *rte,
@@ -1365,7 +1365,7 @@ pull_up_union_leaf_queries(Node *setOp, PlannerInfo *root, int parentRTindex,
* Also create the rather trivial reverse-translation array.
*/
static void
-make_setop_translation_list(Query *query, Index newvarno,
+make_setop_translation_list(Query *query, int newvarno,
AppendRelInfo *appinfo)
{
List *vars = NIL;
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 3719755a0d..12d92fd7a0 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -6941,7 +6941,7 @@ get_variable(Var *var, int levelsup, bool istoplevel, deparse_context *context)
AttrNumber attnum;
int netlevelsup;
deparse_namespace *dpns;
- Index varno;
+ int varno;
AttrNumber varattno;
deparse_columns *colinfo;
char *refname;
@@ -6987,7 +6987,7 @@ get_variable(Var *var, int levelsup, bool istoplevel, deparse_context *context)
*/
if (context->appendparents && dpns->appendrels)
{
- Index pvarno = varno;
+ int pvarno = varno;
AttrNumber pvarattno = varattno;
AppendRelInfo *appinfo = dpns->appendrels[pvarno];
bool found = false;
@@ -7297,7 +7297,7 @@ get_name_for_var_field(Var *var, int fieldno,
AttrNumber attnum;
int netlevelsup;
deparse_namespace *dpns;
- Index varno;
+ int varno;
AttrNumber varattno;
TupleDesc tupleDesc;
Node *expr;
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index 3dc03c913e..cd57a704ad 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -459,7 +459,7 @@ typedef bool (*ExecScanRecheckMtd) (ScanState *node, TupleTableSlot *slot);
extern TupleTableSlot *ExecScan(ScanState *node, ExecScanAccessMtd accessMtd,
ExecScanRecheckMtd recheckMtd);
extern void ExecAssignScanProjectionInfo(ScanState *node);
-extern void ExecAssignScanProjectionInfoWithVarno(ScanState *node, Index varno);
+extern void ExecAssignScanProjectionInfoWithVarno(ScanState *node, int varno);
extern void ExecScanReScan(ScanState *node);
/*
@@ -552,7 +552,7 @@ extern const TupleTableSlotOps *ExecGetResultSlotOps(PlanState *planstate,
extern void ExecAssignProjectionInfo(PlanState *planstate,
TupleDesc inputDesc);
extern void ExecConditionalAssignProjectionInfo(PlanState *planstate,
- TupleDesc inputDesc, Index varno);
+ TupleDesc inputDesc, int varno);
extern void ExecFreeExprContext(PlanState *planstate);
extern void ExecAssignScanType(ScanState *scanstate, TupleDesc tupDesc);
extern void ExecCreateScanSlotFromOuterPlan(EState *estate,
diff --git a/src/include/nodes/makefuncs.h b/src/include/nodes/makefuncs.h
index 48a7ebfe45..eea87f847d 100644
--- a/src/include/nodes/makefuncs.h
+++ b/src/include/nodes/makefuncs.h
@@ -24,18 +24,18 @@ extern A_Expr *makeA_Expr(A_Expr_Kind kind, List *name,
extern A_Expr *makeSimpleA_Expr(A_Expr_Kind kind, char *name,
Node *lexpr, Node *rexpr, int location);
-extern Var *makeVar(Index varno,
+extern Var *makeVar(int varno,
AttrNumber varattno,
Oid vartype,
int32 vartypmod,
Oid varcollid,
Index varlevelsup);
-extern Var *makeVarFromTargetEntry(Index varno,
+extern Var *makeVarFromTargetEntry(int varno,
TargetEntry *tle);
extern Var *makeWholeRowVar(RangeTblEntry *rte,
- Index varno,
+ int varno,
Index varlevelsup,
bool allowScalar);
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index 9ae851d847..3a3d9e7669 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -172,12 +172,12 @@ typedef struct Expr
* in the planner and doesn't correspond to any simple relation column may
* have varnosyn = varattnosyn = 0.
*/
-#define INNER_VAR 65000 /* reference to inner subplan */
-#define OUTER_VAR 65001 /* reference to outer subplan */
-#define INDEX_VAR 65002 /* reference to index column */
-#define ROWID_VAR 65003 /* row identity column during planning */
+#define INNER_VAR (-1) /* reference to inner subplan */
+#define OUTER_VAR (-2) /* reference to outer subplan */
+#define INDEX_VAR (-3) /* reference to index column */
+#define ROWID_VAR (-4) /* row identity column during planning */
-#define IS_SPECIAL_VARNO(varno) ((varno) >= INNER_VAR)
+#define IS_SPECIAL_VARNO(varno) ((varno) < 0)
/* Symbols for the indexes of the special RTE entries in rules */
#define PRS2_OLD_VARNO 1
@@ -186,8 +186,8 @@ typedef struct Expr
typedef struct Var
{
Expr xpr;
- Index varno; /* index of this var's relation in the range
- * table, or INNER_VAR/OUTER_VAR/INDEX_VAR */
+ int varno; /* index of this var's relation in the range
+ * table, or INNER_VAR/OUTER_VAR/etc */
AttrNumber varattno; /* attribute number of this var, or zero for
* all attrs ("whole-row Var") */
Oid vartype; /* pg_type OID for the type of this var */
@@ -1341,6 +1341,7 @@ typedef struct SetToDefault
* of the target relation being constrained; this aids placing the expression
* correctly during planning. We can assume however that its "levelsup" is
* always zero, due to the syntactic constraints on where it can appear.
+ * Also, cvarno will always be a true RT index, never INNER_VAR etc.
*
* The referenced cursor can be represented either as a hardwired string
* or as a reference to a run-time parameter of type REFCURSOR. The latter
On Sat, 3 Jul 2021 at 06:23, Tom Lane <tgl@sss.pgh.pa.us> wrote: > So I'm inclined to propose pushing this and seeing what happens. Is this really sane? As much as I would like to see the 65k limit removed, I just have reservations about fixing it in this way. Even if we get all the cases fixed in core, there's likely a whole bunch of extensions that'll have bugs as a result of this for many years to come. "git grep \sIndex\s -- *.[ch] | wc -l" is showing me 77 matches in the Citus code. That's not the only extension that uses the planner hook. I'm really just not sure it's worth all the dev hours fixing the fallout. To me, it seems much safer to jump bump 65k up to 1m. It'll be a while before anyone complains about that. It's also not that great to see the number of locations that you needed to add run-time checks for negative varnos. That's not going to come for free. David
David Rowley <dgrowleyml@gmail.com> writes:
> Is this really sane?
> As much as I would like to see the 65k limit removed, I just have
> reservations about fixing it in this way. Even if we get all the
> cases fixed in core, there's likely a whole bunch of extensions
> that'll have bugs as a result of this for many years to come.
Maybe. I'm not that concerned about planner hacking: almost all of
the planner is only concerned with pre-setrefs.c representations and
will never see these values. Still, the fact that we had to inject
a couple of explicit IS_SPECIAL_VARNO tests is a bit worrisome.
(I'm more surprised really that noplace in the executor needed it.)
FWIW, experience with those places says that such bugs will be
exposed immediately; it's not like they'd lurk undetected "for years".
You might argue that the int-vs-Index declaration changes are
something that would be much harder to get right, but in reality
those are almost entirely cosmetic. We could make them completely
so by changing the macro to
#define IS_SPECIAL_VARNO(varno) ((int) (varno) < 0)
so that it'd still do the right thing when applied to a variable
declared as Index. (In the light of morning, I'm not sure why
I didn't do that already.) But we've always been extremely
cavalier about whether RT indexes should be declared as int or
Index, so I felt that standardizing on the former was actually
a good side-effect of the patch.
Anyway, to address your point more directly: as I recall, the main
objection to just increasing the values of these constants was the
fear that it'd bloat bitmapsets containing these values. Now on
the one hand, this patch has proven that noplace in the core code
does that today. On the other hand, there's no certainty that
someone might not try to do that tomorrow (if we don't fix it as
per this patch); or extensions might be doing so.
> I'm really just not sure it's worth all the dev hours fixing the
> fallout. To me, it seems much safer to jump bump 65k up to 1m. It'll
> be a while before anyone complains about that.
TBH, if we're to approach it that way, I'd be inclined to go for
broke and raise the values to ~2B. Then (a) we'll be shut of the
problem pretty much permanently, and (b) if someone does try to
make a bitmapset containing these values, hopefully they'll see
performance bad enough to expose the issue immediately.
> It's also not that great to see the number of locations that you
> needed to add run-time checks for negative varnos. That's not going to
> come for free.
Since the test is just "< 0", I pretty much disbelieve that argument.
There are only two such places in the patch, and neither of them
are *that* performance-sensitive.
Anyway, the raise-the-values solution does have the advantage of
being a four-liner, so I can live with it if that's the consensus.
But I do think this way is cleaner in the long run, and I doubt
the argument that it'll create any hard-to-detect bugs.
regards, tom lane
On 2/7/21 21:23, Tom Lane wrote: > So I'm inclined to propose pushing this and seeing what happens. +1 But why the Index type still uses for indexing of range table entries? For example: - we give int resultRelation value to create_modifytable_path() as Index nominalRelation value. - exec_rt_fetch(Index) calls list_nth(int). - generate_subquery_vars() accepts an 'Index varno' value It looks sloppy. Do you plan to change this in the next commits? -- regards, Andrey Lepikhov Postgres Professional
Hi hackers, > > So I'm inclined to propose pushing this and seeing what happens. > > +1 +1. The proposed changes will be beneficial in the long term. They will affect existing extensions. However, the scale of the problem seems to be exaggerated. I can confirm that the patch passes installcheck-world. After some searching through the code, I was unable to identify any places where the logic will break. Although this only proves my inattention, the easiest way to make any further progress seems to apply the patch. -- Best regards, Aleksander Alekseev
Aleksander Alekseev <aleksander@timescale.com> writes:
> +1. The proposed changes will be beneficial in the long term. They
> will affect existing extensions. However, the scale of the problem
> seems to be exaggerated.
Yeah, after thinking more about this I agree we should just do it.
I do not say that David's concerns about effects on extensions are
without merit, but I do think he's overblown it a bit. Most of
the patch is s/Index/int/ for various variables, and as I mentioned
before, that's basically cosmetic; there's no strong reason why
extensions have to follow suit. (In the attached v2, I modified
IS_SPECIAL_VARNO() as discussed, so it will do the right thing
even if the input is declared as Index.) There may be a few
places where extensions need to add explicit IS_SPECIAL_VARNO()
calls, but not many, and I doubt they'll be hard to find.
The alternative of increasing the values of OUTER_VAR et al
is not without risk to extensions either, so on the whole
I don't think this patch is any more problematic than many
other things we commit with little debate.
In any case, since it's still very early in the v15 cycle,
there is plenty of time for people to find problems. If I'm
wrong and there are serious consequences, we can always revert
this and do it the other way.
(v2 below is a rebase up to HEAD; no actual code changes except
for adjusting the definition of IS_SPECIAL_VARNO.)
regards, tom lane
diff --git a/src/backend/executor/execScan.c b/src/backend/executor/execScan.c
index 69ab34573e..9f1d8b6d1e 100644
--- a/src/backend/executor/execScan.c
+++ b/src/backend/executor/execScan.c
@@ -282,7 +282,7 @@ ExecAssignScanProjectionInfo(ScanState *node)
* As above, but caller can specify varno expected in Vars in the tlist.
*/
void
-ExecAssignScanProjectionInfoWithVarno(ScanState *node, Index varno)
+ExecAssignScanProjectionInfoWithVarno(ScanState *node, int varno)
{
TupleDesc tupdesc = node->ss_ScanTupleSlot->tts_tupleDescriptor;
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index ad11392b99..4ab1302313 100644
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -65,7 +65,7 @@
#include "utils/typcache.h"
-static bool tlist_matches_tupdesc(PlanState *ps, List *tlist, Index varno, TupleDesc tupdesc);
+static bool tlist_matches_tupdesc(PlanState *ps, List *tlist, int varno, TupleDesc tupdesc);
static void ShutdownExprContext(ExprContext *econtext, bool isCommit);
@@ -553,7 +553,7 @@ ExecAssignProjectionInfo(PlanState *planstate,
*/
void
ExecConditionalAssignProjectionInfo(PlanState *planstate, TupleDesc inputDesc,
- Index varno)
+ int varno)
{
if (tlist_matches_tupdesc(planstate,
planstate->plan->targetlist,
@@ -579,7 +579,7 @@ ExecConditionalAssignProjectionInfo(PlanState *planstate, TupleDesc inputDesc,
}
static bool
-tlist_matches_tupdesc(PlanState *ps, List *tlist, Index varno, TupleDesc tupdesc)
+tlist_matches_tupdesc(PlanState *ps, List *tlist, int varno, TupleDesc tupdesc)
{
int numattrs = tupdesc->natts;
int attrno;
diff --git a/src/backend/executor/nodeCustom.c b/src/backend/executor/nodeCustom.c
index c82060e6d1..1dfa53c381 100644
--- a/src/backend/executor/nodeCustom.c
+++ b/src/backend/executor/nodeCustom.c
@@ -31,7 +31,7 @@ ExecInitCustomScan(CustomScan *cscan, EState *estate, int eflags)
CustomScanState *css;
Relation scan_rel = NULL;
Index scanrelid = cscan->scan.scanrelid;
- Index tlistvarno;
+ int tlistvarno;
/*
* Allocate the CustomScanState object. We let the custom scan provider
diff --git a/src/backend/executor/nodeForeignscan.c b/src/backend/executor/nodeForeignscan.c
index 1abd906b16..0a549afcc0 100644
--- a/src/backend/executor/nodeForeignscan.c
+++ b/src/backend/executor/nodeForeignscan.c
@@ -138,7 +138,7 @@ ExecInitForeignScan(ForeignScan *node, EState *estate, int eflags)
ForeignScanState *scanstate;
Relation currentRelation = NULL;
Index scanrelid = node->scan.scanrelid;
- Index tlistvarno;
+ int tlistvarno;
FdwRoutine *fdwroutine;
/* check for unsupported flags */
diff --git a/src/backend/nodes/makefuncs.c b/src/backend/nodes/makefuncs.c
index 01c110cd2f..7d1a01d1ed 100644
--- a/src/backend/nodes/makefuncs.c
+++ b/src/backend/nodes/makefuncs.c
@@ -63,7 +63,7 @@ makeSimpleA_Expr(A_Expr_Kind kind, char *name,
* creates a Var node
*/
Var *
-makeVar(Index varno,
+makeVar(int varno,
AttrNumber varattno,
Oid vartype,
int32 vartypmod,
@@ -85,7 +85,7 @@ makeVar(Index varno,
* them, but just initialize them to the given varno/varattno. This
* reduces code clutter and chance of error for most callers.
*/
- var->varnosyn = varno;
+ var->varnosyn = (Index) varno;
var->varattnosyn = varattno;
/* Likewise, we just set location to "unknown" here */
@@ -100,7 +100,7 @@ makeVar(Index varno,
* TargetEntry
*/
Var *
-makeVarFromTargetEntry(Index varno,
+makeVarFromTargetEntry(int varno,
TargetEntry *tle)
{
return makeVar(varno,
@@ -131,7 +131,7 @@ makeVarFromTargetEntry(Index varno,
*/
Var *
makeWholeRowVar(RangeTblEntry *rte,
- Index varno,
+ int varno,
Index varlevelsup,
bool allowScalar)
{
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 36e618611f..464a7f1da5 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -1116,7 +1116,7 @@ _outVar(StringInfo str, const Var *node)
{
WRITE_NODE_TYPE("VAR");
- WRITE_UINT_FIELD(varno);
+ WRITE_INT_FIELD(varno);
WRITE_INT_FIELD(varattno);
WRITE_OID_FIELD(vartype);
WRITE_INT_FIELD(vartypmod);
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index 0dd1ad7dfc..559cf47d7f 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -577,7 +577,7 @@ _readVar(void)
{
READ_LOCALS(Var);
- READ_UINT_FIELD(varno);
+ READ_INT_FIELD(varno);
READ_INT_FIELD(varattno);
READ_OID_FIELD(vartype);
READ_INT_FIELD(vartypmod);
diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index 1fd53b40bb..1e4d404f02 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -5961,7 +5961,8 @@ set_pathtarget_cost_width(PlannerInfo *root, PathTarget *target)
Assert(var->varlevelsup == 0);
/* Try to get data from RelOptInfo cache */
- if (var->varno < root->simple_rel_array_size)
+ if (!IS_SPECIAL_VARNO(var->varno) &&
+ var->varno < root->simple_rel_array_size)
{
RelOptInfo *rel = root->simple_rel_array[var->varno];
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index a5f6d678cc..3dc0176a51 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -4805,7 +4805,8 @@ replace_nestloop_params_mutator(Node *node, PlannerInfo *root)
/* Upper-level Vars should be long gone at this point */
Assert(var->varlevelsup == 0);
/* If not to be replaced, we can just return the Var unmodified */
- if (!bms_is_member(var->varno, root->curOuterRels))
+ if (IS_SPECIAL_VARNO(var->varno) ||
+ !bms_is_member(var->varno, root->curOuterRels))
return node;
/* Replace the Var with a nestloop Param */
return (Node *) replace_nestloop_param_var(root, var);
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index e50624c465..ca22c340a7 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -31,7 +31,7 @@
typedef struct
{
- Index varno; /* RT index of Var */
+ int varno; /* RT index of Var */
AttrNumber varattno; /* attr number of Var */
AttrNumber resno; /* TLE position of Var */
} tlist_vinfo;
@@ -66,7 +66,7 @@ typedef struct
{
PlannerInfo *root;
indexed_tlist *subplan_itlist;
- Index newvarno;
+ int newvarno;
int rtoffset;
double num_exec;
} fix_upper_expr_context;
@@ -143,15 +143,15 @@ static void set_dummy_tlist_references(Plan *plan, int rtoffset);
static indexed_tlist *build_tlist_index(List *tlist);
static Var *search_indexed_tlist_for_var(Var *var,
indexed_tlist *itlist,
- Index newvarno,
+ int newvarno,
int rtoffset);
static Var *search_indexed_tlist_for_non_var(Expr *node,
indexed_tlist *itlist,
- Index newvarno);
+ int newvarno);
static Var *search_indexed_tlist_for_sortgroupref(Expr *node,
Index sortgroupref,
indexed_tlist *itlist,
- Index newvarno);
+ int newvarno);
static List *fix_join_expr(PlannerInfo *root,
List *clauses,
indexed_tlist *outer_itlist,
@@ -163,7 +163,7 @@ static Node *fix_join_expr_mutator(Node *node,
static Node *fix_upper_expr(PlannerInfo *root,
Node *node,
indexed_tlist *subplan_itlist,
- Index newvarno,
+ int newvarno,
int rtoffset, double num_exec);
static Node *fix_upper_expr_mutator(Node *node,
fix_upper_expr_context *context);
@@ -468,16 +468,6 @@ add_rte_to_flat_rtable(PlannerGlobal *glob, RangeTblEntry *rte)
glob->finalrtable = lappend(glob->finalrtable, newrte);
- /*
- * Check for RT index overflow; it's very unlikely, but if it did happen,
- * the executor would get confused by varnos that match the special varno
- * values.
- */
- if (IS_SPECIAL_VARNO(list_length(glob->finalrtable)))
- ereport(ERROR,
- (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
- errmsg("too many range table entries")));
-
/*
* If it's a plain relation RTE, add the table to relationOids.
*
@@ -1924,10 +1914,8 @@ fix_scan_expr_mutator(Node *node, fix_scan_expr_context *context)
{
CurrentOfExpr *cexpr = (CurrentOfExpr *) copyObject(node);
- Assert(cexpr->cvarno != INNER_VAR);
- Assert(cexpr->cvarno != OUTER_VAR);
- if (!IS_SPECIAL_VARNO(cexpr->cvarno))
- cexpr->cvarno += context->rtoffset;
+ Assert(!IS_SPECIAL_VARNO((int) cexpr->cvarno));
+ cexpr->cvarno += context->rtoffset;
return (Node *) cexpr;
}
if (IsA(node, PlaceHolderVar))
@@ -2424,7 +2412,7 @@ build_tlist_index(List *tlist)
* (so nothing other than Vars and PlaceHolderVars can be matched).
*/
static indexed_tlist *
-build_tlist_index_other_vars(List *tlist, Index ignore_rel)
+build_tlist_index_other_vars(List *tlist, int ignore_rel)
{
indexed_tlist *itlist;
tlist_vinfo *vinfo;
@@ -2476,9 +2464,9 @@ build_tlist_index_other_vars(List *tlist, Index ignore_rel)
*/
static Var *
search_indexed_tlist_for_var(Var *var, indexed_tlist *itlist,
- Index newvarno, int rtoffset)
+ int newvarno, int rtoffset)
{
- Index varno = var->varno;
+ int varno = var->varno;
AttrNumber varattno = var->varattno;
tlist_vinfo *vinfo;
int i;
@@ -2516,7 +2504,7 @@ search_indexed_tlist_for_var(Var *var, indexed_tlist *itlist,
*/
static Var *
search_indexed_tlist_for_non_var(Expr *node,
- indexed_tlist *itlist, Index newvarno)
+ indexed_tlist *itlist, int newvarno)
{
TargetEntry *tle;
@@ -2558,7 +2546,7 @@ static Var *
search_indexed_tlist_for_sortgroupref(Expr *node,
Index sortgroupref,
indexed_tlist *itlist,
- Index newvarno)
+ int newvarno)
{
ListCell *lc;
@@ -2776,7 +2764,7 @@ static Node *
fix_upper_expr(PlannerInfo *root,
Node *node,
indexed_tlist *subplan_itlist,
- Index newvarno,
+ int newvarno,
int rtoffset,
double num_exec)
{
diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index 224c5153b1..387a35e112 100644
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -80,7 +80,7 @@ static Node *pull_up_simple_union_all(PlannerInfo *root, Node *jtnode,
static void pull_up_union_leaf_queries(Node *setOp, PlannerInfo *root,
int parentRTindex, Query *setOpQuery,
int childRToffset);
-static void make_setop_translation_list(Query *query, Index newvarno,
+static void make_setop_translation_list(Query *query, int newvarno,
AppendRelInfo *appinfo);
static bool is_simple_subquery(PlannerInfo *root, Query *subquery,
RangeTblEntry *rte,
@@ -1372,7 +1372,7 @@ pull_up_union_leaf_queries(Node *setOp, PlannerInfo *root, int parentRTindex,
* Also create the rather trivial reverse-translation array.
*/
static void
-make_setop_translation_list(Query *query, Index newvarno,
+make_setop_translation_list(Query *query, int newvarno,
AppendRelInfo *appinfo)
{
List *vars = NIL;
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index ccd2835c22..b932a83827 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -6949,7 +6949,7 @@ get_variable(Var *var, int levelsup, bool istoplevel, deparse_context *context)
AttrNumber attnum;
int netlevelsup;
deparse_namespace *dpns;
- Index varno;
+ int varno;
AttrNumber varattno;
deparse_columns *colinfo;
char *refname;
@@ -6995,7 +6995,7 @@ get_variable(Var *var, int levelsup, bool istoplevel, deparse_context *context)
*/
if (context->appendparents && dpns->appendrels)
{
- Index pvarno = varno;
+ int pvarno = varno;
AttrNumber pvarattno = varattno;
AppendRelInfo *appinfo = dpns->appendrels[pvarno];
bool found = false;
@@ -7305,7 +7305,7 @@ get_name_for_var_field(Var *var, int fieldno,
AttrNumber attnum;
int netlevelsup;
deparse_namespace *dpns;
- Index varno;
+ int varno;
AttrNumber varattno;
TupleDesc tupleDesc;
Node *expr;
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index 3dc03c913e..cd57a704ad 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -459,7 +459,7 @@ typedef bool (*ExecScanRecheckMtd) (ScanState *node, TupleTableSlot *slot);
extern TupleTableSlot *ExecScan(ScanState *node, ExecScanAccessMtd accessMtd,
ExecScanRecheckMtd recheckMtd);
extern void ExecAssignScanProjectionInfo(ScanState *node);
-extern void ExecAssignScanProjectionInfoWithVarno(ScanState *node, Index varno);
+extern void ExecAssignScanProjectionInfoWithVarno(ScanState *node, int varno);
extern void ExecScanReScan(ScanState *node);
/*
@@ -552,7 +552,7 @@ extern const TupleTableSlotOps *ExecGetResultSlotOps(PlanState *planstate,
extern void ExecAssignProjectionInfo(PlanState *planstate,
TupleDesc inputDesc);
extern void ExecConditionalAssignProjectionInfo(PlanState *planstate,
- TupleDesc inputDesc, Index varno);
+ TupleDesc inputDesc, int varno);
extern void ExecFreeExprContext(PlanState *planstate);
extern void ExecAssignScanType(ScanState *scanstate, TupleDesc tupDesc);
extern void ExecCreateScanSlotFromOuterPlan(EState *estate,
diff --git a/src/include/nodes/makefuncs.h b/src/include/nodes/makefuncs.h
index 48a7ebfe45..eea87f847d 100644
--- a/src/include/nodes/makefuncs.h
+++ b/src/include/nodes/makefuncs.h
@@ -24,18 +24,18 @@ extern A_Expr *makeA_Expr(A_Expr_Kind kind, List *name,
extern A_Expr *makeSimpleA_Expr(A_Expr_Kind kind, char *name,
Node *lexpr, Node *rexpr, int location);
-extern Var *makeVar(Index varno,
+extern Var *makeVar(int varno,
AttrNumber varattno,
Oid vartype,
int32 vartypmod,
Oid varcollid,
Index varlevelsup);
-extern Var *makeVarFromTargetEntry(Index varno,
+extern Var *makeVarFromTargetEntry(int varno,
TargetEntry *tle);
extern Var *makeWholeRowVar(RangeTblEntry *rte,
- Index varno,
+ int varno,
Index varlevelsup,
bool allowScalar);
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index 7b125904b4..433437643e 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -172,12 +172,12 @@ typedef struct Expr
* in the planner and doesn't correspond to any simple relation column may
* have varnosyn = varattnosyn = 0.
*/
-#define INNER_VAR 65000 /* reference to inner subplan */
-#define OUTER_VAR 65001 /* reference to outer subplan */
-#define INDEX_VAR 65002 /* reference to index column */
-#define ROWID_VAR 65003 /* row identity column during planning */
+#define INNER_VAR (-1) /* reference to inner subplan */
+#define OUTER_VAR (-2) /* reference to outer subplan */
+#define INDEX_VAR (-3) /* reference to index column */
+#define ROWID_VAR (-4) /* row identity column during planning */
-#define IS_SPECIAL_VARNO(varno) ((varno) >= INNER_VAR)
+#define IS_SPECIAL_VARNO(varno) ((int) (varno) < 0)
/* Symbols for the indexes of the special RTE entries in rules */
#define PRS2_OLD_VARNO 1
@@ -186,8 +186,8 @@ typedef struct Expr
typedef struct Var
{
Expr xpr;
- Index varno; /* index of this var's relation in the range
- * table, or INNER_VAR/OUTER_VAR/INDEX_VAR */
+ int varno; /* index of this var's relation in the range
+ * table, or INNER_VAR/OUTER_VAR/etc */
AttrNumber varattno; /* attribute number of this var, or zero for
* all attrs ("whole-row Var") */
Oid vartype; /* pg_type OID for the type of this var */
@@ -1351,6 +1351,7 @@ typedef struct SetToDefault
* of the target relation being constrained; this aids placing the expression
* correctly during planning. We can assume however that its "levelsup" is
* always zero, due to the syntactic constraints on where it can appear.
+ * Also, cvarno will always be a true RT index, never INNER_VAR etc.
*
* The referenced cursor can be represented either as a hardwired string
* or as a reference to a run-time parameter of type REFCURSOR. The latter
Andrey Lepikhov <a.lepikhov@postgrespro.ru> writes:
> But why the Index type still uses for indexing of range table entries?
> For example:
> - we give int resultRelation value to create_modifytable_path() as Index
> nominalRelation value.
> - exec_rt_fetch(Index) calls list_nth(int).
> - generate_subquery_vars() accepts an 'Index varno' value
As I mentioned, the patch only intends to touch code that's possibly
used with post-setrefs Vars. In the parser and most of the planner,
there's little need to do anything because only positive varno values
will appear. So touching that code would just make the patch more
invasive without accomplishing much.
If we'd had any strong convention about whether RT indexes should be
int or Index, I might be worried about maintaining consistency.
But it's always been a horrid mishmash of both ways. Cleaning that
up completely is a task I don't care to undertake right now.
regards, tom lane
On 9/11/21 10:37 PM, Tom Lane wrote: > Aleksander Alekseev <aleksander@timescale.com> writes: > (v2 below is a rebase up to HEAD; no actual code changes except > for adjusting the definition of IS_SPECIAL_VARNO.) I have looked at this code. No problems found. Also, as a test, I used two tables with 1E5 partitions each. I tried to do plain SELECT, JOIN, join with plain table. No errors found, only performance issues. But it is a subject for another research. -- regards, Andrey Lepikhov Postgres Professional
Hi Andrey, > only performance issues That's interesting. Any chance you could share the hardware description, the configuration file, and steps to reproduce with us? -- Best regards, Aleksander Alekseev
"Andrey V. Lepikhov" <a.lepikhov@postgrespro.ru> writes:
> Also, as a test, I used two tables with 1E5 partitions each. I tried to
> do plain SELECT, JOIN, join with plain table. No errors found, only
> performance issues. But it is a subject for another research.
Yeah, there's no expectation that the performance would be any
good yet ;-)
regards, tom lane
On 14/9/21 16:37, Aleksander Alekseev wrote: > Hi Andrey, > >> only performance issues > > That's interesting. Any chance you could share the hardware > description, the configuration file, and steps to reproduce with us? > I didn't control execution time exactly. Because it is a join of two empty tables. As I see, this join used most part of 48GB RAM memory, planned all day on a typical 6 amd cores computer. I guess this is caused by sequental traversal of the partition list in some places in the optimizer. If it makes practical sense, I could investigate reasons for such poor performance. -- regards, Andrey Lepikhov Postgres Professional
Hi Andrey,
> >> only performance issues
> >
> > That's interesting. Any chance you could share the hardware
> > description, the configuration file, and steps to reproduce with us?
> >
> I didn't control execution time exactly. Because it is a join of two
> empty tables. As I see, this join used most part of 48GB RAM memory,
> planned all day on a typical 6 amd cores computer.
> I guess this is caused by sequental traversal of the partition list in
> some places in the optimizer.
> If it makes practical sense, I could investigate reasons for such poor
> performance.
--
Best regards,
Aleksander Alekseev
> >> only performance issues
> >
> > That's interesting. Any chance you could share the hardware
> > description, the configuration file, and steps to reproduce with us?
> >
> I didn't control execution time exactly. Because it is a join of two
> empty tables. As I see, this join used most part of 48GB RAM memory,
> planned all day on a typical 6 amd cores computer.
> I guess this is caused by sequental traversal of the partition list in
> some places in the optimizer.
> If it makes practical sense, I could investigate reasons for such poor
> performance.
Let's say, any information regarding bottlenecks that affect real users with real queries is of interest. Artificially created queries that are unlikely to be ever executed by anyone are not.
--
Best regards,
Aleksander Alekseev