Thread: "RETURNING PRIMARY KEY" syntax extension

"RETURNING PRIMARY KEY" syntax extension

From
Ian Barwick
Date:
Hi,

The JDBC API provides the getGeneratedKeys() method as a way of retrieving
primary key values without the need to explicitly specify the primary key
column(s). This is a widely-used feature, however the implementation has significant
performance drawbacks.

Currently this feature is implemented in the JDBC driver by appending
"RETURNING *" to the supplied statement. However this means all columns of
affected rows will be returned to the client, which causes significant
performance problems, particularly on wide tables. To mitigate this, it would
be desirable to enable the JDBC driver to request only the primary key value(s).

One possible solution would be to have the driver request the primary key for
a table, but this could cause a race condition where the primary key could change,
and even if it does not, it would entail extra overhead.

A more elegant and universal solution, which would allow the JDBC driver to
request the primary key in a single request, would be to extend the RETURNING
clause syntax with the option PRIMARY KEY. This resolves during parse
analysis into the columns of the primary key, which can be done unambiguously
because the table is already locked by that point and the primary key cannot change.

A patch is attached which implements this, and will be added to the next commitfest.
A separate patch will be submitted to the JDBC project. Example usage shown below.


Regards

Ian Barwick

/* ---------------------------------------------- */
     postgres=# CREATE TABLE foo (id SERIAL PRIMARY KEY);
     CREATE TABLE

     postgres=# INSERT INTO foo VALUES(DEFAULT) RETURNING PRIMARY KEY;
      id
     ----
       1
     (1 row)

     INSERT 0 1

     postgres=# CREATE TABLE bar (id1 INT NOT NULL, id2 INT NOT NULL, PRIMARY KEY(id1, id2));
     CREATE TABLE
     postgres=# INSERT INTO bar VALUES(1,2) RETURNING PRIMARY KEY;
      id1 | id2
     -----+-----
        1 |   2
     (1 row)

     INSERT 0 1

     postgres=# INSERT INTO bar VALUES(2,1),(2,2) RETURNING PRIMARY KEY;
      id1 | id2
     -----+-----
        2 |   1
        2 |   2
     (2 rows)

     INSERT 0 2

     postgres=# CREATE TABLE no_pkey (id SERIAL NOT NULL);
     CREATE TABLE
     postgres=# INSERT INTO no_pkey VALUES(DEFAULT) RETURNING id;
      id
     ----
       1
     (1 row)

     INSERT 0 1
     postgres=# INSERT INTO no_pkey VALUES(DEFAULT) RETURNING PRIMARY KEY;
     ERROR:  Relation does not have any primary key(s)

/* ---------------------------------------------- */

--
  Ian Barwick                   http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training & Services

Attachment

Re: "RETURNING PRIMARY KEY" syntax extension

From
David G Johnston
Date:
Ian Barwick wrote
> Hi,
> 
> The JDBC API provides the getGeneratedKeys() method as a way of retrieving
> primary key values without the need to explicitly specify the primary key
> column(s). This is a widely-used feature, however the implementation has
> significant
> performance drawbacks.
> 
> Currently this feature is implemented in the JDBC driver by appending
> "RETURNING *" to the supplied statement. However this means all columns of
> affected rows will be returned to the client, which causes significant
> performance problems, particularly on wide tables. To mitigate this, it
> would
> be desirable to enable the JDBC driver to request only the primary key
> value(s).

Seems like a good idea.


>      ERROR:  Relation does not have any primary key(s)

"Relation does not have a primary key."
or
"Relation has no primary key." (preferred)

By definition it cannot have more than one so it must have none.

It could have multiple unique constraints but I do not believe they are
considered if not tagged as primary.

David J.





--
View this message in context:
http://postgresql.1045698.n5.nabble.com/RETURNING-PRIMARY-KEY-syntax-extension-tp5806462p5806463.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.



Re: "RETURNING PRIMARY KEY" syntax extension

From
David G Johnston
Date:
David G Johnston wrote
> 
> Ian Barwick wrote
>> Hi,
>> 
>> The JDBC API provides the getGeneratedKeys() method as a way of
>> retrieving
>> primary key values without the need to explicitly specify the primary key
>> column(s). This is a widely-used feature, however the implementation has
>> significant
>> performance drawbacks.
>> 
>> Currently this feature is implemented in the JDBC driver by appending
>> "RETURNING *" to the supplied statement. However this means all columns
>> of
>> affected rows will be returned to the client, which causes significant
>> performance problems, particularly on wide tables. To mitigate this, it
>> would
>> be desirable to enable the JDBC driver to request only the primary key
>> value(s).
> Seems like a good idea.
>>      ERROR:  Relation does not have any primary key(s)
> "Relation does not have a primary key."
> or
> "Relation has no primary key." (preferred)
> 
> By definition it cannot have more than one so it must have none.
> 
> It could have multiple unique constraints but I do not believe they are
> considered if not tagged as primary.

Also,

I did see where you account for auto-updatable views but what about complex
views with instead of triggers?

These can still be the target of DML queries but are not guaranteed (though
can possibly) to return a well-defined primary key.  At worse an explicit
error about the view itself, not the apparent lack of primary key, should be
emitted.

David J.



--
View this message in context:
http://postgresql.1045698.n5.nabble.com/RETURNING-PRIMARY-KEY-syntax-extension-tp5806462p5806464.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.



Re: "RETURNING PRIMARY KEY" syntax extension

From
Ian Barwick
Date:

On 09/06/14 14:47, David G Johnston wrote:
> Ian Barwick wrote
>> Hi,
>>
>> The JDBC API provides the getGeneratedKeys() method as a way of retrieving
>> primary key values without the need to explicitly specify the primary key
>> column(s). This is a widely-used feature, however the implementation has
>> significant
>> performance drawbacks.
>>
>> Currently this feature is implemented in the JDBC driver by appending
>> "RETURNING *" to the supplied statement. However this means all columns of
>> affected rows will be returned to the client, which causes significant
>> performance problems, particularly on wide tables. To mitigate this, it
>> would
>> be desirable to enable the JDBC driver to request only the primary key
>> value(s).
>
> Seems like a good idea.
>
>
>>       ERROR:  Relation does not have any primary key(s)
>
> "Relation does not have a primary key."
> or
> "Relation has no primary key." (preferred)
>
> By definition it cannot have more than one so it must have none.

Ah yes, amazing what a fresh pair of eyes does :). The plural is
the vestige of an earlier iteration which said something about
the relation not having any primary key column(s).

Will fix, thanks.

Regards

Ian Barwick


--  Ian Barwick                   http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training &
Services



Re: "RETURNING PRIMARY KEY" syntax extension

From
David Johnston
Date:
On Monday, June 9, 2014, Ian Barwick <ian@2ndquadrant.com> wrote:


On 09/06/14 14:47, David G Johnston wrote:
Ian Barwick wrote
Hi,

The JDBC API provides the getGeneratedKeys() method as a way of retrieving
primary key values without the need to explicitly specify the primary key
column(s). This is a widely-used feature, however the implementation has
significant
performance drawbacks.

Currently this feature is implemented in the JDBC driver by appending
"RETURNING *" to the supplied statement. However this means all columns of
affected rows will be returned to the client, which causes significant
performance problems, particularly on wide tables. To mitigate this, it
would
be desirable to enable the JDBC driver to request only the primary key
value(s).


ISTM that having a non-null returning clause variable when no returning is present in the command makes things more complicated and introduces unnecessary checks in the not uncommon case of multiple non-returning commands being issued in series.

returningList was able to be null and so should returningClause.  Then if non-null first check for the easy column listing and then check for the more expensive PK lookup request.

Then again the extra returning checks may just amount noise.

David J.


Re: "RETURNING PRIMARY KEY" syntax extension

From
Gavin Flower
Date:
On 09/06/14 17:47, David G Johnston wrote:
> Ian Barwick wrote
>> Hi,
>>
>> The JDBC API provides the getGeneratedKeys() method as a way of retrieving
>> primary key values without the need to explicitly specify the primary key
>> column(s). This is a widely-used feature, however the implementation has
>> significant
>> performance drawbacks.
>>
>> Currently this feature is implemented in the JDBC driver by appending
>> "RETURNING *" to the supplied statement. However this means all columns of
>> affected rows will be returned to the client, which causes significant
>> performance problems, particularly on wide tables. To mitigate this, it
>> would
>> be desirable to enable the JDBC driver to request only the primary key
>> value(s).
> Seems like a good idea.
>
>
>>       ERROR:  Relation does not have any primary key(s)
> "Relation does not have a primary key."
> or
> "Relation has no primary key." (preferred)
>
> By definition it cannot have more than one so it must have none.
>
> It could have multiple unique constraints but I do not believe they are
> considered if not tagged as primary.
>
> David J.
>
>
>
>
>
> --
> View this message in context:
http://postgresql.1045698.n5.nabble.com/RETURNING-PRIMARY-KEY-syntax-extension-tp5806462p5806463.html
> Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.
>
>From memory all unique keys can be considered 'candidate primary keys', 
but only one can be designated 'the PRIMARY KEY'.

