Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-12-13 Thread Amit Langote
On 2018/11/09 14:04, Amit Langote wrote: > On 2018/11/09 4:39, Alvaro Herrera wrote: >> I included the test case for collations to the three branches, but no >> code changes. We can patch master for the handling of collations per >> your patch, > > Okay, but should we back-patch it by adding

Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-11-13 Thread Tomas Vondra
On Mon, 2018-11-12 at 11:33 -0500, Tom Lane wrote: > Alvaro Herrera writes: > > One of the guiding principles that I think we should hold for > > partitioning is that operating directly into the partition should > > be > > seen as only an optimization compared to inserting into the parent > >

Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-11-13 Thread Tomas Vondra
On Tue, 2018-11-13 at 13:44 +0100, Jürgen Strobel wrote: > On 2018-11-13 00:57, Tom Lane wrote: > > =?UTF-8?Q?J=c3=bcrgen_Strobel?= > > writes: > > > On 2018-11-12 17:33, Tom Lane wrote: > > > > I'm not entirely convinced that I buy that argument, especially > > > > not in > > > > a case like

Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-11-13 Thread Jürgen Strobel
On 2018-11-13 00:57, Tom Lane wrote: > =?UTF-8?Q?J=c3=bcrgen_Strobel?= writes: >> On 2018-11-12 17:33, Tom Lane wrote: >>> I'm not entirely convinced that I buy that argument, especially not in >>> a case like this where it introduces logical inconsistencies where there >>> otherwise wouldn't be

Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-11-12 Thread Tom Lane
=?UTF-8?Q?J=c3=bcrgen_Strobel?= writes: > On 2018-11-12 17:33, Tom Lane wrote: >> I'm not entirely convinced that I buy that argument, especially not in >> a case like this where it introduces logical inconsistencies where there >> otherwise wouldn't be any. > I think I missed something, what

Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-11-12 Thread Jürgen Strobel
On 2018-11-12 17:33, Tom Lane wrote: > Alvaro Herrera writes: >> One of the guiding principles that I think we should hold for >> partitioning is that operating directly into the partition should be >> seen as only an optimization compared to inserting into the parent table >> -- thus it should

Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-11-12 Thread Tom Lane
Alvaro Herrera writes: > One of the guiding principles that I think we should hold for > partitioning is that operating directly into the partition should be > seen as only an optimization compared to inserting into the parent table > -- thus it should not behave differently. Applying different

Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-11-12 Thread Alvaro Herrera
On 2018-Nov-12, Jürgen Strobel wrote: > With all due respect I suspect your view of implementation issues biases > your judgement here. For me the partition's default handling was > completely unexpected, especially with the documentation silent about > default-applying behavior, and inconsistent

Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-11-12 Thread Jürgen Strobel
On 2018-11-09 23:33, Tom Lane wrote: > Alvaro Herrera writes: >> On 2018-Nov-09, Jürgen Strobel wrote: >>> Regarding your example, what I expected is that *both* inserts would >>> consistently result in a tuple of (1, 42) since p should route the >>> insert to p1 and use p1's defaults. The

Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-11-11 Thread Amit Langote
On 2018/11/12 12:59, Tom Lane wrote: > Amit Langote writes: >> On 2018/11/10 7:33, Tom Lane wrote: >>> I'd argue not, actually. I think there is plausible precedent in >>> updatable views, where what we use is the defaults associated with the >>> view, not the underlying table. Correspondingly,

Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-11-11 Thread Tom Lane
Amit Langote writes: > On 2018/11/10 7:33, Tom Lane wrote: >> I'd argue not, actually. I think there is plausible precedent in >> updatable views, where what we use is the defaults associated with the >> view, not the underlying table. Correspondingly, what ought to govern >> in a partitioned

Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-11-11 Thread Amit Langote
On 2018/11/10 7:33, Tom Lane wrote: > Alvaro Herrera writes: >> On 2018-Nov-09, Jürgen Strobel wrote: >>> Regarding your example, what I expected is that *both* inserts would >>> consistently result in a tuple of (1, 42) since p should route the >>> insert to p1 and use p1's defaults. The current

Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-11-09 Thread Tom Lane
Alvaro Herrera writes: > On 2018-Nov-09, Jürgen Strobel wrote: >> Regarding your example, what I expected is that *both* inserts would >> consistently result in a tuple of (1, 42) since p should route the >> insert to p1 and use p1's defaults. The current inconsistent behavior is >> the heart of

Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-11-09 Thread Alvaro Herrera
On 2018-Nov-09, Jürgen Strobel wrote: > OK got it. I agree about differing COLLATE clauses making no sense and > with the ERROR+WARNING solution, but I didn't report that. Yeah, I happened to notice it on code inspection. > The NULL violation is obvious too. Yeah, IMO that was an obvious

Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-11-09 Thread Jürgen Strobel
On 2018-11-09 22:11, Alvaro Herrera wrote: > On 2018-Nov-09, Jürgen Strobel wrote: > >> I am slightly confused now, I thought this landmine was already fixed in >> master and what remains is only whether to backport it or not? Which I >> guess depends on whether this is classified as a bug or

Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-11-09 Thread Alvaro Herrera
On 2018-Nov-09, Jürgen Strobel wrote: > I am slightly confused now, I thought this landmine was already fixed in > master and what remains is only whether to backport it or not? Which I > guess depends on whether this is classified as a bug or not. Hmm, I changed (and back-patched) what happens

Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-11-09 Thread Jürgen Strobel
On 2018-11-09 18:07, Alvaro Herrera wrote: > On 2018-Nov-09, Jürgen Strobel wrote: > >> Regarding your example, what I expected is that *both* inserts would >> consistently result in a tuple of (1, 42) since p should route the >> insert to p1 and use p1's defaults. The current inconsistent

Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-11-09 Thread Alvaro Herrera
On 2018-Nov-09, Jürgen Strobel wrote: > Regarding your example, what I expected is that *both* inserts would > consistently result in a tuple of (1, 42) since p should route the > insert to p1 and use p1's defaults. The current inconsistent behavior is > the heart of the bug. I think that would

BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-11-09 Thread Jürgen Strobel
On 2018-11-09 16:59, Alvaro Herrera wrote: > On 2018-Nov-09, Amit Langote wrote: > >> Or is it a *bug* of tuple-routing that it doesn't substitute default >> values that may be defined for partitions? It kind of looks like one if >> you see an example like this. >> >> create table p (a int, b

Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-11-09 Thread Alvaro Herrera
On 2018-Nov-09, Amit Langote wrote: > Or is it a *bug* of tuple-routing that it doesn't substitute default > values that may be defined for partitions? It kind of looks like one if > you see an example like this. > > create table p (a int, b int) partition by list (a); > create table p1

Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-11-08 Thread Amit Langote
On 2018/11/09 14:04, Amit Langote wrote: > On 2018/11/09 4:39, Alvaro Herrera wrote: >> and if somebody has it, a change to how defaults are applied >> when routing tuples. > > I haven't written such a patch yet. Do we want such a feature? Or is it a *bug* of tuple-routing that it doesn't

Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-11-08 Thread Amit Langote
On 2018/11/09 4:39, Alvaro Herrera wrote: > Pushed. Thank you for committing. I've closed the CF entry. > I included the test case for collations to the three branches, but no > code changes. We can patch master for the handling of collations per > your patch, Okay, but should we back-patch

Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-11-08 Thread Amit Langote
On 2018/11/09 1:38, Alvaro Herrera wrote: > On 2018-Nov-09, Amit Langote wrote: > >> On Fri, Nov 9, 2018 at 1:03 AM Alvaro Herrera >> wrote: >>> On 2018-Nov-07, Amit Langote wrote: > >>> Hmm, I'm thinking perhaps we shouldn't backpatch this part. It's >>> obviously a bug, but we might break

Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-11-08 Thread Alvaro Herrera
Pushed. I included the test case for collations to the three branches, but no code changes. We can patch master for the handling of collations per your patch, and if somebody has it, a change to how defaults are applied when routing tuples. Thanks to Jürgen for reporting the bug. -- Álvaro

Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-11-08 Thread Alvaro Herrera
On 2018-Nov-09, Amit Langote wrote: > On Fri, Nov 9, 2018 at 1:03 AM Alvaro Herrera > wrote: > > On 2018-Nov-07, Amit Langote wrote: > > Hmm, I'm thinking perhaps we shouldn't backpatch this part. It's > > obviously a bug, but we might break somebody's working apps. Therefore > > I think I'd

Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-11-08 Thread Amit Langote
On Fri, Nov 9, 2018 at 1:03 AM Alvaro Herrera wrote: > On 2018-Nov-07, Amit Langote wrote: > > > I think the result in this case should be an error, just as it would in > > the regular inheritance case. > > > > create table parent (a text); > > create table child (a text collate "en_US") inherits

Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-11-08 Thread Alvaro Herrera
On 2018-Nov-07, Amit Langote wrote: > I think the result in this case should be an error, just as it would in > the regular inheritance case. > > create table parent (a text); > create table child (a text collate "en_US") inherits (parent); > NOTICE: merging column "a" with inherited definition

Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-11-06 Thread Amit Langote
Hi, On 2018/11/07 0:10, Alvaro Herrera wrote: > Looking over the stuff in gram.y (to make sure there's nothing that > could be lost), I noticed that we're losing the COLLATE clause when they > are added to columns in partitions. I would expect part1 to end up with > collation es_CL, or

Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-11-06 Thread Alvaro Herrera
Here's the patch I intend to push to branches 10 and 11. The patch in master is almost identical -- the only difference is that ColumnDef->is_from_parent is removed. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training &

Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-11-06 Thread Alvaro Herrera
Looking over the stuff in gram.y (to make sure there's nothing that could be lost), I noticed that we're losing the COLLATE clause when they are added to columns in partitions. I would expect part1 to end up with collation es_CL, or alternatively that the command throws an error: 55462 10.6

Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-11-06 Thread Amit Langote
Hi, Thank you for looking at this. On 2018/11/06 7:25, Alvaro Herrera wrote: > On 2018-Aug-07, Amit Langote wrote: > >>> But in >>> case of partitioning there is only one parent and hence >>> coldef->constraints is NULL and hence just overwriting it works. I >>> think we need comments

Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-11-05 Thread Alvaro Herrera
On 2018-Aug-07, Amit Langote wrote: > > But in > > case of partitioning there is only one parent and hence > > coldef->constraints is NULL and hence just overwriting it works. I > > think we need comments mentioning this special case. > > That's what I wrote in this comment: > >

Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-08-07 Thread Amit Langote
Thanks Ashutosh, and sorry that I somehow missed replying to this. On 2018/07/13 22:50, Ashutosh Bapat wrote: > On Thu, Jul 12, 2018 at 2:29 PM, Amit Langote wrote: >> I have modified the comments around this code in the updated patch. > > +/* > + * Each member in 'saved_schema'

Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-07-13 Thread Ashutosh Bapat
On Thu, Jul 12, 2018 at 2:29 PM, Amit Langote wrote: > Thanks Ashutosh. > > On 2018/07/10 22:50, Ashutosh Bapat wrote: >> I didn't see any hackers thread linked to this CF entry. Hence sending this >> mail through CF app. > > Hmm, yes. I hadn't posted the patch to -hackers. > >> The patch looks

Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-07-12 Thread Amit Langote
Thanks Ashutosh. On 2018/07/10 22:50, Ashutosh Bapat wrote: > I didn't see any hackers thread linked to this CF entry. Hence sending this > mail through CF app. Hmm, yes. I hadn't posted the patch to -hackers. > The patch looks good to me. It applies cleanly, compiles cleanly and make >

Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

2018-07-10 Thread Ashutosh Bapat
I didn't see any hackers thread linked to this CF entry. Hence sending this mail through CF app. The patch looks good to me. It applies cleanly, compiles cleanly and make check passes. I think you could avoid condition + /* Do not override parent's NOT NULL constraint. */