Horton

Thread

Commits

  1. Fix partitioned index creation with foreign partitions

  2. Doc: Update documentation on partitioning vs. foreign tables.

  1. BUG #15724: Can't create foreign table as partition

    The Post Office <noreply@postgresql.org> — 2019-03-29T12:55:15Z

    The following bug has been logged on the website:

Bug reference:      15724
Logged by:          Stepan Yankevych
Email address:      stepya@ukr.net
PostgreSQL version: 11.1
Operating system:   CentOS Linux release 7.5.1804 (Core)
Description:        

I have two DB 
big_data - PG 11.1
big_data_arch - PG 11.1

On the big_data DB i have following table 

CREATE TABLE msg_json
(   msg_id bigint NOT NULL DEFAULT
nextval('msg_json_msg_id_seq'::regclass),
    date_id integer NOT NULL,
    msg json
    CONSTRAINT pk_msg_json PRIMARY KEY (date_id, msg_id) ) PARTITION BY
RANGE (date_id) 
WITH (
    OIDS = FALSE
)
TABLESPACE pg_default;


CREATE INDEX msg_json_cross_order_id_idx
    ON msg_json USING btree
    (get_jsn_cross_order_id(msg) COLLATE pg_catalog."default")
    TABLESPACE ssd_ts;


CREATE INDEX msg_json_msg_id_idx
    ON msg_json USING btree
    (msg_id, date_id)
    TABLESPACE ssd_ts;

-- Partitions SQL

CREATE TABLE msg_json_20180102 PARTITION OF msg_json
    FOR VALUES FROM (20180102) TO (20180103);

CREATE TABLE msg_json_20180103 PARTITION OF msg_json
    FOR VALUES FROM (20180103) TO (20180104);
...
until today

On the big_data_arch
 I have the same table with the same name, structure and indexes in the
fix_capture schema but containing 2017 year data.

On the big_data DB i run following

CREATE FOREIGN TABLE staging.msg_json PARTITION of msg_json  
    FOR VALUES FROM (20170101) TO (20180101)
    SERVER postgres_big_data_arch
    OPTIONS (schema_name 'fix_capture', table_name 'msg_json');


I got error !!!!

SQL Error [42809]: ERROR: cannot create index on foreign table "msg_json"
  ERROR: cannot create index on foreign table "msg_json"
  ERROR: cannot create index on foreign table "msg_json"


I tried to import foreign table into staging schema and then attach
partition. 
Import looks good. Foreign table contains data and i can query it through
fdw.
But running 
ALTER TABLE msg_json ATTACH PARTITION staging.msg_json FOR VALUES FROM
(20170101) TO (20180101);
Brings me the same error!!!. 

Thanks!

  2. Re: BUG #15724: Can't create foreign table as partition

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-03-29T13:55:19Z

    On 2019-Mar-29, PG Bug reporting form wrote:

> CREATE FOREIGN TABLE staging.msg_json PARTITION of msg_json  
>     FOR VALUES FROM (20170101) TO (20180101)
>     SERVER postgres_big_data_arch
>     OPTIONS (schema_name 'fix_capture', table_name 'msg_json');
> 
> 
> I got error !!!!
> 
> SQL Error [42809]: ERROR: cannot create index on foreign table "msg_json"
>   ERROR: cannot create index on foreign table "msg_json"
>   ERROR: cannot create index on foreign table "msg_json"

Ah, this is because we try to propagate the index to the partition,
which appears to be less than desirable.  Maybe this patch fixes it?  I
have not tested it, only verified that it compiles.

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

  3. Re: BUG #15724: Can't create foreign table as partition

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-03-29T14:01:31Z

    Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> On 2019-Mar-29, PG Bug reporting form wrote:
>> SQL Error [42809]: ERROR: cannot create index on foreign table "msg_json"

> Ah, this is because we try to propagate the index to the partition,
> which appears to be less than desirable.  Maybe this patch fixes it?  I
> have not tested it, only verified that it compiles.

Really, what *ought* to happen in such a case is that the FDW gets
told about the command and has the opportunity to try to do something
corresponding to making an index.  But that smells like a new feature
rather than a bug fix.

I'm not sure that just forcing the case to be a no-op is wise.
What if the index is supposed to be UNIQUE?

			regards, tom lane

  4. Re: BUG #15724: Can't create foreign table as partition

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-03-29T14:10:50Z

    On 2019-Mar-29, Tom Lane wrote:

> Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> > On 2019-Mar-29, PG Bug reporting form wrote:
> >> SQL Error [42809]: ERROR: cannot create index on foreign table "msg_json"
> 
> > Ah, this is because we try to propagate the index to the partition,
> > which appears to be less than desirable.  Maybe this patch fixes it?  I
> > have not tested it, only verified that it compiles.
> 
> Really, what *ought* to happen in such a case is that the FDW gets
> told about the command and has the opportunity to try to do something
> corresponding to making an index.  But that smells like a new feature
> rather than a bug fix.

ENOTIME to get it done for pg12 I suppose (I don't know a thing about
postgres_fdw or the FDW API layer).

> I'm not sure that just forcing the case to be a no-op is wise.
> What if the index is supposed to be UNIQUE?

Hmm, good point, that should be disallowed too.

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

  5. Re[2]: BUG #15724: Can't create foreign table as partition

    Stepan Yankevych <stepya@ukr.net> — 2019-03-29T14:21:15Z

    I think we need at least to mention it in the dochttps://www.postgresql.org/docs/11/ddl-partitioning.html

>> Partitions can also be foreign tables, although they have some limitations that normal tables do not; see CREATE FOREIGN TABLE for more information
For example  
  Partitions can also be foreign tables only in case main table is non indexed and doesn't contain PK, although they have some limitations that normal tables do not; see CREATE FOREIGN TABLE for more information

--- Original message ---
From: "Alvaro Herrera" <alvherre@2ndquadrant.com>
Date: 29 March 2019, 16:11:04

On 2019-Mar-29, Tom Lane wrote:

> Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> > On 2019-Mar-29, PG Bug reporting form wrote:
> >> SQL Error [42809]: ERROR: cannot create index on foreign table "msg_json"
> 
> > Ah, this is because we try to propagate the index to the partition,
> > which appears to be less than desirable.  Maybe this patch fixes it?  I
> > have not tested it, only verified that it compiles.
> 
> Really, what *ought* to happen in such a case is that the FDW gets
> told about the command and has the opportunity to try to do something
> corresponding to making an index.  But that smells like a new feature
> rather than a bug fix.

ENOTIME to get it done for pg12 I suppose (I don't know a thing about
postgres_fdw or the FDW API layer).

> I'm not sure that just forcing the case to be a no-op is wise.
> What if the index is supposed to be UNIQUE?

Hmm, good point, that should be disallowed too.

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

  6. Re[2]: BUG #15724: Can't create foreign table as partition

    Stepan Yankevych <stepya@ukr.net> — 2019-03-29T16:25:42Z

    >> I'm not sure that just forcing the case to be a no-op is wise.
>> What if the index is supposed to be UNIQUE?Anyway it would be better to have no-op in 11.3 than having it not working. We just can put comment in the doc like. Foreign Server should take care about indexing foreign table as partition. 

--- Original message ---
From: "Tom Lane" <tgl@sss.pgh.pa.us>
Date: 29 March 2019, 16:01:41

Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> On 2019-Mar-29, PG Bug reporting form wrote:
>> SQL Error [42809]: ERROR: cannot create index on foreign table "msg_json"

> Ah, this is because we try to propagate the index to the partition,
> which appears to be less than desirable.  Maybe this patch fixes it?  I
> have not tested it, only verified that it compiles.

Really, what *ought* to happen in such a case is that the FDW gets
told about the command and has the opportunity to try to do something
corresponding to making an index.  But that smells like a new feature
rather than a bug fix.

I'm not sure that just forcing the case to be a no-op is wise.
What if the index is supposed to be UNIQUE?

			regards, tom lane

  7. Re: BUG #15724: Can't create foreign table as partition

    Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp> — 2019-04-01T05:35:28Z

    (2019/03/29 23:10), Alvaro Herrera wrote:
> On 2019-Mar-29, Tom Lane wrote:
>
>> Alvaro Herrera<alvherre@2ndquadrant.com>  writes:
>>> On 2019-Mar-29, PG Bug reporting form wrote:
>>>> SQL Error [42809]: ERROR: cannot create index on foreign table "msg_json"