I also like your preferred error message, and to the full extent of my 
decidedly Non-Authority, I hereby authorise it!  :-)


Cheers,
Gavin



Re: "RETURNING PRIMARY KEY" syntax extension

From
Vik Fearing
Date:
On 06/09/2014 09:06 AM, Gavin Flower wrote:
> From memory all unique keys can be considered 'candidate primary keys',
> but only one can be designated 'the PRIMARY KEY'.

Almost.  Candidate keys are also NOT NULL.
-- 
Vik



Re: "RETURNING PRIMARY KEY" syntax extension

From
Kevin Grittner
Date:
David G Johnston <david.g.johnston@gmail.com> wrote:

>>       ERROR:  Relation does not have any primary key(s)
>
> "Relation does not have a primary key."
> or
> "Relation has no primary key." (preferred)

Project style says that the primary message should not capitalize
the first word, nor should it end in a period.  Detail and hints
should be in sentence style, but not the message itself.

http://www.postgresql.org/docs/9.3/interactive/error-style-guide.html#AEN100914

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: "RETURNING PRIMARY KEY" syntax extension

From
Hannu Krosing
Date:
On 06/09/2014 06:58 AM, Ian Barwick wrote:
> Hi,
>
> The JDBC API provides the getGeneratedKeys() method as a way of
> retrieving
> primary key values without the need to explicitly specify the primary key
> column(s).
Is it defined by the standard, to return _only_ generated primary keys,
and not
for example generated alternate keys ?

Cheers

-- 
Hannu Krosing
PostgreSQL Consultant
Performance, Scalability and High Availability
2ndQuadrant Nordic OÜ




Re: "RETURNING PRIMARY KEY" syntax extension

From
Tom Lane
Date:
Ian Barwick <ian@2ndquadrant.com> writes:
> [ RETURNING PRIMARY KEY ]

It looks to me like this is coded to have the expansion of the "primary
key" done at parse time, which seems like fundamentally the wrong thing.
Consider a view or rule containing this clause; the pkey might be
different by the time execution rolls around.  It'd be better probably
if the rewriter or planner did the expansion (and threw the error for
no-primary-key, if necessary).

Alternatively, we could do it like this and consider that the view is
dependent on the primary key constraint, but that seems inflexible.

BTW, it seems like your representation of the clause was rather poorly
chosen: it forces changing a whole lot of code that otherwise would
not need to be changed.  I'd have left returningList alone and put the
returningPrimaryKey flag someplace else.
        regards, tom lane



Re: "RETURNING PRIMARY KEY" syntax extension

From
Gavin Flower
Date:
On 09/06/14 23:42, Vik Fearing wrote:
> On 06/09/2014 09:06 AM, Gavin Flower wrote:
>>  From memory all unique keys can be considered 'candidate primary keys',
>> but only one can be designated 'the PRIMARY KEY'.
> Almost.  Candidate keys are also NOT NULL.
Yeah, obviously!
(Except, I did actually forget that - me bad.)



Re: "RETURNING PRIMARY KEY" syntax extension

From
Andres Freund
Date:
On 2014-06-09 13:42:22 +0200, Vik Fearing wrote:
> On 06/09/2014 09:06 AM, Gavin Flower wrote:
> > From memory all unique keys can be considered 'candidate primary keys',
> > but only one can be designated 'the PRIMARY KEY'.
> 
> Almost.  Candidate keys are also NOT NULL.

The list actually is a bit longer. They also cannot be partial.

There's generally also the restriction that for some contexts - like
e.g. foreign keys - primary/candidate keys may not be deferrable..

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services



Re: "RETURNING PRIMARY KEY" syntax extension

From
Jim Nasby
Date:
On 6/9/14, 8:35 AM, Hannu Krosing wrote:
> On 06/09/2014 06:58 AM, Ian Barwick wrote:
>> Hi,
>>
>> The JDBC API provides the getGeneratedKeys() method as a way of
>> retrieving
>> primary key values without the need to explicitly specify the primary key
>> column(s).
> Is it defined by the standard, to return _only_ generated primary keys,
> and not
> for example generated alternate keys ?

I was wondering that myself. I think it's certainly reasonable to expect someone would wan RETURNING SEQUENCE VALUES,
whichwould return the value of every column that owned a sequence (ie: ALTER SEQUENCE ... OWNED BY). ISTM that would
certainlyhandle the performance aspect of this, and it sounds more in line with what I'd expect getGeneratedKeys() to
do.
-- 
Jim C. Nasby, Data Architect                       jim@nasby.net
512.569.9461 (cell)                         http://jim.nasby.net



Re: "RETURNING PRIMARY KEY" syntax extension

From
Tom Dunstan
Date:
A definite +1 on this feature. A while ago I got partway through hacking the hibernate postgres dialect to make it issue a RETURNING clause to spit out the primary key before I realised that the driver was already doing a RETURNING * internally.

On 10 June 2014 05:53, Jim Nasby <jim@nasby.net> wrote:

> I was wondering that myself. I think it's certainly reasonable to expect
> someone would wan RETURNING SEQUENCE VALUES, which would return the value of
> every column that owned a sequence (ie: ALTER SEQUENCE ... OWNED BY). ISTM
> that would certainly handle the performance aspect of this, and it sounds
> more in line with what I'd expect getGeneratedKeys() to do.

Keep in mind that not all generated keys come from sequences. Many people have custom key generator functions, including UUIDs and other exotic things like Instagram's setup [1].

RETURNING GENERATED KEYS perhaps, but then how do we determine that? Any column that was filled with a default value? But that's potentially returning far more values than the user will want - I bet 99% of users just want their generated primary key.

The spec is a bit vague [2]: 

    Retrieves any auto-generated keys created as a result of executing
    this Statement object. If this Statement object did not generate any
    keys, an empty ResultSet object is returned.

    Note:If the columns which represent the auto-generated keys were
    not specified, the JDBC driver implementation will determine the
    columns which best represent the auto-generated keys.

The second paragraph refers to [3] and [4] where the application can specify which columns it's after. Given that there's a mechanism to specify which keys the application wants returned in the driver, and the driver in that case can just issue a RETURNING clause with a column list, my gut feel would be to just support returning primary keys as that will handle most cases of e.g. middleware like ORMs fetching that without needing to know the specific column names.

Cheers

Tom

Re: "RETURNING PRIMARY KEY" syntax extension

From
Hannu Krosing
Date:
On 06/10/2014 03:19 AM, Tom Dunstan wrote:
A definite +1 on this feature. A while ago I got partway through hacking the hibernate postgres dialect to make it issue a RETURNING clause to spit out the primary key before I realised that the driver was already doing a RETURNING * internally.

On 10 June 2014 05:53, Jim Nasby <jim@nasby.net> wrote:

> I was wondering that myself. I think it's certainly reasonable to expect
> someone would wan RETURNING SEQUENCE VALUES, which would return the value of
> every column that owned a sequence (ie: ALTER SEQUENCE ... OWNED BY). ISTM
> that would certainly handle the performance aspect of this, and it sounds
> more in line with what I'd expect getGeneratedKeys() to do.

Keep in mind that not all generated keys come from sequences. Many people have custom key generator functions, including UUIDs and other exotic things like Instagram's setup [1].

RETURNING GENERATED KEYS perhaps, but then how do we determine that?
What about RETURNING CHANGED FIELDS ?

Might be quite complicated technically, but this is what is probably wanted.
Any column that was filled with a default value? But that's potentially returning far more values than the user will want - I bet 99% of users just want their generated primary key.

Probably not true - you would want your ORM model to be in sync with what is database after you save it if you plan to do any further processing using it.

At least I would :)

The spec is a bit vague [2]: 

    Retrieves any auto-generated keys created as a result of executing
    this Statement object. If this Statement object did not generate any
    keys, an empty ResultSet object is returned.

    Note:If the columns which represent the auto-generated keys were
    not specified, the JDBC driver implementation will determine the
    columns which best represent the auto-generated keys.

The second paragraph refers to [3] and [4] where the application can specify which columns it's after. Given that there's a mechanism to specify which keys the application wants returned in the driver, and the driver in that case can just issue a RETURNING clause with a column list, my gut feel would be to just support returning primary keys as that will handle most cases of e.g. middleware like ORMs fetching that without needing to know the specific column names.
Why not then just leave the whole thing as it is on server side, and let the ORM specify which "generated keys" it wants ?


-- 
Hannu Krosing
PostgreSQL Consultant
Performance, Scalability and High Availability
2ndQuadrant Nordic OÜ

Re: "RETURNING PRIMARY KEY" syntax extension

From
Tom Dunstan
Date:
On 10 June 2014 17:49, Hannu Krosing <hannu@2ndquadrant.com> wrote:
RETURNING GENERATED KEYS perhaps, but then how do we determine that?
What about RETURNING CHANGED FIELDS ?

Might be quite complicated technically, but this is what is probably wanted.

Seems to be getting further away from something that describes the main use case - changed fields sounds like something that would apply to an update statement.
Any column that was filled with a default value? But that's potentially returning far more values than the user will want - I bet 99% of users just want their generated primary key.

Probably not true - you would want your ORM model to be in sync with what is database after you save it if you plan to do any further processing using it.

