Horton

Thread

Commits

  1. Replace enum InhOption with simple boolean.

  2. Remove sql_inheritance GUC.

  1. Re: Declarative partitioning vs. sql_inheritance

    Robert Haas <robertmhaas@gmail.com> — 2016-12-16T16:05:21Z

    On Thu, Dec 15, 2016 at 10:40 AM, Dmitry Ivanov <d.ivanov@postgrespro.ru> wrote:
> Hi everyone,
>
> Looks like "sql_inheritance" GUC is affecting partitioned tables:
>
> explain (costs off) select * from test;
>          QUERY PLAN          ------------------------------
> Append
>   ->  Seq Scan on test
>   ->  Seq Scan on test_1
>   ->  Seq Scan on test_2
>   ->  Seq Scan on test_1_1
>   ->  Seq Scan on test_1_2
>   ->  Seq Scan on test_1_1_1
>   ->  Seq Scan on test_1_2_1
> (8 rows)
>
>
> set sql_inheritance = off;
>
>
> explain (costs off) select * from test;
>    QUERY PLAN    ------------------
> Seq Scan on test
> (1 row)
>
>
> I might be wrong, but IMO this should not happen. Queries involving update,
> delete etc on partitioned tables are basically broken. Moreover, there's no
> point in performing such operations on a parent table that's supposed to be
> empty at all times.

An earlier version of Amit's patches tried to handle this by forcing
sql_inheritance on for partitioned tables, but it wasn't
well-implemented and I don't see the point anyway.  Sure, turning off
sql_inheritance off for partitioned tables produces stupid results.
But turning off sql_inheritance for inheritance hierarchies also
produces stupid results.  If we were going to do anything about this,
my vote would be to remove sql_inheritance.

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

  2. Re: Declarative partitioning vs. sql_inheritance

    Tom Lane <tgl@sss.pgh.pa.us> — 2016-12-16T16:26:00Z

    Robert Haas <robertmhaas@gmail.com> writes:
> An earlier version of Amit's patches tried to handle this by forcing
> sql_inheritance on for partitioned tables, but it wasn't
> well-implemented and I don't see the point anyway.  Sure, turning off
> sql_inheritance off for partitioned tables produces stupid results.
> But turning off sql_inheritance for inheritance hierarchies also
> produces stupid results.  If we were going to do anything about this,
> my vote would be to remove sql_inheritance.

+1.  If memory serves, we invented that GUC as a backwards-compatibility
hack, because once upon a time the default behavior was equivalent to
sql_inheritance = off.  But that was a long time ago; a bit of digging
in the git history suggests we changed it in 2000.  It's hard to believe
that anybody still relies on being able to turn it off.

			regards, tom lane

  3. Re: Declarative partitioning vs. sql_inheritance

    David Fetter <david@fetter.org> — 2016-12-16T19:34:41Z

    On Fri, Dec 16, 2016 at 11:05:21AM -0500, Robert Haas wrote:
> On Thu, Dec 15, 2016 at 10:40 AM, Dmitry Ivanov <d.ivanov@postgrespro.ru> wrote:
> > Hi everyone,
> >
> > Looks like "sql_inheritance" GUC is affecting partitioned tables:
> >
> > [breaks literally everything]
> >
> > I might be wrong, but IMO this should not happen. Queries involving update,
> > delete etc on partitioned tables are basically broken. Moreover, there's no
> > point in performing such operations on a parent table that's supposed to be
> > empty at all times.
> 
> An earlier version of Amit's patches tried to handle this by forcing
> sql_inheritance on for partitioned tables, but it wasn't
> well-implemented and I don't see the point anyway.  Sure, turning
> off sql_inheritance off for partitioned tables produces stupid
> results.  But turning off sql_inheritance for inheritance
> hierarchies also produces stupid results.  If we were going to do
> anything about this, my vote would be to remove sql_inheritance.

+1

It occurs to me this probably isn't the only GUC that's basically just
a foot gun at this point.