>> Really, what *ought* to happen in such a case is that the FDW gets
>> told about the command and has the opportunity to try to do something
>> corresponding to making an index.

Interesting!

>> corresponding to making an index.  But that smells like a new feature
>> rather than a bug fix.

> ENOTIME to get it done for pg12 I suppose (I don't know a thing about
> postgres_fdw or the FDW API layer).

Yeah, I think so.

>> I'm not sure that just forcing the case to be a no-op is wise.
>> What if the index is supposed to be UNIQUE?
>
> Hmm, good point, that should be disallowed too.

Agreed.

Best regards,
Etsuro Fujita

  8. Re: BUG #15724: Can't create foreign table as partition

    Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp> — 2019-04-01T06:01:50Z

    (2019/03/29 23:21), Stepan Yankevych wrote:
> I think we need at least to mention it in the doc
> https://www.postgresql.org/docs/11/ddl-partitioning.html
>
>> > Partitions can also be foreign tables, although they have some
> limitations that normal tables do not; see CREATE FOREIGN TABLE
> <https://www.postgresql.org/docs/11/sql-createforeigntable.html>for more
> information
> For example
> Partitions can also be foreign tables*only in case main table is non
> indexed and doesn't contain PK,*although they have some limitations that
> normal tables do not; see CREATE FOREIGN TABLE
> <https://www.postgresql.org/docs/11/sql-createforeigntable.html>for more
> information

+1 for adding such notes, but I'm not sure if it's a good idea to put it 
there because I think that would make the introductory documentation a 
bit verbose.  How about adding it to the reference pages of CREATE 
FOREIGN TABLE and ALTER TABLE like the attached?

Best regards,
Etsuro Fujita

  9. Re: BUG #15724: Can't create foreign table as partition

    Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2019-04-01T06:23:15Z

    On 2019/04/01 14:35, Etsuro Fujita wrote:
> (2019/03/29 23:10), Alvaro Herrera wrote:
>> On 2019-Mar-29, Tom Lane wrote:
>>
>>> Alvaro Herrera<alvherre@2ndquadrant.com>  writes:
>>>> On 2019-Mar-29, PG Bug reporting form wrote:
>>>>> SQL Error [42809]: ERROR: cannot create index on foreign table
>>>>> "msg_json"
> 
>>> Really, what *ought* to happen in such a case is that the FDW gets
>>> told about the command and has the opportunity to try to do something
>>> corresponding to making an index.
> 
> Interesting!

+1

Maybe, CHECK constraints on foreign tables are similar -- it merely
asserts the condition that all of the rows in the remote table *must*
satisfy so that the local server can do something useful with it, such as
trying to exclude the table with constraint exclusion.

I guess the intended benefit with allowing indexes would be similar -- a
locally-defined index on a foreign table will help an FDW estimate the
cost of a query accessing the table more efficiently than, for example,
having to rely on some other relatively expensive alternative, such as
use_remote_estimate in postgres_fdw's case.

Thanks,
Amit

  10. Re: BUG #15724: Can't create foreign table as partition

    Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp> — 2019-04-01T06:40:49Z

    (2019/03/30 1:25), Stepan Yankevych wrote:
>>>  I'm not sure that just forcing the case to be a no-op is wise.
>>>  What if the index is supposed to be UNIQUE?
>
> Anyway it would be better to have no-op in 11.3 than having it not working.
>
> We just can put comment in the doc like.
>
> Foreign Server should take care about indexing foreign table as partition.

That might be OK, but before doing that, we would need to support eg, 
unique constraints on foreign tables, so this seems more like a feature 
than a fix to me.

Best regards,
Etsuro Fujita

  11. Re: BUG #15724: Can't create foreign table as partition

    Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2019-04-01T06:45:14Z

    Fujita-san,

