agora inbox for [email protected]
help / color / mirror / Atom feedFrom: Andres Freund <[email protected]>
Subject: [PATCH v14 4/7] Compute information about EEOP_*_FETCHSOME at expression init time.
Date: Tue, 13 Nov 2018 12:09:42 -0800
Previously this information was computed when JIT compiling an
expression. But the information is useful for assertions in the
non-JIT case too (for assertions), therefore it makes sense to move
it.
This will, in a followup commit, allow to treat different slot types
differently. E.g. for virtual slots there's no need to generate a JIT
function to deform the slot.
Author: Andres Freund
Discussion: https://postgr.es/m/[email protected]
---
src/backend/executor/execExpr.c | 104 ++++++++++++++++++++++++++++
src/backend/jit/llvm/llvmjit_expr.c | 44 ++++--------
src/include/executor/execExpr.h | 5 ++
3 files changed, 124 insertions(+), 29 deletions(-)
diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index 82b5a0b404a..a4099c23176 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -65,6 +65,7 @@ static void ExecInitFunc(ExprEvalStep *scratch, Expr *node, List *args,
static void ExecInitExprSlots(ExprState *state, Node *node);
static void ExecPushExprSlots(ExprState *state, LastAttnumInfo *info);
static bool get_last_attnums_walker(Node *node, LastAttnumInfo *info);
+static void ExecComputeSlotInfo(ExprState *state, ExprEvalStep *op);
static void ExecInitWholeRowVar(ExprEvalStep *scratch, Var *variable,
ExprState *state);
static void ExecInitArrayRef(ExprEvalStep *scratch, ArrayRef *aref,
@@ -2288,21 +2289,30 @@ ExecPushExprSlots(ExprState *state, LastAttnumInfo *info)
{
scratch.opcode = EEOP_INNER_FETCHSOME;
scratch.d.fetch.last_var = info->last_inner;
+ scratch.d.fetch.fixed = false;
+ scratch.d.fetch.kind = NULL;
scratch.d.fetch.known_desc = NULL;
+ ExecComputeSlotInfo(state, &scratch);
ExprEvalPushStep(state, &scratch);
}
if (info->last_outer > 0)
{
scratch.opcode = EEOP_OUTER_FETCHSOME;
scratch.d.fetch.last_var = info->last_outer;
+ scratch.d.fetch.fixed = false;
+ scratch.d.fetch.kind = NULL;
scratch.d.fetch.known_desc = NULL;
+ ExecComputeSlotInfo(state, &scratch);
ExprEvalPushStep(state, &scratch);
}
if (info->last_scan > 0)
{
scratch.opcode = EEOP_SCAN_FETCHSOME;
scratch.d.fetch.last_var = info->last_scan;
+ scratch.d.fetch.fixed = false;
+ scratch.d.fetch.kind = NULL;
scratch.d.fetch.known_desc = NULL;
+ ExecComputeSlotInfo(state, &scratch);
ExprEvalPushStep(state, &scratch);
}
}
@@ -2355,6 +2365,94 @@ get_last_attnums_walker(Node *node, LastAttnumInfo *info)
(void *) info);
}
+/*
+ * Compute additional information for EEOP_*_FETCHSOME ops.
+ *
+ * The goal is to determine whether a slot is 'fixed', that is, every
+ * evaluation of the the expression will have the same type of slot, with an
+ * equivalent descriptor.
+ */
+static void
+ExecComputeSlotInfo(ExprState *state, ExprEvalStep *op)
+{
+ PlanState *parent = state->parent;
+ TupleDesc desc = NULL;
+ const TupleTableSlotOps *tts_cb = NULL;
+ bool isfixed = false;
+
+ if (op->d.fetch.known_desc != NULL)
+ {
+ desc = op->d.fetch.known_desc;
+ tts_cb = op->d.fetch.kind;
+ isfixed = op->d.fetch.kind != NULL;
+ }
+ else if (!parent)
+ {
+ isfixed = false;
+ }
+ else if (op->opcode == EEOP_INNER_FETCHSOME)
+ {
+ PlanState *is = innerPlanState(parent);
+
+ if (parent->rightopsset && !parent->rightopsfixed)
+ {
+ isfixed = false;
+ }
+ else if (parent->rightopsset && parent->rightops)
+ {
+ isfixed = true;
+ tts_cb = parent->rightops;
+ }
+ else if (is)
+ {
+ tts_cb = ExecGetResultSlotOps(is, &isfixed);
+ desc = ExecGetResultType(is);
+ }
+ }
+ else if (op->opcode == EEOP_OUTER_FETCHSOME)
+ {
+ PlanState *os = outerPlanState(parent);
+
+ if (parent->leftopsset && !parent->leftopsfixed)
+ {
+ isfixed = false;
+ }
+ else if (parent->leftopsset && parent->leftops)
+ {
+ isfixed = true;
+ tts_cb = parent->leftops;
+ }
+ else if (os)
+ {
+ tts_cb = ExecGetResultSlotOps(os, &isfixed);
+ desc = ExecGetResultType(os);
+ }
+ }
+ else if (op->opcode == EEOP_SCAN_FETCHSOME)
+ {
+ desc = parent->scandesc;
+
+ if (parent && parent->scanops)
+ tts_cb = parent->scanops;
+
+ if (parent->scanopsset)
+ isfixed = parent->scanopsfixed;
+ }
+
+ if (isfixed && desc != NULL && tts_cb != NULL)
+ {
+ op->d.fetch.fixed = true;
+ op->d.fetch.kind = tts_cb;
+ op->d.fetch.known_desc = desc;
+ }
+ else
+ {
+ op->d.fetch.fixed = false;
+ op->d.fetch.kind = NULL;
+ op->d.fetch.known_desc = NULL;
+ }
+}
+
/*
* Prepare step for the evaluation of a whole-row variable.
* The caller still has to push the step.
@@ -3254,12 +3352,18 @@ ExecBuildGroupingEqual(TupleDesc ldesc, TupleDesc rdesc,
/* push deform steps */
scratch.opcode = EEOP_INNER_FETCHSOME;
scratch.d.fetch.last_var = maxatt;
+ scratch.d.fetch.fixed = false;
scratch.d.fetch.known_desc = ldesc;
+ scratch.d.fetch.kind = NULL;
+ ExecComputeSlotInfo(state, &scratch);
ExprEvalPushStep(state, &scratch);
scratch.opcode = EEOP_OUTER_FETCHSOME;
scratch.d.fetch.last_var = maxatt;
+ scratch.d.fetch.fixed = false;
scratch.d.fetch.known_desc = rdesc;
+ scratch.d.fetch.kind = NULL;
+ ExecComputeSlotInfo(state, &scratch);
ExprEvalPushStep(state, &scratch);
/*
diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c
index 0dbc1e41062..63b921517c6 100644
--- a/src/backend/jit/llvm/llvmjit_expr.c
+++ b/src/backend/jit/llvm/llvmjit_expr.c
@@ -276,6 +276,8 @@ llvm_compile_expr(ExprState *state)
LLVMValueRef v_slot;
LLVMBasicBlockRef b_fetch;
LLVMValueRef v_nvalid;
+ LLVMValueRef l_jit_deform = NULL;
+ const TupleTableSlotOps *tts_cb = NULL;
b_fetch = l_bb_before_v(opblocks[i + 1],
"op.%d.fetch", i);
@@ -283,40 +285,22 @@ llvm_compile_expr(ExprState *state)
if (op->d.fetch.known_desc)
desc = op->d.fetch.known_desc;
+ if (op->d.fetch.fixed)
+ tts_cb = op->d.fetch.kind;
+
if (opcode == EEOP_INNER_FETCHSOME)
- {
- PlanState *is = innerPlanState(parent);
-
v_slot = v_innerslot;
-
- if (!desc &&
- is &&
- is->ps_ResultTupleSlot &&
- TTS_FIXED(is->ps_ResultTupleSlot))
- desc = is->ps_ResultTupleSlot->tts_tupleDescriptor;
- }
else if (opcode == EEOP_OUTER_FETCHSOME)
- {
- PlanState *os = outerPlanState(parent);
-
v_slot = v_outerslot;
-
- if (!desc &&
- os &&
- os->ps_ResultTupleSlot &&
- TTS_FIXED(os->ps_ResultTupleSlot))
- desc = os->ps_ResultTupleSlot->tts_tupleDescriptor;
- }
else
- {
v_slot = v_scanslot;
- if (!desc && parent)
- desc = parent->scandesc;
- }
/*
* Check if all required attributes are available, or
* whether deforming is required.
+ *
+ * TODO: skip nvalid check if slot is fixed and known to
+ * be a virtual slot.
*/
v_nvalid =
l_load_struct_gep(b, v_slot,
@@ -336,19 +320,21 @@ llvm_compile_expr(ExprState *state)
* function specific to tupledesc and the exact number of
* to-be-extracted attributes.
*/
- if (desc && (context->base.flags & PGJIT_DEFORM))
+ if (tts_cb && desc && (context->base.flags & PGJIT_DEFORM))
{
- LLVMValueRef params[1];
- LLVMValueRef l_jit_deform;
-
l_jit_deform =
slot_compile_deform(context, desc,
op->d.fetch.last_var);
+ }
+
+ if (l_jit_deform)
+ {
+ LLVMValueRef params[1];
+
params[0] = v_slot;
LLVMBuildCall(b, l_jit_deform,
params, lengthof(params), "");
-
}
else
{
diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h
index ac53935d700..194bf46e0f5 100644
--- a/src/include/executor/execExpr.h
+++ b/src/include/executor/execExpr.h
@@ -262,7 +262,12 @@ typedef struct ExprEvalStep
{
/* attribute number up to which to fetch (inclusive) */
int last_var;
+ /* will the type of slot be the same for every invocation */
+ bool fixed;
+ /* tuple descriptor, if known */
TupleDesc known_desc;
+ /* type of slot, can only be relied upon if fixed is set */
+ const TupleTableSlotOps *kind;
} fetch;
/* for EEOP_INNER/OUTER/SCAN_[SYS]VAR[_FIRST] */
--
2.18.0.rc2.dirty
--ll5lqoiis2grgqzk
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v14-0005-Verify-that-expected-slot-types-match-returned-s.patch"
view thread (2+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected]
Subject: Re: [PATCH v14 4/7] Compute information about EEOP_*_FETCHSOME at expression init time.
In-Reply-To: <no-message-id-238627@localhost>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox