CREATE TABLE LIKE INCLUDING INDEXES support

Hi,

This is to get feedback to meet the following TODO:

- Simplify ability to create partitioned tables

This would allow creation of partitioned tables without requiring
creation of rules for INSERT/UPDATE/DELETE, and constraints for rapid
partition selection. Options could include range and hash partition
selection.

There was some discussion on the pgsql mailing lists, which lead to the
above TODO:
http://archives.postgresql.org/pgsql-hackers/2006-09/msg00189.php
http://archives.postgresql.org/pgsql-hackers/2006-08/msg01874.php

We can have the following syntax to support auto creation of partitions in
Postgresql:

CREATE TABLE tabname (
...
...
) PARTITION BY
HASH(expr)
| RANGE(expr)
| LIST(expr)
[PARTITIONS num_partitions] /* will apply to HASH only for now*/
[PARTITION partition_name CHECK(...),
PARTITION partition_name CHECK(...)
...
];

Here "expr" will be one of the column names as specified for the master
table. Once we finalize the syntax, the above statement would end up
carrying out the following activities (disclaimer: we might add or remove
some activities based on the discussion here).

i ) Create master table.
ii) Create children tables based on the number of partitions specified and
make them inherit from the master table.
iii) Auto generate rules (or triggers?) using the checks mentioned for the
partitions, to handle INSERTs/DELETEs/UPDATEs to navigate them to the
appropriate child. Note that checks specified directly on the master table
will get inherited automatically.
iv) Based on the PRIMARY, UNIQUE, REFERENCES information specified, pass it
on to the children tables.
v) If possible add CHECK (false) to the master table to avoid any activity
on it.

Some questions remain as to:

1) Whether we should use triggers/rules for step number (iii) above. Maybe
rules is the way to go.
2) What other attributes (access permissions e.g.) of the master along with
the ones specified in (iv) should be passed on to the children.
3) Some implementation specific issue e.g. whether SPI_execute would be a
good way of creating these rules.

Comments appreciated,
Regards,
Nikhils
EnterpriseDB http://www.enterprisedb.com

NikhilS wrote:
> iv) Based on the PRIMARY, UNIQUE, REFERENCES information specified,
> pass it on to the children tables.

How will you maintain a primary key in such a table, considering that
indexes can't span multiple tables?

> 1) Whether we should use triggers/rules for step number (iii) above.
> Maybe rules is the way to go.

Since this would basically be a case of the updatable rules problem, you
should review those discussions in the past to check whether the issues
mentioned there don't interfere with that plan.

> 2) What other attributes (access permissions e.g.) of the master
> along with the ones specified in (iv) should be passed on to the
> children.

Moreover, how are later changes of those attributes propagated?

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

Hi,

On 3/6/07, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
>
> NikhilS wrote:
> > iv) Based on the PRIMARY, UNIQUE, REFERENCES information specified,
> > pass it on to the children tables.
>
> How will you maintain a primary key in such a table, considering that
> indexes can't span multiple tables?

We will not (I know its a hard thing to do :) ), the intention is to use
this information from the parent and make it a property of the child table.
This will avoid the step for the user having to manually specify CREATE
INDEX and the likes on all the children tables one-by-one.

> 1) Whether we should use triggers/rules for step number (iii) above.
> > Maybe rules is the way to go.
>
> Since this would basically be a case of the updatable rules problem, you
> should review those discussions in the past to check whether the issues
> mentioned there don't interfere with that plan.

The rules mentioned here will be to specify that all the
inserts/updates/deletes should go into proper children tables instead of the
parent. I do not see the updateable rules problem with regards to this, but
will check out the archives for discussion on this related to partitioning.

> 2) What other attributes (access permissions e.g.) of the master
> > along with the ones specified in (iv) should be passed on to the
> > children.
>
> Moreover, how are later changes of those attributes propagated?

Once created, this will be a normal inheritance relationship between the
tables and all the existing commands will apply to both the parent and the
child.

The basic idea here is to automate as many things as possible at partition
creation time. The user is free to make additional changes to the involved
tables later too.

Regards,
Nikhils

--
EnterpriseDB http://www.enterprisedb.com

"NikhilS" <nikkhils(at)gmail(dot)com> writes:

>the intention is to use this information from the parent and make it a
>property of the child table. This will avoid the step for the user having to
>manually specify CREATE INDEX and the likes on all the children tables
>one-by-one.

Missed the start of this thread. A while back I had intended to add WITH
INDEXES to CREATE TABLE LIKE. That would let you create a tale LIKE parent
WITH CONSTRAINTS WITH INDEXES and get pretty much a perfect table ready for
adding to the inheritance structure.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com

Hi,

On 3/6/07, Gregory Stark <stark(at)enterprisedb(dot)com> wrote:
>
> "NikhilS" <nikkhils(at)gmail(dot)com> writes:
>
> >the intention is to use this information from the parent and make it a
> >property of the child table. This will avoid the step for the user having
> to
> >manually specify CREATE INDEX and the likes on all the children tables
> >one-by-one.
>
> Missed the start of this thread. A while back I had intended to add WITH
> INDEXES to CREATE TABLE LIKE. That would let you create a tale LIKE parent
> WITH CONSTRAINTS WITH INDEXES and get pretty much a perfect table ready
> for
> adding to the inheritance structure.

Yeah, this one aims to do pretty much the above as part of the auto creation
of the inheritance-based partitions.

Regards,
Nikhils

--
> Gregory Stark
> EnterpriseDB http://www.enterprisedb.com
>

--
EnterpriseDB http://www.enterprisedb.com

On 3/6/07, NikhilS <nikkhils(at)gmail(dot)com> wrote:
>
> Hi,
>
> On 3/6/07, Gregory Stark <stark(at)enterprisedb(dot)com> wrote:
> >
> > "NikhilS" <nikkhils(at)gmail(dot)com> writes:
> >
> > >the intention is to use this information from the parent and make it a
> > >property of the child table. This will avoid the step for the user
> > having to
> > >manually specify CREATE INDEX and the likes on all the children tables
> > >one-by-one.
> >
> > Missed the start of this thread. A while back I had intended to add WITH
> > INDEXES to CREATE TABLE LIKE. That would let you create a tale LIKE
> > parent
> > WITH CONSTRAINTS WITH INDEXES and get pretty much a perfect table ready
> > for
> > adding to the inheritance structure.
>
>
> Yeah, this one aims to do pretty much the above as part of the auto
> creation of the inheritance-based partitions.
>

And to add, maybe if there is consensus/demand for the WITH INDEXES idea
mentioned above too, I could work on it as well.

Regards,
Nikhils

Regards,
> Nikhils
>
>
> --
> > Gregory Stark
> > EnterpriseDB http://www.enterprisedb.com
> >
>
>
>
> --
> EnterpriseDB http://www.enterprisedb.com
>

--
EnterpriseDB http://www.enterprisedb.com

NikhilS wrote:
> Hi,
>
> On 3/6/07, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
>>
>> NikhilS wrote:
>> > iv) Based on the PRIMARY, UNIQUE, REFERENCES information specified,
>> > pass it on to the children tables.
>>
>> How will you maintain a primary key in such a table, considering that
>> indexes can't span multiple tables?
>
>
> We will not (I know its a hard thing to do :) ), the intention is to use
> this information from the parent and make it a property of the child table.
> This will avoid the step for the user having to manually specify CREATE
> INDEX and the likes on all the children tables one-by-one.

I think a way can be devised to maintain the primary key and unique
constraints.
If a search is done on the parent table, the planner knows to rewrite
the query as a union (or union all) of all child tables that relate to
the where clause, or all child tables if the where clause is not on the
column/s used to partition, then this concept should be able to be
converted to indexes as well, so that when a primary or unique index
from a child table is inserted to, then each of the related child
indexes is consulted to ensure uniqueness.
This would only apply if the partitioning was not done by the primary or
unique column.

>> 1) Whether we should use triggers/rules for step number (iii) above.
>> > Maybe rules is the way to go.
>>
>> Since this would basically be a case of the updatable rules problem, you
>> should review those discussions in the past to check whether the issues
>> mentioned there don't interfere with that plan.
>
> The rules mentioned here will be to specify that all the
> inserts/updates/deletes should go into proper children tables instead of
> the
> parent. I do not see the updateable rules problem with regards to this, but
> will check out the archives for discussion on this related to partitioning.
>

I would think that a trigger would be a better choice as I see the need
(or at least the possibility) for more than just a rewrite. When a value
is inserted that is outside of a condition currently covered by an
existing child table then a new child will need to be spawned to contain
the new data.

So say we partition by year and month of a date column? As new dates are
added new child tables would be created each month. Or is this beyond
the current plan and left to manual creation?

Will ALTER TABLE be extended to handle partitions? This will allow
partitioning existing tables (maybe without blocking access?) and allow
things like ALTER TABLE mytable ADD PARTITION (mydate within 200703)
and ALTER TABLE mytable DROP PARTITION (mydate within 199912) or would
dropping be covered by DELETE FROM mytable where mydate <= 199912 ?

Could such a syntax be devised for date columns? (month of mydate) or
similar to auto create partitions based on the year and month of a date
column? or will we just do CHECK(mydatecol >= 1/3/07 and mydatecol <=
31/3/07) for each month of data? Also (day of mydatecol) to partition
based on year and day of year.

Another syntax possibility - range(myserialcol of 500000) where new
child tables are created every 500000 rows?

Maybe I'm looking at auto-maintenance which is beyond any current planning?

--

Shane Ambler
pgSQL(at)Sheeky(dot)Biz

Get Sheeky @ http://Sheeky.Biz

peter_e(at)gmx(dot)net (Peter Eisentraut) writes:
> NikhilS wrote:
>> iv) Based on the PRIMARY, UNIQUE, REFERENCES information specified,
>> pass it on to the children tables.
>
> How will you maintain a primary key in such a table, considering that
> indexes can't span multiple tables?

On the one hand, I seem to recall seeing "multiple-table-spanning
indices" being on the TODO list.

On the other, it may be that this sort of partitioning is only usable
for scenarios where it is only needful to maintain uniqueness on a
partition-by-partition basis.

>> 1) Whether we should use triggers/rules for step number (iii) above.
>> Maybe rules is the way to go.
>
> Since this would basically be a case of the updatable rules problem, you
> should review those discussions in the past to check whether the issues
> mentioned there don't interfere with that plan.
>
>> 2) What other attributes (access permissions e.g.) of the master
>> along with the ones specified in (iv) should be passed on to the
>> children.
>
> Moreover, how are later changes of those attributes propagated?

I hear rumour of there being a more comprehensive proposal on this in
the works...
--
(reverse (concatenate 'string "ofni.secnanifxunil" "@" "enworbbc"))
http://linuxfinances.info/info/finances.html
Rules of the Evil Overlord #189. "I will never tell the hero "Yes I
was the one who did it, but you'll never be able to prove it to that
incompetent old fool." Chances are, that incompetent old fool is
standing behind the curtain." <http://www.eviloverlord.com/>

On Wed, Mar 07, 2007 at 02:43:39AM +1030, Shane Ambler wrote:
> I think a way can be devised to maintain the primary key and unique
> constraints.
> If a search is done on the parent table, the planner knows to rewrite
> the query as a union (or union all) of all child tables that relate to
> the where clause, or all child tables if the where clause is not on the
> column/s used to partition, then this concept should be able to be
> converted to indexes as well, so that when a primary or unique index
> from a child table is inserted to, then each of the related child
> indexes is consulted to ensure uniqueness.

But that's where it breaks down: you not only need to check that the
row you're inserting is unique, you need to make sure that other people
trying to insert the same value see it.

After all, a unique index does contain multiple instances of any
particular value, it's just that the DB has verified that none of these
are visible to the same transaction. If there is a collision a lock is
taken that waits until the colliding insert commits or rejects.

With b-tree it's easy, every value can only appear in one place, so
it's a very simple lock. The reason why GiST doesn't support unique
indexes is due to the fact that any particular value could appear in
many places, so to stop concurrent inserts you need to lock *every*
page in the tree that another backend might want to insert the record.

I fear this will also be a problem for indexes over multiple tables, you
will need to lock the page in every index that some other DB might want
to insert the value into. All while avoiding deadlocks.

I think the problems are related. If you can solve it for multiple
tables, you can solve it for GiST indexes also.

> This would only apply if the partitioning was not done by the primary or
> unique column.

That's the easy case, no problem there.

Have a nice day,
--
Martijn van Oosterhout <kleptog(at)svana(dot)org> http://svana.org/kleptog/
> From each according to his ability. To each according to his ability to litigate.

Martijn van Oosterhout wrote:
> On Wed, Mar 07, 2007 at 02:43:39AM +1030, Shane Ambler wrote:
>> I think a way can be devised to maintain the primary key and unique
>> constraints.
>> If a search is done on the parent table, the planner knows to rewrite
>> the query as a union (or union all) of all child tables that relate to
>> the where clause, or all child tables if the where clause is not on the
>> column/s used to partition, then this concept should be able to be
>> converted to indexes as well, so that when a primary or unique index
>> from a child table is inserted to, then each of the related child
>> indexes is consulted to ensure uniqueness.
>
> But that's where it breaks down: you not only need to check that the
> row you're inserting is unique, you need to make sure that other people
> trying to insert the same value see it.

This sounds like what is really needed is a way to lock a certain
condition, namely the existance or non-existance of a record with
certain values in certain fields. This would not only help this case,
it would also help RI triggers, because those wouldn't have to acquire
a share lock on the referenced rows anymore.

As you pointed out, this would also make unique GiST indices possible

No real idea how to do this, though :-(

greetings, Florian Pfluge

NikhilS wrote:
> We will not (I know its a hard thing to do :) ), the intention is to
> use this information from the parent and make it a property of the
> child table. This will avoid the step for the user having to manually
> specify CREATE INDEX and the likes on all the children tables
> one-by-one.

But when I say

CREATE TABLE ( a int PRIMARY KEY, ... ) PARTITION blah ...

then I expect that the primary key will be enforced across all
partitions. We currently sidestep that issue by not offering seemingly
transparent partitioning. But if you are planning to offer that, the
unique index issue needs to be solved, and I see nothing in your plan
about that.

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> But when I say
> CREATE TABLE ( a int PRIMARY KEY, ... ) PARTITION blah ...
> then I expect that the primary key will be enforced across all
> partitions. We currently sidestep that issue by not offering seemingly
> transparent partitioning. But if you are planning to offer that, the
> unique index issue needs to be solved, and I see nothing in your plan
> about that.

Agreed, it needs to Just Work. I think it'd still be useful though
if we only support auto-partitioning on the primary key, and that
restriction avoids the indexing problem.

regards, tom lane

Florian,

> This sounds like what is really needed is a way to lock a certain
> condition, namely the existance or non-existance of a record with
> certain values in certain fields. This would not only help this case,
> it would also help RI triggers, because those wouldn't have to acquire
> a share lock on the referenced rows anymore.

That's called "predicate locking" and it's very, very hard to do.

--
Josh Berkus
PostgreSQL @ Sun
San Francisco

> Agreed, it needs to Just Work. I think it'd still be useful though
> if we only support auto-partitioning on the primary key, and that
> restriction avoids the indexing problem.

+1

--
Josh Berkus
PostgreSQL @ Sun
San Francisco

Tom Lane wrote:
> Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
>
>> But when I say
>> CREATE TABLE ( a int PRIMARY KEY, ... ) PARTITION blah ...
>> then I expect that the primary key will be enforced across all
>> partitions. We currently sidestep that issue by not offering seemingly
>> transparent partitioning. But if you are planning to offer that, the
>> unique index issue needs to be solved, and I see nothing in your plan
>> about that.
>>
>
> Agreed, it needs to Just Work. I think it'd still be useful though
> if we only support auto-partitioning on the primary key, and that
> restriction avoids the indexing problem.
>
>

Maybe. The most obvious use for automatic partitioning that I can think
of would be based in the value of a timestamptz field rather than any
PK. Of course I tend to work more in the OLTP field than in DW type
apps, where other considerations might apply.

cheers

andrew

>-----Original Message-----
>From: pgsql-hackers-owner(at)postgresql(dot)org
>[mailto:pgsql-hackers-owner(at)postgresql(dot)org] On Behalf Of Josh Berkus
>Sent: dinsdag 6 maart 2007 19:45
>To: pgsql-hackers(at)postgresql(dot)org
>Cc: Florian G. Pflug; Martijn van Oosterhout; Shane Ambler;
>NikhilS; Peter Eisentraut
>Subject: Re: [HACKERS] Auto creation of Partitions
>
>Florian,
>
>> This sounds like what is really needed is a way to lock a certain
>> condition, namely the existance or non-existance of a record with
>> certain values in certain fields. This would not only help
>this case,
>> it would also help RI triggers, because those wouldn't have
>to acquire
>> a share lock on the referenced rows anymore.
>
>That's called "predicate locking" and it's very, very hard to do.

That's definitely not needed.

Rather something good such that we can finally enforce RI ourselves in
the general case. This is currently not possible to do easily, except in
C code. This means we need to look at all the rows that exists, but are
normally be invisible to our view of the database. Still I'm not sure
about all cases, as the MVCC model is quite tricky and I'm not sure
whether my idea's about it are valid.

The basic idea is that you need to guarentee the constraint for the
'single underlaying model' (with everything visible) and for your view
(under your visibility rules). I believe, but are not certain, that
under these conditions any (valid) snapshot will obey the desired
constraints.

- Joris Dobbelsteen

Greetings all,

I wrote this patch about a week ago to introduce myself to coding on
PostgreSQL. I wasn't entirely sure what the 'INCLUDING INDEXES' option
was meant to do, so I held off submitting it until I could get around to
asking about that and tweaking the documentation to reflect the patch.
By useful coincidence the thread "Auto creation of Partitions" had this
post in it, which made the intent of the option clear enough for me to
go ahead and see what people think of this.

Gregory Stark wrote:
> "NikhilS" <nikkhils(at)gmail(dot)com> writes:
>
>
>> the intention is to use this information from the parent and make it a
>> property of the child table. This will avoid the step for the user having to
>> manually specify CREATE INDEX and the likes on all the children tables
>> one-by-one.
>>
>
> Missed the start of this thread. A while back I had intended to add WITH
> INDEXES to CREATE TABLE LIKE. That would let you create a tale LIKE parent
> WITH CONSTRAINTS WITH INDEXES and get pretty much a perfect table ready for
> adding to the inheritance structure.
>
>
>
So, that's what this patch does. When a table is created with 'CREATE
TABLE ... LIKE parent INCLUDING INDEXES' this iterates over the parent
table indexes looking for constraint indexes, and alters the
CreateStmtContext to include equivalent indexes on the child table.

This is probably a somewhat naive implementation, being a first attempt.
I wasn't sure what sort of lock to place on the parent indexes or what
tablespace the new indexes should be created in. Any help improving it
would be appreciated.

Thank you,
-Trevor Hardcastle

Hi Shane,

Maybe I'm looking at auto-maintenance which is beyond any current planning?

Many of your suggestions are useful, but auto-maintenance will be beyond the
current plan.

Regards,
Nikhils

EnterpriseDB http://www.enterprisedb.com

Hi,

On 3/7/07, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> > But when I say
> > CREATE TABLE ( a int PRIMARY KEY, ... ) PARTITION blah ...
> > then I expect that the primary key will be enforced across all
> > partitions. We currently sidestep that issue by not offering seemingly
> > transparent partitioning. But if you are planning to offer that, the
> > unique index issue needs to be solved, and I see nothing in your plan
> > about that.
>
> Agreed, it needs to Just Work. I think it'd still be useful though
> if we only support auto-partitioning on the primary key, and that
> restriction avoids the indexing problem.
>
> regards, tom lane
>

Sure, but as Chris mentioned earlier, wouldn't it be useful to maintain
uniqueness on a
partition-by-partition basis too?

Regards,
Nikhils
--
EnterpriseDB http://www.enterprisedb.com

Andrew Dunstan wrote:
> Tom Lane wrote:
>> Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
>>
>>> But when I say
>>> CREATE TABLE ( a int PRIMARY KEY, ... ) PARTITION blah ...
>>> then I expect that the primary key will be enforced across all
>>> partitions. We currently sidestep that issue by not offering
>>> seemingly transparent partitioning. But if you are planning to offer
>>> that, the unique index issue needs to be solved, and I see nothing in
>>> your plan about that.
>>>
>>
>> Agreed, it needs to Just Work. I think it'd still be useful though
>> if we only support auto-partitioning on the primary key, and that
>> restriction avoids the indexing problem.
>>
>>
>
> Maybe. The most obvious use for automatic partitioning that I can think
> of would be based in the value of a timestamptz field rather than any
> PK. Of course I tend to work more in the OLTP field than in DW type
> apps, where other considerations might apply.

I second that - partitioning on some kind of timestamp field is a common
usecase here too ...

Stefan

>
>>
>> Maybe. The most obvious use for automatic partitioning that I can
>> think of would be based in the value of a timestamptz field rather
>> than any PK. Of course I tend to work more in the OLTP field than in
>> DW type apps, where other considerations might apply.
>
> I second that - partitioning on some kind of timestamp field is a
> common usecase here too ...
Partitioning period needs to work. It doesn't matter what the user
chooses as their partition key. Timestamp is an obvious
choice but there are others such as serial where you just partition
every million rows (for example) to keep things manageable.

Joshua D. Drake

>
>
> Stefan
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> message can get through to the mailing list cleanly
>

Am Mittwoch, 7. März 2007 07:44 schrieb NikhilS:
> Sure, but as Chris mentioned earlier, wouldn't it be useful to maintain
> uniqueness on a
> partition-by-partition basis too?

Many things might be useful, but the aim of the "table partitioning" venture
is believed to be the provision of a transparent interface to the existing
do-it-yourself partitioning facilities. Therefore, the logical definition of
a table must continue to work unchanged (or alternatively, the use of the
feature must be prohibited if that cannot be guaranteed in a particular
case). Other features such as uniqueness on a partition basis might also be
useful but you can do that today and you don't need partitioning for that.

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

On Tue, 2007-03-06 at 18:31 +0530, NikhilS wrote:

> On 3/6/07, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
> NikhilS wrote:
> > iv) Based on the PRIMARY, UNIQUE, REFERENCES information
> specified,
> > pass it on to the children tables.
>
> How will you maintain a primary key in such a table,
> considering that
> indexes can't span multiple tables?
>
> We will not (I know its a hard thing to do :) ), the intention is to
> use this information from the parent and make it a property of the
> child table. This will avoid the step for the user having to manually
> specify CREATE INDEX and the likes on all the children tables
> one-by-one.

If you know that the constraints on each of the tables is distinct, then
building a UNIQUE index on each of the partitions is sufficient to prove
that all rows in the combined partitioned table are distinct also.

The hard part there is checking that the partition constraints are
distinct. If the partition constraints are added one at a time, you can
use the predicate testing logic to compare the to-be-added partition's
constraint against each of the already added constraints. That becomes
an O(N) problem.

What is really needed is a data structure that allows range partitions
to be accessed more efficiently. This could make adding partitions and
deciding in which partition a specific value goes an O(logN) operation.

--
Simon Riggs
EnterpriseDB http://www.enterprisedb.com

Hi,

If you know that the constraints on each of the tables is distinct, then
> building a UNIQUE index on each of the partitions is sufficient to prove
> that all rows in the combined partitioned table are distinct also.
>
> The hard part there is checking that the partition constraints are
> distinct. If the partition constraints are added one at a time, you can
> use the predicate testing logic to compare the to-be-added partition's
> constraint against each of the already added constraints. That becomes
> an O(N) problem.
>
> What is really needed is a data structure that allows range partitions
> to be accessed more efficiently. This could make adding partitions and
> deciding in which partition a specific value goes an O(logN) operation.

If the unique constraint is supposed to be on a column which is NOT being
used for the partitioning, then all the above becomes much more difficult.
While partitioning, the additional onus on the user is to specify
non-conflicting CHECKs for the range/list partitions.

Regards,
Nikhils

--
> Simon Riggs
> EnterpriseDB http://www.enterprisedb.com
>
>
>

--
EnterpriseDB http://www.enterprisedb.com

> > iv) Based on the PRIMARY, UNIQUE, REFERENCES information specified,
> > pass it on to the children tables.
>
> How will you maintain a primary key in such a table,
> considering that indexes can't span multiple tables?

Many partitioning schemes have (or can be made to have) only one
possible target partition for the primary key. Thus if you create
separate unique indexes on each partition the problem is solved.
For a first version I opt, that it is sufficient to disallow creation of
a unique index on the master, when the constraints (on columns of this
index) do not nail down a specific partition for each row (e.g. a hash
or a range on one of the index columns that does not overlap).
Later, global index, or indexes with separate partitioning rules can be
implemented, that cover the other cases.

Andreas

Hi,

On 3/7/07, Zeugswetter Andreas ADI SD <ZeugswetterA(at)spardat(dot)at> wrote:
>
>
> > > iv) Based on the PRIMARY, UNIQUE, REFERENCES information specified,
> > > pass it on to the children tables.
> >
> > How will you maintain a primary key in such a table,
> > considering that indexes can't span multiple tables?
>
> Many partitioning schemes have (or can be made to have) only one
> possible target partition for the primary key. Thus if you create
> separate unique indexes on each partition the problem is solved.
> For a first version I opt, that it is sufficient to disallow creation of
> a unique index on the master, when the constraints (on columns of this
> index) do not nail down a specific partition for each row (e.g. a hash
> or a range on one of the index columns that does not overlap).
> Later, global index, or indexes with separate partitioning rules can be
> implemented, that cover the other cases.
>
> Andreas
>

Yes, I agree. For version 1, UNIQUE/PRIMARY indexes will cascade down to the
child table, only if the indexed column is present as part of the
partitioning rule.

Regards,
Nikhils

--
EnterpriseDB http://www.enterprisedb.com

I am wondering if we can implement unique indexes across several tables
(inheritance hierarchy) not by using a single, big index covering all
the tables, but rather by inserting a dummy entry into each partition's
unique index. This dummy entry would have an expanded CTID which would
include the tableoid, so it's possible to check it (albeit there is a
problem in that we may require the opening of another heap to do the
actual checking). These dummy entries could be removed by bulkcleanup
as soon as the inserting transaction is no longer running, to avoid
bloating the index too much. All said dummy index entries would be
located at either the rightmost or the leftmost leaf, or close to it, so
another idea is to have future inserters reuse the entry for a different
key.

The obvious problem with this is, naturally, the excess I/O that extra
index traversing causes. The not so obvious ones are locking,
deadlocking and the opening of other heaps and indexes while you do the
insertion, which may be too expensive. On the other hand, maybe this
idea is easier to implement than full-fledged cross-table indexes, so we
could have richer partitioning earlier than when somebody finally bites
the bullet and implements cross-table indexes.

Or maybe this is just a dumb idea, but I had to let it out anyway :-)

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Simon,

On 3/7/07 5:26 AM, "Simon Riggs" <simon(at)2ndquadrant(dot)com> wrote:

> What is really needed is a data structure that allows range partitions
> to be accessed more efficiently. This could make adding partitions and
> deciding in which partition a specific value goes an O(logN) operation.

I think we need to re-evaluate the inheritance mechanism for partitioning
and implement something much closer to the storage layer, similar to Oracle.

Having the constraint exclusion occur in the planner is not flexible enough
to allow more advanced solutions.

- Luke

On Wed, 2007-03-07 at 10:23 -0500, Luke Lonergan wrote:
> Simon,
>
> On 3/7/07 5:26 AM, "Simon Riggs" <simon(at)2ndquadrant(dot)com> wrote:
>
> > What is really needed is a data structure that allows range partitions
> > to be accessed more efficiently. This could make adding partitions and
> > deciding in which partition a specific value goes an O(logN) operation.
>
> I think we need to re-evaluate the inheritance mechanism for partitioning
> and implement something much closer to the storage layer, similar to Oracle.

Oracle's implementation is fairly close to ours, actually. Each
partition is a segment. SQLServer's is fairly low level.

> Having the constraint exclusion occur in the planner is not flexible enough
> to allow more advanced solutions.

It depends what those advanced solutions are. I definitely want to be
able to do run-time exclusion, push down merge joins and parallelism,
but I also think that being able to index only the first 3 partitions is
a useful feature too.

ISTM that making the Append node responsible for exclusion might be a
way to go with this, but various ways are possible, I'm sure. The trick
is to find one that does everything you need and that will take some
deeper analysis.

However you do it, you'll still need a way of deciding which partitions
to include/exclude that doesn't involve a sequential scan of all
partition constraints. So my comments about a different index structure
are still going to be relevant, wherever that lives/executes.

I'm not doing anything in this area personally for 8.3 though.

--
Simon Riggs
EnterpriseDB http://www.enterprisedb.com

> > What is really needed is a data structure that allows range
partitions
> > to be accessed more efficiently. This could make adding partitions
and
> > deciding in which partition a specific value goes an O(logN)
operation.
>
> I think we need to re-evaluate the inheritance mechanism for
> partitioning and implement something much closer to the
> storage layer, similar to Oracle.
>
> Having the constraint exclusion occur in the planner is not
> flexible enough to allow more advanced solutions.

Whoa, do you have anything to back that up ?
You would need to elaborate what you actually mean, but I think it is
moot.
Sure, the constraint technique can be further extended (e.g. during
runtime), but imho the approach is very good.

Andreas

>-----Original Message-----
>From: pgsql-hackers-owner(at)postgresql(dot)org
>[mailto:pgsql-hackers-owner(at)postgresql(dot)org] On Behalf Of Alvaro Herrera
>Sent: woensdag 7 maart 2007 15:59
>To: NikhilS
>Cc: Zeugswetter Andreas ADI SD; Peter Eisentraut;
>pgsql-hackers(at)postgresql(dot)org
>Subject: Re: [HACKERS] Auto creation of Partitions
>
>I am wondering if we can implement unique indexes across
>several tables (inheritance hierarchy) not by using a single,
>big index covering all the tables, but rather by inserting a
>dummy entry into each partition's unique index.

I think that one big index is much better in this case. You are already
replicating the data and need a lot of work when updating the partition
structure (e.g. removing one).

[snip]

Rather:
If we have the partition relations (aka partition), then we can define a
unique index on it. This guarentees that there is at most one tuple with
the same value (for the specified columns) for every individual
partition.

Now for the inserts.
We already guarentee that is unique within the partition it lives. So we
must insert it first.
Next its to check the other partitions (in order!) for existence of a
row with a similar context. Of course we require full visibility of the
data.
We do the insert first, as this ensures other will find it. The order is
also important, otherwise there can be situations where we can't
guarentee the constraint.

Updates are exactly the same.
Deletes are trivial.

Perhaps you can do it without an index, but this is nothing different
than for a normal table.

- Joris Dobbelsteen

Andreas,

On 3/7/07 11:45 AM, "Zeugswetter Andreas ADI SD" <ZeugswetterA(at)spardat(dot)at>
wrote:

> Whoa, do you have anything to back that up ?

Sure - when we start to consider designs that implement advanced data
management features, we run into problems with the architecture of
"tables->tables->tables...". Here are some examples:
1 - people think of partitions as a logical building block for tables, they
would like to move partitions around underneath a table without the table
definition being involved. In the current implementation, there are
explicit linkages between the table definition and the child tables -
imagine an "ALTER TABLE foo_parent ADD COLUMN" and how it would need to
cascade to 1,000 child tables and you get the beginning of it - this
connection should not exist.

2 - INSERT/UPDATE/DELETE processing through the SQL rewrite layer (rules) is
terribly slow and gets slower as you add more partitions. If done closer to
the storage layer, this can be done in ways that use access methods shared
with other storage entities, e.g. Indices, and the code path would flow more
naturally.

3 - Parallel query can be accomplished more easily by separating scans
across relations split among tablespaces. This is more natural than trying
to parallelize APPEND nodes within existing plans

> You would need to elaborate what you actually mean, but I think it is
> moot.
> Sure, the constraint technique can be further extended (e.g. during
> runtime), but imho the approach is very good.

Well, it's being used and that's good, but it needs to be better IMO and I
think that before we go too far down the current path we should consider the
alternatives more carefully.

- Luke

On Mar 6, 2007, at 9:13 AM, Shane Ambler wrote:
> NikhilS wrote:
>> On 3/6/07, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
>>> 1) Whether we should use triggers/rules for step number (iii) above.
>>> > Maybe rules is the way to go.
>>>
>>> Since this would basically be a case of the updatable rules
>>> problem, you
>>> should review those discussions in the past to check whether the
>>> issues
>>> mentioned there don't interfere with that plan.
>> The rules mentioned here will be to specify that all the
>> inserts/updates/deletes should go into proper children tables
>> instead of the
>> parent. I do not see the updateable rules problem with regards to
>> this, but
>> will check out the archives for discussion on this related to
>> partitioning.
>
> I would think that a trigger would be a better choice as I see the
> need (or at least the possibility) for more than just a rewrite.
> When a value is inserted that is outside of a condition currently
> covered by an existing child table then a new child will need to be
> spawned to contain the new data.

There's no reason a new child has to be spawned, and I don't think
that having a DML statement 'automagically' generating DDL is such a
hot idea, either.

Also, there's nothing inherently wrong with having an 'overflow
partition' (to use Oracle syntax) that holds values that don't fall
in the range of any other tables. The obvious place to do that with
our partitioning is in the parent table.

There are 2 other reasons to favor triggers though:

1) People (Josh Drake comes to mind) have found that if you get over
a tiny number of partitions, the performance of rules is abysmal.

2) I believe it should be possible to construct an update trigger
that allows you to perform updates that will place the row in
question into a new partition. While I can see cases for simply
disallowing updates to the partitioning key, I think there are also
times when being able to do that would be very useful.

> Will ALTER TABLE be extended to handle partitions? This will allow
> partitioning existing tables (maybe without blocking access?) and
> allow things like ALTER TABLE mytable ADD PARTITION (mydate within
> 200703)
> and ALTER TABLE mytable DROP PARTITION (mydate within 199912) or
> would dropping be covered by DELETE FROM mytable where mydate <=
> 199912 ?

I think it'd be great to make adding and removing partitions as
simple as ALTER TABLE. I don't think that DELETE should be the
mechanism to drop a partition, though. Again, DML statements
shouldn't be performing DDL.

> Could such a syntax be devised for date columns? (month of mydate)
> or similar to auto create partitions based on the year and month of
> a date column? or will we just do CHECK(mydatecol >= 1/3/07 and
> mydatecol <= 31/3/07) for each month of data? Also (day of
> mydatecol) to partition based on year and day of year.
>
> Another syntax possibility - range(myserialcol of 500000) where new
> child tables are created every 500000 rows?
>
> Maybe I'm looking at auto-maintenance which is beyond any current
> planning?

I don't think it's worth it to burden the database with auto-creating
time partitions; it's easy enough to setup a cron job to handle it.

It might be more useful to have the database handle things like
partitioning on a SERIAL column, though I agree with Nikhils that
this should wait.

Does any other database support 'automagic' partition creation? I
know Oracle 9i doesn't... not sure about 10g or DB2 or MSSQL...
--
Jim Nasby jim(at)nasby(dot)net
EnterpriseDB http://enterprisedb.com 512.569.9461 (cell)

On Mar 6, 2007, at 4:57 AM, NikhilS wrote:
> iv) Based on the PRIMARY, UNIQUE, REFERENCES information specified,
> pass it on to the children tables.

Since we want to eventually support 'global' indexes, I think we need
to be really careful here. There's actually 3 possible cases:

1) Index* should be global (spanning multiple tables)
2) Index* should be inherited by all partitions as they're created
3) Index* should exist only on the parent table

* Note that there's really no reason this has to be limit to indexes;
it could certainly apply to constraints, or even triggers.

IIRC, Oracle has a special syntax for global indexes; any other index
defined on a partitioned table is picked up *by newly created
partitions*. If you want to add indexes to existing partitions, you
have to explicitly add it to each partition.

I'd like to eventually see us supporting all 3 options. I'm not sure
how much we want to clutter the grammar, though.
--
Jim Nasby jim(at)nasby(dot)net
EnterpriseDB http://enterprisedb.com 512.569.9461 (cell)

On Mar 7, 2007, at 3:26 AM, Simon Riggs wrote:
> If you know that the constraints on each of the tables is distinct,
> then
> building a UNIQUE index on each of the partitions is sufficient to
> prove
> that all rows in the combined partitioned table are distinct also.
>
> The hard part there is checking that the partition constraints are
> distinct. If the partition constraints are added one at a time, you
> can
> use the predicate testing logic to compare the to-be-added partition's
> constraint against each of the already added constraints. That becomes
> an O(N) problem.
>
> What is really needed is a data structure that allows range partitions
> to be accessed more efficiently. This could make adding partitions and
> deciding in which partition a specific value goes an O(logN)
> operation.

Directing data to child tables with triggers pretty much necessitates
having some way to codify what partition a particular row belongs in.
IE: for partitioning by month, you'll see things like naming the
partition tables "parent_table_name_$YEAR_$MONTH", so the
'partitioning function' takes a date or timestamp and then returns
what partition it belongs to. Perhaps there is some way to use that
mapping to drive the selection of what partitions could contain a
given value?

One possibility would be to require 3 functions for a partitioned
table: one accepts the partitioning key and tells you what partition
it's in, one that tells you what the minimum partitioning key for a
partition would be, and one that tells you what the maximum would be.
If the user supplied those 3 functions, I think it would be possibly
to automatically generate code for the triggers and check
constraints. The min/max partition key functions might allow you to
more efficiently do partition elimination, too.
--
Jim Nasby jim(at)nasby(dot)net
EnterpriseDB http://enterprisedb.com 512.569.9461 (cell)

Hi,

Maybe, the dummy entry could be extended to contain the bounds (max/min) for
each of the other involved partitions and they could be updated each time a
DML happens across the partitions. That ways, an update to a particular
partition needs to lock out the others, examine the dummy entries in its own
index and follow it up with dummy entries update into other partitions if
the need be.

Ofcourse as you have mentioned all of this so needs to be done after a
careful think on the locking/deadlocking etc issues.

Regards,
Nikhils

On 3/7/07, Alvaro Herrera <alvherre(at)commandprompt(dot)com> wrote:
>
> I am wondering if we can implement unique indexes across several tables
> (inheritance hierarchy) not by using a single, big index covering all
> the tables, but rather by inserting a dummy entry into each partition's
> unique index. This dummy entry would have an expanded CTID which would
> include the tableoid, so it's possible to check it (albeit there is a
> problem in that we may require the opening of another heap to do the
> actual checking). These dummy entries could be removed by bulkcleanup
> as soon as the inserting transaction is no longer running, to avoid
> bloating the index too much. All said dummy index entries would be
> located at either the rightmost or the leftmost leaf, or close to it, so
> another idea is to have future inserters reuse the entry for a different
> key.
>
> The obvious problem with this is, naturally, the excess I/O that extra
> index traversing causes. The not so obvious ones are locking,
> deadlocking and the opening of other heaps and indexes while you do the
> insertion, which may be too expensive. On the other hand, maybe this
> idea is easier to implement than full-fledged cross-table indexes, so we
> could have richer partitioning earlier than when somebody finally bites
> the bullet and implements cross-table indexes.
>
> Or maybe this is just a dumb idea, but I had to let it out anyway :-)
>
> --
> Alvaro Herrera
> http://www.CommandPrompt.com/
> The PostgreSQL Company - Command Prompt, Inc.
>

--
EnterpriseDB http://www.enterprisedb.com

Hi,

> There are 2 other reasons to favor triggers though:
>
> 1) People (Josh Drake comes to mind) have found that if you get over
> a tiny number of partitions, the performance of rules is abysmal.
>
> 2) I believe it should be possible to construct an update trigger
> that allows you to perform updates that will place the row in
> question into a new partition. While I can see cases for simply
> disallowing updates to the partitioning key, I think there are also
> times when being able to do that would be very useful.

The consensus seems to be veering towards triggers.

>
> I think it'd be great to make adding and removing partitions as
> simple as ALTER TABLE. I don't think that DELETE should be the
> mechanism to drop a partition, though. Again, DML statements
> shouldn't be performing DDL.

Since partition is inheritance-based, a simple DROP or "NO INHERIT" will do
the job to deal with the partition. Do we want to reinvent additional syntax
when these are around and are documented?

Regards,
Nikhils
--
EnterpriseDB http://www.enterprisedb.com

On Wed, 2007-03-07 at 22:32 -0500, Luke Lonergan wrote:
> Andreas,
>
> On 3/7/07 11:45 AM, "Zeugswetter Andreas ADI SD" <ZeugswetterA(at)spardat(dot)at>
> wrote:
>
> > Whoa, do you have anything to back that up ?
>
> Sure - when we start to consider designs that implement advanced data
> management features, we run into problems with the architecture of
> "tables->tables->tables...". Here are some examples:
> 1 - people think of partitions as a logical building block for tables, they
> would like to move partitions around underneath a table without the table
> definition being involved. In the current implementation, there are
> explicit linkages between the table definition and the child tables -
> imagine an "ALTER TABLE foo_parent ADD COLUMN" and how it would need to
> cascade to 1,000 child tables and you get the beginning of it - this
> connection should not exist.

The inheritance can work at multiple levels, so its up to you how you
manage things. If you want to add a column to only the newest tables,
you just add a new mid-level table, add the new column only to that and
then make all new partitions inherit from that table rather than the
main table. So I don't see the objection here, I see a benefit.

> 2 - INSERT/UPDATE/DELETE processing through the SQL rewrite layer (rules) is
> terribly slow and gets slower as you add more partitions. If done closer to
> the storage layer, this can be done in ways that use access methods shared
> with other storage entities, e.g. Indices, and the code path would flow more
> naturally.

That bit is just syntactic sugar and unnecessary, for most applications,
in my experience - especially because it doesn't work with COPY.

People do seem to want it, so as I said upthread, we need a way of
speeding up the selection of the appropriate partition, so we can get
this to work for individual INSERTs.

This needs substantial improvement, but I don't see this challenging the
partition == table assumption.

> 3 - Parallel query can be accomplished more easily by separating scans
> across relations split among tablespaces. This is more natural than trying
> to parallelize APPEND nodes within existing plans

Tables-> Partitions -> Tablespaces

Changing Partitions from Tables to SomethingElse won't increase the
current capability to define the disk layout for concurrent I/O.

Parallel Query needs to be planner-aware, so if we change partitions
from being tables, then we'd need to reintroduce them to the planner.
APPEND is clearly not the only thing that's needed for parallel query.

Note to Nikhil: Make sure the new syntax doesn't prevent partitions from
being placed upon multiple tablespaces in some manner, at CREATE TABLE
time.

> > You would need to elaborate what you actually mean, but I think it is
> > moot.
> > Sure, the constraint technique can be further extended (e.g. during
> > runtime), but imho the approach is very good.
>
> Well, it's being used and that's good, but it needs to be better IMO and I
> think that before we go too far down the current path we should consider the
> alternatives more carefully.

I'm happy to reconsider things, but we need to do that with some clear
analysis of what doesn't work yet and how best to implement that.

None of what's been mentioned requires us to reconsider the
Partition == Table assumption. There were other ways considered, but
they didn't win out in the analysis, for documented reasons.

If there are benefits to having partitions act like tables, then maybe
we can make them behave differently in just those circumstances. Pack
animals behave differently in a group, so why not tables, if need be?
But when is that exactly?

There is a ton of work to make partitioning the elegant beast we'd like
it to be, but that seems like extension only, not rip and replace.

--
Simon Riggs
EnterpriseDB http://www.enterprisedb.com

On Wed, 2007-03-07 at 21:27 -0700, Jim Nasby wrote:
> On Mar 7, 2007, at 3:26 AM, Simon Riggs wrote:
> > If you know that the constraints on each of the tables is distinct,
> > then
> > building a UNIQUE index on each of the partitions is sufficient to
> > prove
> > that all rows in the combined partitioned table are distinct also.
> >
> > The hard part there is checking that the partition constraints are
> > distinct. If the partition constraints are added one at a time, you
> > can
> > use the predicate testing logic to compare the to-be-added partition's
> > constraint against each of the already added constraints. That becomes
> > an O(N) problem.
> >
> > What is really needed is a data structure that allows range partitions
> > to be accessed more efficiently. This could make adding partitions and
> > deciding in which partition a specific value goes an O(logN)
> > operation.
>
> Directing data to child tables with triggers pretty much necessitates
> having some way to codify what partition a particular row belongs in.
> IE: for partitioning by month, you'll see things like naming the
> partition tables "parent_table_name_$YEAR_$MONTH", so the
> 'partitioning function' takes a date or timestamp and then returns
> what partition it belongs to. Perhaps there is some way to use that
> mapping to drive the selection of what partitions could contain a
> given value?
>
> One possibility would be to require 3 functions for a partitioned
> table: one accepts the partitioning key and tells you what partition
> it's in, one that tells you what the minimum partitioning key for a
> partition would be, and one that tells you what the maximum would be.
> If the user supplied those 3 functions, I think it would be possibly
> to automatically generate code for the triggers and check
> constraints. The min/max partition key functions might allow you to
> more efficiently do partition elimination, too.

ISTM this is a good idea.

SQLServer uses partitioning functions and I like that approach. It makes
it much easier to do partition-wise joins between tables that share
partitioning functions.

--
Simon Riggs
EnterpriseDB http://www.enterprisedb.com

"Jim Nasby" <decibel(at)decibel(dot)org> writes:

> One possibility would be to require 3 functions for a partitioned table: one
> accepts the partitioning key and tells you what partition it's in, one that
> tells you what the minimum partitioning key for a partition would be, and one
> that tells you what the maximum would be. If the user supplied those 3
> functions, I think it would be possibly to automatically generate code for the
> triggers and check constraints. The min/max partition key functions might
> allow you to more efficiently do partition elimination, too.

But then it would be harder to tell whether a clause implied a given
partition. That is, if you have a partition constraint of "col OP const" then
we can test whether a query clause of "col OP2 const2" implies that constraint
when planning (or actually whether it implies it's false to exclude the
partition). If you have a constraint like "P1(const)" it'll be pretty hard to
do much with that.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com

> > The hard part there is checking that the partition constraints are
> > distinct. If the partition constraints are added one at a time, you
> > can use the predicate testing logic to compare the to-be-added
> > partition's constraint against each of the already added
constraints.
> > That becomes an O(N) problem.

Yes, we could preevaluate that check (per index) in the DDL phase and
keep the info in a flag.
Also interesting info is if there is an order the partitions can be read
in to satisfy a particular order by.

> > What is really needed is a data structure that allows range
partitions
> > to be accessed more efficiently. This could make adding partitions
and
> > deciding in which partition a specific value goes an O(logN)
> > operation.

I do not really see a problem with O(N) since typical N currently range
from 10 to 200.
N = 1000 is already good for a >= 10 TB table.
If a 10 GB partition were too large we should imho invest more in the
advanced indexing methods that are currently beeing developed.

> Directing data to child tables with triggers pretty much
> necessitates having some way to codify what partition a
> particular row belongs in.
> IE: for partitioning by month, you'll see things like naming
> the partition tables "parent_table_name_$YEAR_$MONTH", so the
> 'partitioning function' takes a date or timestamp and then
> returns what partition it belongs to. Perhaps there is some
> way to use that mapping to drive the selection of what
> partitions could contain a given value?

You put it in the first partition that has matching constraints.

> One possibility would be to require 3 functions for a partitioned
> table: one accepts the partitioning key and tells you what
> partition it's in, one that tells you what the minimum
> partitioning key for a partition would be, and one that tells
> you what the maximum would be.
> If the user supplied those 3 functions, I think it would be
> possibly to automatically generate code for the triggers and
> check constraints. The min/max partition key functions might
> allow you to more efficiently do partition elimination, too.

I can see this as a good optional addition, but it can only be optional
else it would pretty much limit the methods that can be used for
partitioning. e.g. hash, modulo do not have a min,max per partition.

Andreas

On Thu, Mar 08, 2007 at 01:52:27PM +0530, NikhilS wrote:
> >I think it'd be great to make adding and removing partitions as
> >simple as ALTER TABLE. I don't think that DELETE should be the
> >mechanism to drop a partition, though. Again, DML statements
> >shouldn't be performing DDL.
>
>
> Since partition is inheritance-based, a simple DROP or "NO INHERIT" will do
> the job to deal with the partition. Do we want to reinvent additional syntax
> when these are around and are documented?

Well, if the syntax for adding a new partition eventually ends up as
ALTER TABLE ADD PARTITION, then it would make more sense that you remove
a partition via ALTER TABLE DROP PARTITION.
--
Jim Nasby jim(at)nasby(dot)net
EnterpriseDB http://enterprisedb.com 512.569.9461 (cell)

On Thu, Mar 08, 2007 at 10:12:30AM +0000, Gregory Stark wrote:
> "Jim Nasby" <decibel(at)decibel(dot)org> writes:
>
> > One possibility would be to require 3 functions for a partitioned table: one
> > accepts the partitioning key and tells you what partition it's in, one that
> > tells you what the minimum partitioning key for a partition would be, and one
> > that tells you what the maximum would be. If the user supplied those 3
> > functions, I think it would be possibly to automatically generate code for the
> > triggers and check constraints. The min/max partition key functions might
> > allow you to more efficiently do partition elimination, too.
>
> But then it would be harder to tell whether a clause implied a given
> partition. That is, if you have a partition constraint of "col OP const" then
> we can test whether a query clause of "col OP2 const2" implies that constraint
> when planning (or actually whether it implies it's false to exclude the
> partition). If you have a constraint like "P1(const)" it'll be pretty hard to
> do much with that.

Well, you could tell what partition 'const' was in; I would think that
plus knowledge about OP2 would allow you to decide what partitions you
need to look at.

There's also nothing to prevent us from also adding the constraints and
using constraint exclusion as well. In fact, I think we'd want to have
the constraints just so we know that a given partition only contains the
data we want it to.
--
Jim Nasby jim(at)nasby(dot)net
EnterpriseDB http://enterprisedb.com 512.569.9461 (cell)

One other thought... a lot of this discussion seems to parallel the
debate from a while ago about whether SERIAL should act like a macro
(ie: it just sets everything up and users are free to monkey under the
hood afterwards), or whether it should be it's own 'closed-box'
construct.

Currently, we seem to be leaning towards partition management being a
'macro', with child tables very exposed, etc. I don't know if that's
good or bad, but it's probably worth some thought.
--
Jim Nasby jim(at)nasby(dot)net
EnterpriseDB http://enterprisedb.com 512.569.9461 (cell)

> Note to Nikhil: Make sure the new syntax doesn't prevent partitions from
> being placed upon multiple tablespaces in some manner, at CREATE TABLE
> time.

What if the syntax was something like -

CREATE TABLE tabname (
...
...
) PARTITION BY
HASH(expr)
| RANGE(expr)
| LIST(expr)
[PARTITIONS num_partitions] /* will apply to HASH only for now*/
[PARTITION partition_name CHECK(...) [USING TABLESPACE tblspcname],
PARTITION partition_name CHECK(...) [USING TABLESPACE tblspcname]
...
];

And (if we use the ALTER TABLE to add partitions)

ALTER TABLE tabname
ADD PARTITION partition_name CHECK(...)
[USING TABLESPACE tblspcname];

Of course ALTER TABLE childtable SET TABLESPACE tblspcname; should not
cause any probs.

--

Shane Ambler
pgSQL(at)Sheeky(dot)Biz

Get Sheeky @ http://Sheeky.Biz

Jim C. Nasby wrote:
> On Thu, Mar 08, 2007 at 01:52:27PM +0530, NikhilS wrote:
>>> I think it'd be great to make adding and removing partitions as
>>> simple as ALTER TABLE. I don't think that DELETE should be the
>>> mechanism to drop a partition, though. Again, DML statements
>>> shouldn't be performing DDL.
>>
>> Since partition is inheritance-based, a simple DROP or "NO INHERIT" will do
>> the job to deal with the partition. Do we want to reinvent additional syntax
>> when these are around and are documented?
>
> Well, if the syntax for adding a new partition eventually ends up as
> ALTER TABLE ADD PARTITION, then it would make more sense that you remove
> a partition via ALTER TABLE DROP PARTITION.

This follows on from the suggestion I made - taken along the lines of
the subject "auto creation of partitions" where I suggested the syntax
of partition check(month of mydatecol) and have a new partition created
as data was entered. With this scenario dropping the partition when it
was empty would complement the creation of a new partition as needed.

Given that there seems to be no real support of going with "auto
maintenance" were new partitions are added as needed, then the auto
dropping of empty partitions would also not apply.

Leaving us with only specific add partition / drop partition commands.
And have the parent table pick up rows not matching any partition check
criteria.

--

Shane Ambler
pgSQL(at)Sheeky(dot)Biz

Get Sheeky @ http://Sheeky.Biz

Hi,