Is 10 a good time to sweep and clear them?

Best,
David.
-- 
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter      XMPP: david(dot)fetter(at)gmail(dot)com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

  4. Re: Declarative partitioning vs. sql_inheritance

    Robert Haas <robertmhaas@gmail.com> — 2016-12-16T20:15:42Z

    On Fri, Dec 16, 2016 at 2:34 PM, David Fetter <david@fetter.org> wrote:
> It occurs to me this probably isn't the only GUC that's basically just
> a foot gun at this point.
>
> Is 10 a good time to sweep and clear them?

We never make any progress trying to do these things "in bulk".  If
you think there are other GUCs that need to be removed, start a thread
for each one, or closely related group, and let's talk about it on the
merits.  On this thread, let's just decide whether or not to remove
sql_inheritance.

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

  5. Re: Declarative partitioning vs. sql_inheritance

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2016-12-16T20:33:02Z

    On 12/16/16 11:05 AM, Robert Haas wrote:
> If we were going to do anything about this,
> my vote would be to remove sql_inheritance.

Go for it.

Let's also remove the table* syntax then.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  6. Re: Declarative partitioning vs. sql_inheritance

    David Steele <david@pgmasters.net> — 2016-12-16T20:40:29Z

    On 12/16/16 11:05 AM, Robert Haas wrote:

> If we were going to do anything about this,
> my vote would be to remove sql_inheritance.

+1.  This option is long past the intended shelf life.

-- 
-David
david@pgmasters.net

  7. Re: Declarative partitioning vs. sql_inheritance

    Tom Lane <tgl@sss.pgh.pa.us> — 2016-12-17T00:39:46Z

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> On 12/16/16 11:05 AM, Robert Haas wrote:
>> If we were going to do anything about this,
>> my vote would be to remove sql_inheritance.

> Go for it.

> Let's also remove the table* syntax then.

Meh --- that might break existing queries, to what purpose?

We certainly shouldn't remove query syntax without a deprecation period.
I'm less concerned about that for GUCs.

			regards, tom lane

  8. Re: Declarative partitioning vs. sql_inheritance

    Robert Haas <robertmhaas@gmail.com> — 2016-12-17T01:40:50Z

    On Fri, Dec 16, 2016 at 7:39 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
>> On 12/16/16 11:05 AM, Robert Haas wrote:
>>> If we were going to do anything about this,
>>> my vote would be to remove sql_inheritance.
>
>> Go for it.
>
>> Let's also remove the table* syntax then.
>
> Meh --- that might break existing queries, to what purpose?
>
> We certainly shouldn't remove query syntax without a deprecation period.
> I'm less concerned about that for GUCs.

I agree.  Patch attached, just removing the GUC and a fairly minimal
amount of the supporting infrastructure.

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

  9. Re: Declarative partitioning vs. sql_inheritance

    Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2016-12-19T05:25:56Z

    On 2016/12/17 10:40, Robert Haas wrote:
> On Fri, Dec 16, 2016 at 7:39 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
>>> On 12/16/16 11:05 AM, Robert Haas wrote:
>>>> If we were going to do anything about this,
>>>> my vote would be to remove sql_inheritance.
>>
>>> Go for it.
>>
>>> Let's also remove the table* syntax then.
>>
>> Meh --- that might break existing queries, to what purpose?
>>
>> We certainly shouldn't remove query syntax without a deprecation period.
>> I'm less concerned about that for GUCs.
> 
> I agree.  Patch attached, just removing the GUC and a fairly minimal
> amount of the supporting infrastructure.

+1 to removing the sql_inheritance GUC.  The patch looks good to me.

Thanks,
Amit

  10. Re: Declarative partitioning vs. sql_inheritance

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2016-12-19T16:48:48Z

    Robert Haas wrote:
> On Fri, Dec 16, 2016 at 7:39 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> >> On 12/16/16 11:05 AM, Robert Haas wrote:
> >>> If we were going to do anything about this,
> >>> my vote would be to remove sql_inheritance.
> >
> >> Go for it.
> >
> >> Let's also remove the table* syntax then.
> >
> > Meh --- that might break existing queries, to what purpose?
> >
> > We certainly shouldn't remove query syntax without a deprecation period.
> > I'm less concerned about that for GUCs.
> 
> I agree.  Patch attached, just removing the GUC and a fairly minimal
> amount of the supporting infrastructure.

Any particular reason not to change inhOpt to be a simple boolean, and
remove the enum?

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  11. Re: Declarative partitioning vs. sql_inheritance

    Robert Haas <robertmhaas@gmail.com> — 2016-12-19T16:53:05Z

    On Mon, Dec 19, 2016 at 11:48 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> Any particular reason not to change inhOpt to be a simple boolean, and
> remove the enum?

No, no particular reason.  I thought about it, but I didn't really see
any advantage in getting rid of the typedef.

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

  12. Re: Declarative partitioning vs. sql_inheritance

    Robert Haas <robertmhaas@gmail.com> — 2016-12-23T12:37:09Z

    On Mon, Dec 19, 2016 at 12:25 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
>> I agree.  Patch attached, just removing the GUC and a fairly minimal
>> amount of the supporting infrastructure.
>
> +1 to removing the sql_inheritance GUC.  The patch looks good to me.

Great, committed.  I realize just now that I forgot to credit anyone
as a reviewer, but hopefully nobody's going to mind that too much
considering this is a purely mechanical patch I wrote in 20 minutes.

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

  13. Re: Declarative partitioning vs. sql_inheritance

    Tom Lane <tgl@sss.pgh.pa.us> — 2016-12-23T16:59:18Z

    Robert Haas <robertmhaas@gmail.com> writes:
> Great, committed.  I realize just now that I forgot to credit anyone
> as a reviewer, but hopefully nobody's going to mind that too much
> considering this is a purely mechanical patch I wrote in 20 minutes.

Do you have any particular objection to taking the next step of removing
enum InhOption in favor of making inhOpt a bool?  It seems to me that
stuff like

-       bool        recurse = interpretInhOption(rv->inhOpt);
+       bool        recurse = (rv->inhOpt == INH_YES);

just begs the question of why it's not simply

        bool        recurse = rv->inh;

Certainly a reader who did not know the history would be confused at
the useless-looking complexity.

			regards, tom lane

  14. Re: Declarative partitioning vs. sql_inheritance

    Robert Haas <robertmhaas@gmail.com> — 2016-12-23T17:25:44Z

    On Fri, Dec 23, 2016 at 11:59 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> Great, committed.  I realize just now that I forgot to credit anyone
>> as a reviewer, but hopefully nobody's going to mind that too much
>> considering this is a purely mechanical patch I wrote in 20 minutes.
>
> Do you have any particular objection to taking the next step of removing
> enum InhOption in favor of making inhOpt a bool?  It seems to me that
> stuff like
>
> -       bool        recurse = interpretInhOption(rv->inhOpt);
> +       bool        recurse = (rv->inhOpt == INH_YES);
>
> just begs the question of why it's not simply
>
>         bool        recurse = rv->inh;
>
> Certainly a reader who did not know the history would be confused at
> the useless-looking complexity.

No, not really.  I don't feel like it's an improvement, but you and
Alvaro obviously do, so have at it.

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

  15. Re: Declarative partitioning vs. sql_inheritance

    Tom Lane <tgl@sss.pgh.pa.us> — 2016-12-23T17:30:27Z

    Robert Haas <robertmhaas@gmail.com> writes:
> On Fri, Dec 23, 2016 at 11:59 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Do you have any particular objection to taking the next step of removing
>> enum InhOption in favor of making inhOpt a bool?

> No, not really.  I don't feel like it's an improvement, but you and
> Alvaro obviously do, so have at it.

OK, will do.

			regards, tom lane