On 2019/04/01 15:01, Etsuro Fujita wrote:
> (2019/03/29 23:21), Stepan Yankevych wrote:
>> I think we need at least to mention it in the doc
>> https://www.postgresql.org/docs/11/ddl-partitioning.html
>>
>>> > Partitions can also be foreign tables, although they have some
>> limitations that normal tables do not; see CREATE FOREIGN TABLE
>> <https://www.postgresql.org/docs/11/sql-createforeigntable.html>for more
>> information
>> For example
>> Partitions can also be foreign tables*only in case main table is non
>> indexed and doesn't contain PK,*although they have some limitations that
>> normal tables do not; see CREATE FOREIGN TABLE
>> <https://www.postgresql.org/docs/11/sql-createforeigntable.html>for more
>> information
> 
> +1 for adding such notes, but I'm not sure if it's a good idea to put it
> there because I think that would make the introductory documentation a bit
> verbose.  How about adding it to the reference pages of CREATE FOREIGN
> TABLE and ALTER TABLE like the attached?

Thanks for creating the patch.  Looks good to me.

Thanks,
Amit

  12. Re: BUG #15724: Can't create foreign table as partition

    Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp> — 2019-04-01T06:57:29Z

    (2019/04/01 15:23), Amit Langote wrote:
> On 2019/04/01 14:35, Etsuro Fujita wrote:
>> (2019/03/29 23:10), Alvaro Herrera wrote:
>>> On 2019-Mar-29, Tom Lane wrote:
>>>
>>>> Alvaro Herrera<alvherre@2ndquadrant.com>   writes:
>>>>> On 2019-Mar-29, PG Bug reporting form wrote:
>>>>>> SQL Error [42809]: ERROR: cannot create index on foreign table
>>>>>> "msg_json"
>>
>>>> Really, what *ought* to happen in such a case is that the FDW gets
>>>> told about the command and has the opportunity to try to do something
>>>> corresponding to making an index.
>>
>> Interesting!
>
> +1
>
> Maybe, CHECK constraints on foreign tables are similar -- it merely
> asserts the condition that all of the rows in the remote table *must*
> satisfy so that the local server can do something useful with it, such as
> trying to exclude the table with constraint exclusion.
>
> I guess the intended benefit with allowing indexes would be similar -- a
> locally-defined index on a foreign table will help an FDW estimate the
> cost of a query accessing the table more efficiently than, for example,
> having to rely on some other relatively expensive alternative, such as
> use_remote_estimate in postgres_fdw's case.

Yeah, I think so.  Another benefit I can think of would be: that would 
probably allow us to support ON CONFLICT DO UPDATE for foreign tables. 
I think the intended benefit would be large!

Best regards,
Etsuro Fujita

  13. Re: BUG #15724: Can't create foreign table as partition

    Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp> — 2019-04-05T12:07:47Z

    (2019/04/01 15:45), Amit Langote wrote:
> On 2019/04/01 15:01, Etsuro Fujita wrote:
>> +1 for adding such notes, but I'm not sure if it's a good idea to put it
>> there because I think that would make the introductory documentation a bit
>> verbose.  How about adding it to the reference pages of CREATE FOREIGN
>> TABLE and ALTER TABLE like the attached?
>
> Thanks for creating the patch.  Looks good to me.

Pushed after fixing a copy-pasto: s/sql-createtable/sql-createforeigntable/

Thanks for reviewing, Amit-san!

Best regards,
Etsuro Fujita

  14. Re: BUG #15724: Can't create foreign table as partition

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-06-20T00:36:44Z

    On 2019-Mar-29, Alvaro Herrera wrote:

> On 2019-Mar-29, PG Bug reporting form wrote:
> 
> > CREATE FOREIGN TABLE staging.msg_json PARTITION of msg_json  
> >     FOR VALUES FROM (20170101) TO (20180101)
> >     SERVER postgres_big_data_arch
> >     OPTIONS (schema_name 'fix_capture', table_name 'msg_json');
> > 
> > I got error !!!!
> > 
> > SQL Error [42809]: ERROR: cannot create index on foreign table "msg_json"
> >   ERROR: cannot create index on foreign table "msg_json"
> >   ERROR: cannot create index on foreign table "msg_json"
> 
> Ah, this is because we try to propagate the index to the partition,
> which appears to be less than desirable.  Maybe this patch fixes it?  I
> have not tested it, only verified that it compiles.

The previously proposed patch doesn't work; there are a few other places
that needed patching.  Here's a proposed patch.  I'm not sure whether
this is a bug fix or a new feature.