On 3/9/07, Shane Ambler <pgsql(at)sheeky(dot)biz> wrote:
>
>
> > Note to Nikhil: Make sure the new syntax doesn't prevent partitions from
> > being placed upon multiple tablespaces in some manner, at CREATE TABLE
> > time.
>
> What if the syntax was something like -
>
> CREATE TABLE tabname (
> ...
> ...
> ) PARTITION BY
> HASH(expr)
> | RANGE(expr)
> | LIST(expr)
> [PARTITIONS num_partitions] /* will apply to HASH only for now*/
> [PARTITION partition_name CHECK(...) [USING TABLESPACE tblspcname],
> PARTITION partition_name CHECK(...) [USING TABLESPACE tblspcname]
> ...
> ];
>
>
> And (if we use the ALTER TABLE to add partitions)
>
> ALTER TABLE tabname
> ADD PARTITION partition_name CHECK(...)
> [USING TABLESPACE tblspcname];
>
>
We could as well drop the USING part.

Regards,
Nikhils
--
EnterpriseDB http://www.enterprisedb.com

Hi,

> This follows on from the suggestion I made - taken along the lines of
> the subject "auto creation of partitions" where I suggested the syntax
> of partition check(month of mydatecol) and have a new partition created
> as data was entered. With this scenario dropping the partition when it
> was empty would complement the creation of a new partition as needed.
>
> Given that there seems to be no real support of going with "auto
> maintenance" were new partitions are added as needed, then the auto
> dropping of empty partitions would also not apply.
>
> Leaving us with only specific add partition / drop partition commands.
> And have the parent table pick up rows not matching any partition check
> criteria.

I was thinking along the lines of what Jim had suggested earlier regarding
overflow partition. Instead of dumping unmatched rows to the master table,
we could put them into a default "DUMP/DUMB" partition.

Given that Simon wants to do away with having the master table APPENDed in
the planning phase, this would be better.

Regards,
Nikhils

--
>
> Shane Ambler
> pgSQL(at)Sheeky(dot)Biz
>
> Get Sheeky @ http://Sheeky.Biz
>

--
EnterpriseDB http://www.enterprisedb.com

> > Since partition is inheritance-based, a simple DROP or "NO
> INHERIT"
> > will do the job to deal with the partition. Do we want to reinvent
> > additional syntax when these are around and are documented?
>
> Well, if the syntax for adding a new partition eventually
> ends up as ALTER TABLE ADD PARTITION, then it would make more
> sense that you remove a partition via ALTER TABLE DROP PARTITION.

But DROP PARTITION usually moves the data from this partition to other
partitions,
so it is something different.

Andreas

On Fri, 2007-03-09 at 11:48 +0530, NikhilS wrote:
> Hi,
>
> On 3/9/07, Shane Ambler <pgsql(at)sheeky(dot)biz> wrote:
>
> > Note to Nikhil: Make sure the new syntax doesn't prevent
> partitions from
> > being placed upon multiple tablespaces in some manner, at
> CREATE TABLE
> > time.
>
> What if the syntax was something like -
>
> CREATE TABLE tabname (
> ...
> ...
> ) PARTITION BY
> HASH(expr)
> | RANGE(expr)
> | LIST(expr)
> [PARTITIONS num_partitions] /* will apply to HASH only for
> now*/
> [PARTITION partition_name CHECK(...) [USING TABLESPACE
> tblspcname],
> PARTITION partition_name CHECK(...) [USING TABLESPACE
> tblspcname]
> ...
> ];
>
>
> And (if we use the ALTER TABLE to add partitions)
>
> ALTER TABLE tabname
> ADD PARTITION partition_name CHECK(...)
> [USING TABLESPACE tblspcname];
>
>
>
> We could as well drop the USING part.

Why would we support HASH partitions?
If you did, the full syntax for hash clusters should be supported.

If we do the CHECK clauses like that then we still have don't have a
guaranteed non-overlap between partitions. It would be easier to use
Oracle syntax and then construct the CHECK clauses from that.

Also, the syntax needs to be fairly complex to allow for a mixture of
modes, e.g. range and list partitioning. That is currently possible
today and the syntax for doing that is IMHO much simpler than the Oracle
"simple" way of specifying it.

An alternative is to provide a partitioning function which decides which
partition each values goes into.

PARTITION FUNCTION which_partition(date_col)

The partition function must return an unsigned integer > 0, which would
correspond to particular partitions. Partitions would be numbered 1..N,
and named tablename_partM where 1 <= M <= N.

The input and contents of the partition function would be up to the
user. e.g.

CREATE FUNCTION range_partition(date date_col)
{
if (date_col < D1)
return 1;
else if (date_col < D2)
return 2;
else if (date_col < D3)
return 3;

return 4;
}

Doing it this way would allow us to easily join two tables based upon a
common partition function.

In time, I would suggest we support both ways: declarative and
functional.

--
Simon Riggs
EnterpriseDB http://www.enterprisedb.com

Hi,

> Why would we support HASH partitions?
> If you did, the full syntax for hash clusters should be supported.

In MySQL, Oracle, the syntax for HASH partitions seems to be similar to the
one mentioned. I do not know much about hash clusters though.

If we do the CHECK clauses like that then we still have don't have a
> guaranteed non-overlap between partitions. It would be easier to use
> Oracle syntax and then construct the CHECK clauses from that.

Again Oracle, MySQL use "VALUES LESS THAN (expr)" format for RANGE
partitions. So you mean that they end up creating ranges like "MININT -
Range1", "Range1+1 - Range2" etc for each of the partitions?

I think Postgres users are used to the CHECK clauses and I still feel that
the onus of distinct partitions lies on the partition creator.

Also, the syntax needs to be fairly complex to allow for a mixture of
> modes, e.g. range and list partitioning. That is currently possible
> today and the syntax for doing that is IMHO much simpler than the Oracle
> "simple" way of specifying it.

Subpartitioning is not being targeted right now, but could be put on the
TODO list for further enhancements.

An alternative is to provide a partitioning function which decides which
> partition each values goes into.
>
> PARTITION FUNCTION which_partition(date_col)
>
> The partition function must return an unsigned integer > 0, which would
> correspond to particular partitions. Partitions would be numbered 1..N,
> and named tablename_partM where 1 <= M <= N.
>
> The input and contents of the partition function would be up to the
> user. e.g.
>
> CREATE FUNCTION range_partition(date date_col)
> {
> if (date_col < D1)
> return 1;
> else if (date_col < D2)
> return 2;
> else if (date_col < D3)
> return 3;
>
> return 4;
> }
>
> Doing it this way would allow us to easily join two tables based upon a
> common partition function.
>
> In time, I would suggest we support both ways: declarative and
> functional.

Till now, we are going the declarative way.

Regards,
Nikhils
--
EnterpriseDB http://www.enterprisedb.com

On Mar 9, 2007, at 3:31 AM, Zeugswetter Andreas ADI SD wrote:
>>> Since partition is inheritance-based, a simple DROP or "NO
>> INHERIT"
>>> will do the job to deal with the partition. Do we want to reinvent
>>> additional syntax when these are around and are documented?
>>
>> Well, if the syntax for adding a new partition eventually
>> ends up as ALTER TABLE ADD PARTITION, then it would make more
>> sense that you remove a partition via ALTER TABLE DROP PARTITION.
>
> But DROP PARTITION usually moves the data from this partition to other
> partitions,
> so it is something different.

It does? IIRC every partitioning system I've seen DROP PARTITION
drops the data as well. It's up to you to move it somewhere else if
you want to keep it.
--
Jim Nasby jim(at)nasby(dot)net
EnterpriseDB http://enterprisedb.com 512.569.9461 (cell)

Ühel kenal päeval, R, 2007-03-09 kell 15:41, kirjutas Jim Nasby:
> On Mar 9, 2007, at 3:31 AM, Zeugswetter Andreas ADI SD wrote:
> >>> Since partition is inheritance-based, a simple DROP or "NO
> >> INHERIT"
> >>> will do the job to deal with the partition. Do we want to reinvent
> >>> additional syntax when these are around and are documented?
> >>
> >> Well, if the syntax for adding a new partition eventually
> >> ends up as ALTER TABLE ADD PARTITION, then it would make more
> >> sense that you remove a partition via ALTER TABLE DROP PARTITION.
> >
> > But DROP PARTITION usually moves the data from this partition to other
> > partitions,
> > so it is something different.
>
> It does? IIRC every partitioning system I've seen DROP PARTITION
> drops the data as well. It's up to you to move it somewhere else if
> you want to keep it.

Will this proposed DROP PARTITION just disassociate the table from the
master, or will it actually drop the partitions table from the whole
database ?

--
----------------
Hannu Krosing
Database Architect
Skype Technologies OÜ
Akadeemia tee 21 F, Tallinn, 12618, Estonia

Skype me: callto:hkrosing
Get Skype for free: http://www.skype.com

On Friday 09 March 2007 01:23, NikhilS wrote:
> Hi,
>
> > This follows on from the suggestion I made - taken along the lines of
> > the subject "auto creation of partitions" where I suggested the syntax
> > of partition check(month of mydatecol) and have a new partition created
> > as data was entered. With this scenario dropping the partition when it
> > was empty would complement the creation of a new partition as needed.
> >
> > Given that there seems to be no real support of going with "auto
> > maintenance" were new partitions are added as needed, then the auto
> > dropping of empty partitions would also not apply.
> >
> > Leaving us with only specific add partition / drop partition commands.
> > And have the parent table pick up rows not matching any partition check
> > criteria.
>
> I was thinking along the lines of what Jim had suggested earlier regarding
> overflow partition. Instead of dumping unmatched rows to the master table,
> we could put them into a default "DUMP/DUMB" partition.
>

I'm quite content dumping unmatched rows into the master table. This makes it
very easy to scan partitioned tables for busted partition setups. Having a
DUMP tables seems only different semantically, so why learn new semantics?

> Given that Simon wants to do away with having the master table APPENDed in
> the planning phase, this would be better.
>

ISTM you're trading appending the master table for appending the DUMP
partition, which afaict would give you no gain.

--
Robert Treat
Build A Brighter LAMP :: Linux Apache {middleware} PostgreSQL

Hi,

On 3/10/07, Hannu Krosing <hannu(at)skype(dot)net> wrote:
>
> Ühel kenal päeval, R, 2007-03-09 kell 15:41, kirjutas Jim Nasby:
> > On Mar 9, 2007, at 3:31 AM, Zeugswetter Andreas ADI SD wrote:
> > >>> Since partition is inheritance-based, a simple DROP or "NO
> > >> INHERIT"
> > >>> will do the job to deal with the partition. Do we want to reinvent
> > >>> additional syntax when these are around and are documented?
> > >>
> > >> Well, if the syntax for adding a new partition eventually
> > >> ends up as ALTER TABLE ADD PARTITION, then it would make more
> > >> sense that you remove a partition via ALTER TABLE DROP PARTITION.
> > >
> > > But DROP PARTITION usually moves the data from this partition to other
> > > partitions,
> > > so it is something different.
> >
> > It does? IIRC every partitioning system I've seen DROP PARTITION
> > drops the data as well. It's up to you to move it somewhere else if
> > you want to keep it.
>
> Will this proposed DROP PARTITION just disassociate the table from the
> master, or will it actually drop the partitions table from the whole
> database ?

Thats why I would prefer the existing mechanism, there a DROP on the child
removes it and a NO INHERIT disassociates it. There might be situations
where we would want to just disassociate and not drop.

Regards,
Nikhils

--
> ----------------
> Hannu Krosing
> Database Architect
> Skype Technologies OÜ
> Akadeemia tee 21 F, Tallinn, 12618, Estonia
>
> Skype me: callto:hkrosing
> Get Skype for free: http://www.skype.com
>
>
>

--
EnterpriseDB http://www.enterprisedb.com

Hi,

> > Given that Simon wants to do away with having the master table APPENDed
> in
> > the planning phase, this would be better.
> >
>
> ISTM you're trading appending the master table for appending the DUMP
> partition, which afaict would give you no gain.

If there are entries in the master table, I think it would get appended for
all queries regardless of whether we need to examine its contents or not.
Segregating dump data into a partition will avoid that.

I have seen examples in some other databases wherein a partition specifies a
range of "someval - MAXINT" for instance, to catch such cases.

That again means that the onus is on the partition creator most of the
times..
Regards,
Nikhils

> --
> Robert Treat
> Build A Brighter LAMP :: Linux Apache {middleware} PostgreSQL
>

--
EnterpriseDB http://www.enterprisedb.com

On Saturday 10 March 2007 00:13, NikhilS wrote:
> Hi,
>
> > > Given that Simon wants to do away with having the master table APPENDed
> >
> > in
> >
> > > the planning phase, this would be better.
> >
> > ISTM you're trading appending the master table for appending the DUMP
> > partition, which afaict would give you no gain.
>
> If there are entries in the master table, I think it would get appended for
> all queries regardless of whether we need to examine its contents or not.
> Segregating dump data into a partition will avoid that.
>
> I have seen examples in some other databases wherein a partition specifies
> a range of "someval - MAXINT" for instance, to catch such cases.
>
> That again means that the onus is on the partition creator most of the
> times..

*shrug*... we can do that now in pgsql

--
Robert Treat
Build A Brighter LAMP :: Linux Apache {middleware} PostgreSQL

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.

---------------------------------------------------------------------------

Trevor Hardcastle wrote:
> Greetings all,
>
> I wrote this patch about a week ago to introduce myself to coding on
> PostgreSQL. I wasn't entirely sure what the 'INCLUDING INDEXES' option
> was meant to do, so I held off submitting it until I could get around to
> asking about that and tweaking the documentation to reflect the patch.
> By useful coincidence the thread "Auto creation of Partitions" had this
> post in it, which made the intent of the option clear enough for me to
> go ahead and see what people think of this.
>
> Gregory Stark wrote:
> > "NikhilS" <nikkhils(at)gmail(dot)com> writes:
> >
> >
> >> the intention is to use this information from the parent and make it a
> >> property of the child table. This will avoid the step for the user having to
> >> manually specify CREATE INDEX and the likes on all the children tables
> >> one-by-one.
> >>
> >
> > Missed the start of this thread. A while back I had intended to add WITH
> > INDEXES to CREATE TABLE LIKE. That would let you create a tale LIKE parent
> > WITH CONSTRAINTS WITH INDEXES and get pretty much a perfect table ready for
> > adding to the inheritance structure.
> >
> >
> >
> So, that's what this patch does. When a table is created with 'CREATE
> TABLE ... LIKE parent INCLUDING INDEXES' this iterates over the parent
> table indexes looking for constraint indexes, and alters the
> CreateStmtContext to include equivalent indexes on the child table.
>
> This is probably a somewhat naive implementation, being a first attempt.
> I wasn't sure what sort of lock to place on the parent indexes or what
> tablespace the new indexes should be created in. Any help improving it
> would be appreciated.
>
> Thank you,
> -Trevor Hardcastle
>

