Horton

Thread

Commits

  1. Don't allow partitioned index on foreign-table partitions

  1. Indexes on partitioned tables and foreign partitions

    Arseny Sher <a.sher@postgrespro.ru> — 2018-05-09T11:50:04Z

    Hi,

8b08f7d4 added propagation of indexes on partitioned tables to
partitions, which is very cool. However, index creation also recurses
down to foreign tables. I doubt this is intentional, as such indexes are
forbidden as not making much sense; attempt to create index on
partitioned table with foreign partition leads to an error
now. Attached lines fix this.

  2. Re: Indexes on partitioned tables and foreign partitions

    Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> — 2018-05-09T12:52:15Z

    On Wed, May 9, 2018 at 5:20 PM, Arseny Sher <a.sher@postgrespro.ru> wrote:
> Hi,
>
> 8b08f7d4 added propagation of indexes on partitioned tables to
> partitions, which is very cool. However, index creation also recurses
> down to foreign tables. I doubt this is intentional, as such indexes are
> forbidden as not making much sense; attempt to create index on
> partitioned table with foreign partition leads to an error
> now. Attached lines fix this.
>

The patch looks good, but I guess that we have to do some tricks with
the validity of index on the partitioned table since not all
partitions have a given index when one of those is a foreign
partition.
-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

  3. Re: Indexes on partitioned tables and foreign partitions

    Simon Riggs <simon@2ndquadrant.com> — 2018-05-09T13:08:49Z

    On 9 May 2018 at 12:50, Arseny Sher <a.sher@postgrespro.ru> wrote:
> Hi,
>
> 8b08f7d4 added propagation of indexes on partitioned tables to
> partitions, which is very cool. However, index creation also recurses
> down to foreign tables. I doubt this is intentional, as such indexes are
> forbidden as not making much sense; attempt to create index on
> partitioned table with foreign partition leads to an error
> now. Attached lines fix this.

"Fix"?

How much sense is it to have a partitioned table with a mix of local
and foreign tables?

Shouldn't the fix be to allow creation of indexes on foreign tables?
(Maybe they would be virtual or foreign indexes??)

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

  4. Re: Indexes on partitioned tables and foreign partitions

    Arseny Sher <a.sher@postgrespro.ru> — 2018-05-09T14:26:34Z

    Simon Riggs <simon@2ndquadrant.com> writes:

> How much sense is it to have a partitioned table with a mix of local
> and foreign tables?

Well, as much sense as fdw-based sharding has, for instance. It is
arguable, but it exists.

> Shouldn't the fix be to allow creation of indexes on foreign tables?
> (Maybe they would be virtual or foreign indexes??)

Similar ideas were discussed at [1]. There was no wide consensus of even
what problems such feature would solve. Since currently indexes on
foreign tables are just forbidden, it seems to me that the best what
partitioning code can do today is just not creating them.

[1] https://www.postgresql.org/message-id/flat/4F62FD69.2060007%40lab.ntt.co.jp#4F62FD69.2060007@lab.ntt.co.jp

--
Arseny Sher
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

  5. Re: Indexes on partitioned tables and foreign partitions

    Simon Riggs <simon@2ndquadrant.com> — 2018-05-09T14:45:13Z

    On 9 May 2018 at 15:26, Arseny Sher <a.sher@postgrespro.ru> wrote:
>
> Simon Riggs <simon@2ndquadrant.com> writes:
>
>> How much sense is it to have a partitioned table with a mix of local
>> and foreign tables?
>
> Well, as much sense as fdw-based sharding has, for instance. It is
> arguable, but it exists.
>
>> Shouldn't the fix be to allow creation of indexes on foreign tables?
>> (Maybe they would be virtual or foreign indexes??)
>
> Similar ideas were discussed at [1]. There was no wide consensus of even
> what problems such feature would solve. Since currently indexes on
> foreign tables are just forbidden, it seems to me that the best what
> partitioning code can do today is just not creating them.
>
> [1] https://www.postgresql.org/message-id/flat/4F62FD69.2060007%40lab.ntt.co.jp#4F62FD69.2060007@lab.ntt.co.jp