With this patch, an index creation will no longer fail in the presence
of a partition that is a foreign table, as long as the index is not a
constraint index (not unique, not primary key).  Conversely,
creating/attaching a partition that is a foreign table does not fail if
the partitioned table only has non-constraint indexes.

With the current code, these commands would all fail, and the resulting
behavior is pretty unhelpful, as reported in this thread and elsewhere.

Any opinions on backpatching this?  I'm considering pg11, but also
considering not to backpatch at all.

(There *is* a much smaller bugfix here, which is that we don't raise an
error when unique/pk indexes exist and a foreign table is attached as
partition.)

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

  15. Re: BUG #15724: Can't create foreign table as partition

    Pavan Deolasee <pavan.deolasee@gmail.com> — 2019-06-21T06:37:54Z

    On Thu, Jun 20, 2019 at 6:07 AM Alvaro Herrera <alvherre@2ndquadrant.com>
wrote:

>
>
> With this patch, an index creation will no longer fail in the presence
> of a partition that is a foreign table, as long as the index is not a
> constraint index (not unique, not primary key).  Conversely,
> creating/attaching a partition that is a foreign table does not fail if
> the partitioned table only has non-constraint indexes.
>
>
Like others suggested above, I also think that we should make this is a
no-op on the foreign tables i.e. not fail even when there exists a UNIQUE
or PRIMARY KEY on the parent table. We simply assume that the appropriate
constraints will be defined on the foreign side and violations will be
caught. This is same as CHECK constraints on the foreign partitions, that
we assume the foreign server will enforce.

Thanks,
Pavan

-- 
 Pavan Deolasee                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

  16. Re: BUG #15724: Can't create foreign table as partition

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-06-21T15:49:15Z

    On 2019-Jun-21, Pavan Deolasee wrote:

> On Thu, Jun 20, 2019 at 6:07 AM Alvaro Herrera <alvherre@2ndquadrant.com>
> wrote:
> 
> > With this patch, an index creation will no longer fail in the presence
> > of a partition that is a foreign table, as long as the index is not a
> > constraint index (not unique, not primary key).  Conversely,
> > creating/attaching a partition that is a foreign table does not fail if
> > the partitioned table only has non-constraint indexes.
>
> Like others suggested above, I also think that we should make this is a
> no-op on the foreign tables i.e. not fail even when there exists a UNIQUE
> or PRIMARY KEY on the parent table. We simply assume that the appropriate
> constraints will be defined on the foreign side and violations will be
> caught. This is same as CHECK constraints on the foreign partitions, that
> we assume the foreign server will enforce.

I don't oppose making such a change in pg13, but it seems too much of a
behavior change to be making even in pg12, let alone in the stable
branches.

Therefore, my proposal --in response to this bug report-- is to
backpatch the previously proposed patch, which allows indexes to be
created [on partitioned tables containing foreign tables as partitions],
as long as they are not UNIQUE/PKs.

If others want to explore the implications of UNIQUE/PK indexes on
partitioned tables with foreign partitions, I won't stand in their way,
though it doesn't seem material for this thread.

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

  17. Re: BUG #15724: Can't create foreign table as partition

    Amit Langote <amitlangote09@gmail.com> — 2019-06-25T08:56:00Z

    Hi Alvaro,

On Sat, Jun 22, 2019 at 12:49 AM Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> On 2019-Jun-21, Pavan Deolasee wrote:
> > On Thu, Jun 20, 2019 at 6:07 AM Alvaro Herrera <alvherre@2ndquadrant.com>
> > wrote:
> >
> > > With this patch, an index creation will no longer fail in the presence
> > > of a partition that is a foreign table, as long as the index is not a
> > > constraint index (not unique, not primary key).  Conversely,
> > > creating/attaching a partition that is a foreign table does not fail if
> > > the partitioned table only has non-constraint indexes.
> >
> > Like others suggested above, I also think that we should make this is a
> > no-op on the foreign tables i.e. not fail even when there exists a UNIQUE
> > or PRIMARY KEY on the parent table. We simply assume that the appropriate
> > constraints will be defined on the foreign side and violations will be
> > caught. This is same as CHECK constraints on the foreign partitions, that
> > we assume the foreign server will enforce.
>
> I don't oppose making such a change in pg13, but it seems too much of a
> behavior change to be making even in pg12, let alone in the stable
> branches.

