agora inbox for [email protected]
help / color / mirror / Atom feedFrom: Andres Freund <[email protected]>
Subject: [PATCH v14 5/7] Verify that expected slot types match returned slot types.
Date: Tue, 13 Nov 2018 12:13:20 -0800
This is important so JIT compilation knows what kind of tuple slot the
deforming routine can expect. There's also optimization potential for
expression initialization without JIT compilation. It e.g. seems
plausible to elide EEOP_*_FETCHSOME ops entirely when dealing with
virtual slots.
Author: Andres Freund
Discussion: https://postgr.es/m/[email protected]
---
src/backend/executor/execExprInterp.c | 52 +++++++++++++++++++++++++++
1 file changed, 52 insertions(+)
diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index f7eac2a5723..357eae41cc3 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -143,6 +143,7 @@ static void ExecInitInterpreter(void);
/* support functions */
static void CheckVarSlotCompatibility(TupleTableSlot *slot, int attnum, Oid vartype);
+static void CheckOpSlotCompatibility(ExprEvalStep *op, TupleTableSlot *slot);
static TupleDesc get_cached_rowtype(Oid type_id, int32 typmod,
TupleDesc *cache_field, ExprContext *econtext);
static void ShutdownTupleDescRef(Datum arg);
@@ -425,6 +426,8 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
EEO_CASE(EEOP_INNER_FETCHSOME)
{
+ CheckOpSlotCompatibility(op, innerslot);
+
/* XXX: worthwhile to check tts_nvalid inline first? */
slot_getsomeattrs(innerslot, op->d.fetch.last_var);
@@ -433,6 +436,8 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
EEO_CASE(EEOP_OUTER_FETCHSOME)
{
+ CheckOpSlotCompatibility(op, outerslot);
+
slot_getsomeattrs(outerslot, op->d.fetch.last_var);
EEO_NEXT();
@@ -440,6 +445,8 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
EEO_CASE(EEOP_SCAN_FETCHSOME)
{
+ CheckOpSlotCompatibility(op, scanslot);
+
slot_getsomeattrs(scanslot, op->d.fetch.last_var);
EEO_NEXT();
@@ -1854,6 +1861,39 @@ CheckVarSlotCompatibility(TupleTableSlot *slot, int attnum, Oid vartype)
}
}
+/*
+ * Verify that the slot is compatible with a EEOP_*_FETCHSOME operation.
+ */
+static void
+CheckOpSlotCompatibility(ExprEvalStep *op, TupleTableSlot *slot)
+{
+#ifdef USE_ASSERT_CHECKING
+ /* there's nothing to check */
+ if (!op->d.fetch.fixed)
+ return;
+
+ /*
+ * Should probably fixed at some point, but for now it's easier to allow
+ * buffer and heap tuples to be used interchangably.
+ */
+ if (slot->tts_cb == &TTSOpsBufferTuple &&
+ op->d.fetch.kind == &TTSOpsHeapTuple)
+ return;
+ if (slot->tts_cb == &TTSOpsHeapTuple &&
+ op->d.fetch.kind == &TTSOpsBufferTuple)
+ return;
+
+ /*
+ * At the moment we consider it OK if a virtual slot is used instead of a
+ * specific type of slot, as a virtual slot never needs to be deformed.
+ */
+ if (slot->tts_cb == &TTSOpsVirtual)
+ return;
+
+ Assert(op->d.fetch.kind == slot->tts_cb);
+#endif
+}
+
/*
* get_cached_rowtype: utility function to lookup a rowtype tupdesc
*
@@ -1921,6 +1961,8 @@ ExecJustInnerVar(ExprState *state, ExprContext *econtext, bool *isnull)
int attnum = op->d.var.attnum + 1;
TupleTableSlot *slot = econtext->ecxt_innertuple;
+ CheckOpSlotCompatibility(&state->steps[0], slot);
+
/*
* Since we use slot_getattr(), we don't need to implement the FETCHSOME
* step explicitly, and we also needn't Assert that the attnum is in range
@@ -1937,6 +1979,8 @@ ExecJustOuterVar(ExprState *state, ExprContext *econtext, bool *isnull)
int attnum = op->d.var.attnum + 1;
TupleTableSlot *slot = econtext->ecxt_outertuple;
+ CheckOpSlotCompatibility(&state->steps[0], slot);
+
/* See comments in ExecJustInnerVar */
return slot_getattr(slot, attnum, isnull);
}
@@ -1949,6 +1993,8 @@ ExecJustScanVar(ExprState *state, ExprContext *econtext, bool *isnull)
int attnum = op->d.var.attnum + 1;
TupleTableSlot *slot = econtext->ecxt_scantuple;
+ CheckOpSlotCompatibility(&state->steps[0], slot);
+
/* See comments in ExecJustInnerVar */
return slot_getattr(slot, attnum, isnull);
}
@@ -1973,6 +2019,8 @@ ExecJustAssignInnerVar(ExprState *state, ExprContext *econtext, bool *isnull)
TupleTableSlot *inslot = econtext->ecxt_innertuple;
TupleTableSlot *outslot = state->resultslot;
+ CheckOpSlotCompatibility(&state->steps[0], inslot);
+
/*
* We do not need CheckVarSlotCompatibility here; that was taken care of
* at compilation time.
@@ -1996,6 +2044,8 @@ ExecJustAssignOuterVar(ExprState *state, ExprContext *econtext, bool *isnull)
TupleTableSlot *inslot = econtext->ecxt_outertuple;
TupleTableSlot *outslot = state->resultslot;
+ CheckOpSlotCompatibility(&state->steps[0], inslot);
+
/* See comments in ExecJustAssignInnerVar */
outslot->tts_values[resultnum] =
slot_getattr(inslot, attnum, &outslot->tts_isnull[resultnum]);
@@ -2012,6 +2062,8 @@ ExecJustAssignScanVar(ExprState *state, ExprContext *econtext, bool *isnull)
TupleTableSlot *inslot = econtext->ecxt_scantuple;
TupleTableSlot *outslot = state->resultslot;
+ CheckOpSlotCompatibility(&state->steps[0], inslot);
+
/* See comments in ExecJustAssignInnerVar */
outslot->tts_values[resultnum] =
slot_getattr(inslot, attnum, &outslot->tts_isnull[resultnum]);
--
2.18.0.rc2.dirty
--ll5lqoiis2grgqzk
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v14-0006-Restructure-TupleTableSlot-to-allow-tuples-other.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 5/7] Verify that expected slot types match returned slot types.
In-Reply-To: <no-message-id-238628@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