On 4/3/17 14:19, Andres Freund wrote:
> Are you going to try to merge this soon, or are you pushing this to 11?

I plan to commit it this week.  It was basically ready weeks ago, but
had to be adjusted after the executor changes.

>> +            case T_NextValueExpr:
>> +                    {
>> +                            NextValueExpr *nve = (NextValueExpr *) node;
>> +
>> +                            scratch.opcode = EEOP_NEXTVALUEEXPR;
>> +                            scratch.d.nextvalueexpr.seqid = nve->seqid;
>> +
>> +                            ExprEvalPushStep(state, &scratch);
>> +                            break;
>> +                    }
> 
> Hm - that's probably been answered somewhere, but why do we need a
> special expression type for this?

Because that's the way to evade the separate permission check that
nextval the function would otherwise do.

>> diff --git a/src/backend/executor/execExprInterp.c 
>> b/src/backend/executor/execExprInterp.c
>> index 4fbb5c1e74..5935b9ef75 100644
>> --- a/src/backend/executor/execExprInterp.c
>> +++ b/src/backend/executor/execExprInterp.c
>> @@ -60,6 +60,7 @@
>>  
>>  #include "access/tuptoaster.h"
>>  #include "catalog/pg_type.h"
>> +#include "commands/sequence.h"
>>  #include "executor/execExpr.h"
>>  #include "executor/nodeSubplan.h"
>>  #include "funcapi.h"
>> @@ -337,6 +338,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, 
>> bool *isnull)
>>              &&CASE_EEOP_NULLIF,
>>              &&CASE_EEOP_SQLVALUEFUNCTION,
>>              &&CASE_EEOP_CURRENTOFEXPR,
>> +            &&CASE_EEOP_NEXTVALUEEXPR,
>>              &&CASE_EEOP_ARRAYEXPR,
>>              &&CASE_EEOP_ARRAYCOERCE,
>>              &&CASE_EEOP_ROW,
>> @@ -1228,6 +1230,14 @@ ExecInterpExpr(ExprState *state, ExprContext 
>> *econtext, bool *isnull)
>>                      EEO_NEXT();
>>              }
>>  
>> +            EEO_CASE(EEOP_NEXTVALUEEXPR)
>> +            {
>> +                    *op->resvalue = 
>> Int64GetDatum(nextval_internal(op->d.nextvalueexpr.seqid, false));
>> +                    *op->resnull = false;
>> +
>> +                    EEO_NEXT();
>> +            }
> 
> Is it guaranteed that the caller expects an int64?  I saw that
> nextvalueexpr's have a typeid field.

It expects one of the integer types.  We could cast the result of
Int64GetDatum() to the appropriate type, but that wouldn't actually do
anything.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to