> Index: src/backend/parser/analyze.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/backend/parser/analyze.c,v
> retrieving revision 1.361
> diff -c -r1.361 analyze.c
> *** src/backend/parser/analyze.c 20 Feb 2007 17:32:16 -0000 1.361
> --- src/backend/parser/analyze.c 7 Mar 2007 01:43:12 -0000
> ***************
> *** 14,19 ****
> --- 14,20 ----
> #include "postgres.h"
>
> #include "access/heapam.h"
> + #include "access/genam.h"
> #include "catalog/heap.h"
> #include "catalog/index.h"
> #include "catalog/namespace.h"
> ***************
> *** 40,45 ****
> --- 41,47 ----
> #include "utils/acl.h"
> #include "utils/builtins.h"
> #include "utils/lsyscache.h"
> + #include "utils/relcache.h"
> #include "utils/syscache.h"
>
>
> ***************
> *** 1345,1355 ****
> }
> }
>
> - if (including_indexes)
> - ereport(ERROR,
> - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> - errmsg("LIKE INCLUDING INDEXES is not implemented")));
> -
> /*
> * Insert the copied attributes into the cxt for the new table
> * definition.
> --- 1347,1352 ----
> ***************
> *** 1448,1453 ****
> --- 1445,1519 ----
> }
>
> /*
> + * Clone constraint indexes if requested.
> + */
> + if (including_indexes && relation->rd_rel->relhasindex)
> + {
> + List *parent_index_list = RelationGetIndexList(relation);
> + ListCell *parent_index_scan;
> +
> + foreach(parent_index_scan, parent_index_list)
> + {
> + Oid parent_index_oid = lfirst_oid(parent_index_scan);
> + Relation parent_index;
> +
> + parent_index = index_open(parent_index_oid, AccessShareLock);
> +
> + /*
> + * Create new unique or primary key indexes on the child.
> + */
> + if (parent_index->rd_index->indisunique || parent_index->rd_index->indisprimary)
> + {
> + IndexInfo *parent_index_info;
> + Constraint *n = makeNode(Constraint);
> + AttrNumber parent_attno;
> +
> + parent_index_info = BuildIndexInfo(parent_index);
> +
> + if (parent_index->rd_index->indisprimary)
> + {
> + n->contype = CONSTR_PRIMARY;
> + }
> + else
> + {
> + n->contype = CONSTR_UNIQUE;
> + }
> + /* Let DefineIndex name it */
> + n->name = NULL;
> + n->raw_expr = NULL;
> + n->cooked_expr = NULL;
> +
> + /*
> + * Search through the possible index keys, and append
> + * the names of simple columns to the new index key list.
> + */
> + for (parent_attno = 1; parent_attno <= parent_index->rd_att->natts;
> + parent_attno++)
> + {
> + Form_pg_attribute attribute = parent_index->rd_att->attrs[parent_attno - 1];
> + char *attributeName = NameStr(attribute->attname);
> +
> + /*
> + * Ignore dropped columns in the parent.
> + */
> + if (!attribute->attisdropped)
> + n->keys = lappend(n->keys,
> + makeString(attributeName));
> + }
> +
> + /* Add the new index constraint to the create context */
> + cxt->ixconstraints = lappend(cxt->ixconstraints, n);
> +
> + ereport(NOTICE,
> + (errmsg("Index \"%s\" cloned.",
> + RelationGetRelationName(parent_index))));
> + }
> +
> + relation_close(parent_index, AccessShareLock);
> + }
> + }
> +
> + /*
> * Close the parent rel, but keep our AccessShareLock on it until xact
> * commit. That will prevent someone else from deleting or ALTERing the
> * parent before the child is committed.
> Index: doc/src/sgml/ref/create_table.sgml
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/doc/src/sgml/ref/create_table.sgml,v
> retrieving revision 1.107
> diff -c -r1.107 create_table.sgml
> *** doc/src/sgml/ref/create_table.sgml 1 Feb 2007 00:28:18 -0000 1.107
> --- doc/src/sgml/ref/create_table.sgml 7 Mar 2007 01:43:13 -0000
> ***************
> *** 23,29 ****
> CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } ] TABLE <replaceable class="PARAMETER">table_name</replaceable> ( [
> { <replaceable class="PARAMETER">column_name</replaceable> <replaceable class="PARAMETER">data_type</replaceable> [ DEFAULT <replaceable>default_expr</> ] [ <replaceable class="PARAMETER">column_constraint</replaceable> [ ... ] ]
> | <replaceable>table_constraint</replaceable>
> ! | LIKE <replaceable>parent_table</replaceable> [ { INCLUDING | EXCLUDING } { DEFAULTS | CONSTRAINTS } ] ... }
> [, ... ]
> ] )
> [ INHERITS ( <replaceable>parent_table</replaceable> [, ... ] ) ]
> --- 23,29 ----
> CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } ] TABLE <replaceable class="PARAMETER">table_name</replaceable> ( [
> { <replaceable class="PARAMETER">column_name</replaceable> <replaceable class="PARAMETER">data_type</replaceable> [ DEFAULT <replaceable>default_expr</> ] [ <replaceable class="PARAMETER">column_constraint</replaceable> [ ... ] ]
> | <replaceable>table_constraint</replaceable>
> ! | LIKE <replaceable>parent_table</replaceable> [ { INCLUDING | EXCLUDING } { DEFAULTS | CONSTRAINTS | INDEXES } ] ... }
> [, ... ]
> ] )
> [ INHERITS ( <replaceable>parent_table</replaceable> [, ... ] ) ]
> ***************
> *** 237,243 ****
> </varlistentry>
>
> <varlistentry>
> ! <term><literal>LIKE <replaceable>parent_table</replaceable> [ { INCLUDING | EXCLUDING } { DEFAULTS | CONSTRAINTS } ]</literal></term>
> <listitem>
> <para>
> The <literal>LIKE</literal> clause specifies a table from which
> --- 237,243 ----
> </varlistentry>
>
> <varlistentry>
> ! <term><literal>LIKE <replaceable>parent_table</replaceable> [ { INCLUDING | EXCLUDING | INDEXES } { DEFAULTS | CONSTRAINTS } ]</literal></term>
> <listitem>
> <para>
> The <literal>LIKE</literal> clause specifies a table from which
> ***************
> *** 260,269 ****
> <para>
> Not-null constraints are always copied to the new table.
> <literal>CHECK</literal> constraints will only be copied if
> ! <literal>INCLUDING CONSTRAINTS</literal> is specified; other types of
> ! constraints will never be copied. Also, no distinction is made between
> ! column constraints and table constraints &mdash; when constraints are
> ! requested, all check constraints are copied.
> </para>
> <para>
> Note also that unlike <literal>INHERITS</literal>, copied columns and
> --- 260,271 ----
> <para>
> Not-null constraints are always copied to the new table.
> <literal>CHECK</literal> constraints will only be copied if
> ! <literal>INCLUDING CONSTRAINTS</literal> is specified. UNIQUE and
> ! PRIMARY KEY constraints will only be copied if
> ! <literal>INCLUDING INDEXES</literal> is specified. Also, no
> ! distinction is made between column constraints and table constraints
> ! &mdash; when constraints are requested, all check constraints are
> ! copied.
> </para>
> <para>
> Note also that unlike <literal>INHERITS</literal>, copied columns and
> Index: src/test/regress/sql/inherit.sql
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/test/regress/sql/inherit.sql,v
> retrieving revision 1.10
> diff -c -r1.10 inherit.sql
> *** src/test/regress/sql/inherit.sql 27 Jun 2006 03:43:20 -0000 1.10
> --- src/test/regress/sql/inherit.sql 7 Mar 2007 01:43:13 -0000
> ***************
> *** 155,160 ****
> --- 155,164 ----
> INSERT INTO inhg VALUES ('x', 'foo', 'y'); /* fails due to constraint */
> SELECT * FROM inhg; /* Two records with three columns in order x=x, xx=text, y=y */
> DROP TABLE inhg;
> + CREATE TABLE inhg (x text, LIKE inhx INCLUDING INDEXES, y text); /* Copies indexes */
> + INSERT INTO inhg VALUES ('x', 'text', 'y'); /* Succeeds */
> + INSERT INTO inhg VALUES ('x', 'text', 'y'); /* Fails -- Unique constraints are copied */
> + DROP TABLE inhg;
>
>
> -- Test changing the type of inherited columns
> Index: src/test/regress/expected/inherit.out
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/test/regress/expected/inherit.out,v
> retrieving revision 1.20
> diff -c -r1.20 inherit.out
> *** src/test/regress/expected/inherit.out 27 Jun 2006 03:43:20 -0000 1.20
> --- src/test/regress/expected/inherit.out 7 Mar 2007 01:43:14 -0000
> ***************
> *** 633,638 ****
> --- 633,645 ----
> (2 rows)
>
> DROP TABLE inhg;
> + CREATE TABLE inhg (x text, LIKE inhx INCLUDING INDEXES, y text); /* Copies indexes */
> + NOTICE: Index "inhx_pkey" cloned.
> + NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "inhg_pkey" for table "inhg"
> + INSERT INTO inhg VALUES ('x', 'text', 'y'); /* Succeeds */
> + INSERT INTO inhg VALUES ('x', 'text', 'y'); /* Fails -- Unique constraints are copied */
> + ERROR: duplicate key violates unique constraint "inhg_pkey"
> + DROP TABLE inhg;
> -- Test changing the type of inherited columns
> insert into d values('test','one','two','three');
> alter table a alter column aa type integer using bit_length(aa);

>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> message can get through to the mailing list cleanly

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Uh, shouldn't CREATE TABLE LIKE INCLUDING CONSTRAINTS already be including
any indexes in the parent table?

---------------------------------------------------------------------------

Trevor Hardcastle wrote:
> Greetings all,
>
> I wrote this patch about a week ago to introduce myself to coding on
> PostgreSQL. I wasn't entirely sure what the 'INCLUDING INDEXES' option
> was meant to do, so I held off submitting it until I could get around to
> asking about that and tweaking the documentation to reflect the patch.
> By useful coincidence the thread "Auto creation of Partitions" had this
> post in it, which made the intent of the option clear enough for me to
> go ahead and see what people think of this.
>
> Gregory Stark wrote:
> > "NikhilS" <nikkhils(at)gmail(dot)com> writes:
> >
> >
> >> the intention is to use this information from the parent and make it a
> >> property of the child table. This will avoid the step for the user having to
> >> manually specify CREATE INDEX and the likes on all the children tables
> >> one-by-one.
> >>
> >
> > Missed the start of this thread. A while back I had intended to add WITH
> > INDEXES to CREATE TABLE LIKE. That would let you create a tale LIKE parent
> > WITH CONSTRAINTS WITH INDEXES and get pretty much a perfect table ready for
> > adding to the inheritance structure.
> >
> >
> >
> So, that's what this patch does. When a table is created with 'CREATE
> TABLE ... LIKE parent INCLUDING INDEXES' this iterates over the parent
> table indexes looking for constraint indexes, and alters the
> CreateStmtContext to include equivalent indexes on the child table.
>
> This is probably a somewhat naive implementation, being a first attempt.
> I wasn't sure what sort of lock to place on the parent indexes or what
> tablespace the new indexes should be created in. Any help improving it
> would be appreciated.
>
> Thank you,
> -Trevor Hardcastle
>

> Index: src/backend/parser/analyze.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/backend/parser/analyze.c,v
> retrieving revision 1.361
> diff -c -r1.361 analyze.c
> *** src/backend/parser/analyze.c 20 Feb 2007 17:32:16 -0000 1.361
> --- src/backend/parser/analyze.c 7 Mar 2007 01:43:12 -0000
> ***************
> *** 14,19 ****
> --- 14,20 ----
> #include "postgres.h"
>
> #include "access/heapam.h"
> + #include "access/genam.h"
> #include "catalog/heap.h"
> #include "catalog/index.h"
> #include "catalog/namespace.h"
> ***************
> *** 40,45 ****
> --- 41,47 ----
> #include "utils/acl.h"
> #include "utils/builtins.h"
> #include "utils/lsyscache.h"
> + #include "utils/relcache.h"
> #include "utils/syscache.h"
>
>
> ***************
> *** 1345,1355 ****
> }
> }
>
> - if (including_indexes)
> - ereport(ERROR,
> - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> - errmsg("LIKE INCLUDING INDEXES is not implemented")));
> -
> /*
> * Insert the copied attributes into the cxt for the new table
> * definition.
> --- 1347,1352 ----
> ***************
> *** 1448,1453 ****
> --- 1445,1519 ----
> }
>
> /*
> + * Clone constraint indexes if requested.
> + */
> + if (including_indexes && relation->rd_rel->relhasindex)
> + {
> + List *parent_index_list = RelationGetIndexList(relation);
> + ListCell *parent_index_scan;
> +
> + foreach(parent_index_scan, parent_index_list)
> + {
> + Oid parent_index_oid = lfirst_oid(parent_index_scan);
> + Relation parent_index;
> +
> + parent_index = index_open(parent_index_oid, AccessShareLock);
> +
> + /*
> + * Create new unique or primary key indexes on the child.
> + */
> + if (parent_index->rd_index->indisunique || parent_index->rd_index->indisprimary)
> + {
> + IndexInfo *parent_index_info;
> + Constraint *n = makeNode(Constraint);
> + AttrNumber parent_attno;
> +
> + parent_index_info = BuildIndexInfo(parent_index);
> +
> + if (parent_index->rd_index->indisprimary)
> + {
> + n->contype = CONSTR_PRIMARY;
> + }
> + else
> + {
> + n->contype = CONSTR_UNIQUE;
> + }
> + /* Let DefineIndex name it */
> + n->name = NULL;
> + n->raw_expr = NULL;
> + n->cooked_expr = NULL;
> +
> + /*
> + * Search through the possible index keys, and append
> + * the names of simple columns to the new index key list.
> + */
> + for (parent_attno = 1; parent_attno <= parent_index->rd_att->natts;
> + parent_attno++)
> + {
> + Form_pg_attribute attribute = parent_index->rd_att->attrs[parent_attno - 1];
> + char *attributeName = NameStr(attribute->attname);
> +
> + /*
> + * Ignore dropped columns in the parent.
> + */
> + if (!attribute->attisdropped)
> + n->keys = lappend(n->keys,
> + makeString(attributeName));
> + }
> +
> + /* Add the new index constraint to the create context */
> + cxt->ixconstraints = lappend(cxt->ixconstraints, n);
> +
> + ereport(NOTICE,
> + (errmsg("Index \"%s\" cloned.",
> + RelationGetRelationName(parent_index))));
> + }
> +
> + relation_close(parent_index, AccessShareLock);
> + }
> + }
> +
> + /*
> * Close the parent rel, but keep our AccessShareLock on it until xact
> * commit. That will prevent someone else from deleting or ALTERing the
> * parent before the child is committed.
> Index: doc/src/sgml/ref/create_table.sgml
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/doc/src/sgml/ref/create_table.sgml,v
> retrieving revision 1.107
> diff -c -r1.107 create_table.sgml
> *** doc/src/sgml/ref/create_table.sgml 1 Feb 2007 00:28:18 -0000 1.107
> --- doc/src/sgml/ref/create_table.sgml 7 Mar 2007 01:43:13 -0000
> ***************
> *** 23,29 ****
> CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } ] TABLE <replaceable class="PARAMETER">table_name</replaceable> ( [
> { <replaceable class="PARAMETER">column_name</replaceable> <replaceable class="PARAMETER">data_type</replaceable> [ DEFAULT <replaceable>default_expr</> ] [ <replaceable class="PARAMETER">column_constraint</replaceable> [ ... ] ]
> | <replaceable>table_constraint</replaceable>
> ! | LIKE <replaceable>parent_table</replaceable> [ { INCLUDING | EXCLUDING } { DEFAULTS | CONSTRAINTS } ] ... }
> [, ... ]
> ] )
> [ INHERITS ( <replaceable>parent_table</replaceable> [, ... ] ) ]
> --- 23,29 ----
> CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } ] TABLE <replaceable class="PARAMETER">table_name</replaceable> ( [
> { <replaceable class="PARAMETER">column_name</replaceable> <replaceable class="PARAMETER">data_type</replaceable> [ DEFAULT <replaceable>default_expr</> ] [ <replaceable class="PARAMETER">column_constraint</replaceable> [ ... ] ]
> | <replaceable>table_constraint</replaceable>
> ! | LIKE <replaceable>parent_table</replaceable> [ { INCLUDING | EXCLUDING } { DEFAULTS | CONSTRAINTS | INDEXES } ] ... }
> [, ... ]
> ] )
> [ INHERITS ( <replaceable>parent_table</replaceable> [, ... ] ) ]
> ***************
> *** 237,243 ****
> </varlistentry>
>
> <varlistentry>
> ! <term><literal>LIKE <replaceable>parent_table</replaceable> [ { INCLUDING | EXCLUDING } { DEFAULTS | CONSTRAINTS } ]</literal></term>
> <listitem>
> <para>
> The <literal>LIKE</literal> clause specifies a table from which
> --- 237,243 ----
> </varlistentry>
>
> <varlistentry>
> ! <term><literal>LIKE <replaceable>parent_table</replaceable> [ { INCLUDING | EXCLUDING | INDEXES } { DEFAULTS | CONSTRAINTS } ]</literal></term>
> <listitem>
> <para>
> The <literal>LIKE</literal> clause specifies a table from which
> ***************
> *** 260,269 ****
> <para>
> Not-null constraints are always copied to the new table.
> <literal>CHECK</literal> constraints will only be copied if
> ! <literal>INCLUDING CONSTRAINTS</literal> is specified; other types of
> ! constraints will never be copied. Also, no distinction is made between
> ! column constraints and table constraints &mdash; when constraints are
> ! requested, all check constraints are copied.
> </para>
> <para>
> Note also that unlike <literal>INHERITS</literal>, copied columns and
> --- 260,271 ----
> <para>
> Not-null constraints are always copied to the new table.
> <literal>CHECK</literal> constraints will only be copied if
> ! <literal>INCLUDING CONSTRAINTS</literal> is specified. UNIQUE and
> ! PRIMARY KEY constraints will only be copied if
> ! <literal>INCLUDING INDEXES</literal> is specified. Also, no
> ! distinction is made between column constraints and table constraints
> ! &mdash; when constraints are requested, all check constraints are
> ! copied.
> </para>
> <para>
> Note also that unlike <literal>INHERITS</literal>, copied columns and
> Index: src/test/regress/sql/inherit.sql
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/test/regress/sql/inherit.sql,v
> retrieving revision 1.10
> diff -c -r1.10 inherit.sql
> *** src/test/regress/sql/inherit.sql 27 Jun 2006 03:43:20 -0000 1.10
> --- src/test/regress/sql/inherit.sql 7 Mar 2007 01:43:13 -0000
> ***************
> *** 155,160 ****
> --- 155,164 ----
> INSERT INTO inhg VALUES ('x', 'foo', 'y'); /* fails due to constraint */
> SELECT * FROM inhg; /* Two records with three columns in order x=x, xx=text, y=y */
> DROP TABLE inhg;
> + CREATE TABLE inhg (x text, LIKE inhx INCLUDING INDEXES, y text); /* Copies indexes */
> + INSERT INTO inhg VALUES ('x', 'text', 'y'); /* Succeeds */
> + INSERT INTO inhg VALUES ('x', 'text', 'y'); /* Fails -- Unique constraints are copied */
> + DROP TABLE inhg;
>
>
> -- Test changing the type of inherited columns
> Index: src/test/regress/expected/inherit.out
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/test/regress/expected/inherit.out,v
> retrieving revision 1.20
> diff -c -r1.20 inherit.out
> *** src/test/regress/expected/inherit.out 27 Jun 2006 03:43:20 -0000 1.20
> --- src/test/regress/expected/inherit.out 7 Mar 2007 01:43:14 -0000
> ***************
> *** 633,638 ****
> --- 633,645 ----
> (2 rows)
>
> DROP TABLE inhg;
> + CREATE TABLE inhg (x text, LIKE inhx INCLUDING INDEXES, y text); /* Copies indexes */
> + NOTICE: Index "inhx_pkey" cloned.
> + NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "inhg_pkey" for table "inhg"
> + INSERT INTO inhg VALUES ('x', 'text', 'y'); /* Succeeds */
> + INSERT INTO inhg VALUES ('x', 'text', 'y'); /* Fails -- Unique constraints are copied */
> + ERROR: duplicate key violates unique constraint "inhg_pkey"
> + DROP TABLE inhg;
> -- Test changing the type of inherited columns
> insert into d values('test','one','two','three');
> alter table a alter column aa type integer using bit_length(aa);

>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> message can get through to the mailing list cleanly

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

"Bruce Momjian" <bruce(at)momjian(dot)us> writes:

> Uh, shouldn't CREATE TABLE LIKE INCLUDING CONSTRAINTS already be including
> any indexes in the parent table?

You could argue it should for unique indexes since our unique indexes are how
we implement unique constraints. But I see no particular reason to expect it
to copy random other indexes. At least its name doesn't lead one to expect it
to.

I also thought it was sort of strange to have a command that otherwise is just
copying definitions suddenly start building whole new objects. I think I was
thinking it would be a long slow operation but I suppose creating an empty
index isn't really noticeably slow. It could print a NOTICE similar to what's
printed when you create a primary key or unique constraint.

