Re: Dump-restore loosing 'attnotnull' bit for DEFERRABLE PRIMARY KEY column(s).

2024-05-14 Thread Robert Haas
On Tue, May 14, 2024 at 11:11 AM Alvaro Herrera wrote: > > So, are you saying this should be marked Committed in the commitfest? > > Yeah. I've done so. Great, thanks. -- Robert Haas EDB: http://www.enterprisedb.com

Re: Dump-restore loosing 'attnotnull' bit for DEFERRABLE PRIMARY KEY column(s).

2024-05-14 Thread Alvaro Herrera
On 2024-May-14, Robert Haas wrote: > On Tue, May 14, 2024 at 10:42 AM Alvaro Herrera > wrote: > > This had already been committed as 270af6f0df76 (the day before it was > > sent to the next commitfest). This commit wasn't included in the > > reverted set, though, so you still get deferrable

Re: Dump-restore loosing 'attnotnull' bit for DEFERRABLE PRIMARY KEY column(s).

2024-05-14 Thread Robert Haas
On Tue, May 14, 2024 at 10:42 AM Alvaro Herrera wrote: > This had already been committed as 270af6f0df76 (the day before it was > sent to the next commitfest). This commit wasn't included in the > reverted set, though, so you still get deferrable PKs from > RelationGetIndexList. I don't think

Re: Dump-restore loosing 'attnotnull' bit for DEFERRABLE PRIMARY KEY column(s).

2024-05-14 Thread Alvaro Herrera
On 2024-May-14, Robert Haas wrote: > On Thu, Mar 7, 2024 at 12:32 PM Alvaro Herrera > wrote: > > On 2024-Mar-07, Alvaro Herrera wrote: > > > Maybe we can add a flag RelationData->rd_ispkdeferred, so that > > > RelationGetPrimaryKeyIndex returned InvalidOid for deferrable PKs; then > > > logical

Re: Dump-restore loosing 'attnotnull' bit for DEFERRABLE PRIMARY KEY column(s).

2024-05-14 Thread Robert Haas
On Thu, Mar 7, 2024 at 12:32 PM Alvaro Herrera wrote: > On 2024-Mar-07, Alvaro Herrera wrote: > > Maybe we can add a flag RelationData->rd_ispkdeferred, so that > > RelationGetPrimaryKeyIndex returned InvalidOid for deferrable PKs; then > > logical replication would continue to not know about

Re: Dump-restore loosing 'attnotnull' bit for DEFERRABLE PRIMARY KEY column(s).

2024-03-08 Thread Dean Rasheed
On Thu, 7 Mar 2024 at 17:32, Alvaro Herrera wrote: > > This seems to work okay. > Yes, this looks good. I tested it against CREATE TABLE ... LIKE, and it worked as expected. It might be worth adding a test case for that, to ensure that it doesn't get broken in the future. Do we also want a test

Re: Dump-restore loosing 'attnotnull' bit for DEFERRABLE PRIMARY KEY column(s).

2024-03-07 Thread Amul Sul
On Thu, Mar 7, 2024 at 11:02 PM Alvaro Herrera wrote: > On 2024-Mar-07, Alvaro Herrera wrote: > > > Maybe we can add a flag RelationData->rd_ispkdeferred, so that > > RelationGetPrimaryKeyIndex returned InvalidOid for deferrable PKs; then > > logical replication would continue to not know about

Re: Dump-restore loosing 'attnotnull' bit for DEFERRABLE PRIMARY KEY column(s).

2024-03-07 Thread Alvaro Herrera
On 2024-Mar-07, Alvaro Herrera wrote: > Maybe we can add a flag RelationData->rd_ispkdeferred, so that > RelationGetPrimaryKeyIndex returned InvalidOid for deferrable PKs; then > logical replication would continue to not know about this PK, which > perhaps is what we want. I'll do some testing

Re: Dump-restore loosing 'attnotnull' bit for DEFERRABLE PRIMARY KEY column(s).

2024-03-07 Thread Alvaro Herrera
On 2024-Mar-07, Dean Rasheed wrote: > On Thu, 7 Mar 2024 at 13:00, Alvaro Herrera wrote: > > > > So I think the original developers of REPLICA IDENTITY had the right > > idea here (commit 07cacba983ef), and we mustn't change this aspect, > > because it'll lead to data corruption in replication.

Re: Dump-restore loosing 'attnotnull' bit for DEFERRABLE PRIMARY KEY column(s).

2024-03-07 Thread Dean Rasheed
On Thu, 7 Mar 2024 at 13:00, Alvaro Herrera wrote: > > So I think the original developers of REPLICA IDENTITY had the right > idea here (commit 07cacba983ef), and we mustn't change this aspect, > because it'll lead to data corruption in replication. Using a deferred > PK for DDL considerations

Re: Dump-restore loosing 'attnotnull' bit for DEFERRABLE PRIMARY KEY column(s).