Well, yes, but since RETURNING is non-standard most ORMs are unlikely to support fetching other generated values that way anyway. The ones that I've dealt with will do an insert, then a select to get the extra fields. I don't know if other JDBC drivers allow applications to just specify any old list of non-key columns to the execute method, but I suspect not, given that the way they fetch those columns is rather less general-purpose than our RETURNING syntax.
 

The second paragraph refers to [3] and [4] where the application can specify which columns it's after. Given that there's a mechanism to specify which keys the application wants returned in the driver, and the driver in that case can just issue a RETURNING clause with a column list, my gut feel would be to just support returning primary keys as that will handle most cases of e.g. middleware like ORMs fetching that without needing to know the specific column names.
Why not then just leave the whole thing as it is on server side, and let the ORM specify which "generated keys" it wants ?

Because java-based ORMs (at least) mostly don't have to - other server/driver combos manage to implement getGeneratedKeys() without being explicitly given a column list, they just do the sane thing and return the appropriate identity column or whatever for the inserted row.
 
I agree that in hand-crafted JDBC there's no particular problem in making a user specify a column list, (although I don't think I've EVER seen anyone actually do that in the wild), but most middleware will expect getGeneratedKeys() to just work and we should try to do something about making that case work a bit more efficiently than it does now.

Cheers

Tom

Re: "RETURNING PRIMARY KEY" syntax extension

From
Hannu Krosing
Date:
On 06/10/2014 11:02 AM, Tom Dunstan wrote:
On 10 June 2014 17:49, Hannu Krosing <hannu@2ndquadrant.com> wrote:
RETURNING GENERATED KEYS perhaps, but then how do we determine that?
What about RETURNING CHANGED FIELDS ?

Might be quite complicated technically, but this is what is probably wanted.

Seems to be getting further away from something that describes the main use
case - changed fields sounds like something that would apply to an update statement.
Not really - it applies to both INSERT and UPDATE if there are any triggers and/or default values

The use-case is an extended version of getting the key, with the main aim of making sure
that your ORM model is the same as what is saved in database.
Any column that was filled with a default value? But that's potentially returning far more values than the user will want - I bet 99% of users just want their generated primary key.

Probably not true - you would want your ORM model to be in sync with what is database after you save it if you plan to do any further processing using it.

Well, yes, but since RETURNING is non-standard most ORMs are unlikely to support fetching other generated values that way anyway. The ones that I've dealt with will do an insert, then a select to get the extra fields. I don't know if other JDBC drivers allow applications to just specify any old list of non-key columns to the execute method, but I suspect not, given that the way they fetch those columns is rather less general-purpose than our RETURNING syntax.
 

The second paragraph refers to [3] and [4] where the application can specify which columns it's after. Given that there's a mechanism to specify which keys the application wants returned in the driver, and the driver in that case can just issue a RETURNING clause with a column list, my gut feel would be to just support returning primary keys as that will handle most cases of e.g. middleware like ORMs fetching that without needing to know the specific column names.
Why not then just leave the whole thing as it is on server side, and let the ORM specify which "generated keys" it wants ?

Because java-based ORMs (at least) mostly don't have to - other server/driver combos manage to implement getGeneratedKeys() without being explicitly given a column list, they just do the sane thing and return the appropriate identity column or whatever for the inserted row.
 
I agree that in hand-crafted JDBC there's no particular problem in making a user specify a column list, (although I don't think I've EVER seen anyone actually do that in the wild), but most middleware will expect getGeneratedKeys() to just work and we should try to do something about making that case work a bit more efficiently than it does now.
But does the ORM already not "know" the names of auto-generated keys and thus could easily replace them for * in RETURNING ?

Cheers

Tom


-- 
Hannu Krosing
PostgreSQL Consultant
Performance, Scalability and High Availability
2ndQuadrant Nordic OÜ

Re: "RETURNING PRIMARY KEY" syntax extension

From
Vik Fearing
Date:
On 06/09/2014 07:13 PM, Andres Freund wrote:
> On 2014-06-09 13:42:22 +0200, Vik Fearing wrote:
>> On 06/09/2014 09:06 AM, Gavin Flower wrote:
>>> From memory all unique keys can be considered 'candidate primary keys',
>>> but only one can be designated 'the PRIMARY KEY'.
>>
>> Almost.  Candidate keys are also NOT NULL.
> 
> The list actually is a bit longer. They also cannot be partial.

What?  AFAIK, that only applies to an index.  How can the data itself be
partial?

> There's generally also the restriction that for some contexts - like
> e.g. foreign keys - primary/candidate keys may not be deferrable..

Again, what is deferrable data?
-- 
Vik



Re: "RETURNING PRIMARY KEY" syntax extension

From
Andres Freund
Date:
On 2014-06-11 00:21:58 +0200, Vik Fearing wrote:
> On 06/09/2014 07:13 PM, Andres Freund wrote:
> > On 2014-06-09 13:42:22 +0200, Vik Fearing wrote:
> >> On 06/09/2014 09:06 AM, Gavin Flower wrote:
> >>> From memory all unique keys can be considered 'candidate primary keys',
> >>> but only one can be designated 'the PRIMARY KEY'.
> >>
> >> Almost.  Candidate keys are also NOT NULL.
> > 
> > The list actually is a bit longer. They also cannot be partial.
> 
> What?  AFAIK, that only applies to an index.  How can the data itself be
> partial?

I don't follow? Gavin above talked about unique keys - which in postgres
you can create using CREATE UNIQUE INDEX. And if you make those partial
they can't be used for this purpose.

> > There's generally also the restriction that for some contexts - like
> > e.g. foreign keys - primary/candidate keys may not be deferrable..
> 
> Again, what is deferrable data?

You can define primary/unique constraints to be deferrable. c.f. CREATE
TABLE docs.

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services



Re: "RETURNING PRIMARY KEY" syntax extension

From
Tom Lane
Date:
Andres Freund <andres@2ndquadrant.com> writes:
> On 2014-06-11 00:21:58 +0200, Vik Fearing wrote:
>> What?  AFAIK, that only applies to an index.  How can the data itself be
>> partial?

> I don't follow? Gavin above talked about unique keys - which in postgres
> you can create using CREATE UNIQUE INDEX. And if you make those partial
> they can't be used for this purpose.

I have a feeling this conversation is going in the wrong direction.
ISTM that to be useful at all, the set of columns that would be returned
by a clause like this has to be *extremely* predictable; otherwise the
application won't know what to do with the results.  If the app has to
examine the table's metadata to identify what it's getting, what's the
point of the feature at all as opposed to just listing the columns you
want explicitly?  So I doubt that the use-case for anything more
complicated than returning the primary key, full stop, is really there.

I'm not even 100% sold that automatically returning the primary key
is going to save any application logic.  Could somebody point out
*exactly* where an app is going to save effort with this type of
syntax, compared to requesting the columns it wants by name?
Is it going to save enough to justify depending on a syntax that won't
be universal for a long time to come?
        regards, tom lane



Re: "RETURNING PRIMARY KEY" syntax extension

From
Tom Dunstan
Date:
On 11 June 2014 10:09, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I'm not even 100% sold that automatically returning the primary key
is going to save any application logic.  Could somebody point out
*exactly* where an app is going to save effort with this type of
syntax, compared to requesting the columns it wants by name?
Is it going to save enough to justify depending on a syntax that won't
be universal for a long time to come?

Well, in e.g. Hibernate there's piece of code which calls getGeneratedKeys() to fetch the inserted primary key (it only supports fetching a single generated key column in this way) if the underlying database supports that. The postgresql dialect specifies that it does support that code path, so at the moment any hibernate users who aren't explicitly specifying the "sequence" type for their id generation will be calling that, and the JDBC driver will be appending "RETURNING *" under the hood for all inserts.

Looking at the oracle hibernate dialect is instructive as to the state of support for the explicit-column-list variation:

// Oracle driver reports to support getGeneratedKeys(), but they only
// support the version taking an array of the names of the columns to
// be returned (via its RETURNING clause). No other driver seems to
// support this overloaded version.

And so hibernate doesn't support the explicit-column-list version at all since apparently no-one else supports it, and just marks that code path as unsupported for oracle. I presume that the situation is similar in other java-based ORMs.

Looking at some other drivers that I would expect to support getGeneratedKeys() in a sane way given their identity/auto-increment semantics reveals:

 - JTDS driver for MSSQL/Sybase piggybacks a second query to do "SELECT SCOPE_IDENTITY() AS blah" / "SELECT @@IDENTITY AS blah" to fetch the key if that was requested. It looks like this driver does support specifying the column name, but it only allows a single column to be given, and it promptly ignores the passed in value and calls the non-specified version.

 - MySQL driver internally returns a single ID with the query result, and the driver then appears to add an auto-increment amount to calculate the rest of the values. I guess MySQL must allocate the ids in guaranteed-sequential chunks. MySQL only supports a single auto-increment key. If called with the explicit column version, the passed-in column names are ignored.

So looks like other JDBC driver/server combos only support this for single-column primary keys. But for those cases things pretty much work as expected. It would be nice to be able to support at least primary keys with this feature.