It does mean that users would be unable to create a partition, load data, then
build indexes. Perhaps it would be nice to have an ALTER TABLE foo LIKE bar
INCLUDING CONSTRAINTS as well.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com

Gregory Stark wrote:
> "Bruce Momjian" <bruce(at)momjian(dot)us> writes:
>
> > Uh, shouldn't CREATE TABLE LIKE INCLUDING CONSTRAINTS already be including
> > any indexes in the parent table?
>
> You could argue it should for unique indexes since our unique indexes are how
> we implement unique constraints. But I see no particular reason to expect it
> to copy random other indexes. At least its name doesn't lead one to expect it
> to.
>
> I also thought it was sort of strange to have a command that otherwise is just
> copying definitions suddenly start building whole new objects. I think I was
> thinking it would be a long slow operation but I suppose creating an empty
> index isn't really noticeably slow. It could print a NOTICE similar to what's
> printed when you create a primary key or unique constraint.
>
> It does mean that users would be unable to create a partition, load data, then
> build indexes. Perhaps it would be nice to have an ALTER TABLE foo LIKE bar
> INCLUDING CONSTRAINTS as well.

The patch already _only_ does constraint(unique) indexes:

> So, that's what this patch does. When a table is created with 'CREATE
> TABLE ... LIKE parent INCLUDING INDEXES' this iterates over the parent
> table indexes looking for constraint indexes, and alters the
> CreateStmtContext to include equivalent indexes on the child table.

so I am just suggesting it do that always for INCLUDING CONSTRAINTS,
with a notice as you suggest.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Added to TODO:

o Have WITH CONSTRAINTS also create constraint indexes
http://archives.postgresql.org/pgsql-patches/2007-04/msg00149.php

---------------------------------------------------------------------------

Trevor Hardcastle wrote:
> Greetings all,
>
> I wrote this patch about a week ago to introduce myself to coding on
> PostgreSQL. I wasn't entirely sure what the 'INCLUDING INDEXES' option
> was meant to do, so I held off submitting it until I could get around to
> asking about that and tweaking the documentation to reflect the patch.
> By useful coincidence the thread "Auto creation of Partitions" had this
> post in it, which made the intent of the option clear enough for me to
> go ahead and see what people think of this.
>
> Gregory Stark wrote:
> > "NikhilS" <nikkhils(at)gmail(dot)com> writes:
> >
> >
> >> the intention is to use this information from the parent and make it a
> >> property of the child table. This will avoid the step for the user having to
> >> manually specify CREATE INDEX and the likes on all the children tables
> >> one-by-one.
> >>
> >
> > Missed the start of this thread. A while back I had intended to add WITH
> > INDEXES to CREATE TABLE LIKE. That would let you create a tale LIKE parent
> > WITH CONSTRAINTS WITH INDEXES and get pretty much a perfect table ready for
> > adding to the inheritance structure.
> >
> >
> >
> So, that's what this patch does. When a table is created with 'CREATE
> TABLE ... LIKE parent INCLUDING INDEXES' this iterates over the parent
> table indexes looking for constraint indexes, and alters the
> CreateStmtContext to include equivalent indexes on the child table.
>
> This is probably a somewhat naive implementation, being a first attempt.
> I wasn't sure what sort of lock to place on the parent indexes or what
> tablespace the new indexes should be created in. Any help improving it
> would be appreciated.
>
> Thank you,
> -Trevor Hardcastle
>

> Index: src/backend/parser/analyze.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/backend/parser/analyze.c,v
> retrieving revision 1.361
> diff -c -r1.361 analyze.c
> *** src/backend/parser/analyze.c 20 Feb 2007 17:32:16 -0000 1.361
> --- src/backend/parser/analyze.c 7 Mar 2007 01:43:12 -0000
> ***************
> *** 14,19 ****
> --- 14,20 ----
> #include "postgres.h"
>
> #include "access/heapam.h"
> + #include "access/genam.h"
> #include "catalog/heap.h"
> #include "catalog/index.h"
> #include "catalog/namespace.h"
> ***************
> *** 40,45 ****
> --- 41,47 ----
> #include "utils/acl.h"
> #include "utils/builtins.h"
> #include "utils/lsyscache.h"
> + #include "utils/relcache.h"
> #include "utils/syscache.h"
>
>
> ***************
> *** 1345,1355 ****
> }
> }
>
> - if (including_indexes)
> - ereport(ERROR,
> - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> - errmsg("LIKE INCLUDING INDEXES is not implemented")));
> -
> /*
> * Insert the copied attributes into the cxt for the new table
> * definition.
> --- 1347,1352 ----
> ***************
> *** 1448,1453 ****
> --- 1445,1519 ----
> }
>
> /*
> + * Clone constraint indexes if requested.
> + */
> + if (including_indexes && relation->rd_rel->relhasindex)
> + {
> + List *parent_index_list = RelationGetIndexList(relation);
> + ListCell *parent_index_scan;
> +
> + foreach(parent_index_scan, parent_index_list)
> + {
> + Oid parent_index_oid = lfirst_oid(parent_index_scan);
> + Relation parent_index;
> +
> + parent_index = index_open(parent_index_oid, AccessShareLock);
> +
> + /*
> + * Create new unique or primary key indexes on the child.
> + */
> + if (parent_index->rd_index->indisunique || parent_index->rd_index->indisprimary)
> + {
> + IndexInfo *parent_index_info;
> + Constraint *n = makeNode(Constraint);
> + AttrNumber parent_attno;
> +
> + parent_index_info = BuildIndexInfo(parent_index);
> +
> + if (parent_index->rd_index->indisprimary)
> + {
> + n->contype = CONSTR_PRIMARY;
> + }
> + else
> + {
> + n->contype = CONSTR_UNIQUE;
> + }
> + /* Let DefineIndex name it */
> + n->name = NULL;
> + n->raw_expr = NULL;
> + n->cooked_expr = NULL;
> +
> + /*
> + * Search through the possible index keys, and append
> + * the names of simple columns to the new index key list.
> + */
> + for (parent_attno = 1; parent_attno <= parent_index->rd_att->natts;
> + parent_attno++)
> + {
> + Form_pg_attribute attribute = parent_index->rd_att->attrs[parent_attno - 1];
> + char *attributeName = NameStr(attribute->attname);
> +
> + /*
> + * Ignore dropped columns in the parent.
> + */
> + if (!attribute->attisdropped)
> + n->keys = lappend(n->keys,
> + makeString(attributeName));
> + }
> +
> + /* Add the new index constraint to the create context */
> + cxt->ixconstraints = lappend(cxt->ixconstraints, n);
> +
> + ereport(NOTICE,
> + (errmsg("Index \"%s\" cloned.",
> + RelationGetRelationName(parent_index))));
> + }
> +
> + relation_close(parent_index, AccessShareLock);
> + }
> + }
> +
> + /*
> * Close the parent rel, but keep our AccessShareLock on it until xact
> * commit. That will prevent someone else from deleting or ALTERing the
> * parent before the child is committed.
> Index: doc/src/sgml/ref/create_table.sgml
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/doc/src/sgml/ref/create_table.sgml,v
> retrieving revision 1.107
> diff -c -r1.107 create_table.sgml
> *** doc/src/sgml/ref/create_table.sgml 1 Feb 2007 00:28:18 -0000 1.107
> --- doc/src/sgml/ref/create_table.sgml 7 Mar 2007 01:43:13 -0000
> ***************
> *** 23,29 ****
> CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } ] TABLE <replaceable class="PARAMETER">table_name</replaceable> ( [
> { <replaceable class="PARAMETER">column_name</replaceable> <replaceable class="PARAMETER">data_type</replaceable> [ DEFAULT <replaceable>default_expr</> ] [ <replaceable class="PARAMETER">column_constraint</replaceable> [ ... ] ]
> | <replaceable>table_constraint</replaceable>
> ! | LIKE <replaceable>parent_table</replaceable> [ { INCLUDING | EXCLUDING } { DEFAULTS | CONSTRAINTS } ] ... }
> [, ... ]
> ] )
> [ INHERITS ( <replaceable>parent_table</replaceable> [, ... ] ) ]
> --- 23,29 ----
> CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } ] TABLE <replaceable class="PARAMETER">table_name</replaceable> ( [
> { <replaceable class="PARAMETER">column_name</replaceable> <replaceable class="PARAMETER">data_type</replaceable> [ DEFAULT <replaceable>default_expr</> ] [ <replaceable class="PARAMETER">column_constraint</replaceable> [ ... ] ]
> | <replaceable>table_constraint</replaceable>
> ! | LIKE <replaceable>parent_table</replaceable> [ { INCLUDING | EXCLUDING } { DEFAULTS | CONSTRAINTS | INDEXES } ] ... }
> [, ... ]
> ] )
> [ INHERITS ( <replaceable>parent_table</replaceable> [, ... ] ) ]
> ***************
> *** 237,243 ****
> </varlistentry>
>
> <varlistentry>
> ! <term><literal>LIKE <replaceable>parent_table</replaceable> [ { INCLUDING | EXCLUDING } { DEFAULTS | CONSTRAINTS } ]</literal></term>
> <listitem>
> <para>
> The <literal>LIKE</literal> clause specifies a table from which
> --- 237,243 ----
> </varlistentry>
>
> <varlistentry>
> ! <term><literal>LIKE <replaceable>parent_table</replaceable> [ { INCLUDING | EXCLUDING | INDEXES } { DEFAULTS | CONSTRAINTS } ]</literal></term>
> <listitem>
> <para>
> The <literal>LIKE</literal> clause specifies a table from which
> ***************
> *** 260,269 ****
> <para>
> Not-null constraints are always copied to the new table.
> <literal>CHECK</literal> constraints will only be copied if
> ! <literal>INCLUDING CONSTRAINTS</literal> is specified; other types of
> ! constraints will never be copied. Also, no distinction is made between
> ! column constraints and table constraints &mdash; when constraints are
> ! requested, all check constraints are copied.
> </para>
> <para>
> Note also that unlike <literal>INHERITS</literal>, copied columns and
> --- 260,271 ----
> <para>
> Not-null constraints are always copied to the new table.
> <literal>CHECK</literal> constraints will only be copied if
> ! <literal>INCLUDING CONSTRAINTS</literal> is specified. UNIQUE and
> ! PRIMARY KEY constraints will only be copied if
> ! <literal>INCLUDING INDEXES</literal> is specified. Also, no
> ! distinction is made between column constraints and table constraints
> ! &mdash; when constraints are requested, all check constraints are
> ! copied.
> </para>
> <para>
> Note also that unlike <literal>INHERITS</literal>, copied columns and
> Index: src/test/regress/sql/inherit.sql
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/test/regress/sql/inherit.sql,v
> retrieving revision 1.10
> diff -c -r1.10 inherit.sql
> *** src/test/regress/sql/inherit.sql 27 Jun 2006 03:43:20 -0000 1.10
> --- src/test/regress/sql/inherit.sql 7 Mar 2007 01:43:13 -0000
> ***************
> *** 155,160 ****
> --- 155,164 ----
> INSERT INTO inhg VALUES ('x', 'foo', 'y'); /* fails due to constraint */
> SELECT * FROM inhg; /* Two records with three columns in order x=x, xx=text, y=y */
> DROP TABLE inhg;
> + CREATE TABLE inhg (x text, LIKE inhx INCLUDING INDEXES, y text); /* Copies indexes */
> + INSERT INTO inhg VALUES ('x', 'text', 'y'); /* Succeeds */
> + INSERT INTO inhg VALUES ('x', 'text', 'y'); /* Fails -- Unique constraints are copied */
> + DROP TABLE inhg;
>
>
> -- Test changing the type of inherited columns
> Index: src/test/regress/expected/inherit.out
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/test/regress/expected/inherit.out,v
> retrieving revision 1.20
> diff -c -r1.20 inherit.out
> *** src/test/regress/expected/inherit.out 27 Jun 2006 03:43:20 -0000 1.20
> --- src/test/regress/expected/inherit.out 7 Mar 2007 01:43:14 -0000
> ***************
> *** 633,638 ****
> --- 633,645 ----
> (2 rows)
>
> DROP TABLE inhg;
> + CREATE TABLE inhg (x text, LIKE inhx INCLUDING INDEXES, y text); /* Copies indexes */
> + NOTICE: Index "inhx_pkey" cloned.
> + NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "inhg_pkey" for table "inhg"
> + INSERT INTO inhg VALUES ('x', 'text', 'y'); /* Succeeds */
> + INSERT INTO inhg VALUES ('x', 'text', 'y'); /* Fails -- Unique constraints are copied */
> + ERROR: duplicate key violates unique constraint "inhg_pkey"
> + DROP TABLE inhg;
> -- Test changing the type of inherited columns
> insert into d values('test','one','two','three');
> alter table a alter column aa type integer using bit_length(aa);

>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> message can get through to the mailing list cleanly

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Hi,

On 4/10/07, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>
>
> Added to TODO:
>
> o Have WITH CONSTRAINTS also create constraint indexes
>
> http://archives.postgresql.org/pgsql-patches/2007-04/msg00149.php

Trevor's patch does add unique/primary indexes. This would mean that we have
to remove the syntax support for "INCLUDING INDEXES" and just add code to
the existing WITH CONSTRAINTs code path from his patch.

Is there something else and hence we have the above TODO?

Regards,
Nikhils

--
EnterpriseDB http://www.enterprisedb.com

NikhilS wrote:
> Hi,
>
> On 4/10/07, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> >
> >
> > Added to TODO:
> >
> > o Have WITH CONSTRAINTS also create constraint indexes
> >
> > http://archives.postgresql.org/pgsql-patches/2007-04/msg00149.php
>
>
> Trevor's patch does add unique/primary indexes. This would mean that we have
> to remove the syntax support for "INCLUDING INDEXES" and just add code to
> the existing WITH CONSTRAINTs code path from his patch.

That is all that is required.

> Is there something else and hence we have the above TODO?

If someone wants to work on this item and submit it, we can review it
for 8.3, but if not, it waits until 8.4.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:
> NikhilS wrote:
>
>> Hi,
>>
>> On 4/10/07, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>>
>>> Added to TODO:
>>>
>>> o Have WITH CONSTRAINTS also create constraint indexes
>>>
>>> http://archives.postgresql.org/pgsql-patches/2007-04/msg00149.php
>>>
>> Trevor's patch does add unique/primary indexes. This would mean that we have
>> to remove the syntax support for "INCLUDING INDEXES" and just add code to
>> the existing WITH CONSTRAINTs code path from his patch.
>>
>
> That is all that is required.
>
>
>> Is there something else and hence we have the above TODO?
>>
>
> If someone wants to work on this item and submit it, we can review it
> for 8.3, but if not, it waits until 8.4.
>
>
I've updated my patch to merge the INDEXES behavior implemented into the
CONSTRAINTS option, and restore the current error triggered when you try
to use the INDEXES option. Attached is the updated patch.

I didn't remove the INDEXES syntax, just undocumented it again and put
the error it raised back in. It seems like an implementation of copying
all of the indexes could still use that syntax.

Thank you for all the comments,
-Trevor Hardcastle

Hi Trevor,

> +
> + parent_index_info =
> BuildIndexInfo(parent_index);

The above is not used anywhere else in the code and seems redundant.

>
> +
> + ereport(NOTICE,
> + (errmsg("Index
> \"%s\" cloned.",
> +
> RelationGetRelationName(parent_index))));

DefineIndex will give out a message anyways for unique/primary keys. The
above seems additional to it.

Regards,
Nikhils

--
EnterpriseDB http://www.enterprisedb.com

NikhilS wrote:
> Hi Trevor,
>
>
> +
> + parent_index_info =
> BuildIndexInfo(parent_index);
>
>
> The above is not used anywhere else in the code and seems redundant.
Yep, pulled that out.
>
> +
> + ereport(NOTICE,
> +
> (errmsg("Index \"%s\" cloned.",
> +
> RelationGetRelationName(parent_index))));
>
>
> DefineIndex will give out a message anyways for unique/primary keys.
> The above seems additional to it.
The original reason for this was the support for copying all indexes,
but it doesn't make much sense now. I've pulled it too.

Thanks for pointing those out. An updated patch is attached.

-Trevor Hardcastle

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.

---------------------------------------------------------------------------

Trevor Hardcastle wrote:
> NikhilS wrote:
> > Hi Trevor,
> >
> >
> > +
> > + parent_index_info =
> > BuildIndexInfo(parent_index);
> >
> >
> > The above is not used anywhere else in the code and seems redundant.
> Yep, pulled that out.
> >
> > +
> > + ereport(NOTICE,
> > +
> > (errmsg("Index \"%s\" cloned.",
> > +
> > RelationGetRelationName(parent_index))));
> >
> >
> > DefineIndex will give out a message anyways for unique/primary keys.
> > The above seems additional to it.
> The original reason for this was the support for copying all indexes,
> but it doesn't make much sense now. I've pulled it too.
>
> Thanks for pointing those out. An updated patch is attached.
>
> -Trevor Hardcastle
>