Indexes on foreign tables cause an ERROR, so yes, we already just
don't create them.

You're suggesting silently skipping the ERROR. I can't see a reason for that.

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

  6. Re: Indexes on partitioned tables and foreign partitions

    Robert Haas <robertmhaas@gmail.com> — 2018-05-09T14:57:33Z

    On Wed, May 9, 2018 at 9:08 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
> How much sense is it to have a partitioned table with a mix of local
> and foreign tables?

Fair question, but we put some effort into making it work, so I think
it should keep working.

> Shouldn't the fix be to allow creation of indexes on foreign tables?
> (Maybe they would be virtual or foreign indexes??)

It might be useful to invent the concept of a foreign index, but not
for v11 a month after feature freeze.

For right now, I think the options are (1) throw an ERROR if we
encounter a foreign table or (2) silently skip the foreign table.  I
think (2) is defensible for non-UNIQUE indexes, because the index is
just a performance optimization.  However, for UNIQUE indexes, at
least, it seems like we'd better do (1), because a major point of such
an index is to enforce a constraint; we can't allege that we have such
a constraint if foreign tables are just silently skipped.

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

  7. Re: Indexes on partitioned tables and foreign partitions

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-05-09T15:10:43Z

    Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, May 9, 2018 at 9:08 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
>> Shouldn't the fix be to allow creation of indexes on foreign tables?
>> (Maybe they would be virtual or foreign indexes??)

> It might be useful to invent the concept of a foreign index, but not
> for v11 a month after feature freeze.

Yeah.  That's a can of worms we can *not* open at this stage.

> For right now, I think the options are (1) throw an ERROR if we
> encounter a foreign table or (2) silently skip the foreign table.  I
> think (2) is defensible for non-UNIQUE indexes, because the index is
> just a performance optimization.  However, for UNIQUE indexes, at
> least, it seems like we'd better do (1), because a major point of such
> an index is to enforce a constraint; we can't allege that we have such
> a constraint if foreign tables are just silently skipped.

Agreed about unique indexes.  I suggest that we throw an error for both
cases, because (1) having unique and non-unique indexes behave differently
for this purpose seems pretty weird; (2) throwing an error now preserves
our options to do something different later.  Given where we are in the
release cycle, we should be taking the most conservative path we can.

			regards, tom lane

  8. Re: Indexes on partitioned tables and foreign partitions

    Simon Riggs <simon@2ndquadrant.com> — 2018-05-09T15:12:38Z

    On 9 May 2018 at 15:57, Robert Haas <robertmhaas@gmail.com> wrote:

> For right now, I think the options are (1) throw an ERROR if we
> encounter a foreign table or (2) silently skip the foreign table.  I
> think (2) is defensible for non-UNIQUE indexes, because the index is
> just a performance optimization. However, for UNIQUE indexes, at
> least, it seems like we'd better do (1), because a major point of such
> an index is to enforce a constraint; we can't allege that we have such
> a constraint if foreign tables are just silently skipped.

If we can assume an index exists on a foreign table, why can we not
just assume a unique index exists?? Why the difference?

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

  9. Re: Indexes on partitioned tables and foreign partitions

    Simon Riggs <simon@2ndquadrant.com> — 2018-05-09T15:14:30Z

    On 9 May 2018 at 16:10, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Wed, May 9, 2018 at 9:08 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
>>> Shouldn't the fix be to allow creation of indexes on foreign tables?
>>> (Maybe they would be virtual or foreign indexes??)
>
>> It might be useful to invent the concept of a foreign index, but not
>> for v11 a month after feature freeze.
>
> Yeah.  That's a can of worms we can *not* open at this stage.

Lucky nobody suggested that then, eh? Robert's just making a joke.

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

  10. Re: Indexes on partitioned tables and foreign partitions

    Robert Haas <robertmhaas@gmail.com> — 2018-05-09T15:15:09Z

    On Wed, May 9, 2018 at 11:14 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