We could try to teach every ORM out there to call the explicit column-list version, but given other lack of support for it I doubt they'll be interested, especially if the reason is because we don't want to add enough support to make getGeneratedKeys() work efficiently.

FWIW I reckon for most users of ORMs at least it will be enough to support this for direct inserts to tables - views is a nice-to-have but I'd take tables-only over not at all.

Cheers

Tom

Re: "RETURNING PRIMARY KEY" syntax extension

From
Tom Dunstan
Date:
Is it going to save enough to justify depending on a syntax that won't
be universal for a long time to come?

Oh, and on the won't-be-universal-for-a-while point - the status quo works fine, it's just less efficient than it should be. Once someone upgrades to 9.5 or whatever, and upgrades to the matching JDBC driver version, they'll get the newer efficient call for free.

In the python world PEP249 has a lastrowid property that drivers can implement, but I don't know how much it's used or our support for it. Any python devs out there want to chime in? I don't know about other drivers.

Obviously anyone hand-crafting their queries won't be able to do that until they know it's safe. But that's always the case with new syntax.

Cheers

Tom

Re: "RETURNING PRIMARY KEY" syntax extension

From
Jochem van Dieten
Date:
On Wed, Jun 11, 2014 at 2:39 AM, Tom Lane wrote:
I'm not even 100% sold that automatically returning the primary key
is going to save any application logic.  Could somebody point out
*exactly* where an app is going to save effort with this type of
syntax, compared to requesting the columns it wants by name?

I haven't checked the code, but I am hoping it will help with the problem where a RETURNING * is added to a statement that is not an insert or update by the JDBC driver. That has been reported on the JDBC list at least twice, and the proposed workaround is neither very elegant nor very robust: https://groups.google.com/forum/#!msg/pgsql.interfaces.jdbc/7WY60JX3qyo/-v1fqDqLQKwJ 

Jochem


--
Jochem van Dieten
http://jochem.vandieten.net/

Re: "RETURNING PRIMARY KEY" syntax extension

From
Ian Barwick
Date:
On 14/06/12 18:46, Jochem van Dieten wrote:
> On Wed, Jun 11, 2014 at 2:39 AM, Tom Lane wrote:
> 
>     I'm not even 100% sold that automatically returning the primary key
>     is going to save any application logic.  Could somebody point out
>     *exactly* where an app is going to save effort with this type of
>     syntax, compared to requesting the columns it wants by name?
> 
> 
> I haven't checked the code, but I am hoping it will help with the problem 
> where a RETURNING * is added to a statement that is not an insert or update
> by the JDBC driver. That has been reported on the JDBC list at least twice,
> and the proposed workaround is neither very elegant nor very robust:
> https://groups.google.com/forum/#!msg/pgsql.interfaces.jdbc/7WY60JX3qyo/-v1fqDqLQKwJ

Unfortunately that seems to be a JDBC-specific issue, which is outside
of the scope of this particular patch (which proposes additional server-side
syntax intended to make RETURNING * operations more efficient for
certain use cases, but which is in itself not a JDBC change).


Regards

Ian Barwick

-- Ian Barwick                   http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services



Re: "RETURNING PRIMARY KEY" syntax extension

From
Jochem van Dieten
Date:
On Thu, Jun 12, 2014 at 12:25 PM, Ian Barwick wrote:
On 14/06/12 18:46, Jochem van Dieten wrote:
> I haven't checked the code, but I am hoping it will help with the problem
> where a RETURNING * is added to a statement that is not an insert or update
> by the JDBC driver. That has been reported on the JDBC list at least twice,
> and the proposed workaround is neither very elegant nor very robust:
> https://groups.google.com/forum/#!msg/pgsql.interfaces.jdbc/7WY60JX3qyo/-v1fqDqLQKwJ

Unfortunately that seems to be a JDBC-specific issue, which is outside
of the scope of this particular patch (which proposes additional server-side
syntax intended to make RETURNING * operations more efficient for
certain use cases, but which is in itself not a JDBC change).

But the obvious way to fix the JDBC issue is not to fix it by adding a 'mini parser' on the JDBC side, but to make SELECT ... RETURNING PRIMARY KEY a regular select that silently ignores the returning clause and doesn't throw an error on the server-side.

That might still be outside the scope of this particular patch, but it would provide (additional) justification if it were supported.

Jochem


--
Jochem van Dieten
http://jochem.vandieten.net/

Re: "RETURNING PRIMARY KEY" syntax extension

From
Andres Freund
Date:
On 2014-06-12 13:58:31 +0200, Jochem van Dieten wrote:
> But the obvious way to fix the JDBC issue is not to fix it by adding a
> 'mini parser' on the JDBC side, but to make SELECT ... RETURNING PRIMARY
> KEY a regular select that silently ignores the returning clause and doesn't
> throw an error on the server-side.

Brr. Then it'd need to be added to all statements, not just SELECT. I've
my doubts that's going to happen.

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services



Re: "RETURNING PRIMARY KEY" syntax extension

From
Ian Barwick
Date:
On 14/06/12 20:58, Jochem van Dieten wrote:
> On Thu, Jun 12, 2014 at 12:25 PM, Ian Barwick wrote:
> 
>     On 14/06/12 18:46, Jochem van Dieten wrote:
>     > I haven't checked the code, but I am hoping it will help with the problem
>     > where a RETURNING * is added to a statement that is not an insert or update
>     > by the JDBC driver. That has been reported on the JDBC list at least twice,
>     > and the proposed workaround is neither very elegant nor very robust:
>     > https://groups.google.com/forum/#!msg/pgsql.interfaces.jdbc/7WY60JX3qyo/-v1fqDqLQKwJ
> 
>     Unfortunately that seems to be a JDBC-specific issue, which is outside
>     of the scope of this particular patch (which proposes additional server-side
>     syntax intended to make RETURNING * operations more efficient for
>     certain use cases, but which is in itself not a JDBC change).
> 
> 
> But the obvious way to fix the JDBC issue is not to fix it by adding a 'mini parser' on
> the JDBC side, but to make SELECT ... RETURNING PRIMARY KEY a regular select that silently
> ignores the returning clause and doesn't throw an error on the server-side.
> 
> That might still be outside the scope of this particular patch, but it would provide 
> (additional) justification if it were supported.