2024-03-07 Thread Alvaro Herrera
On 2024-Mar-05, Dean Rasheed wrote: > So I think RelationGetIndexAttrBitmap() should include deferrable PKs, I tried this, but it doesn't actually lead to a good place, because if we allow deferrable PKs to identify rows, then they are not useful to find the tuple to update when replicating.

Re: Dump-restore loosing 'attnotnull' bit for DEFERRABLE PRIMARY KEY column(s).

2024-03-05 Thread Alvaro Herrera
On 2024-Mar-05, Dean Rasheed wrote: > Looking at the other places that call RelationGetIndexAttrBitmap() > with INDEX_ATTR_BITMAP_PRIMARY_KEY, they all appear to want to include > deferrable PKs, since they are relying on the result to see which > columns are not nullable. Hmm, I find this

Re: Dump-restore loosing 'attnotnull' bit for DEFERRABLE PRIMARY KEY column(s).

2024-03-05 Thread Dean Rasheed
On Tue, 5 Mar 2024 at 12:36, Alvaro Herrera wrote: > > Yeah. As I said upthread, a good fix seems to require no longer relying > on RelationGetIndexAttrBitmap to obtain the columns in the primary key, > because that function does not include deferred primary keys. I came up > with the attached

Re: Dump-restore loosing 'attnotnull' bit for DEFERRABLE PRIMARY KEY column(s).

2024-03-05 Thread Alvaro Herrera
On 2024-Mar-04, Dean Rasheed wrote: > I don't think that this is the right fix. ISTM that the real issue is > that dropping a NOT NULL constraint should not mark the column as > nullable if it is part of a PK, whether or not that PK is deferrable > -- a deferrable PK still marks a column as not

Re: Dump-restore loosing 'attnotnull' bit for DEFERRABLE PRIMARY KEY column(s).

2024-03-04 Thread Dean Rasheed
On Mon, 4 Mar 2024 at 12:34, Aleksander Alekseev wrote: > > > This was an experimental patch, I was looking for the comment on the > > proposed > > approach -- whether we could simply skip the throwaway NOT NULL constraint > > for > > deferred PK constraint. Moreover, skip that for any PK

Re: Dump-restore loosing 'attnotnull' bit for DEFERRABLE PRIMARY KEY column(s).

2024-03-04 Thread Aleksander Alekseev
Hi hackers, >> I did not see any test addition for this, can we add it? > > > Ok, added it in the attached version. > > This was an experimental patch, I was looking for the comment on the proposed > approach -- whether we could simply skip the throwaway NOT NULL constraint for > deferred PK

Re: Dump-restore loosing 'attnotnull' bit for DEFERRABLE PRIMARY KEY column(s).

2024-01-23 Thread Amul Sul
On Sat, Jan 20, 2024 at 7:55 AM vignesh C wrote: > On Fri, 22 Sept 2023 at 18:45, Amul Sul wrote: > > > > > > > > On Wed, Sep 20, 2023 at 8:29 PM Alvaro Herrera > wrote: > >> > >> On 2023-Sep-20, Amul Sul wrote: > >> > >> > On the latest master head, I can see a $subject bug that seems to be >

Re: Dump-restore loosing 'attnotnull' bit for DEFERRABLE PRIMARY KEY column(s).

2024-01-19 Thread vignesh C
On Fri, 22 Sept 2023 at 18:45, Amul Sul wrote: > > > > On Wed, Sep 20, 2023 at 8:29 PM Alvaro Herrera > wrote: >> >> On 2023-Sep-20, Amul Sul wrote: >> >> > On the latest master head, I can see a $subject bug that seems to be >> > related >> > commit #b0e96f311985: >> > >> > Here is the table

Re: Dump-restore loosing 'attnotnull' bit for DEFERRABLE PRIMARY KEY column(s).

2023-09-22 Thread Amul Sul
On Wed, Sep 20, 2023 at 8:29 PM Alvaro Herrera wrote: > On 2023-Sep-20, Amul Sul wrote: > > > On the latest master head, I can see a $subject bug that seems to be > related > > commit #b0e96f311985: > > > > Here is the table definition: > > create table foo(i int, j int, CONSTRAINT pk PRIMARY

Re: Dump-restore loosing 'attnotnull' bit for DEFERRABLE PRIMARY KEY column(s).

2023-09-20 Thread Alvaro Herrera
On 2023-Sep-20, Amul Sul wrote: > On the latest master head, I can see a $subject bug that seems to be related > commit #b0e96f311985: > > Here is the table definition: > create table foo(i int, j int, CONSTRAINT pk PRIMARY KEY(i) DEFERRABLE); Interesting, thanks for the report. Your

Dump-restore loosing 'attnotnull' bit for DEFERRABLE PRIMARY KEY column(s).

2023-09-20 Thread Amul Sul
Hi, On the latest master head, I can see a $subject bug that seems to be related commit #b0e96f311985: Here is the table definition: create table foo(i int, j int, CONSTRAINT pk PRIMARY KEY(i) DEFERRABLE); And after restore from the dump, it shows a descriptor where column 'i' not marked NOT