> On 9 May 2018 at 16:10, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Robert Haas <robertmhaas@gmail.com> writes:
>>> On Wed, May 9, 2018 at 9:08 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
>>>> Shouldn't the fix be to allow creation of indexes on foreign tables?
>>>> (Maybe they would be virtual or foreign indexes??)
>>
>>> It might be useful to invent the concept of a foreign index, but not
>>> for v11 a month after feature freeze.
>>
>> Yeah.  That's a can of worms we can *not* open at this stage.
>
> Lucky nobody suggested that then, eh? Robert's just making a joke.

Someone did suggest that.  It was you.

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

  11. Re: Indexes on partitioned tables and foreign partitions

    Simon Riggs <simon@2ndquadrant.com> — 2018-05-09T15:20:02Z

    On 9 May 2018 at 16:15, Robert Haas <robertmhaas@gmail.com> wrote:
> On Wed, May 9, 2018 at 11:14 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
>> On 9 May 2018 at 16:10, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> Robert Haas <robertmhaas@gmail.com> writes:
>>>> On Wed, May 9, 2018 at 9:08 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
>>>>> Shouldn't the fix be to allow creation of indexes on foreign tables?
>>>>> (Maybe they would be virtual or foreign indexes??)
>>>
>>>> It might be useful to invent the concept of a foreign index, but not
>>>> for v11 a month after feature freeze.
>>>
>>> Yeah.  That's a can of worms we can *not* open at this stage.
>>
>> Lucky nobody suggested that then, eh? Robert's just making a joke.
>
> Someone did suggest that.  It was you.

Oh, you weren't joking. I think we're having serious problems with
people putting words in my mouth again then.

Please show me where I suggested doing anything for v11?

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

  12. Re: Indexes on partitioned tables and foreign partitions

    Robert Haas <robertmhaas@gmail.com> — 2018-05-09T15:33:25Z

    On Wed, May 9, 2018 at 11:20 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
> On 9 May 2018 at 16:15, Robert Haas <robertmhaas@gmail.com> wrote:
>> On Wed, May 9, 2018 at 11:14 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
>>> On 9 May 2018 at 16:10, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>>> Robert Haas <robertmhaas@gmail.com> writes:
>>>>> On Wed, May 9, 2018 at 9:08 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
>>>>>> Shouldn't the fix be to allow creation of indexes on foreign tables?
>>>>>> (Maybe they would be virtual or foreign indexes??)
>>>>
>>>>> It might be useful to invent the concept of a foreign index, but not
>>>>> for v11 a month after feature freeze.
>>>>
>>>> Yeah.  That's a can of worms we can *not* open at this stage.
>>>
>>> Lucky nobody suggested that then, eh? Robert's just making a joke.
>>
>> Someone did suggest that.  It was you.
>
> Oh, you weren't joking. I think we're having serious problems with
> people putting words in my mouth again then.
>
> Please show me where I suggested doing anything for v11?

Come on, Simon.  It's in the quoted text.  I realize you didn't say
v11 specifically, but this is the context of a patch that is proposed
a bug-fix for v11.  If you meant that we should apply the patch as
proposed now, or some other one, and do the other thing later, you
could have said so.

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

  13. Re: Indexes on partitioned tables and foreign partitions

    Robert Haas <robertmhaas@gmail.com> — 2018-05-09T15:33:46Z

    On Wed, May 9, 2018 at 11:12 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
> If we can assume an index exists on a foreign table, why can we not
> just assume a unique index exists?? Why the difference?

We can't assume either of those things, and I didn't say that we should.

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

  14. Re: Indexes on partitioned tables and foreign partitions

    Arseny Sher <a.sher@postgrespro.ru> — 2018-05-09T15:48:37Z

    Simon Riggs <simon@2ndquadrant.com> writes:

> Indexes on foreign tables cause an ERROR, so yes, we already just
> don't create them.
>
> You're suggesting silently skipping the ERROR. I can't see a reason for that.