That would be adding superfluous, unused and unusable syntax of no potential value
(there is no SELECT ... RETURNING and it wouldn't make any sense if there was) as a
workaround for a driver issue - not going to happen.

Regards

Ian Barwick


-- Ian Barwick                   http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services



Re: "RETURNING PRIMARY KEY" syntax extension

From
Rushabh Lathia
Date:
Hello All,

I assigned my self as reviewer of the patch. I gone through the
mail chain discussion and in that question has been raised about
the feature and its implementation, so would like to know what is
the current status of this project/patch.

Regards,


On Thu, Jun 12, 2014 at 5:53 PM, Ian Barwick <ian@2ndquadrant.com> wrote:
On 14/06/12 20:58, Jochem van Dieten wrote:
> On Thu, Jun 12, 2014 at 12:25 PM, Ian Barwick wrote:
>
>     On 14/06/12 18:46, Jochem van Dieten wrote:
>     > I haven't checked the code, but I am hoping it will help with the problem
>     > where a RETURNING * is added to a statement that is not an insert or update
>     > by the JDBC driver. That has been reported on the JDBC list at least twice,
>     > and the proposed workaround is neither very elegant nor very robust:
>     > https://groups.google.com/forum/#!msg/pgsql.interfaces.jdbc/7WY60JX3qyo/-v1fqDqLQKwJ
>
>     Unfortunately that seems to be a JDBC-specific issue, which is outside
>     of the scope of this particular patch (which proposes additional server-side
>     syntax intended to make RETURNING * operations more efficient for
>     certain use cases, but which is in itself not a JDBC change).
>
>
> But the obvious way to fix the JDBC issue is not to fix it by adding a 'mini parser' on
> the JDBC side, but to make SELECT ... RETURNING PRIMARY KEY a regular select that silently
> ignores the returning clause and doesn't throw an error on the server-side.
>
> That might still be outside the scope of this particular patch, but it would provide
> (additional) justification if it were supported.

That would be adding superfluous, unused and unusable syntax of no potential value
(there is no SELECT ... RETURNING and it wouldn't make any sense if there was) as a
workaround for a driver issue - not going to happen.

Regards

Ian Barwick


--
 Ian Barwick                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, 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



--
Rushabh Lathia

Re: "RETURNING PRIMARY KEY" syntax extension

From
Ian Barwick
Date:
Hi

On 14/06/25 15:13, Rushabh Lathia wrote:
> Hello All,
> 
> I assigned my self as reviewer of the patch. I gone through the
> mail chain discussion and in that question has been raised about
> the feature and its implementation, so would like to know what is
> the current status of this project/patch.
> 
> Regards,

I'll be submitting a revised version of this patch very shortly.


Regards

Ian Barwick

-- Ian Barwick                   http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services



Re: "RETURNING PRIMARY KEY" syntax extension

From
Ian Barwick
Date:
On 25/06/14 16:04, Ian Barwick wrote:
> Hi
>
> On 14/06/25 15:13, Rushabh Lathia wrote:
>> Hello All,
>>
>> I assigned my self as reviewer of the patch. I gone through the
>> mail chain discussion and in that question has been raised about
>> the feature and its implementation, so would like to know what is
>> the current status of this project/patch.
>>
>> Regards,
>
> I'll be submitting a revised version of this patch very shortly.

Revised version of the patch attached, which implements the expansion
of "primary key" in the rewrite phase per Tom Lane's suggestion upthread [*]

[*] http://www.postgresql.org/message-id/28583.1402325169@sss.pgh.pa.us


Regards

Ian Barwick

--
  Ian Barwick                   http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training & Services

Attachment

Re: "RETURNING PRIMARY KEY" syntax extension

From
Rushabh Lathia
Date:
Thanks for sharing latest patch.

For me this syntax is limiting the current returning clause implementation.
Because its not allowing the returning PRIMARY KEYS with other columns, and
if user or application require that they need to do RETURNING *. Following
syntax not working, which i think is very useful in term if someone want to use
returning clause with PRIMARY KEY as well as some other table columns.

INSERT INTO dept VALUES (10,'ACCOUNTING','NEW YORK') returning primary key, dname;

I think allowing other columns with PRIMARY KEY would be more useful syntax.
Even in later versions if we want to extend this syntax to return UNIQUE KEY,
SEQUENCE VALUES, etc.. comma separation syntax will be more handy.

Others please share your inputs/thoughts.



On Thu, Jun 26, 2014 at 8:11 AM, Ian Barwick <ian@2ndquadrant.com> wrote:
On 25/06/14 16:04, Ian Barwick wrote:
Hi

On 14/06/25 15:13, Rushabh Lathia wrote:
Hello All,

I assigned my self as reviewer of the patch. I gone through the
mail chain discussion and in that question has been raised about
the feature and its implementation, so would like to know what is
the current status of this project/patch.

Regards,

I'll be submitting a revised version of this patch very shortly.

Revised version of the patch attached, which implements the expansion
of "primary key" in the rewrite phase per Tom Lane's suggestion upthread [*]

[*] http://www.postgresql.org/message-id/28583.1402325169@sss.pgh.pa.us



Regards

Ian Barwick

--
 Ian Barwick                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services



--
Rushabh Lathia

Re: "RETURNING PRIMARY KEY" syntax extension

From
Gavin Flower
Date:
On 27/06/14 00:12, Rushabh Lathia wrote:
> Thanks for sharing latest patch.
>
> For me this syntax is limiting the current returning clause 
> implementation.
> Because its not allowing the returning PRIMARY KEYS with other 
> columns, and
> if user or application require that they need to do RETURNING *. Following
> syntax not working, which i think is very useful in term if someone 
> want to use
> returning clause with PRIMARY KEY as well as some other table columns.
>
> INSERT INTO dept VALUES (10,'ACCOUNTING','NEW YORK') returning primary 
> key, dname;
>
> I think allowing other columns with PRIMARY KEY would be more useful 
> syntax.
> Even in later versions if we want to extend this syntax to return 
> UNIQUE KEY,
> SEQUENCE VALUES, etc.. comma separation syntax will be more handy.
>
> Others please share your inputs/thoughts.
>
>
[...]

I agree 100%.

I note that DELETE & INSERT have a RETURNING clause.  So the semantics 
and syntax should be the same, as much as is practicable.


Cheers,
Gavin



Re: "RETURNING PRIMARY KEY" syntax extension

From
Tom Dunstan
Date:
On 27 June 2014 06:14, Gavin Flower <GavinFlower@archidevsys.co.nz> wrote:
On 27/06/14 00:12, Rushabh Lathia wrote:
INSERT INTO dept VALUES (10,'ACCOUNTING','NEW YORK') returning primary key, dname;

I think allowing other columns with PRIMARY KEY would be more useful syntax.
Even in later versions if we want to extend this syntax to return UNIQUE KEY,
SEQUENCE VALUES, etc.. comma separation syntax will be more handy.


I agree 100%.

If the query is being hand-crafted, what's to stop the query writer from just listing the id columns in the returning clause? And someone specifying RETURNING * is getting all the columns anyway.

The target use-case for this feature is a database driver that has just done an insert and doesn't know what the primary key columns are - in that case mixing them with any other columns is actually counter-productive as the driver won't know which columns are which. What use cases are there where the writer of the query knows enough to write specific columns in the RETURNING clause but not enough to know which column is the id column?

Consistency is nice, and I can understand wanting to treat the PRIMARY KEY bit as just another set of columns in the list to return, but I'd hate to see this feature put on the back-burner to support use-cases that are already handled by the current RETURNING feature. Maybe it's easy to do, though.. I haven't looked into the implementation at all.

Cheers

Tom

Re: "RETURNING PRIMARY KEY" syntax extension

From
Ian Barwick
Date:

On 27/06/14 09:09, Tom Dunstan wrote:
> On 27 June 2014 06:14, Gavin Flower <GavinFlower@archidevsys.co.nz <mailto:GavinFlower@archidevsys.co.nz>> wrote:
>
>     On 27/06/14 00:12, Rushabh Lathia wrote:
>
>         INSERT INTO dept VALUES (10,'ACCOUNTING','NEW YORK') returning primary key, dname;
>
>         I think allowing other columns with PRIMARY KEY would be more useful syntax.
>         Even in later versions if we want to extend this syntax to return UNIQUE KEY,
>         SEQUENCE VALUES, etc.. comma separation syntax will be more handy.
>
>
>     I agree 100%.
>
>
> If the query is being hand-crafted, what's to stop the query writer from just listing the> id columns in the
returningclause? And someone specifying RETURNING * is getting all the> columns anyway.
 
>
> The target use-case for this feature is a database driver that has just done an insert and> doesn't know what the
primarykey columns are - in that case mixing them with any other> columns is actually counter-productive as the driver
won'tknow which columns are which.> What use cases are there where the writer of the query knows enough to write
specificcolumns> in the RETURNING clause but not enough to know which column is the id column?
 
>
> Consistency is nice, and I can understand wanting to treat the PRIMARY KEY bit as just
> another set of columns in the list to return, but I'd hate to see this feature put on> the back-burner to support
use-casesthat are already handled by the current RETURNING> feature. Maybe it's easy to do, though.. I haven't looked
intothe implementation at all.
 

Normal columns are injected into the query's returning list at parse time, whereas
this version of the patch handles expansion of PRIMARY KEY at the rewrite stage, which
would make handling a mix of PRIMARY KEY and normal output expressions somewhat tricky
to handle. (In order to maintain the columns in their expected position you'd
have to add some sort of placeholder/dummy TargetEntry to the returning list at parse
time, then rewrite it later with the expanded primary key columns, or something
equally messy).

On the other hand, it should be fairly straightforward to handle a list of keywords
for expansion (e.g. "RETURNING PRIMARY KEY, UNIQUE KEYS, SEQUENCE VALUES") should
the need arise.


Regards

Ian Barwick

--  Ian Barwick                   http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training &
Services



Re: "RETURNING PRIMARY KEY" syntax extension

From
Rushabh Lathia
Date:

I spent some more time on the patch and here are my review comments.

.) Patch gets applied through patch -p1 (git apply fails)

.) trailing whitespace in the patch at various places

.) Unnecessary new line + and - in the patch.
(src/backend/rewrite/rewriteManip.c::getInsertSelectQuery())
(src/include/rewrite/rewriteManip.h)

.) patch has proper test coverage and regression running cleanly.

.) In map_primary_key_to_list() patch using INDEX_ATTR_BITMAP_IDENTITY_KEY
bitmap to get the keycols. In IndexAttrBitmapKind there is also
INDEX_ATTR_BITMAP_KEY, so was confuse between INDEX_ATTR_BITMAP_KEY and
INDEX_ATTR_BITMAP_IDENTITY_KEY and also haven't found related comments in
the code. Later with use of testcase and debugging found confirmed that
INDEX_ATTR_BITMAP_IDENTITY_KEY only returns the Primary key.

.) At present in patch when RETURNING PRIMARY KEY is used on table having no
primary key it throw an error. If I am not wrong JDBC will be using that into
getGeneratedKeys(). Currently this feature is implemented in the JDBC driver by
appending "RETURNING *" to the supplied statement. With this implementation
it will replace "RETURNING *" with "RETURNING PRIMARY KEY", right ? So just
wondering what JDBC expect getGeneratedKeys() to return when query don't
have primary key and user called executeUpdate() with
Statement.RETURN_GENERATED_KEYS? I looked at JDBC specification but its not
clear what it will return when table don't have keys. Can you please let us
know your finding on this ?

.) Already raised my concern regarding syntax limiting the current returning
clause implementation.




On Fri, Jun 27, 2014 at 6:22 AM, Ian Barwick <ian@2ndquadrant.com> wrote:


On 27/06/14 09:09, Tom Dunstan wrote:

On 27 June 2014 06:14, Gavin Flower <GavinFlower@archidevsys.co.nz <mailto:GavinFlower@archidevsys.co.nz>> wrote:

    On 27/06/14 00:12, Rushabh Lathia wrote:

        INSERT INTO dept VALUES (10,'ACCOUNTING','NEW YORK') returning primary key, dname;

        I think allowing other columns with PRIMARY KEY would be more useful syntax.
        Even in later versions if we want to extend this syntax to return UNIQUE KEY,
        SEQUENCE VALUES, etc.. comma separation syntax will be more handy.


    I agree 100%.


If the query is being hand-crafted, what's to stop the query writer from just listing the
> id columns in the returning clause? And someone specifying RETURNING * is getting all the
> columns anyway.

The target use-case for this feature is a database driver that has just done an insert and
> doesn't know what the primary key columns are - in that case mixing them with any other
> columns is actually counter-productive as the driver won't know which columns are which.
> What use cases are there where the writer of the query knows enough to write specific columns
> in the RETURNING clause but not enough to know which column is the id column?

Consistency is nice, and I can understand wanting to treat the PRIMARY KEY bit as just
another set of columns in the list to return, but I'd hate to see this feature put on
> the back-burner to support use-cases that are already handled by the current RETURNING
> feature. Maybe it's easy to do, though.. I haven't looked into the implementation at all.

Normal columns are injected into the query's returning list at parse time, whereas
this version of the patch handles expansion of PRIMARY KEY at the rewrite stage, which
would make handling a mix of PRIMARY KEY and normal output expressions somewhat tricky
to handle. (In order to maintain the columns in their expected position you'd
have to add some sort of placeholder/dummy TargetEntry to the returning list at parse
time, then rewrite it later with the expanded primary key columns, or something
equally messy).

On the other hand, it should be fairly straightforward to handle a list of keywords
for expansion (e.g. "RETURNING PRIMARY KEY, UNIQUE KEYS, SEQUENCE VALUES") should
the need arise.



Regards

Ian Barwick

--
 Ian Barwick                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services



--
Rushabh Lathia

Re: "RETURNING PRIMARY KEY" syntax extension

From
Robert Haas
Date:
On Tue, Jul 1, 2014 at 8:00 AM, Rushabh Lathia <rushabh.lathia@gmail.com> wrote:
> .) In map_primary_key_to_list() patch using INDEX_ATTR_BITMAP_IDENTITY_KEY
> bitmap to get the keycols. In IndexAttrBitmapKind there is also
> INDEX_ATTR_BITMAP_KEY, so was confuse between INDEX_ATTR_BITMAP_KEY and
> INDEX_ATTR_BITMAP_IDENTITY_KEY and also haven't found related comments in
> the code. Later with use of testcase and debugging found confirmed that
> INDEX_ATTR_BITMAP_IDENTITY_KEY only returns the Primary key.

Actually, that depends on how REPLICA IDENTITY is set.  IOW, you can't
assume that will give you the primary key.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: "RETURNING PRIMARY KEY" syntax extension

From
Ian Barwick
Date:
On 14/07/01 23:13, Robert Haas wrote:
> On Tue, Jul 1, 2014 at 8:00 AM, Rushabh Lathia <rushabh.lathia@gmail.com> wrote:
>> .) In map_primary_key_to_list() patch using INDEX_ATTR_BITMAP_IDENTITY_KEY
>> bitmap to get the keycols. In IndexAttrBitmapKind there is also
>> INDEX_ATTR_BITMAP_KEY, so was confuse between INDEX_ATTR_BITMAP_KEY and
>> INDEX_ATTR_BITMAP_IDENTITY_KEY and also haven't found related comments in
>> the code. Later with use of testcase and debugging found confirmed that
>> INDEX_ATTR_BITMAP_IDENTITY_KEY only returns the Primary key.
> 
> Actually, that depends on how REPLICA IDENTITY is set.  IOW, you can't
> assume that will give you the primary key.

Damn, fooled by the name. Thanks for the info; I'll rework the patch
accordingly.

Regards


Ian Barwick


-- Ian Barwick                   http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services



Re: "RETURNING PRIMARY KEY" syntax extension

From
Ian Barwick
Date:
On 02/07/14 15:16, Ian Barwick wrote:
> On 14/07/01 23:13, Robert Haas wrote:
>> On Tue, Jul 1, 2014 at 8:00 AM, Rushabh Lathia <rushabh.lathia@gmail.com> wrote:
>>> .) In map_primary_key_to_list() patch using INDEX_ATTR_BITMAP_IDENTITY_KEY
>>> bitmap to get the keycols. In IndexAttrBitmapKind there is also
>>> INDEX_ATTR_BITMAP_KEY, so was confuse between INDEX_ATTR_BITMAP_KEY and
>>> INDEX_ATTR_BITMAP_IDENTITY_KEY and also haven't found related comments in
>>> the code. Later with use of testcase and debugging found confirmed that
>>> INDEX_ATTR_BITMAP_IDENTITY_KEY only returns the Primary key.
>>
>> Actually, that depends on how REPLICA IDENTITY is set.  IOW, you can't
>> assume that will give you the primary key.
>
> Damn, fooled by the name. Thanks for the info; I'll rework the patch
> accordingly.

Attached version implements an IndexAttrBitmapKind "INDEX_ATTR_BITMAP_PRIMARY_KEY",
which will return the primary key column(s). Note this would require a catalog
version bump.


Regards

Ian Barwick



--
  Ian Barwick                   http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training & Services

Attachment

Re: "RETURNING PRIMARY KEY" syntax extension

From
Ian Barwick
Date:
On 01/07/14 21:00, Rushabh Lathia wrote:
>
> I spent some more time on the patch and here are my review comments.
>
> .) Patch gets applied through patch -p1 (git apply fails)
>
> .) trailing whitespace in the patch at various places

Not sure where you see this, unless it's in the tests, where it's
required.

> .) Unnecessary new line + and - in the patch.
> (src/backend/rewrite/rewriteManip.c::getInsertSelectQuery())
> (src/include/rewrite/rewriteManip.h)

Fixed.

> .) patch has proper test coverage and regression running cleanly.
>
> .) In map_primary_key_to_list() patch using INDEX_ATTR_BITMAP_IDENTITY_KEY
> bitmap to get the keycols. In IndexAttrBitmapKind there is also
> INDEX_ATTR_BITMAP_KEY, so was confuse between INDEX_ATTR_BITMAP_KEY and
> INDEX_ATTR_BITMAP_IDENTITY_KEY and also haven't found related comments in
> the code. Later with use of testcase and debugging found confirmed that
> INDEX_ATTR_BITMAP_IDENTITY_KEY only returns the Primary key.

Revised patch version (see other mail) fixes this by introducing
INDEX_ATTR_BITMAP_PRIMARY_KEY.

> .) At present in patch when RETURNING PRIMARY KEY is used on table having no
> primary key it throw an error. If I am not wrong JDBC will be using that into
> getGeneratedKeys(). Currently this feature is implemented in the JDBC driver by
> appending "RETURNING *" to the supplied statement. With this implementation
> it will replace "RETURNING *" with "RETURNING PRIMARY KEY", right ? So just
> wondering what JDBC expect getGeneratedKeys() to return when query don't
> have primary key and user called executeUpdate() with
> Statement.RETURN_GENERATED_KEYS? I looked at JDBC specification but its not
> clear what it will return when table don't have keys. Can you please let us
> know your finding on this ?

The spec [*] is indeed frustratingly vague:
    The method Statement.getGeneratedKeys, which can be called to retrieve the generated    value, returns a ResultSet
objectwith a column for each automatically generated value.    The methods execute, executeUpdate or
Connection.prepareStatementaccept an optional    parameter, which can be used to indicate that any auto generated
valuesshould be    returned when the statement is executed or prepared.
 

[*] http://download.oracle.com/otn-pub/jcp/jdbc-4_1-mrel-spec/jdbc4.1-fr-spec.pdf

I understand this to mean that no rows will be returned if no auto-generated values
are not present.

As-is, the patch will raise an error if the target table does not have a primary key,
which makes sense from the point of view of the proposed syntax, but which will
make it impossible for the JDBC driver to implement the above understanding of the spec
(i.e. return nothing if no primary key exists).

It would be simple enough not to raise an error in this case, but that means the
query would be effectively failing silently and I don't think that's desirable
behaviour.

A better solution would be to have an optional "IF EXISTS" clause:
  RETURNING PRIMARY KEY [ IF EXISTS ]

which would be easy enough to implement.


Thoughts?


Ian Barwick

--  Ian Barwick                   http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training &
Services



Re: "RETURNING PRIMARY KEY" syntax extension

From
Rushabh Lathia
Date:



On Thu, Jul 3, 2014 at 6:56 AM, Ian Barwick <ian@2ndquadrant.com> wrote:
On 01/07/14 21:00, Rushabh Lathia wrote:

I spent some more time on the patch and here are my review comments.

.) Patch gets applied through patch -p1 (git apply fails)

.) trailing whitespace in the patch at various places

Not sure where you see this, unless it's in the tests, where it's
required.


Please ignore the whitespace comment. Don't know somehow that was due to
my local changes in the branch.
 

.) Unnecessary new line + and - in the patch.
(src/backend/rewrite/rewriteManip.c::getInsertSelectQuery())
(src/include/rewrite/rewriteManip.h)