> Index: src/backend/parser/analyze.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/backend/parser/analyze.c,v
> retrieving revision 1.362
> diff -c -r1.362 analyze.c
> *** src/backend/parser/analyze.c 13 Mar 2007 00:33:41 -0000 1.362
> --- src/backend/parser/analyze.c 13 Apr 2007 16:41:46 -0000
> ***************
> *** 28,33 ****
> --- 28,34 ----
> #include "postgres.h"
>
> #include "access/heapam.h"
> + #include "access/genam.h"
> #include "catalog/heap.h"
> #include "catalog/index.h"
> #include "catalog/namespace.h"
> ***************
> *** 54,59 ****
> --- 55,61 ----
> #include "utils/acl.h"
> #include "utils/builtins.h"
> #include "utils/lsyscache.h"
> + #include "utils/relcache.h"
> #include "utils/syscache.h"
>
>
> ***************
> *** 1331,1338 ****
> }
>
> /*
> ! * Copy CHECK constraints if requested, being careful to adjust
> ! * attribute numbers
> */
> if (including_constraints && tupleDesc->constr)
> {
> --- 1333,1340 ----
> }
>
> /*
> ! * Copy CHECK based constraints if requested, being careful to adjust
> ! * attribute numbers. Also duplicate unique index constraints.
> */
> if (including_constraints && tupleDesc->constr)
> {
> ***************
> *** 1355,1360 ****
> --- 1357,1424 ----
> n->indexspace = NULL;
> cxt->ckconstraints = lappend(cxt->ckconstraints, (Node *) n);
> }
> +
> + /*
> + * Clone constraint indexes if any exist.
> + */
> + if (relation->rd_rel->relhasindex)
> + {
> + List *parent_index_list = RelationGetIndexList(relation);
> + ListCell *parent_index_scan;
> +
> + foreach(parent_index_scan, parent_index_list)
> + {
> + Oid parent_index_oid = lfirst_oid(parent_index_scan);
> + Relation parent_index;
> +
> + parent_index = index_open(parent_index_oid, AccessShareLock);
> +
> + /*
> + * Create new unique or primary key indexes on the child.
> + */
> + if (parent_index->rd_index->indisunique || parent_index->rd_index->indisprimary)
> + {
> + Constraint *n = makeNode(Constraint);
> + AttrNumber parent_attno;
> +
> + if (parent_index->rd_index->indisprimary)
> + {
> + n->contype = CONSTR_PRIMARY;
> + }
> + else
> + {
> + n->contype = CONSTR_UNIQUE;
> + }
> + /* Let DefineIndex name it */
> + n->name = NULL;
> + n->raw_expr = NULL;
> + n->cooked_expr = NULL;
> +
> + /*
> + * Search through the possible index keys, and append
> + * the names of simple columns to the new index key list.
> + */
> + for (parent_attno = 1; parent_attno <= parent_index->rd_att->natts;
> + parent_attno++)
> + {
> + Form_pg_attribute attribute = parent_index->rd_att->attrs[parent_attno - 1];
> + char *attributeName = NameStr(attribute->attname);
> +
> + /*
> + * Ignore dropped columns in the parent.
> + */
> + if (!attribute->attisdropped)
> + n->keys = lappend(n->keys,
> + makeString(attributeName));
> + }
> +
> + /* Add the new index constraint to the create context */
> + cxt->ixconstraints = lappend(cxt->ixconstraints, n);
> + }
> +
> + relation_close(parent_index, AccessShareLock);
> + }
> + }
> }
>
> /*
> Index: src/test/regress/sql/inherit.sql
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/test/regress/sql/inherit.sql,v
> retrieving revision 1.10
> diff -c -r1.10 inherit.sql
> *** src/test/regress/sql/inherit.sql 27 Jun 2006 03:43:20 -0000 1.10
> --- src/test/regress/sql/inherit.sql 13 Apr 2007 16:41:46 -0000
> ***************
> *** 151,160 ****
> DROP TABLE inhg;
> CREATE TABLE inhg (x text, LIKE inhx INCLUDING CONSTRAINTS, y text); /* Copies constraints */
> INSERT INTO inhg VALUES ('x', 'text', 'y'); /* Succeeds */
> ! INSERT INTO inhg VALUES ('x', 'text', 'y'); /* Succeeds -- Unique constraints not copied */
> INSERT INTO inhg VALUES ('x', 'foo', 'y'); /* fails due to constraint */
> SELECT * FROM inhg; /* Two records with three columns in order x=x, xx=text, y=y */
> DROP TABLE inhg;
>
>
> -- Test changing the type of inherited columns
> --- 151,161 ----
> DROP TABLE inhg;
> CREATE TABLE inhg (x text, LIKE inhx INCLUDING CONSTRAINTS, y text); /* Copies constraints */
> INSERT INTO inhg VALUES ('x', 'text', 'y'); /* Succeeds */
> ! INSERT INTO inhg VALUES ('x', 'text', 'y'); /* Fails -- Unique constraints copied */
> INSERT INTO inhg VALUES ('x', 'foo', 'y'); /* fails due to constraint */
> SELECT * FROM inhg; /* Two records with three columns in order x=x, xx=text, y=y */
> DROP TABLE inhg;
> + CREATE TABLE inhg (x text, LIKE inhx INCLUDING INDEXES, y text); /* Unimplemented */
>
>
> -- Test changing the type of inherited columns
> Index: src/test/regress/expected/inherit.out
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/test/regress/expected/inherit.out,v
> retrieving revision 1.20
> diff -c -r1.20 inherit.out
> *** src/test/regress/expected/inherit.out 27 Jun 2006 03:43:20 -0000 1.20
> --- src/test/regress/expected/inherit.out 13 Apr 2007 16:41:46 -0000
> ***************
> *** 621,638 ****
> INSERT INTO inhg VALUES ('foo');
> DROP TABLE inhg;
> CREATE TABLE inhg (x text, LIKE inhx INCLUDING CONSTRAINTS, y text); /* Copies constraints */
> INSERT INTO inhg VALUES ('x', 'text', 'y'); /* Succeeds */
> ! INSERT INTO inhg VALUES ('x', 'text', 'y'); /* Succeeds -- Unique constraints not copied */
> INSERT INTO inhg VALUES ('x', 'foo', 'y'); /* fails due to constraint */
> ERROR: new row for relation "inhg" violates check constraint "foo"
> SELECT * FROM inhg; /* Two records with three columns in order x=x, xx=text, y=y */
> x | xx | y
> ---+------+---
> x | text | y
> ! x | text | y
> ! (2 rows)
>
> DROP TABLE inhg;
> -- Test changing the type of inherited columns
> insert into d values('test','one','two','three');
> alter table a alter column aa type integer using bit_length(aa);
> --- 621,641 ----
> INSERT INTO inhg VALUES ('foo');
> DROP TABLE inhg;
> CREATE TABLE inhg (x text, LIKE inhx INCLUDING CONSTRAINTS, y text); /* Copies constraints */
> + NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "inhg_pkey" for table "inhg"
> INSERT INTO inhg VALUES ('x', 'text', 'y'); /* Succeeds */
> ! INSERT INTO inhg VALUES ('x', 'text', 'y'); /* Fails -- Unique constraints copied */
> ! ERROR: duplicate key violates unique constraint "inhg_pkey"
> INSERT INTO inhg VALUES ('x', 'foo', 'y'); /* fails due to constraint */
> ERROR: new row for relation "inhg" violates check constraint "foo"
> SELECT * FROM inhg; /* Two records with three columns in order x=x, xx=text, y=y */
> x | xx | y
> ---+------+---
> x | text | y
> ! (1 row)
>
> DROP TABLE inhg;
> + CREATE TABLE inhg (x text, LIKE inhx INCLUDING INDEXES, y text); /* Unimplemented */
> + ERROR: LIKE INCLUDING INDEXES is not implemented
> -- Test changing the type of inherited columns
> insert into d values('test','one','two','three');
> alter table a alter column aa type integer using bit_length(aa);
> Index: doc/src/sgml/ref/create_table.sgml
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/doc/src/sgml/ref/create_table.sgml,v
> retrieving revision 1.107
> diff -c -r1.107 create_table.sgml
> *** doc/src/sgml/ref/create_table.sgml 1 Feb 2007 00:28:18 -0000 1.107
> --- doc/src/sgml/ref/create_table.sgml 13 Apr 2007 16:41:46 -0000
> ***************
> *** 259,269 ****
> </para>
> <para>
> Not-null constraints are always copied to the new table.
> ! <literal>CHECK</literal> constraints will only be copied if
> ! <literal>INCLUDING CONSTRAINTS</literal> is specified; other types of
> ! constraints will never be copied. Also, no distinction is made between
> ! column constraints and table constraints &mdash; when constraints are
> ! requested, all check constraints are copied.
> </para>
> <para>
> Note also that unlike <literal>INHERITS</literal>, copied columns and
> --- 259,268 ----
> </para>
> <para>
> Not-null constraints are always copied to the new table.
> ! <literal>CHECK, UNIQUE, and PRIMARY KEY</literal> constraints will only
> ! be copied if <literal>INCLUDING CONSTRAINTS</literal> is specified. Also,
> ! no distinction is made between column constraints and table constraints
> ! &mdash; when constraints are requested, all check constraints are copied.
> </para>
> <para>
> Note also that unlike <literal>INHERITS</literal>, copied columns and

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

This patch needs more work. How carefully did you test it?

* the patch failed to copy index constraints if there was not also at
least one CHECK constraint defined on the table

* the patch is broken for expressional indexes, and silently omits
copying the predicate that may be associated with an index. It also
doesn't copy the index's amoptions (WITH clause), or the NULLS
FIRST/etc. options that may be associated with any of the index's
columns.

In other words, copying column names does not suffice to duplicate the
index constraint. Perhaps the right fix is to implement support for
INCLUDING INDEXES, and then use that code to copy the definition of any
constraint indexes in INCLUDING INDEXES?

* should we copy invalid indexes? I suppose there's nothing wrong with
copying them...

* we should probably hold the AccessShareLock on copied indexes till end
of xact, per the comments at the end of transformInhRelation()

* index_open() should be matched with index_close(), not
relation_close(). (In the current implementation, index_close() and
relation_close() happen to be identical, so it still worked.)

I've attached a revised version of the patch, but I didn't fix the
second or third bullets in the list.

-Neil

Hi Neil,

>
> * the patch is broken for expressional indexes, and silently omits
> copying the predicate that may be associated with an index. It also
> doesn't copy the index's amoptions (WITH clause), or the NULLS
> FIRST/etc. options that may be associated with any of the index's
> columns.

Since this patch is only supposed to copy unique/primary indexes, I dont
think we will ever have predicates associated to such indexes?

Regards,
Nikhils

--
EnterpriseDB http://www.enterprisedb.com

NikhilS <nikkhils(at)gmail(dot)com> writes:
> Hi Neil,
>> * the patch is broken for expressional indexes, and silently omits
>> copying the predicate that may be associated with an index.

> Since this patch is only supposed to copy unique/primary indexes, I dont
> think we will ever have predicates associated to such indexes?

Huh? I would expect a clause "INCLUDING INDEXES" to mean copying *all*
indexes. A clause "INCLUDING CONSTRAINTS" would reasonably act as you
suggest, ie copy only indexes derived from SQL constraint clauses.

regards, tom lane

On Wed, 2007-02-05 at 17:09 +0530, NikhilS wrote:
> Since this patch is only supposed to copy unique/primary indexes, I
> dont think we will ever have predicates associated to such indexes?

Nope:

neilc=# create table t1 (a int, b int);
CREATE TABLE
neilc=# create unique index t1_a_idx on t1 ((a + b)) where (a > 5);
CREATE INDEX

-Neil

Hi,

On 5/3/07, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> NikhilS <nikkhils(at)gmail(dot)com> writes:
> > Hi Neil,
> >> * the patch is broken for expressional indexes, and silently omits
> >> copying the predicate that may be associated with an index.
>
> > Since this patch is only supposed to copy unique/primary indexes, I dont
> > think we will ever have predicates associated to such indexes?
>
> Huh? I would expect a clause "INCLUDING INDEXES" to mean copying *all*
> indexes. A clause "INCLUDING CONSTRAINTS" would reasonably act as you
> suggest, ie copy only indexes derived from SQL constraint clauses.

But this patch is not for the "INCLUDING INDEXES" case. As mentioned by
Bruce earlier in this thread, this patch is for the following TODO:

o Have WITH CONSTRAINTS also create constraint indexes
http://archives.postgresql.org/pgsql-patches/2007-04/msg00149.php

Regards,
Nikhils

regards, tom lane
>

--
EnterpriseDB http://www.enterprisedb.com

NikhilS <nikkhils(at)gmail(dot)com> writes:
> But this patch is not for the "INCLUDING INDEXES" case.