Truly, I was inaccurate. I mean that index propagation is a nice feature,
and making it available for mix of foreign and local partitions skipping
the foreign ones is useful thing for users who understand the
consequences (of course there should be a warning in the docs -- my
patch was just an illustration of the idea). As I said, FDW-based
sharding often involves mix of foreign and local partitions, even in
simple manual forms. Imagine that you have a table which is too big to
fit into one server, so you partition it and move some partiitons to
another machine; you still would like to access the whole table to avoid
manual tuple routing. Or, you partition table by timestamps and keep
recent data on fast primary server, gradually moving old partitions to
another box. Again, it would be nice to access table as a whole to
e.g. calculate some analytics.

Anyway, given other opinions, I assume that the community has chosen
more conservative approach. Indeed, we can't guarantee uniqueness this
way; that's fully users responsiblity.

--
Arseny Sher
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

  15. Re: Indexes on partitioned tables and foreign partitions

    Michael Paquier <michael@paquier.xyz> — 2018-05-10T01:02:03Z

    On Wed, May 09, 2018 at 11:10:43AM -0400, Tom Lane wrote:
> Agreed about unique indexes.  I suggest that we throw an error for both
> cases, because (1) having unique and non-unique indexes behave differently
> for this purpose seems pretty weird; (2) throwing an error now preserves
> our options to do something different later.  Given where we are in the
> release cycle, we should be taking the most conservative path we can.

Heartfully agreed to throw an error for all cases for now.  Something
that I find confusing on HEAD though is that DefineIndex calls itself
around line 1006 and cascades through each children but there is no
context about the error.

For example if I have this partition layer:
CREATE TABLE measurement (
     logdate         date not null,
     peaktemp        int,
     unitsales       int
) PARTITION BY RANGE (logdate);
CREATE FOREIGN TABLE measurement_y2016m07
    PARTITION OF measurement FOR VALUES FROM ('2016-07-01') TO
    ('2016-08-01')
    SERVER postgres_server;

Then by creating an index on the parent I get that:
=# create index measurement_idx on measurement (peaktemp);
ERROR:  42809: cannot create index on foreign table "measurement_y2016m07"
LOCATION:  DefineIndex, indexcmds.c:442

This may be confusing for the user as the DDL does not complain directly
about the relation worked on.  Perhaps we could add an error context?
We surely don't want multiple contexts either when working on long
chains, but we could build a chained list of relation names in the error
message.
--
Michael

  16. Re: Indexes on partitioned tables and foreign partitions

    Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2018-05-10T01:15:05Z

    On 2018/05/09 23:57, Robert Haas wrote:
> For right now, I think the options are (1) throw an ERROR if we
> encounter a foreign table or (2) silently skip the foreign table.  I
> think (2) is defensible for non-UNIQUE indexes, because the index is
> just a performance optimization.

Along with others, my +1 to option 1.

> However, for UNIQUE indexes, at
> least, it seems like we'd better do (1), because a major point of such
> an index is to enforce a constraint; we can't allege that we have such
> a constraint if foreign tables are just silently skipped.

While I agree with this, let me point out that we do allow inherited check
constraints on foreign tables that are not actually enforced locally.

create table p (a int) partition by range (a);
create table p1 partition of p for values from (minvalue) to (1);
create table p2base (a int);
create foreign table p2 partition of p for values from (1) to (maxvalue)
server loopback options (table_name 'p2base');

alter table p add check (a between -1000 and 1000);

-- routed to foreign partition, which doesn't enforce check constraints
insert into p values (1001);
INSERT 0 1

-- whereas, local partition does
insert into p values (-1001);
ERROR:  new row for relation "p1" violates check constraint "p_a_check"
DETAIL:  Failing row contains (-1001).

We have to do the following to prevent that.

alter table p2base add check (a between -1000 and 1000);
insert into p values (1001);
ERROR:  new row for relation "p2base" violates check constraint
"p2base_a_check"
DETAIL:  Failing row contains (1001).
CONTEXT:  remote SQL command: INSERT INTO public.p2base(a) VALUES ($1)

Thanks,
Amit

  17. Re: Indexes on partitioned tables and foreign partitions

    Michael Paquier <michael@paquier.xyz> — 2018-05-10T01:37:11Z

    On Thu, May 10, 2018 at 10:15:05AM +0900, Amit Langote wrote:
> While I agree with this, let me point out that we do allow inherited check
> constraints on foreign tables that are not actually enforced locally.
> 
> create table p (a int) partition by range (a);
> create table p1 partition of p for values from (minvalue) to (1);
> create table p2base (a int);
> create foreign table p2 partition of p for values from (1) to (maxvalue)
> server loopback options (table_name 'p2base');
> 
> alter table p add check (a between -1000 and 1000);
> 
> -- routed to foreign partition, which doesn't enforce check constraints
> insert into p values (1001);
> INSERT 0 1

That's not actually a surprise, right?  Since foreign tables can be part
of inheritance trees in 9.5, CHECK constraints on foreign tables are not
enforced locally, but used as planner hints to guess how a query would
work remotely.  So getting partition children to work the same way is
consistent.

> We have to do the following to prevent that.
> 
> alter table p2base add check (a between -1000 and 1000);
> insert into p values (1001);
> ERROR:  new row for relation "p2base" violates check constraint
> "p2base_a_check"
> DETAIL:  Failing row contains (1001).
> CONTEXT:  remote SQL command: INSERT INTO public.p2base(a) VALUES ($1)

This bit looks natural to me as well.
--
Michael

  18. Re: Indexes on partitioned tables and foreign partitions

    Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2018-05-10T02:20:56Z

    On 2018/05/10 10:37, Michael Paquier wrote:
> On Thu, May 10, 2018 at 10:15:05AM +0900, Amit Langote wrote:
>> While I agree with this, let me point out that we do allow inherited check
>> constraints on foreign tables that are not actually enforced locally.
>>
>> create table p (a int) partition by range (a);
>> create table p1 partition of p for values from (minvalue) to (1);
>> create table p2base (a int);
>> create foreign table p2 partition of p for values from (1) to (maxvalue)
>> server loopback options (table_name 'p2base');
>>
>> alter table p add check (a between -1000 and 1000);
>>
>> -- routed to foreign partition, which doesn't enforce check constraints
>> insert into p values (1001);
>> INSERT 0 1
> 
> That's not actually a surprise, right?  Since foreign tables can be part
> of inheritance trees in 9.5, CHECK constraints on foreign tables are not
> enforced locally, but used as planner hints to guess how a query would
> work remotely.  So getting partition children to work the same way is
> consistent.
> 
>> We have to do the following to prevent that.
>>
>> alter table p2base add check (a between -1000 and 1000);
>> insert into p values (1001);
>> ERROR:  new row for relation "p2base" violates check constraint
>> "p2base_a_check"
>> DETAIL:  Failing row contains (1001).
>> CONTEXT:  remote SQL command: INSERT INTO public.p2base(a) VALUES ($1)
> 
> This bit looks natural to me as well.

Yes, I know it is working as designed and documented.  I was just trying
to comment on this bit of Robert's email:

"...because a major point of such an index is to enforce a constraint; we
can't allege that we have such a constraint if foreign tables are just
silently skipped."

So if someday we go ahead and allow indexes to be created on partitioned
tables even if there are foreign partitions, how would we choose to deal
with a unique constraint?  Will it be same as CHECK constraints such that
we don't enforce it locally but only assume it to be enforced by the
remote data source using whatever method?

But it seems I've misinterpreted what he was saying.  He doesn't seem to
be saying anything about how or whether we enforce the unique constraint
on foreign tables.  Only that if someone creates a constraint index on the
partitioned table, all partitions *including* foreign partitions, must get
a copy.

So for now, we give users an error if they try to create an index on a
partitioned table with a mix of local and foreign partitions.  Once we
figure out how to allow creating indexes (constraint-enforcing or not) on
foreign tables, we can then think of relaxing that restriction.

Thanks,
Amit

  19. Re: Indexes on partitioned tables and foreign partitions

    Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2018-05-10T03:00:13Z

    On 2018/05/10 10:02, Michael Paquier wrote:
> Something
> that I find confusing on HEAD though is that DefineIndex calls itself
> around line 1006 and cascades through each children but there is no
> context about the error.
> 
> For example if I have this partition layer:
> CREATE TABLE measurement (
>      logdate         date not null,
>      peaktemp        int,
>      unitsales       int
> ) PARTITION BY RANGE (logdate);
> CREATE FOREIGN TABLE measurement_y2016m07
>     PARTITION OF measurement FOR VALUES FROM ('2016-07-01') TO
>     ('2016-08-01')
>     SERVER postgres_server;
> 
> Then by creating an index on the parent I get that:
> =# create index measurement_idx on measurement (peaktemp);
> ERROR:  42809: cannot create index on foreign table "measurement_y2016m07"
> LOCATION:  DefineIndex, indexcmds.c:442
> 
> This may be confusing for the user as the DDL does not complain directly
> about the relation worked on.  Perhaps we could add an error context?
> We surely don't want multiple contexts either when working on long
> chains, but we could build a chained list of relation names in the error
> message.

How about we error out even *before* calling DefineIndex for the 1st time?
 I see that ProcessUtilitySlow() gets a list of all partitions when
locking them for index creation before calling DefineIndex.  Maybe, just
go through the list and error out if one of them is a partition that we
don't support creating an index on?

For example, with the attached patch doing the above, I get:

create table p (a int) partition by range (a);
create table p1 partition of p for values from (minvalue) to (1);
create foreign table p2 partition of p for values from (1) to (maxvalue)
server loopback options (table_name 'p2base');
create index on p (a);
ERROR:  cannot create index on partitioned table containing foreign partitions

Thanks,
Amit

  20. Re: Indexes on partitioned tables and foreign partitions

    Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> — 2018-05-10T13:19:00Z

    On Thu, May 10, 2018 at 8:30 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
>
> How about we error out even *before* calling DefineIndex for the 1st time?
>  I see that ProcessUtilitySlow() gets a list of all partitions when
> locking them for index creation before calling DefineIndex.  Maybe, just
> go through the list and error out if one of them is a partition that we
> don't support creating an index on?

+1 for the idea. We don't want to spend efforts in DefineIndex for
some relations only to throw away that work later. I quickly looked at
the patch and I think it's on right track.

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

  21. Re: Indexes on partitioned tables and foreign partitions

    Robert Haas <robertmhaas@gmail.com> — 2018-05-10T14:28:16Z

    On Wed, May 9, 2018 at 10:20 PM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
> But it seems I've misinterpreted what he was saying.  He doesn't seem to
> be saying anything about how or whether we enforce the unique constraint
> on foreign tables.  Only that if someone creates a constraint index on the
> partitioned table, all partitions *including* foreign partitions, must get
> a copy.

Honestly, I hadn't quite gotten that far in my thinking.  That's a
really useful distinction, and I completely agree with it.

> So for now, we give users an error if they try to create an index on a
> partitioned table with a mix of local and foreign partitions.  Once we
> figure out how to allow creating indexes (constraint-enforcing or not) on
> foreign tables, we can then think of relaxing that restriction.

Yeah, that sounds exactly right.

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

  22. Re: Indexes on partitioned tables and foreign partitions

    Simon Riggs <simon@2ndquadrant.com> — 2018-05-11T12:52:24Z

    On 9 May 2018 at 17:33, Robert Haas <robertmhaas@gmail.com> wrote:
> On Wed, May 9, 2018 at 11:20 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
>> On 9 May 2018 at 16:15, Robert Haas <robertmhaas@gmail.com> wrote:
>>> On Wed, May 9, 2018 at 11:14 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
>>>> On 9 May 2018 at 16:10, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>>>> Robert Haas <robertmhaas@gmail.com> writes:
>>>>>> On Wed, May 9, 2018 at 9:08 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
>>>>>>> Shouldn't the fix be to allow creation of indexes on foreign tables?
>>>>>>> (Maybe they would be virtual or foreign indexes??)
>>>>>
>>>>>> It might be useful to invent the concept of a foreign index, but not
>>>>>> for v11 a month after feature freeze.
>>>>>
>>>>> Yeah.  That's a can of worms we can *not* open at this stage.
>>>>
>>>> Lucky nobody suggested that then, eh? Robert's just making a joke.
>>>
>>> Someone did suggest that.  It was you.
>>
>> Oh, you weren't joking. I think we're having serious problems with
>> people putting words in my mouth again then.
>>
>> Please show me where I suggested doing anything for v11?
>
> Come on, Simon.  It's in the quoted text.