Fixed.


.) patch has proper test coverage and regression running cleanly.

.) In map_primary_key_to_list() patch using INDEX_ATTR_BITMAP_IDENTITY_KEY
bitmap to get the keycols. In IndexAttrBitmapKind there is also
INDEX_ATTR_BITMAP_KEY, so was confuse between INDEX_ATTR_BITMAP_KEY and
INDEX_ATTR_BITMAP_IDENTITY_KEY and also haven't found related comments in
the code. Later with use of testcase and debugging found confirmed that
INDEX_ATTR_BITMAP_IDENTITY_KEY only returns the Primary key.

Revised patch version (see other mail) fixes this by introducing
INDEX_ATTR_BITMAP_PRIMARY_KEY.


Looks good.
 

.) At present in patch when RETURNING PRIMARY KEY is used on table having no
primary key it throw an error. If I am not wrong JDBC will be using that into
getGeneratedKeys(). Currently this feature is implemented in the JDBC driver by
appending "RETURNING *" to the supplied statement. With this implementation
it will replace "RETURNING *" with "RETURNING PRIMARY KEY", right ? So just
wondering what JDBC expect getGeneratedKeys() to return when query don't
have primary key and user called executeUpdate() with
Statement.RETURN_GENERATED_KEYS? I looked at JDBC specification but its not
clear what it will return when table don't have keys. Can you please let us
know your finding on this ?

The spec [*] is indeed frustratingly vague:

    The method Statement.getGeneratedKeys, which can be called to retrieve the generated
    value, returns a ResultSet object with a column for each automatically generated value.
    The methods execute, executeUpdate or Connection.prepareStatement accept an optional
    parameter, which can be used to indicate that any auto generated values should be
    returned when the statement is executed or prepared.

[*] http://download.oracle.com/otn-pub/jcp/jdbc-4_1-mrel-spec/jdbc4.1-fr-spec.pdf

I understand this to mean that no rows will be returned if no auto-generated values
are not present.

As-is, the patch will raise an error if the target table does not have a primary key,
which makes sense from the point of view of the proposed syntax, but which will
make it impossible for the JDBC driver to implement the above understanding of the spec
(i.e. return nothing if no primary key exists).

The basic reason for this patch is to allow JDBC to use this syntax for getGeneratedKeys().
Now because this patch throw an error when table don't have any primary key, this patch
won't be useful for the getGeneratedKeys() implementation.
 
It would be simple enough not to raise an error in this case, but that means the
query would be effectively failing silently and I don't think that's desirable
behaviour.


Yes, not to raise an error won't be desirable behavior.
 
A better solution would be to have an optional "IF EXISTS" clause:

  RETURNING PRIMARY KEY [ IF EXISTS ]

which would be easy enough to implement.


Thoughts?


Hmm liked the idea about adding [ IF EXISTS ]. Looking at the grammar so far we
having [ IF EXISTS ] option only for DDL commands (mainly DROP) not sure whether
its ok to use such syntax for DML commands.

Others please share your thoughts/comments.



Ian Barwick

--
 Ian Barwick                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services



--
Rushabh Lathia

Re: "RETURNING PRIMARY KEY" syntax extension

From
Tom Lane
Date:
Rushabh Lathia <rushabh.lathia@gmail.com> writes:
> On Thu, Jul 3, 2014 at 6:56 AM, Ian Barwick <ian@2ndquadrant.com> wrote:
>> A better solution would be to have an optional "IF EXISTS" clause:
>> RETURNING PRIMARY KEY [ IF EXISTS ]

> Hmm liked the idea about adding [ IF EXISTS ]. Looking at the grammar so
> far we
> having [ IF EXISTS ] option only for DDL commands (mainly DROP) not sure
> whether
> its ok to use such syntax for DML commands.

> Others please share your thoughts/comments.

TBH, I thought that RETURNING PRIMARY KEY was already fairly iffy
in that the application would have little idea what it was getting back.
IF EXISTS would make it so spongy as to be useless, IMO.

It sounds to me like designing this for JDBC's getGeneratedKeys method
is a mistake.  There isn't going to be any way that the driver can support
that without having looked at the table's metadata for itself, and if
it's going to do that then it doesn't need a crutch that only partially
solves the problem anyhow.
        regards, tom lane



Re: "RETURNING PRIMARY KEY" syntax extension

From
Greg Stark
Date:
On Thu, Jul 3, 2014 at 7:37 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> TBH, I thought that RETURNING PRIMARY KEY was already fairly iffy
> in that the application would have little idea what it was getting back.
> IF EXISTS would make it so spongy as to be useless, IMO.

Seems easy enough to look at the count of columns in the result set.
But it seems like noise words -- if you don't put IF EXISTS then
surely you'll get the same behaviour anyways, no?

Fwiw when I wrote ORM-like layers I used to describe the output of the
query, including sometimes issuing WHERE 1<>0 queries and looking at
the output column types when I needed that before executing the query.
Using table metadata would have required a much more in depth
understanding of how the database worked and also wouldn't handle
expressions, joins, set operations, etc.

-- 
greg



Re: "RETURNING PRIMARY KEY" syntax extension

From
Tom Lane
Date:
Greg Stark <stark@mit.edu> writes:
> On Thu, Jul 3, 2014 at 7:37 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> TBH, I thought that RETURNING PRIMARY KEY was already fairly iffy
>> in that the application would have little idea what it was getting back.
>> IF EXISTS would make it so spongy as to be useless, IMO.

> Seems easy enough to look at the count of columns in the result set.

Sure, if you know how many columns are in the table to begin with; but
that goes back to having table metadata.  If you don't, you're probably
going to be issuing "RETURNING *", and AFAICS "RETURNING *, PRIMARY KEY"
would be entirely useless (even without IF EXISTS :-().

I'm still unconvinced that there's a robust use-case here.
        regards, tom lane



Re: "RETURNING PRIMARY KEY" syntax extension

From
Tom Dunstan
Date:
On 4 July 2014 00:07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
TBH, I thought that RETURNING PRIMARY KEY was already fairly iffy
in that the application would have little idea what it was getting back.
IF EXISTS would make it so spongy as to be useless, IMO.

IF EXISTS is pretty pointless - while the behaviour of getGeneratedKeys() isn't defined for cases where there aren't any, it's only meaningful if the caller has previously asked for the keys to be returned, and someone asking to do that where it doesn't make sense can get an error as far as I'm concerned. No-one does this in practice.

Looking at the feature as a more general SQL construct, ISTM that if someone requests RETURNING PRIMARY KEY where there isn't one, an error is appropriate. And for the IF EXISTS case, when on earth will someone request a primary key even if they're not sure one exists?
 
It sounds to me like designing this for JDBC's getGeneratedKeys method
is a mistake.  There isn't going to be any way that the driver can support
that without having looked at the table's metadata for itself, and if
it's going to do that then it doesn't need a crutch that only partially
solves the problem anyhow.

Sure it can - it append RETURNING PRIMARY KEY and hand back a ResultSet from whatever was returned. It's CURRENTLY doing that, but it's appending RETURNING * and leaving it up to the caller of getGeneratedKeys() to work out which columns the caller is interested in.

Turns out that's actually ok - most Java-based ORMs have more than enough metadata about the tables they manage to know which columns are the primary key. It's the driver that doesn't have that information, which is where RETURNING PRIMARY KEY can help by not forcing the driver to request that every single column is returned.

The only downside that I see is cases where someone requests the keys to be returned but already has a RETURNING clause in their insert statement - what if the requested columns don't include the primary key? IMO it's probably enough to document that if the caller wants to issue a RETURNING clause then they better make sure it contains the columns they're interested in. This is already way outside anything that standard ORMs will do (they don't know about RETURNING) so anyone doing this is hand-crafting anyway.

Cheers

Tom

Re: "RETURNING PRIMARY KEY" syntax extension

From
Tom Lane
Date:
Tom Dunstan <pgsql@tomd.cc> writes:
> On 4 July 2014 00:07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> It sounds to me like designing this for JDBC's getGeneratedKeys method
>> is a mistake.  There isn't going to be any way that the driver can support
>> that without having looked at the table's metadata for itself, and if
>> it's going to do that then it doesn't need a crutch that only partially
>> solves the problem anyhow.

> Sure it can - it append RETURNING PRIMARY KEY and hand back a ResultSet
> from whatever was returned. It's CURRENTLY doing that, but it's appending
> RETURNING * and leaving it up to the caller of getGeneratedKeys() to work
> out which columns the caller is interested in.

I might be mistaken, but as I read the spec for getGeneratedKeys, whether
or not a column is in a primary key has got nothing to do with whether
that column should be returned.  It looks to me like what you're supposed
to return is columns with computed default values, eg, serial columns.
(Whether we should define it as being *exactly* columns created by the
SERIAL macro is an interesting question, but for sure those ought to be
included.)  Now, the pkey might be a serial column ... but it equally
well might not be; it might not have any default expression at all.
And there certainly could be serial columns that weren't in the pkey.

The fact that the spec is kinda fuzzy doesn't entitle us to ignore the
plain meaning of the term "generated key".
        regards, tom lane



