Move EEOP_*_SYSVAR evaluation out of line.

This mainly de-duplicates code. As evaluating a system variable isn't
the hottest path and the current inline implementation ends up calling
out to an external function anyway, this is OK from a performance POV.

The main motivation for de-duplicating is the upcoming slot
abstraction work, after which there's not guaranteed to be a HeapTuple
backing the slot.

Author: Andres Freund, Amit Khandekar
Discussion: https://postgr.es/m/20181105210039.hh4vvi4vwoq5ba2q@alap3.anarazel.de

diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index c08df6f162d1c3ebbac35ed22133c547a617f673..f7eac2a5723135c35919b2c304245dba9e6b1337 100644 (file)
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -490,55 +490,19 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
 
                EEO_CASE(EEOP_INNER_SYSVAR)
                {
-                       int                     attnum = op->d.var.attnum;
-                       Datum           d;
-
-                       /* these asserts must match defenses in slot_getattr */
-                       Assert(innerslot->tts_tuple != NULL);
-                       Assert(innerslot->tts_tuple != &(innerslot->tts_minhdr));
-
-                       /* heap_getsysattr has sufficient defenses against bad attnums */
-                       d = heap_getsysattr(innerslot->tts_tuple, attnum,
-                                                               innerslot->tts_tupleDescriptor,
-                                                               op->resnull);
-                       *op->resvalue = d;
-
+                       ExecEvalSysVar(state, op, econtext, innerslot);
                        EEO_NEXT();
                }
 
                EEO_CASE(EEOP_OUTER_SYSVAR)
                {
-                       int                     attnum = op->d.var.attnum;
-                       Datum           d;
-
-                       /* these asserts must match defenses in slot_getattr */
-                       Assert(outerslot->tts_tuple != NULL);
-                       Assert(outerslot->tts_tuple != &(outerslot->tts_minhdr));
-
-                       /* heap_getsysattr has sufficient defenses against bad attnums */
-                       d = heap_getsysattr(outerslot->tts_tuple, attnum,
-                                                               outerslot->tts_tupleDescriptor,
-                                                               op->resnull);
-                       *op->resvalue = d;
-
+                       ExecEvalSysVar(state, op, econtext, outerslot);
                        EEO_NEXT();
                }
 
                EEO_CASE(EEOP_SCAN_SYSVAR)
                {
-                       int                     attnum = op->d.var.attnum;
-                       Datum           d;
-
-                       /* these asserts must match defenses in slot_getattr */
-                       Assert(scanslot->tts_tuple != NULL);
-                       Assert(scanslot->tts_tuple != &(scanslot->tts_minhdr));
-
-                       /* heap_getsysattr has sufficient defenses against bad attnums */
-                       d = heap_getsysattr(scanslot->tts_tuple, attnum,
-                                                               scanslot->tts_tupleDescriptor,
-                                                               op->resnull);
-                       *op->resvalue = d;
-
+                       ExecEvalSysVar(state, op, econtext, scanslot);
                        EEO_NEXT();
                }
 
@@ -4006,6 +3970,22 @@ ExecEvalWholeRowVar(ExprState *state, ExprEvalStep *op, ExprContext *econtext)
        *op->resnull = false;
 }
 