No, its not.

>  I realize you didn't say
> v11 specifically, but this is the context of a patch that is proposed
> a bug-fix for v11.  If you meant that we should apply the patch as
> proposed now, or some other one, and do the other thing later, you
> could have said so.

I think its reasonable to expect you interpret my words sensibly,
rather than in some more dramatic form where I seem to break rules
with every phrase.

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

  23. Re: Indexes on partitioned tables and foreign partitions

    Robert Haas <robertmhaas@gmail.com> — 2018-05-13T12:59:45Z

    On Fri, May 11, 2018 at 8:52 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
> I think its reasonable to expect you interpret my words sensibly,
> rather than in some more dramatic form where I seem to break rules
> with every phrase.

Sure, I agree.  I try to interpret the words of everyone here sensibly
and without unnecessary drama.

I don't really know why we're still talking about this.  My principle
concern here has never been to understand whether or not you were
proposing foreign indexes for v11 -- although I have now got a very
clear idea that you weren't -- but to resolve the actual problem --
which seems to be a bug in 8b08f7d4.  I regret that we seem to have
gotten sucked into a conversation about whether I was being
unreasonable in thinking that you might have meant what I thought you
might have meant for a period of 3 hours last Wednesday, rather than
about the technical issue.  I apologize to you and to the list for my
contribution to that state of affairs.

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

  24. Re: Indexes on partitioned tables and foreign partitions

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2018-05-14T17:29:10Z

    On 2018-May-10, Amit Langote wrote:

> How about we error out even *before* calling DefineIndex for the 1st time?
>  I see that ProcessUtilitySlow() gets a list of all partitions when
> locking them for index creation before calling DefineIndex.  Maybe, just
> go through the list and error out if one of them is a partition that we
> don't support creating an index on?

The overwhelming consensus seems to be for this option, so I pushed your
patch after some small tweaks -- mostly to simplify unnecessarily
baroque code.  (I must have been thinking that recursion would happen
right in ProcessUtilitySlow.  That doesn't match my memories, but I
can't explain this code otherwise.)  I added a very small test too.

I think it'd be better to take this out of ProcessUtility also and into
DefineInde, for cleanliness sake; maybe add a 'recursing' flag to
DefineIndex.  Not for pg11, though.

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

  25. Re: Indexes on partitioned tables and foreign partitions

    Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2018-05-15T00:47:24Z

    On 2018/05/15 2:29, Alvaro Herrera wrote:
> On 2018-May-10, Amit Langote wrote:
> 
>> How about we error out even *before* calling DefineIndex for the 1st time?
>>  I see that ProcessUtilitySlow() gets a list of all partitions when
>> locking them for index creation before calling DefineIndex.  Maybe, just
>> go through the list and error out if one of them is a partition that we
>> don't support creating an index on?
> 
> The overwhelming consensus seems to be for this option, so I pushed your
> patch after some small tweaks -- mostly to simplify unnecessarily
> baroque code.  (I must have been thinking that recursion would happen
> right in ProcessUtilitySlow.  That doesn't match my memories, but I
> can't explain this code otherwise.)  I added a very small test too.

Thank you.

> I think it'd be better to take this out of ProcessUtility also and into
> DefineInde, for cleanliness sake; maybe add a 'recursing' flag to
> DefineIndex.  Not for pg11, though.

Agreed.  Most of the stuff in utility.c is for command dispatch and there
is no reason for this partitioning-related bit to be sitting here.  Moving
it will require perhaps non-trivial adjustment of indexcmds.c code, so
let's leave it for later as you say.

Thanks,
Amit