Pardon me for being misled by the thread title :-(

regards, tom lane

On Fri, 2007-27-04 at 18:59 -0400, Neil Conway wrote:
> This patch needs more work.

Has a revised version of this patch been submitted?

-Neil

Hi,

>
> Nope:
>
> neilc=# create table t1 (a int, b int);
> CREATE TABLE
> neilc=# create unique index t1_a_idx on t1 ((a + b)) where (a > 5);
> CREATE INDEX

I just now realized that even though we allow the above. We do not allow:

pg=# create table t1 (a int, b int, unique(a+b));

nor the where clause syntax.

Any specific reason for this behaviour?

If we want to pass such kinds of expr, predicate based constraints via the
"LIKE .. INCLUDING CONSTRAINTS" statements, transformIndexConstraints as
well as the Constraint structure might need modifications.

Regards,
Nikhils

--
EnterpriseDB http://www.enterprisedb.com

NikhilS <nikkhils(at)gmail(dot)com> writes:
> I just now realized that even though we allow the above. We do not allow:

> pg=# create table t1 (a int, b int, unique(a+b));

> Any specific reason for this behaviour?

It'd be contrary to SQL spec. The UNIQUE constraint takes a list of
column names, full stop.

regards, tom lane

"Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:

> NikhilS <nikkhils(at)gmail(dot)com> writes:
>> I just now realized that even though we allow the above. We do not allow:
>
>> pg=# create table t1 (a int, b int, unique(a+b));
>
>> Any specific reason for this behaviour?
>
> It'd be contrary to SQL spec. The UNIQUE constraint takes a list of
> column names, full stop.

Does the SQL spec actually specify what happens if you provide an
non-compliant table definition like this?

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com

Gregory Stark <stark(at)enterprisedb(dot)com> writes:
> "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:
>> It'd be contrary to SQL spec. The UNIQUE constraint takes a list of
>> column names, full stop.

> Does the SQL spec actually specify what happens if you provide an
> non-compliant table definition like this?

It does not. We could accept expressions there, and pray that the SQL
committee never extends the spec syntax in a direction incompatible with
that. That seems like a pretty risky thing to do though.

[ thinks... ] I think actually the prior discussions about this have
suggested importing our CREATE INDEX argument-list syntax into the
UNIQUE/PRIMARY KEY syntax, lock stock and barrel. Between the optional
opclass and sort direction and the slightly klugy requirement to
parenthesize expressions, that seemed clearly at huge risk of putting us
behind the compatibility eight-ball. You could make an argument that
the only plausible way that the committee would extend the syntax would
be to allow plain expressions instead of column names, and thus that
allowing that but not our other extensions would be safe. You might be
right about that ... or not. The fact that the committee still hasn't
made this seemingly-obvious extension makes me wonder if they have plans
we don't know about. There are certainly a ton of bells and whistles
in SQL2003 that're far less useful than this would be.

In any case there will always be features in CREATE INDEX that we
daren't drop into the standard constraint syntax, so the argument
for importing just this one seems a bit weak.

BTW, has anyone checked to see if Oracle and/or DB2 allow expressions
here? I think it's unlikely the committee would do anything to break
the standards compliance of those guys, so we might be safe in following
their lead if there is one.

regards, tom lane

I wrote:
> Gregory Stark <stark(at)enterprisedb(dot)com> writes:
>> Does the SQL spec actually specify what happens if you provide an
>> non-compliant table definition like this?

> It does not. We could accept expressions there, and pray that the SQL
> committee never extends the spec syntax in a direction incompatible with
> that. That seems like a pretty risky thing to do though.

[ remembering previous discussions more clearly... ] Actually there
is a concrete problem here: unique constraints are supposed to be
represented in the information_schema views, and there is no
spec-compliant way to do that for a constraint on something other than
a column. We'd have to guess at what the SQL committee would do about
that, and the odds of guessing exactly right don't seem encouraging.

regards, tom lane

On Mon, 2007-14-05 at 22:58 -0400, Neil Conway wrote:
> Has a revised version of this patch been submitted?

In the absence of a revised patch, I can finish the feature myself, but
I won't get the free cycles until after PGCon. I can commit to getting
it done before the end of May, or else we can just push this to 8.4.

-Neil

Neil Conway wrote:
> On Mon, 2007-14-05 at 22:58 -0400, Neil Conway wrote:
> > Has a revised version of this patch been submitted?
>
> In the absence of a revised patch, I can finish the feature myself, but
> I won't get the free cycles until after PGCon. I can commit to getting
> it done before the end of May, or else we can just push this to 8.4.

We will keep it in the queue then. We can always push it for 8.4 if it
if we are near beta.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Hi,

> [ remembering previous discussions more clearly... ] Actually there
> is a concrete problem here: unique constraints are supposed to be
> represented in the information_schema views, and there is no
> spec-compliant way to do that for a constraint on something other than
> a column. We'd have to guess at what the SQL committee would do about
> that, and the odds of guessing exactly right don't seem encouraging.

Considering that a unique index is a unique constraint, then isn't allowing
expressional unique indexes contradictory to the above?

It seems that "CREATE UNIQUE INDEX" currently does not pass isconstraint as
true to DefineIndex, otherwise index_create() would have cribbed with:

"constraints cannot have index expressions" error.

Since this patch is going to consider creating unique/primary indexes
assuming them to be constraints, IMHO we should be uniform about unique
constraints semantics.

That might mean that we only support expressionless, non-predicate indexes
via "INCLUDING CONSTRAINTS"?

Regards,
Nikhils

--
EnterpriseDB http://www.enterprisedb.com

NikhilS <nikkhils(at)gmail(dot)com> writes:
>> [ remembering previous discussions more clearly... ] Actually there
>> is a concrete problem here: unique constraints are supposed to be
>> represented in the information_schema views, and there is no
>> spec-compliant way to do that for a constraint on something other than
>> a column. We'd have to guess at what the SQL committee would do about
>> that, and the odds of guessing exactly right don't seem encouraging.

> Considering that a unique index is a unique constraint,

No, it isn't. You are confusing definition and implementation. The
spec requires us to do X, Y, and Z in response to the unique-constraint
syntax. It says nothing about what CREATE INDEX does.

> Since this patch is going to consider creating unique/primary indexes
> assuming them to be constraints,

If it does that it will be rejected. There is a difference here and
that difference has to be maintained.

The correct way to think about this is that a pg_constraint entry of
type "unique" or "primary key" has an associated index that is part of
its implementation (and therefore has an "internal" dependency on the
constraint). But they are far from being the same thing.

regression=# create table foo (f1 int unique);
NOTICE: CREATE TABLE / UNIQUE will create implicit index "foo_f1_key" for table "foo"
CREATE TABLE
regression=# drop index foo_f1_key;
ERROR: cannot drop index foo_f1_key because constraint foo_f1_key on table foo requires it
HINT: You can drop constraint foo_f1_key on table foo instead.
regression=# alter table foo drop constraint foo_f1_key;
ALTER TABLE
regression=#

regards, tom lane

Hi,

> > Since this patch is going to consider creating unique/primary indexes
> > assuming them to be constraints,
>
> If it does that it will be rejected. There is a difference here and
> that difference has to be maintained.
>
> The correct way to think about this is that a pg_constraint entry of
> type "unique" or "primary key" has an associated index that is part of
> its implementation (and therefore has an "internal" dependency on the
> constraint). But they are far from being the same thing.

Thanks Tom, I understand the difference now. I have a working patch and will
send it to Neil for review tommorrow.

Regards,
Nikhils

--
EnterpriseDB http://www.enterprisedb.com

Hi Neil,

On 5/18/07, Neil Conway <neilc(at)samurai(dot)com> wrote:
>
> On Mon, 2007-14-05 at 22:58 -0400, Neil Conway wrote:
> > Has a revised version of this patch been submitted?
>
> In the absence of a revised patch, I can finish the feature myself, but
> I won't get the free cycles until after PGCon. I can commit to getting
> it done before the end of May, or else we can just push this to 8.4.

I had spent some time on this earlier so decided to complete and send the
patch to you for review. This patch supports copying of expressions,
predicates, opclass, amorder, reloptions etc. The test case also contains
some more additions with this patch. Please let me know if there are any
issues.

Also, if this patch is acceptable, I think the mechanism provided here can
be used to support "INCLUDING INDEXES" case easily too.

Regards,
Nikhils
--
EnterpriseDB http://www.enterprisedb.com

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.

---------------------------------------------------------------------------

NikhilS wrote:
> Hi Neil,
>
> On 5/18/07, Neil Conway <neilc(at)samurai(dot)com> wrote:
> >
> > On Mon, 2007-14-05 at 22:58 -0400, Neil Conway wrote:
> > > Has a revised version of this patch been submitted?
> >
> > In the absence of a revised patch, I can finish the feature myself, but
> > I won't get the free cycles until after PGCon. I can commit to getting
> > it done before the end of May, or else we can just push this to 8.4.
>
>
> I had spent some time on this earlier so decided to complete and send the
> patch to you for review. This patch supports copying of expressions,
> predicates, opclass, amorder, reloptions etc. The test case also contains
> some more additions with this patch. Please let me know if there are any
> issues.
>
> Also, if this patch is acceptable, I think the mechanism provided here can
> be used to support "INCLUDING INDEXES" case easily too.
>
> Regards,
> Nikhils
> --
> EnterpriseDB http://www.enterprisedb.com

[ Attachment, skipping... ]

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian escribió:
>
> Your patch has been added to the PostgreSQL unapplied patches list at:
>
> http://momjian.postgresql.org/cgi-bin/pgpatches
>
> It will be applied as soon as one of the PostgreSQL committers reviews
> and approves it.

I noticed that this patch uses names for some things (for example it
gets the name of the access method), and then builds a IndexStmt which
contains the name. I don't think this is a good idea. I think what
should happen here is that the function to build indexes should be split
in two: one to resolve the names and fill a structure with Oids of
things, and another to get that structure and actually build the index
or constraint. For example look into how GrantStmt is turned into
InternalGrant, and the stuff in aclchk.c to work with that.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Hi,

>
> I noticed that this patch uses names for some things (for example it
> gets the name of the access method), and then builds a IndexStmt which
> contains the name. I don't think this is a good idea. I think what
> should happen here is that the function to build indexes should be split
> in two: one to resolve the names and fill a structure with Oids of
> things, and another to get that structure and actually build the index
> or constraint. For example look into how GrantStmt is turned into
> InternalGrant, and the stuff in aclchk.c to work with that.

The index creation happens after the new table (which is LIKE the parent)
has been created, by appending the cxt.alist information to "extras_after".
The entry point for the index creation is thus via ProcessUtility which
expects an IndexStmt structure. That is why the current patch does all the
Oid to name mapping exercise to populate the relevant fields in IndexStmt
some of which are char pointers. The internal DefineIndex function also
expects most of the fields to be in IndexStmt like format.

If we want to follow the above suggestion, as I understand it, we might have
to devise a new structure to contain Oids and make ProcessUtility accept a
new nodeTag. We will also not be able to use existing Index definition
functions and this will lead to more coding IMHO. Do we want to go down this
path? Or is there something else that has been suggested above and that I am
missing completely?

Please let me know.
Regards,
Nikhils
--
EnterpriseDB http://www.enterprisedb.com

Hi,

> The index creation happens after the new table (which is LIKE the parent)
> has been created, by appending the cxt.alist information to
> "extras_after". The entry point for the index creation is thus via
> ProcessUtility which expects an IndexStmt structure. That is why the current
> patch does all the Oid to name mapping exercise to populate the relevant
> fields in IndexStmt some of which are char pointers. The internal
> DefineIndex function also expects most of the fields to be in IndexStmt like
> format.
>
> If we want to follow the above suggestion, as I understand it, we might
> have to devise a new structure to contain Oids and make ProcessUtility
> accept a new nodeTag. We will also not be able to use existing Index
> definition functions and this will lead to more coding IMHO. Do we want to
> go down this path? Or is there something else that has been suggested above
> and that I am missing completely?

OTOH, we can populate a new structure with the relevant Oids, IndexInfo
information from parent relation indexes and call index_create directly
from within ProcessUtility. Guess, it should be cleaner than the current
approach.

Regards,
Nikhils
--
EnterpriseDB http://www.enterprisedb.com

Hi,

On 5/23/07, NikhilS <nikkhils(at)gmail(dot)com> wrote:
>
> Hi,
>
> > The index creation happens after the new table (which is LIKE the
> > parent) has been created, by appending the cxt.alist information to
> > "extras_after". The entry point for the index creation is thus via
> > ProcessUtility which expects an IndexStmt structure. That is why the current
> > patch does all the Oid to name mapping exercise to populate the relevant
> > fields in IndexStmt some of which are char pointers. The internal
> > DefineIndex function also expects most of the fields to be in IndexStmt like
> > format.
> >
> > If we want to follow the above suggestion, as I understand it, we might
> > have to devise a new structure to contain Oids and make ProcessUtility
> > accept a new nodeTag. We will also not be able to use existing Index
> > definition functions and this will lead to more coding IMHO. Do we want to
> > go down this path? Or is there something else that has been suggested above
> > and that I am missing completely?
>
>
> OTOH, we can populate a new structure with the relevant Oids, IndexInfo
> information from parent relation indexes and call index_create directly
> from within ProcessUtility. Guess, it should be cleaner than the current
> approach.
>
Sorry for the barrage of emails. But as I looked closely at the current
patch there are only 2 fields (accessMethod and tableSpace) in IndexStmt
structure that we populate by doing the conversion from OIDs to name. For
the other fields, the current transformations will remain.

If so, I think we can introduce 2 Oid fields in the IndexStmt structure and
store the Oids there. In DefineIndex we can use these Oids if they are not
invalid.

IMHO, all this is less work and the bulk of the changes remain localized in
mostly one or two functions as in the current patch.

Comments?

Regards,
Nikhils
--
EnterpriseDB http://www.enterprisedb.com

NikhilS escribió:

> Sorry for the barrage of emails. But as I looked closely at the current
> patch there are only 2 fields (accessMethod and tableSpace) in IndexStmt
> structure that we populate by doing the conversion from OIDs to name. For
> the other fields, the current transformations will remain.
>
> If so, I think we can introduce 2 Oid fields in the IndexStmt structure and
> store the Oids there. In DefineIndex we can use these Oids if they are not
> invalid.

Sounds reasonable. This is what we do for example in VacuumStmt. Make
sure that the OIDs are set to Invalid in the parser.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

NikhilS <nikkhils(at)gmail(dot)com> writes:
> If so, I think we can introduce 2 Oid fields in the IndexStmt structure and
> store the Oids there. In DefineIndex we can use these Oids if they are not
> invalid.

I think this is just make-work that causes the patch to complicate parts
of the system it didn't need to touch. The original suggestion was to
actively refactor existing code, which might or might not have been
worthwhile. But this isn't an appropriate substitute --- it's just
making the API uglier for no particular benefit.

regards, tom lane

Hi,

On 5/23/07, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> NikhilS <nikkhils(at)gmail(dot)com> writes:
> > If so, I think we can introduce 2 Oid fields in the IndexStmt structure
> and
> > store the Oids there. In DefineIndex we can use these Oids if they are
> not
> > invalid.
>
> I think this is just make-work that causes the patch to complicate parts
> of the system it didn't need to touch. The original suggestion was to
> actively refactor existing code, which might or might not have been
> worthwhile. But this isn't an appropriate substitute --- it's just
> making the API uglier for no particular benefit.

I agree this will unnecessary add arguments to the DefineIndex API. If we
stick to the patch's earlier way of converting the Oid to names for just
these 2 arguments, we can avoid this IMO.

Considering that we will be generating this information from existing valid
index information, I think converting the Oids to names is safe enough.
Alvaro, do you think we should stick to the existing patch mechanism then
considering that it avoids polluting the API?

Regards,
Nikhils
--
EnterpriseDB http://www.enterprisedb.com

NikhilS escribió:

> On 5/23/07, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >
> >NikhilS <nikkhils(at)gmail(dot)com> writes:
> >> If so, I think we can introduce 2 Oid fields in the IndexStmt
> >> structure and store the Oids there. In DefineIndex we can use these
> >> Oids if they are not invalid.
> >
> >I think this is just make-work that causes the patch to complicate parts
> >of the system it didn't need to touch. The original suggestion was to
> >actively refactor existing code, which might or might not have been
> >worthwhile. But this isn't an appropriate substitute --- it's just
> >making the API uglier for no particular benefit.
>
> I agree this will unnecessary add arguments to the DefineIndex API. If we
> stick to the patch's earlier way of converting the Oid to names for just
> these 2 arguments, we can avoid this IMO.
>
> Considering that we will be generating this information from existing valid
> index information, I think converting the Oids to names is safe enough.
> Alvaro, do you think we should stick to the existing patch mechanism then
> considering that it avoids polluting the API?

Not sure. Is it possible that the schema is renamed while the operation
is being executed? If it's not then this not a problem at all so the
existing patch is fine.

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> Not sure. Is it possible that the schema is renamed while the operation
> is being executed? If it's not then this not a problem at all so the
> existing patch is fine.

There are hazards of that type in CREATE TABLE right now; it's hardly
fair to hold LIKE INCLUDING INDEXES to a higher standard.

regards, tom lane

Hi,

> I agree this will unnecessary add arguments to the DefineIndex API. If we
> > stick to the patch's earlier way of converting the Oid to names for just
> > these 2 arguments, we can avoid this IMO.
> >
> > Considering that we will be generating this information from existing
> valid
> > index information, I think converting the Oids to names is safe enough.
> > Alvaro, do you think we should stick to the existing patch mechanism
> then
> > considering that it avoids polluting the API?
>
> Not sure. Is it possible that the schema is renamed while the operation
> is being executed? If it's not then this not a problem at all so the
> existing patch is fine.

I doubt if accessMethod name will change. The tableSpace name can change,
but the possibility is no worse to doing a [CREATE TABLE table_name ...
TABLESPACE tablespace]. So this should be reasonably ok.

Regards,
Nikhils
--
EnterpriseDB http://www.enterprisedb.com

OK, so the patch is ready to be applied?

---------------------------------------------------------------------------

NikhilS wrote:
> Hi,
>
> > I agree this will unnecessary add arguments to the DefineIndex API. If we
> > > stick to the patch's earlier way of converting the Oid to names for just
> > > these 2 arguments, we can avoid this IMO.
> > >
> > > Considering that we will be generating this information from existing
> > valid
> > > index information, I think converting the Oids to names is safe enough.
> > > Alvaro, do you think we should stick to the existing patch mechanism
> > then
> > > considering that it avoids polluting the API?
> >
> > Not sure. Is it possible that the schema is renamed while the operation
> > is being executed? If it's not then this not a problem at all so the
> > existing patch is fine.
>
>
> I doubt if accessMethod name will change. The tableSpace name can change,
> but the possibility is no worse to doing a [CREATE TABLE table_name ...
> TABLESPACE tablespace]. So this should be reasonably ok.
>
> Regards,
> Nikhils
> --
> EnterpriseDB http://www.enterprisedb.com

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> OK, so the patch is ready to be applied?

Neil's still reviewing it, last I heard.

regards, tom lane

On Mon, 2007-21-05 at 12:23 +0530, NikhilS wrote:
> I had spent some time on this earlier so decided to complete and send
> the patch to you for review. This patch supports copying of
> expressions, predicates, opclass, amorder, reloptions etc. The test
> case also contains some more additions with this patch. Please let me
> know if there are any issues.

Attached is a revised version of this patch. Note that this pattern is
always unsafe:

ht_am = SearchSysCache(AMOID, ...);
if (!HeapTupleIsValid(ht_am))
elog(ERROR, "...");
amrec = (Form_pg_am) GETSTRUCT(ht_am);
index->accessMethod = NameStr(amrec->amname);

/* ... */
ReleaseSysCache(ht_am);

return index;

Before calling ReleaseSysCache(), all the data you need from the
syscache entry needs to be deep-copied to allow subsequent access, but
NameStr() doesn't do a deep-copy. Adding "-DFORCE_CATCACHE_RELEASE" is a
useful way to catch these kinds of problems (I wonder if this is worth
adding to the default CFLAGS when assertions are enabled?)

I also made a bunch of editorial changes, including moving the
varattnos_map_schema() call out of the loop in transformInhRelation().

BTW, comments like "This function is based on code from ruleutils.c"
would be helpful for reviewers (whether in the patch itself or just in
the email containing the patch).

There's still a few things I need to fix in the patch, but I'll apply a
revised version of the attached patch to HEAD tomorrow, barring any
objections.

-Neil

Neil Conway <neilc(at)samurai(dot)com> writes:
> Attached is a revised version of this patch.

This still seems to fundamentally misunderstand the difference between
an index and a constraint. IMHO it should not be examining pg_index
(or specifically, the index Relations) at all.

regards, tom lane

Hi,

On 6/3/07, Neil Conway <neilc(at)samurai(dot)com> wrote:
>
> On Mon, 2007-21-05 at 12:23 +0530, NikhilS wrote:
> > I had spent some time on this earlier so decided to complete and send
> > the patch to you for review. This patch supports copying of
> > expressions, predicates, opclass, amorder, reloptions etc. The test
> > case also contains some more additions with this patch. Please let me
> > know if there are any issues.
>
> Attached is a revised version of this patch. Note that this pattern is
> always unsafe:
>
> ht_am = SearchSysCache(AMOID, ...);
> if (!HeapTupleIsValid(ht_am))
> elog(ERROR, "...");
> amrec = (Form_pg_am) GETSTRUCT(ht_am);
> index->accessMethod = NameStr(amrec->amname);
>
> /* ... */
> ReleaseSysCache(ht_am);
>
> return index;
>
> Before calling ReleaseSysCache(), all the data you need from the
> syscache entry needs to be deep-copied to allow subsequent access, but
> NameStr() doesn't do a deep-copy. Adding "-DFORCE_CATCACHE_RELEASE" is a
> useful way to catch these kinds of problems (I wonder if this is worth
> adding to the default CFLAGS when assertions are enabled?)

I should have delved deep into the NameStr functionality myself for this. I
too agree that adding the flag makes sense when assertions are enabled.

I also made a bunch of editorial changes, including moving the
> varattnos_map_schema() call out of the loop in transformInhRelation().
>
> BTW, comments like "This function is based on code from ruleutils.c"
> would be helpful for reviewers (whether in the patch itself or just in
> the email containing the patch).

Yes I had this in mind but somehow forgot to mention this when I posted the
patch. I had done some changes in ruleutils.c specificly to take cognizance
of this fact.

Regards,
Nikhils
--
EnterpriseDB http://www.enterprisedb.com

Hi,

On 6/3/07, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Neil Conway <neilc(at)samurai(dot)com> writes:
> > Attached is a revised version of this patch.
>
> This still seems to fundamentally misunderstand the difference between
> an index and a constraint. IMHO it should not be examining pg_index
> (or specifically, the index Relations) at all.

But as you had mentioned earlier, if we look at index entries as part of the
implementation of "unique" or "primary key" pg_constraint entries, then
examining pg_index is required, right?

Anyways, this patch and the functionality introduced herein will be useful
in the "CREATE .. INCLUDING INDEXES" case too.

Regards,
Nikhils

--
EnterpriseDB http://www.enterprisedb.com

NikhilS <nikkhils(at)gmail(dot)com> writes:
> On 6/3/07, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> This still seems to fundamentally misunderstand the difference between
>> an index and a constraint. IMHO it should not be examining pg_index
>> (or specifically, the index Relations) at all.

> Anyways, this patch and the functionality introduced herein will be useful
> in the "CREATE .. INCLUDING INDEXES" case too.

No doubt. But those are different features and we shouldn't confuse
'em; in particular not put behavior into the INCLUDING CONSTRAINTS case
that shouldn't be there.