I agree that index constraints on foreign tables would be a new
feature, because to support them we'd first need to support defining
(dummy) indexes on foreign tables.

> Therefore, my proposal --in response to this bug report-- is to
> backpatch the previously proposed patch, which allows indexes to be
> created [on partitioned tables containing foreign tables as partitions],
> as long as they are not UNIQUE/PKs.

I've looked at the patch you posted on Jun 20 and here are some comments.

@@ -15705,6 +15721,32 @@ AttachPartitionEnsureIndexes(Relation rel,
Relation attachrel)
        i++;
    }

+   /*
+    * If we're attaching a foreign table, we must fail if any of the indexes
+    * is a constraint index; otherwise, there's nothing to do here.
+    */
+   if (attachrel->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
+   {
+       foreach(cell, idxes)
+       {
+           Oid         idx = lfirst_oid(cell);

Why not add the is-foreign-table check in the loop that already exists
just below the above added code?  That's what the patch does for
DefineRelation() and if you do so, there's no need for the goto label
that's also added by the patch.

@@ -1347,11 +1347,20 @@ ProcessUtilitySlow(ParseState *pstate,

                            if (relkind != RELKIND_RELATION &&
                                relkind != RELKIND_MATVIEW &&
-                               relkind != RELKIND_PARTITIONED_TABLE)
+                               relkind != RELKIND_PARTITIONED_TABLE &&
+                               relkind != RELKIND_FOREIGN_TABLE)
                                ereport(ERROR,

(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
                                         errmsg("cannot create index
on partitioned table \"%s\"",
                                                stmt->relation->relname),
+                                        errdetail("Table \"%s\"
contains weird partitions or something.",
+                                                  stmt->relation->relname)));
+                           if (relkind == RELKIND_FOREIGN_TABLE &&
+                               (stmt->unique || stmt->primary))
+                               ereport(ERROR,
+
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                                        errmsg("cannot create unique
index on partitioned table \"%s\"",
+                                               stmt->relation->relname),

The "...weird partitions or something" message wouldn't be very
useful, but maybe you intended to rewrite it before committing?  I
suppose we could turn that particular ereport into elog(ERROR, ...),
because finding children of a partitioned that are neither of
RELATION, PARTITIONED_TABLE, FOREIGN_TABLE should be an internal
error.

Also, +1 to back-patching, fwiw.  Thanks for working on the patch.

Regards,
Amit

  18. Re: BUG #15724: Can't create foreign table as partition

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-06-25T12:56:57Z

    Hi Amit, thanks for reviewing.

On 2019-Jun-25, Amit Langote wrote:

> @@ -15705,6 +15721,32 @@ AttachPartitionEnsureIndexes(Relation rel,
> Relation attachrel)
>         i++;
>     }
> 
> +   /*
> +    * If we're attaching a foreign table, we must fail if any of the indexes
> +    * is a constraint index; otherwise, there's nothing to do here.
> +    */
> +   if (attachrel->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
> +   {
> +       foreach(cell, idxes)
> +       {
> +           Oid         idx = lfirst_oid(cell);
> 
> Why not add the is-foreign-table check in the loop that already exists
> just below the above added code?  That's what the patch does for
> DefineRelation() and if you do so, there's no need for the goto label
> that's also added by the patch.

Because if you do that, you might build a few indexes on regular
partitions before coming across a foreign one, which is very unfriendly.
I'll add a comment to this effect.

> (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
>                                          errmsg("cannot create index
> on partitioned table \"%s\"",
>                                                 stmt->relation->relname),
> +                                        errdetail("Table \"%s\"
> contains weird partitions or something.",
> +                                                  stmt->relation->relname)));

> The "...weird partitions or something" message wouldn't be very
> useful, but maybe you intended to rewrite it before committing?

Hah, yeah, I did :-)

> I suppose we could turn that particular ereport into elog(ERROR, ...),
> because finding children of a partitioned that are neither of
> RELATION, PARTITIONED_TABLE, FOREIGN_TABLE should be an internal
> error.

Yeah, an elog() sounds a good idea.  I suppose "unexpected relkind
\"%c\" on partition \"%s\"" should be good.


BTW I'm now thinking that those ERRCODE_INVALID_OBJECT_DEFINITION codes
are not really the correct ones; I mean, it would be the right one to
use for the unexpected relkind condition, but for the other cases I
think we should use ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE instead.

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

  19. Re: BUG #15724: Can't create foreign table as partition

    Amit Langote <amitlangote09@gmail.com> — 2019-06-26T05:50:49Z

    On Tue, Jun 25, 2019 at 9:57 PM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> On 2019-Jun-25, Amit Langote wrote:
> > @@ -15705,6 +15721,32 @@ AttachPartitionEnsureIndexes(Relation rel,
> > Relation attachrel)
> >         i++;
> >     }
> >
> > +   /*
> > +    * If we're attaching a foreign table, we must fail if any of the indexes
> > +    * is a constraint index; otherwise, there's nothing to do here.
> > +    */
> > +   if (attachrel->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
> > +   {
> > +       foreach(cell, idxes)
> > +       {
> > +           Oid         idx = lfirst_oid(cell);
> >
> > Why not add the is-foreign-table check in the loop that already exists
> > just below the above added code?  That's what the patch does for
> > DefineRelation() and if you do so, there's no need for the goto label
> > that's also added by the patch.
>
> Because if you do that, you might build a few indexes on regular
> partitions before coming across a foreign one, which is very unfriendly.
> I'll add a comment to this effect.

Hmm, why would other partitions be involved?
AttachPartitionEnsureIndexes() only considers the partition being
attached, like DefineRelation only considers the partition being
created.

> > (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
> >                                          errmsg("cannot create index
> > on partitioned table \"%s\"",
> >                                                 stmt->relation->relname),
> > +                                        errdetail("Table \"%s\"
> > contains weird partitions or something.",
> > +                                                  stmt->relation->relname)));
>
> > The "...weird partitions or something" message wouldn't be very
> > useful, but maybe you intended to rewrite it before committing?
>
> Hah, yeah, I did :-)
>
> > I suppose we could turn that particular ereport into elog(ERROR, ...),
> > because finding children of a partitioned that are neither of
> > RELATION, PARTITIONED_TABLE, FOREIGN_TABLE should be an internal
> > error.
>
> Yeah, an elog() sounds a good idea.  I suppose "unexpected relkind
> \"%c\" on partition \"%s\"" should be good.

Yeah, that'll suffice.

> BTW I'm now thinking that those ERRCODE_INVALID_OBJECT_DEFINITION codes
> are not really the correct ones; I mean, it would be the right one to
> use for the unexpected relkind condition, but for the other cases I
> think we should use ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE instead.

Agreed.

Thanks,
Amit

  20. Re: BUG #15724: Can't create foreign table as partition

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-06-26T20:18:43Z

    On 2019-Jun-26, Amit Langote wrote:

> On Tue, Jun 25, 2019 at 9:57 PM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:

> > Because if you do that, you might build a few indexes on regular
> > partitions before coming across a foreign one, which is very unfriendly.
> > I'll add a comment to this effect.
> 
> Hmm, why would other partitions be involved?
> AttachPartitionEnsureIndexes() only considers the partition being
> attached, like DefineRelation only considers the partition being
> created.

Sorry, I meant other *indexes*.  You might build a few regular
(non-unique) indexes before coming across a unique index, at which point
you throw the error.

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

  21. Re: BUG #15724: Can't create foreign table as partition

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-06-27T16:33:32Z

    On 2019-Jun-26, Amit Langote wrote:

> On Tue, Jun 25, 2019 at 9:57 PM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:

> > BTW I'm now thinking that those ERRCODE_INVALID_OBJECT_DEFINITION codes
> > are not really the correct ones; I mean, it would be the right one to
> > use for the unexpected relkind condition, but for the other cases I
> > think we should use ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE instead.
> 
> Agreed.

I had second thoughts about this one -- ERRCODE_WRONG_OBJECT_TYPE seems
more appropriate, so I used that.

I also tweaked Fujita-san's recently committed documentation addition.

Pushed to 11 and master.  Thanks!

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

  22. Re: BUG #15724: Can't create foreign table as partition

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-06-27T16:47:50Z

    Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> Pushed to 11 and master.  Thanks!

Don't actually see any push from here?

			regards, tom lane

  23. Re: BUG #15724: Can't create foreign table as partition

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-06-27T17:04:41Z

    On 2019-Jun-27, Tom Lane wrote:

> Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> > Pushed to 11 and master.  Thanks!
> 
> Don't actually see any push from here?

Sorry, that was a delayed email on which I had not yet pushed the "send"
keystroke.  The push was yesterday.

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

  24. RE: BUG #15724: Can't create foreign table as partition

    Stepan Yankevych <stepan_yankevych@epam.com> — 2019-09-25T06:48:36Z

    Hi Guys!

As far as I can see  the issue has been fixed with following resolution 
Ignore partitions that are foreign tables when creating indexes on partitioned tables (Álvaro Herrera)
Previously an error was thrown on encountering a foreign-table partition, but that's unhelpful and doesn't protect against any actual problem.

But I still can't create Pk on partitioned table

See details: 
SELECT version();
PostgreSQL 11.5 on x86_64-pc-linux-gnu, compiled by gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-36), 64-bit

ALTER TABLE fix_capture.fix_message_json ADD CONSTRAINT fix_message_json_pk PRIMARY KEY (fix_message_id,date_id);
SQL Error [42809]: ERROR: cannot create unique index on partitioned table "fix_message_json"
  Detail: Table "fix_message_json" contains partitions that are foreign tables.

Have I missed something? 

STEPAN YANKEVYCH 
Software Engineering Team Leader 
Software Engineering Manager
OCA 
 
Office: +380 322 424 642 x 58840   Cell: +380 96 915 9551   Email: stepan_yankevych@epam.com 
Lviv, Ukraine   epam.com 
 
 
CONFIDENTIALITY CAUTION AND DISCLAIMER
This message is intended only for the use of the individual(s) or entity(ies) to which it is addressed and contains information that is legally privileged and confidential. If you are not the intended recipient, or the person responsible for delivering the message to the intended recipient, you are hereby notified that any dissemination, distribution or copying of this communication is strictly prohibited. All unintended recipients are obliged to delete this message and destroy any printed copies. 
 

-----Original Message-----
From: Alvaro Herrera <alvherre@2ndquadrant.com> 
Sent: Thursday, June 27, 2019 20:05
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Amit Langote <amitlangote09@gmail.com>; Pavan Deolasee <pavan.deolasee@gmail.com>; stepya@ukr.net; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Subject: Re: BUG #15724: Can't create foreign table as partition

On 2019-Jun-27, Tom Lane wrote:

> Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> > Pushed to 11 and master.  Thanks!
> 
> Don't actually see any push from here?

Sorry, that was a delayed email on which I had not yet pushed the "send"
keystroke.  The push was yesterday.

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

  25. Re: BUG #15724: Can't create foreign table as partition

    Etsuro Fujita <etsuro.fujita@gmail.com> — 2019-09-25T07:43:46Z

    On Wed, Sep 25, 2019 at 4:16 PM Stepan Yankevych
<Stepan_Yankevych@epam.com> wrote:
> As far as I can see  the issue has been fixed with following resolution
> Ignore partitions that are foreign tables when creating indexes on partitioned tables (Álvaro Herrera)
> Previously an error was thrown on encountering a foreign-table partition, but that's unhelpful and doesn't protect against any actual problem.

That's right.

> But I still can't create Pk on partitioned table
>
> See details:
> SELECT version();
> PostgreSQL 11.5 on x86_64-pc-linux-gnu, compiled by gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-36), 64-bit
>
> ALTER TABLE fix_capture.fix_message_json ADD CONSTRAINT fix_message_json_pk PRIMARY KEY (fix_message_id,date_id);
> SQL Error [42809]: ERROR: cannot create unique index on partitioned table "fix_message_json"
>   Detail: Table "fix_message_json" contains partitions that are foreign tables.
>
> Have I missed something?

Unfortunately, that is still a restriction: it's allowed to create
regular indexes, but not unique or primary key indexes [1].

Best regards,
Etsuro Fujita

[1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=55ed3defc966cf718fe1e8c0efe964580bb23351