Re: [HACKERS] identity columns

On Fri, Apr 28, 2017 at 2:49 PM, Peter Eisentraut wrote: > On 4/27/17 16:10, Robert Haas wrote: >> I still think you should consider improving the psql output, though. >> Vitaly's examples upthread indicate that for a serial sequence, >> there's psql output

Re: [HACKERS] identity columns

On 4/27/17 16:10, Robert Haas wrote: > I still think you should consider improving the psql output, though. > Vitaly's examples upthread indicate that for a serial sequence, > there's psql output showing the linkage between the table and sequence > in both directions, but not when GENERATED is

Re: [HACKERS] identity columns

On Thu, Apr 27, 2017 at 3:42 PM, Peter Eisentraut wrote: > On 4/27/17 10:03, Robert Haas wrote: >>> So we could make up new syntax >>> >>> ALTER TABLE t1 ALTER COLUMN c1 SET GENERATED ALWAYS AS IDENTITY; >>> >>> and let that be set-or-add, but then the argument

Re: [HACKERS] identity columns

On 4/27/17 10:03, Robert Haas wrote: >> So we could make up new syntax >> >> ALTER TABLE t1 ALTER COLUMN c1 SET GENERATED ALWAYS AS IDENTITY; >> >> and let that be set-or-add, but then the argument for staying within the >> SQL standard goes out the window. > > What does the SQL standard actually

Re: [HACKERS] identity columns

On Wed, Apr 26, 2017 at 10:03 PM, Peter Eisentraut wrote: > On 4/23/17 16:58, Robert Haas wrote: >> I agree that ADD is a little odd here, but it doesn't seem terrible. >> But why do we need it? Instead of: >> >> ADD GENERATED { ALWAYS | BY DEFAULT } AS IDENTITY

Re: [HACKERS] identity columns

On 4/23/17 16:58, Robert Haas wrote: > I agree that ADD is a little odd here, but it doesn't seem terrible. > But why do we need it? Instead of: > > ADD GENERATED { ALWAYS | BY DEFAULT } AS IDENTITY > SET GENERATED { ALWAYS | BY DEFAULT } > DROP IDENTITY [ IF EXISTS ] > > Why not just: > > SET

Re: [HACKERS] identity columns

