Thread: 12.1 Partitioned Table Creation Bug

Chris Drawater <Chris.Drawater@viavisolutions.com> writes:
> geo=> create table junk ( a int, b int) PARTITION BY RANGE (a);
> ERROR:  cannot specify default tablespace for partitioned relations

I'm guessing you've got default_tablespace set to something nonempty.

I don't know why the decision was made to throw an error rather than
silently ignoring the setting ...

            regards, tom lane

"David G. Johnston" <david.g.johnston@gmail.com> writes:
> On Wed, Jan 22, 2020 at 12:48 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Chris Drawater <Chris.Drawater@viavisolutions.com> writes:
>>> geo=> create table junk ( a int, b int) PARTITION BY RANGE (a);
>>> ERROR:  cannot specify default tablespace for partitioned relations

>> I'm guessing you've got default_tablespace set to something nonempty.
>> I don't know why the decision was made to throw an error rather than
>> silently ignoring the setting ...

> Ignoring the setting seems to be what the documentation says we do:
> https://www.postgresql.org/docs/12/sql-createtable.html
> "For partitioned tables, since no storage is required for the table itself,
> the tablespace specified overrides default_tablespace as the default
> tablespace to use for any newly created partitions when no other tablespace
> is explicitly specified."
> So this seems like a regression in v12.

Poking into it, I can reproduce Chris' failure in the specific case
where default_tablespace is explicitly set to "pg_default" rather
than being left empty.

You're right that this is new behavior in v12; it seems to have been
introduced by commit 87259588d, which quoth
    
    * Setting a partitioned rel's tablespace to the database default is
      confusing; if it worked, it would direct the partitions to that
      tablespace regardless of default_tablespace.  But in reality it does
      not work, and making it work is a larger project.  Therefore, throw
      an error when this condition is detected, to alert the unwary.

I wouldn't say that this behavior is "alerting the unwary"; it's just
confusing them --- and the case that this is less confusing than what
happened before seems pretty thin.

Maybe a better error message would improve matters.  But I'm inclined
to say that reverting this aspect of the patch would be a better idea.
I find it particularly bletcherous that it made GetDefaultTablespace()
know whether it's operating to select a tablespace for a partitioned
relation or some other case --- that is just awful design, and it doesn't
make me feel that the whole idea was especially well thought out.

            regards, tom lane

On 2020-Jan-22, Tom Lane wrote:

> Poking into it, I can reproduce Chris' failure in the specific case
> where default_tablespace is explicitly set to "pg_default" rather
> than being left empty.
> 
> You're right that this is new behavior in v12; it seems to have been
> introduced by commit 87259588d, which quoth
>     
>     * Setting a partitioned rel's tablespace to the database default is
>       confusing; if it worked, it would direct the partitions to that
>       tablespace regardless of default_tablespace.  But in reality it does
>       not work, and making it work is a larger project.  Therefore, throw
>       an error when this condition is detected, to alert the unwary.
> 
> I wouldn't say that this behavior is "alerting the unwary"; it's just
> confusing them --- and the case that this is less confusing than what
> happened before seems pretty thin.
> 
> Maybe a better error message would improve matters.  But I'm inclined
> to say that reverting this aspect of the patch would be a better idea.
> I find it particularly bletcherous that it made GetDefaultTablespace()
> know whether it's operating to select a tablespace for a partitioned
> relation or some other case --- that is just awful design, and it doesn't
> make me feel that the whole idea was especially well thought out.

A partitioned relation behaves differently from other relations in the
storage regard, so I don't find particularly surprising that
GetDefaultTablespace behaves differently for them.

I think randomly, carelessly reverting parts of the patch would be a
terrible idea, because it was difficult to arrive at the current
behavior; all other behaviors had different problematic corner cases.
I invite people to read the discussion to led to the current design.

As the quoted commit message indicates, it *is* possible to make this
configuration/command combination work, but it's not as trivial as
reverting a few lines of a commit.  Again, I suggest to reread the old
thread for some insight.  In particular,
https://postgr.es/m/20190423222633.GA8364@alvherre.pgsql

The fact remains that partitioned tables were left to work poorly in
many cases, and we made great efforts to improve them.  Some casualties
were left along the way, mostly in the area of backwards compatibility.
I didn't shed tears for those, and nobody should either: in pg10 we got
partitioning, which was a huge effort and I applaud that, but it was
just the beginning and we knew full well that it had wrinkles that we'd
have to iron out later on.  That is what we were doing in the discussion
that led to commit 87259588d.

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