Re: "RETURNING PRIMARY KEY" syntax extension

From
Tom Dunstan
Date:
On 4 July 2014 11:37, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Sure it can - it append RETURNING PRIMARY KEY and hand back a ResultSet
> from whatever was returned. It's CURRENTLY doing that, but it's appending
> RETURNING * and leaving it up to the caller of getGeneratedKeys() to work
> out which columns the caller is interested in.

I might be mistaken, but as I read the spec for getGeneratedKeys, whether
or not a column is in a primary key has got nothing to do with whether
that column should be returned.  It looks to me like what you're supposed
to return is columns with computed default values, eg, serial columns.
(Whether we should define it as being *exactly* columns created by the
SERIAL macro is an interesting question, but for sure those ought to be
included.)  Now, the pkey might be a serial column ... but it equally
well might not be; it might not have any default expression at all.
And there certainly could be serial columns that weren't in the pkey.

The fact that the spec is kinda fuzzy doesn't entitle us to ignore the
plain meaning of the term "generated key".

Well, strictly speaking no, but in reality it doesn't look like other databases that support this feature support anything other than returning auto-increment fields on primary keys, often only supporting a single column (see [1]). Oracle doesn't support even that - callers have to call the variant that specifies a column list, since historically there was no real support for recognising such columns, oracle being sequence based.

As such, there can't be any callers in the wild that will expect this to return anything other than the primary key, because there's no support for doing anything else. And if the caller really DOES want other columns returned, they can always call the variant that allows them to specify those columns, or just issue a normal query with a returning clause. This feature really exists to support the default case where those columns aren't specified by the caller, and given current driver support (and sanity) the only thing any caller will expect from such a result is the primary key.

Cheers

Tom

Re: "RETURNING PRIMARY KEY" syntax extension

From
Rushabh Lathia
Date:



On Fri, Jul 4, 2014 at 7:37 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Tom Dunstan <pgsql@tomd.cc> writes:
> On 4 July 2014 00:07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> It sounds to me like designing this for JDBC's getGeneratedKeys method
>> is a mistake.  There isn't going to be any way that the driver can support
>> that without having looked at the table's metadata for itself, and if
>> it's going to do that then it doesn't need a crutch that only partially
>> solves the problem anyhow.

> Sure it can - it append RETURNING PRIMARY KEY and hand back a ResultSet
> from whatever was returned. It's CURRENTLY doing that, but it's appending
> RETURNING * and leaving it up to the caller of getGeneratedKeys() to work
> out which columns the caller is interested in.

I might be mistaken, but as I read the spec for getGeneratedKeys, whether
or not a column is in a primary key has got nothing to do with whether
that column should be returned.  It looks to me like what you're supposed
to return is columns with computed default values, eg, serial columns.
(Whether we should define it as being *exactly* columns created by the
SERIAL macro is an interesting question, but for sure those ought to be
included.)  Now, the pkey might be a serial column ... but it equally
well might not be; it might not have any default expression at all.
And there certainly could be serial columns that weren't in the pkey.


100% agree with Tom.
 
The fact that the spec is kinda fuzzy doesn't entitle us to ignore the
plain meaning of the term "generated key".

                        regards, tom lane



--
Rushabh Lathia

Re: "RETURNING PRIMARY KEY" syntax extension

From
Robert Haas
Date:
On Fri, Jul 4, 2014 at 12:36 AM, Rushabh Lathia
<rushabh.lathia@gmail.com> wrote:
> On Fri, Jul 4, 2014 at 7:37 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Tom Dunstan <pgsql@tomd.cc> writes:
>> > On 4 July 2014 00:07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> >> It sounds to me like designing this for JDBC's getGeneratedKeys method
>> >> is a mistake.  There isn't going to be any way that the driver can
>> >> support
>> >> that without having looked at the table's metadata for itself, and if
>> >> it's going to do that then it doesn't need a crutch that only partially
>> >> solves the problem anyhow.
>>
>> > Sure it can - it append RETURNING PRIMARY KEY and hand back a ResultSet
>> > from whatever was returned. It's CURRENTLY doing that, but it's
>> > appending
>> > RETURNING * and leaving it up to the caller of getGeneratedKeys() to
>> > work
>> > out which columns the caller is interested in.
>>
>> I might be mistaken, but as I read the spec for getGeneratedKeys, whether
>> or not a column is in a primary key has got nothing to do with whether
>> that column should be returned.  It looks to me like what you're supposed
>> to return is columns with computed default values, eg, serial columns.
>> (Whether we should define it as being *exactly* columns created by the
>> SERIAL macro is an interesting question, but for sure those ought to be
>> included.)  Now, the pkey might be a serial column ... but it equally
>> well might not be; it might not have any default expression at all.
>> And there certainly could be serial columns that weren't in the pkey.
>
> 100% agree with Tom.

Well, we could have a RETURNING GENERATED COLUMNS construct, or
something like that.  I can certainly see the usefulness of such a
thing.

As a general note not necessarily related to this specific issue, I
think we should be careful not to lose sight of the point of this
feature, which is to allow pgsql-jdbc to more closely adhere to the
JDBC specification.  While it's great if this feature has general
utility, if it doesn't help pgsql-jdbc, then it fails to achieve the
author's original intention.  We need to make sure we're not throwing
up too many obstacles in the way of better driver compliance if we
want to have good drivers.

As a code-level comment, I am not too find of adding yet another value
for IndexAttrBitmapKind; every values we compute in
RelationGetIndexAttrBitmap adds overhead for everyone, even people not
using the feature.  Admittedly, it's only paid once per relcache load,
but does
map_primary_key_to_list() really need this information cached, or can
it just look through the index list for the primary key, and then work
directly from that?

Also, map_primary_key_to_list() tacitly assumes that an auto-updatable
view has only 1 from-list item.  That seems like a good thing to check
with an Assert(), just in case we should support some other case in
the future.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: "RETURNING PRIMARY KEY" syntax extension

From
Rushabh Lathia
Date:



On Mon, Jul 7, 2014 at 5:50 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Fri, Jul 4, 2014 at 12:36 AM, Rushabh Lathia
<rushabh.lathia@gmail.com> wrote:
> On Fri, Jul 4, 2014 at 7:37 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Tom Dunstan <pgsql@tomd.cc> writes:
>> > On 4 July 2014 00:07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> >> It sounds to me like designing this for JDBC's getGeneratedKeys method
>> >> is a mistake.  There isn't going to be any way that the driver can
>> >> support
>> >> that without having looked at the table's metadata for itself, and if
>> >> it's going to do that then it doesn't need a crutch that only partially
>> >> solves the problem anyhow.
>>
>> > Sure it can - it append RETURNING PRIMARY KEY and hand back a ResultSet
>> > from whatever was returned. It's CURRENTLY doing that, but it's
>> > appending
>> > RETURNING * and leaving it up to the caller of getGeneratedKeys() to
>> > work
>> > out which columns the caller is interested in.
>>
>> I might be mistaken, but as I read the spec for getGeneratedKeys, whether
>> or not a column is in a primary key has got nothing to do with whether
>> that column should be returned.  It looks to me like what you're supposed
>> to return is columns with computed default values, eg, serial columns.
>> (Whether we should define it as being *exactly* columns created by the
>> SERIAL macro is an interesting question, but for sure those ought to be
>> included.)  Now, the pkey might be a serial column ... but it equally
>> well might not be; it might not have any default expression at all.
>> And there certainly could be serial columns that weren't in the pkey.
>
> 100% agree with Tom.

Well, we could have a RETURNING GENERATED COLUMNS construct, or
something like that.  I can certainly see the usefulness of such a
thing.

As a general note not necessarily related to this specific issue, I
think we should be careful not to lose sight of the point of this
feature, which is to allow pgsql-jdbc to more closely adhere to the
JDBC specification.  While it's great if this feature has general
utility, if it doesn't help pgsql-jdbc, then it fails to achieve the
author's original intention.  We need to make sure we're not throwing
up too many obstacles in the way of better driver compliance if we
want to have good drivers.

As a code-level comment, I am not too find of adding yet another value
for IndexAttrBitmapKind; every values we compute in
RelationGetIndexAttrBitmap adds overhead for everyone, even people not
using the feature.  Admittedly, it's only paid once per relcache load,
but does
map_primary_key_to_list() really need this information cached, or can
it just look through the index list for the primary key, and then work
directly from that?

Also, map_primary_key_to_list() tacitly assumes that an auto-updatable
view has only 1 from-list item.  That seems like a good thing to check
with an Assert(), just in case we should support some other case in
the future.


Another code-level comment is, why we need RowExclusiveLock before calling
map_primary_key_to_list() ? I think we should have explanation for that.

+            /* Handle RETURNING PRIMARY KEY */
+            if(query->returningPK)
+            {
+                RangeTblEntry *rte = (RangeTblEntry *) list_nth(query->rtable, query->resultRelation - 1);
+                Relation rel = heap_open(rte->relid, RowExclusiveLock);
+                query->returningList = map_primary_key_to_list(rel, query);
+                heap_close(rel, NoLock);
+            }
 

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



--
Rushabh Lathia