On Sun, Apr 23, 2017 at 9:03 PM, Vitaly Burovoy wrote: >> But why do we need it? Instead of: >> >> ADD GENERATED { ALWAYS | BY DEFAULT } AS IDENTITY >> SET GENERATED { ALWAYS | BY DEFAULT } >> DROP IDENTITY [ IF EXISTS ] >> >> Why not just: >> >> SET GENERATED { ALWAYS

Re: [HACKERS] identity columns

On Mon, Apr 24, 2017 at 10:03 AM, Vitaly Burovoy wrote: > On 4/23/17, Robert Haas wrote: >> On Thu, Apr 20, 2017 at 12:05 AM, Vitaly Burovoy >> wrote: >> But why do we need it? Instead of: >> >> ADD GENERATED { ALWAYS |

Re: [HACKERS] identity columns

On 4/23/17, Robert Haas wrote: > On Thu, Apr 20, 2017 at 12:05 AM, Vitaly Burovoy > wrote: >> OK. Let's go through it again. >> IDENTITY is a property of a column. There are no syntax to change any >> property of any DB object via the "ADD"

Re: [HACKERS] identity columns

On Thu, Apr 20, 2017 at 12:05 AM, Vitaly Burovoy wrote: >> I am still not fond of this change. There is precedent all over the >> place for having separate commands for creating a structure, changing a >> structure, and removing a structure. I don't understand what the

Re: [HACKERS] identity columns

On Tue, Apr 18, 2017 at 11:53:36AM -0400, Peter Eisentraut wrote: > On 4/18/17 02:07, Noah Misch wrote: > > This PostgreSQL 10 open item is past due for your status update. Kindly > > send > > a status update within 24 hours, and include a date for your subsequent > > status > > update. Refer

Re: [HACKERS] identity columns

On 4/18/17, Peter Eisentraut wrote: > On 4/7/17 01:26, Vitaly Burovoy wrote: >> I've implement SET GENERATED ... IF NOT EXISTS. It must be placed >> before other SET options but fortunately it conforms with the >> standard. >> Since that form always changes the

Re: [HACKERS] identity columns

On 4/18/17 02:07, Noah Misch wrote: > This PostgreSQL 10 open item is past due for your status update. Kindly send > a status update within 24 hours, and include a date for your subsequent status > update. Refer to the policy on open item ownership: >

Re: [HACKERS] identity columns

On 4/7/17 01:26, Vitaly Burovoy wrote: > I've implement SET GENERATED ... IF NOT EXISTS. It must be placed > before other SET options but fortunately it conforms with the > standard. > Since that form always changes the sequence behind the column, I > decided to explicitly write "[NO] CACHE" in

Re: [HACKERS] identity columns

On Fri, Apr 14, 2017 at 05:56:44AM +, Noah Misch wrote: > On Thu, Apr 06, 2017 at 10:26:16PM -0700, Vitaly Burovoy wrote: > > On 4/6/17, Peter Eisentraut wrote: > > > On 4/4/17 22:53, Vitaly Burovoy wrote: > > >> The next nitpickings to the last patch. I try

Re: [HACKERS] identity columns

On Thu, Apr 06, 2017 at 10:26:16PM -0700, Vitaly Burovoy wrote: > On 4/6/17, Peter Eisentraut wrote: > > On 4/4/17 22:53, Vitaly Burovoy wrote: > >> The next nitpickings to the last patch. I try to get places with > >> lacking of variables' initialization. > >>

Re: [HACKERS] identity columns

On Thu, Apr 13, 2017 at 7:40 PM, Vitaly Burovoy wrote: > On 4/6/17, Vitaly Burovoy wrote: >> On 4/6/17, Peter Eisentraut wrote: >>> As I tried to mention earlier, it is very difficult to implement the IF >>>

Re: [HACKERS] identity columns

On 4/6/17, Vitaly Burovoy wrote: > On 4/6/17, Peter Eisentraut wrote: >> As I tried to mention earlier, it is very difficult to implement the IF >> NOT EXISTS behavior here, because we need to run the commands the create >> the sequence

Re: [HACKERS] identity columns

On 4/6/17, Peter Eisentraut wrote: > On 4/4/17 22:53, Vitaly Burovoy wrote: >> The next nitpickings to the last patch. I try to get places with >> lacking of variables' initialization. >> All other things seem good for me now. I'll continue to review the >> patch

Re: [HACKERS] identity columns

On 4/4/17 22:53, Vitaly Burovoy wrote: > The next nitpickings to the last patch. I try to get places with > lacking of variables' initialization. > All other things seem good for me now. I'll continue to review the > patch while you're fixing the current notes. Committed with your changes (see

Re: [HACKERS] identity columns

On 4/4/17, Vitaly Burovoy wrote: > On 4/3/17, Peter Eisentraut wrote: >> On 3/30/17 22:57, Vitaly Burovoy wrote: >>> Why do you still want to leave "ADD IF NOT EXISTS" instead of using >>> "SET IF NOT EXISTS"? >>> If someone wants to

Re: [HACKERS] identity columns

On 4/3/17, Peter Eisentraut wrote: > On 3/30/17 22:57, Vitaly Burovoy wrote: >> Why do you still want to leave "ADD IF NOT EXISTS" instead of using >> "SET IF NOT EXISTS"? >> If someone wants to follow the standard he can simply not to use "IF >> NOT EXISTS" at

Re: [HACKERS] identity columns

Peter Eisentraut writes: > On 4/3/17 14:19, Andres Freund wrote: > + *op->resvalue = > Int64GetDatum(nextval_internal(op->d.nextvalueexpr.seqid, false)); >> Is it guaranteed that the caller expects an int64? I saw that >> nextvalueexpr's

Re: [HACKERS] identity columns

On 2017-04-03 16:31:27 -0400, Peter Eisentraut wrote: > > 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

Re: [HACKERS] identity columns

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: >> +{ >>

Re: [HACKERS] identity columns

On 3/30/17 22:57, Vitaly Burovoy wrote: > Why do you still want to leave "ADD IF NOT EXISTS" instead of using > "SET IF NOT EXISTS"? > If someone wants to follow the standard he can simply not to use "IF > NOT EXISTS" at all. Without it error should be raised. As I tried to mention earlier, it is

Re: [HACKERS] identity columns

On 2017-03-29 13:40:29 -0400, Peter Eisentraut wrote: > On 3/24/17 05:29, Vitaly Burovoy wrote: > > It would be great if "DROP IDENTITY IF EXISTS" is in the current patch > > since we don't have any disagreements about "DROP IDENTITY" behavior > > and easiness of implementation of "IF EXISTS"

Re: [HACKERS] identity columns

On 3/29/17, Peter Eisentraut wrote: > On 3/24/17 05:29, Vitaly Burovoy wrote: >> It would be great if "DROP IDENTITY IF EXISTS" is in the current patch >> since we don't have any disagreements about "DROP IDENTITY" behavior >> and easiness of implementation of

Re: [HACKERS] identity columns

On 3/23/17, Peter Eisentraut wrote: > On 3/23/17 06:09, Vitaly Burovoy wrote: >> I think we'll end up with "DROP IDENTITY IF EXISTS" to avoid raising >> an exception and "ADD OR SET" if your grammar remains. > > That sounds reasonable to me. It would be great if

Re: [HACKERS] identity columns

On 3/23/17 06:09, Vitaly Burovoy wrote: > I think we'll end up with "DROP IDENTITY IF EXISTS" to avoid raising > an exception and "ADD OR SET" if your grammar remains. That sounds reasonable to me. > Right. From that PoV IDENTITY also changes a default value: "SET (ADD > ... AS?) IDENTITY" works

Re: [HACKERS] identity columns

On 3/22/17, Peter Eisentraut wrote: > On 3/22/17 03:59, Vitaly Burovoy wrote: >> Column's IDENTITY behavior is very similar to a DEFAULT one. We write >> "SET DEFAULT" and don't care whether it was set before or not, because >> we can't have many of them for a

Re: [HACKERS] identity columns

On 3/22/17 03:59, Vitaly Burovoy wrote: > Column's IDENTITY behavior is very similar to a DEFAULT one. We write > "SET DEFAULT" and don't care whether it was set before or not, because > we can't have many of them for a single column. Why should we do that > for IDENTITY? One indication is that

Re: [HACKERS] identity columns

On 3/21/17, Peter Eisentraut wrote: > On 3/21/17 16:11, Vitaly Burovoy wrote: >> My argument is consistency. >> Since IDENTITY is a property of a column (similar to DEFAULT, NOT >> NULL, attributes, STORAGE, etc.), it follows a different rule: it is >> either set

Re: [HACKERS] identity columns

On 3/21/17 16:11, Vitaly Burovoy wrote: > I've just checked and still get an error about a type, not about > absence of a column: > test=# CREATE TABLE itest3 (a int generated by default as identity, b text); > CREATE TABLE > test=# ALTER TABLE itest3 ALTER COLUMN o ADD GENERATED ALWAYS AS

Re: [HACKERS] identity columns

I haven't seen a patch (I'll do it later), just few notes: On 3/21/17, Peter Eisentraut wrote: 3. Strange error (not about absence of a column; but see pp.5 and 8): test=# ALTER TABLE idnt ALTER COLUMN o ADD GENERATED ALWAYS AS IDENTITY;

Re: [HACKERS] identity columns

On 2/28/17, Peter Eisentraut wrote: > New patch that fixes everything. ;-) Great work! > On 1/4/17 19:34, Vitaly Burovoy wrote: >> 1. The fact COPY ignores GENERATED ALWAYS constraint (treats as >> GENERATED BY DEFAULT) should be mentioned as well as rules. >

Re: [HACKERS] identity columns

On 3/15/17, Peter Eisentraut wrote: > Vitaly, will you be able to review this again? > > -- > Peter Eisentraut http://www.2ndQuadrant.com/ I apologize for a delay. Yes, I'm going to do it by Sunday. -- Best regards, Vitaly Burovoy -- Sent via

Re: [HACKERS] identity columns

Vitaly, will you be able to review this again? On 2/28/17 21:23, Peter Eisentraut wrote: > New patch that fixes everything. ;-) Besides hopefully addressing all > your issues, this version also no longer requires separate privileges on > the internal sequence, which was the last outstanding