+void
+ExecEvalSysVar(ExprState *state, ExprEvalStep *op, ExprContext *econtext,
+                          TupleTableSlot *slot)
+{
+       bool success;
+
+       /* slot_getsysattr has sufficient defenses against bad attnums */
+       success = slot_getsysattr(slot,
+                                                         op->d.var.attnum,
+                                                         op->resvalue,
+                                                         op->resnull);
+       /* this ought to be unreachable, but it's cheap enough to check */
+       if (unlikely(!success))
+               elog(ERROR, "failed to fetch attribute from slot");
+}
+
 /*
  * Transition value has not been initialized. This is the first non-NULL input
  * value for a group. We use it as the initial value for transValue.

diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c
index a543dd371107fbab19c68670fbe3f7eb558514d9..168072afd2f4c2389b242833598680cab30802dc 100644 (file)
--- a/src/backend/jit/llvm/llvmjit.c
+++ b/src/backend/jit/llvm/llvmjit.c
@@ -81,9 +81,9 @@ LLVMValueRef FuncStrlen;
 LLVMValueRef FuncVarsizeAny;
 LLVMValueRef FuncSlotGetsomeattrs;
 LLVMValueRef FuncSlotGetmissingattrs;
-LLVMValueRef FuncHeapGetsysattr;
 LLVMValueRef FuncMakeExpandedObjectReadOnlyInternal;
 LLVMValueRef FuncExecEvalArrayRefSubscript;
+LLVMValueRef FuncExecEvalSysVar;
 LLVMValueRef FuncExecAggTransReparent;
 LLVMValueRef FuncExecAggInitGroup;
 
@@ -822,9 +822,9 @@ llvm_create_types(void)
        FuncVarsizeAny = LLVMGetNamedFunction(mod, "varsize_any");
        FuncSlotGetsomeattrs = LLVMGetNamedFunction(mod, "slot_getsomeattrs");
        FuncSlotGetmissingattrs = LLVMGetNamedFunction(mod, "slot_getmissingattrs");
-       FuncHeapGetsysattr = LLVMGetNamedFunction(mod, "heap_getsysattr");
        FuncMakeExpandedObjectReadOnlyInternal = LLVMGetNamedFunction(mod, "MakeExpandedObjectReadOnlyInternal");
        FuncExecEvalArrayRefSubscript = LLVMGetNamedFunction(mod, "ExecEvalArrayRefSubscript");
+       FuncExecEvalSysVar = LLVMGetNamedFunction(mod, "ExecEvalSysVar");
        FuncExecAggTransReparent = LLVMGetNamedFunction(mod, "ExecAggTransReparent");
        FuncExecAggInitGroup = LLVMGetNamedFunction(mod, "ExecAggInitGroup");
 

diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c
index 42258774783f31db2b4deae2c07c4e857b50c49c..13e981e5641b4cc8f97b49009005f9f951b8054d 100644 (file)
--- a/src/backend/jit/llvm/llvmjit_expr.c
+++ b/src/backend/jit/llvm/llvmjit_expr.c
@@ -406,13 +406,8 @@ llvm_compile_expr(ExprState *state)
                        case EEOP_OUTER_SYSVAR:
                        case EEOP_SCAN_SYSVAR:
                                {
-                                       int                     attnum = op->d.var.attnum;
-                                       LLVMValueRef v_attnum;
-                                       LLVMValueRef v_tuple;
-                                       LLVMValueRef v_tupleDescriptor;
-                                       LLVMValueRef v_params[4];
-                                       LLVMValueRef v_syscol;
                                        LLVMValueRef v_slot;
+                                       LLVMValueRef v_params[4];
 
                                        if (opcode == EEOP_INNER_SYSVAR)
                                                v_slot = v_innerslot;
@@ -421,31 +416,14 @@ llvm_compile_expr(ExprState *state)
                                        else
                                                v_slot = v_scanslot;
 
-                                       Assert(op->d.var.attnum < 0);
-
-                                       v_tuple = l_load_struct_gep(b, v_slot,
-                                                                                               FIELDNO_TUPLETABLESLOT_TUPLE,
-                                                                                               "v.tuple");
+                                       v_params[0] = v_state;
+                                       v_params[1] = l_ptr_const(op, l_ptr(StructExprEvalStep));
+                                       v_params[2] = v_econtext;
+                                       v_params[3] = v_slot;
 
-                                       /*
-                                        * Could optimize this a bit for fixed descriptors, but
-                                        * this shouldn't be that critical a path.
-                                        */
-                                       v_tupleDescriptor =
-                                               l_load_struct_gep(b, v_slot,
-                                                                                 FIELDNO_TUPLETABLESLOT_TUPLEDESCRIPTOR,
-                                                                                 "v.tupledesc");
-                                       v_attnum = l_int32_const(attnum);
-
-                                       v_params[0] = v_tuple;
-                                       v_params[1] = v_attnum;
-                                       v_params[2] = v_tupleDescriptor;
-                                       v_params[3] = v_resnullp;
-                                       v_syscol = LLVMBuildCall(b,
-                                                                                        llvm_get_decl(mod, FuncHeapGetsysattr),
-                                                                                        v_params, lengthof(v_params),
-                                                                                        "");
-                                       LLVMBuildStore(b, v_syscol, v_resvaluep);
+                                       LLVMBuildCall(b,
+                                                                 FuncExecEvalSysVar,
+                                                                 v_params, lengthof(v_params), "");
 
                                        LLVMBuildBr(b, opblocks[i + 1]);
                                        break;

diff --git a/src/backend/jit/llvm/llvmjit_types.c b/src/backend/jit/llvm/llvmjit_types.c
index 58316a760d44c3e2c55ecdff5b3e5f7fb11abbc6..855a6977ee5fa2986b439146f5bb46fd176928fb 100644 (file)
--- a/src/backend/jit/llvm/llvmjit_types.c
+++ b/src/backend/jit/llvm/llvmjit_types.c
@@ -99,9 +99,9 @@ void     *referenced_functions[] =
        varsize_any,
        slot_getsomeattrs,
        slot_getmissingattrs,
-       heap_getsysattr,
        MakeExpandedObjectReadOnlyInternal,
        ExecEvalArrayRefSubscript,
+       ExecEvalSysVar,
        ExecAggTransReparent,
        ExecAggInitGroup
 };

diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h
index 02af68f2c25d779e24ec09bcbcd3ad13a354264e..ac53935d700b686debea905894da4b93e0991dec 100644 (file)
--- a/src/include/executor/execExpr.h
+++ b/src/include/executor/execExpr.h
@@ -734,6 +734,8 @@ extern void ExecEvalAlternativeSubPlan(ExprState *state, ExprEvalStep *op,
                                                   ExprContext *econtext);
 extern void ExecEvalWholeRowVar(ExprState *state, ExprEvalStep *op,
                                        ExprContext *econtext);
+extern void ExecEvalSysVar(ExprState *state, ExprEvalStep *op,
+                          ExprContext *econtext, TupleTableSlot *slot);
 
 extern void ExecAggInitGroup(AggState *aggstate, AggStatePerTrans pertrans, AggStatePerGroup pergroup);
 extern Datum ExecAggTransReparent(AggState *aggstate, AggStatePerTrans pertrans,

diff --git a/src/include/jit/llvmjit.h b/src/include/jit/llvmjit.h
index c81cff8a35f2bec8cc4adf13482634f084343543..f3ea2492835a2aaafb4014e007c0b3046023b3a5 100644 (file)
--- a/src/include/jit/llvmjit.h
+++ b/src/include/jit/llvmjit.h
@@ -79,9 +79,9 @@ extern LLVMValueRef FuncStrlen;
 extern LLVMValueRef FuncVarsizeAny;
 extern LLVMValueRef FuncSlotGetsomeattrs;
 extern LLVMValueRef FuncSlotGetmissingattrs;
-extern LLVMValueRef FuncHeapGetsysattr;
 extern LLVMValueRef FuncMakeExpandedObjectReadOnlyInternal;
 extern LLVMValueRef FuncExecEvalArrayRefSubscript;
+extern LLVMValueRef FuncExecEvalSysVar;
 extern LLVMValueRef FuncExecAggTransReparent;
 extern LLVMValueRef FuncExecAggInitGroup;