Subject: Re: identity columns
To: Andres Freund <andres@anarazel.de>
References: 
 <CAKOSWNmKbthEdV0m1J=V7r6nDyPk8diKwHW93jYbB+wnXd-c1A@mail.gmail.com>
 <ed41b243-63b2-f287-e7b0-8b2ac7266d66@2ndquadrant.com>
 <CAKOSWNnMp0sKZzcX0zSKJOTSb1Fas02+xTW=kvGMu2NhRiOAbg@mail.gmail.com>
 <ec772da8-3443-b39d-e25e-138ef01a71c1@2ndquadrant.com>
 <CAKOSWNnXmM6YBXNzGnXtZQMPjDgJF+a3Wx53Wzmrq5wqDyRX7Q@mail.gmail.com>
 <c05eefac-0b55-74df-6343-71d232c15bc5@2ndquadrant.com>
 <CAKOSWNmRm0ydv1Yp8OzZ_GPh6Kfyhk+o-fxRb6onGfp433afFA@mail.gmail.com>
 <06a416db-e35b-36d1-7e2f-0affdee24745@2ndquadrant.com>
 <CAKOSWN=U0wfqjPuUbSEMAn_dcP7QnD_U0KskYf7OVhkVqs8CXA@mail.gmail.com>
 <4b1d703f-818f-00bf-4169-3279af377f3d@2ndquadrant.com>
 <20170403181918.55rci53cll6fhcq6@alap3.anarazel.de>
Cc: Vitaly Burovoy <vitaly.burovoy@gmail.com>,
 pgsql-hackers <pgsql-hackers@postgresql.org>
From: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Organization: 2ndQuadrant
Message-ID: <38c3769c-eec5-19c6-ce33-16838de77ccc@2ndquadrant.com>
Date: Mon, 3 Apr 2017 16:31:27 -0400
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:45.0)
 Gecko/20100101 Thunderbird/45.8.0
MIME-Version: 1.0
In-Reply-To: <20170403181918.55rci53cll6fhcq6@alap3.anarazel.de>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

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