Re: [HACKERS] identity columns

On Thu, Jan 5, 2017 at 9:34 AM, Vitaly Burovoy wrote: > I apologize for a silence since the last CF. > I've tested your last patch and have several nitpickings: Last update is a review from 3 weeks ago, this patch is marked as returned with feedback. -- Michael --

Re: [HACKERS] identity columns

Hello, Peter, I apologize for a silence since the last CF. I've tested your last patch and have several nitpickings: 1. The fact COPY ignores GENERATED ALWAYS constraint (treats as GENERATED BY DEFAULT) should be mentioned as well as rules. 2. Usually error message for identical columns (with

Re: [HACKERS] identity columns

Updated patch with some merge conflicts resolved and some minor bug fixes. Vitaly's earlier reviews were very comprehensive, and I believe I have fixed all the issues that have been pointed out, so just double-checking that would be helpful at this point. I still don't have a solution for

Re: [HACKERS] identity columns

On Tue, Nov 22, 2016 at 10:51 PM, Haribabu Kommi wrote: > Hi Vitaly, > > > This is a gentle reminder. > > you assigned as reviewer to the current patch in the 11-2016 commitfest. > But you haven't shared your review yet on the new patch in this commitfest > . > Please

Re: [HACKERS] identity columns

Hi Vitaly, This is a gentle reminder. you assigned as reviewer to the current patch in the 11-2016 commitfest. But you haven't shared your review yet on the new patch in this commitfest. Please share your review about the patch. This will help us in smoother operation of commitfest. Please

Re: [HACKERS] identity columns

New patch. On 9/9/16 11:45 PM, Vitaly Burovoy wrote: > 1. The standard requires "... ALTER COLUMN ... SET GENERATED { ALWAYS > | BY DEFAULT }" (9075-2:2011 subcl 11.20), but the patch implements it > as "... ALTER COLUMN ... ADD GENERATED { ALWAYS | BY DEFAULT } AS > IDENTITY" Has both now.

Re: [HACKERS] identity columns

On Mon, Sep 12, 2016 at 5:02 PM, Peter Eisentraut wrote: > Thank you for this extensive testing. I will work on getting the bugs > fixed. It looks like the patch has not been updated; since the CommitFest is (hopefully) wrapping up, I am marking this "Returned

Re: [HACKERS] identity columns

On 9/12/16, Peter Eisentraut wrote: > Thank you for this extensive testing. I will work on getting the bugs > fixed. Just a couple of comments on some of your points: > > On 9/9/16 11:45 PM, Vitaly Burovoy wrote: >> It compiles and passes "make check" tests,

Re: [HACKERS] identity columns

Thank you for this extensive testing. I will work on getting the bugs fixed. Just a couple of comments on some of your points: On 9/9/16 11:45 PM, Vitaly Burovoy wrote: > It compiles and passes "make check" tests, but fails with "make check-world" > at: > test foreign_data ...

Re: [HACKERS] identity columns

Hello Peter, I have reviewed the patch. Currently it does not applies at the top of master, the last commit without a conflict is 975768f It compiles and passes "make check" tests, but fails with "make check-world" at: test foreign_data ... FAILED It tries to implement SQL:2011

Re: [HACKERS] identity columns

Hello, The first look at the patch: On 8/30/16, Peter Eisentraut wrote: > Here is another attempt to implement identity columns. This is a > standard-conforming variant of PostgreSQL's serial columns. > > ... > > Some comments on the implementation, and where

[HACKERS] identity columns

Here is another attempt to implement identity columns. This is a standard-conforming variant of PostgreSQL's serial columns. It also fixes a few usability issues that serial columns have: - need to set permissions on sequence in addition to table (*) - CREATE TABLE / LIKE copies default but