Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
"Jim C. Nasby" <[EMAIL PROTECTED]> writes: > On Fri, Jul 20, 2007 at 12:40:46PM +0100, Gregory Stark wrote: > >> One of the main use cases I envision is wanting to create new partitions >> suitable for being added to a partitioned table. > > Except that's normally done with CREATE TABLE INHERITS, which seems > cleaner to me. I think the patch would be more useful if it added > support for INHERITS, but I don't object to it as-is. Well there are two different approaches to creating a new partition. For some use cases you want to create a new empty partition directly in the table and start using it right away. For other use cases you want to create a table into which you need to load data, possibly even massage the data with updates and deletes or additional inserts, then add the partition with the data all ready directly into the master table. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
On Fri, Jul 20, 2007 at 12:40:46PM +0100, Gregory Stark wrote: > "Tatsuo Ishii" <[EMAIL PROTECTED]> writes: > > > Can someone enligten me what the usecase for CREATE TABLE LIKE at this > > moment is? > > One of the main use cases I envision is wanting to create new partitions > suitable for being added to a partitioned table. Except that's normally done with CREATE TABLE INHERITS, which seems cleaner to me. I think the patch would be more useful if it added support for INHERITS, but I don't object to it as-is. -- Jim Nasby [EMAIL PROTECTED] EnterpriseDB http://enterprisedb.com 512.569.9461 (cell) pgp8t3gcRjs6u.pgp Description: PGP signature
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
"Tatsuo Ishii" <[EMAIL PROTECTED]> writes: > Can someone enligten me what the usecase for CREATE TABLE LIKE at this > moment is? One of the main use cases I envision is wanting to create new partitions suitable for being added to a partitioned table. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Can someone enligten me what the usecase for CREATE TABLE LIKE at this moment is? I read the doc and followed the discssion in this thread in pgsql-patches list but it was hard for me to figure out. -- Tatsuo Ishii SRA OSS, Inc. Japan > I've applied the latest version of the patch to HEAD. Thanks for the > patches, Nikhil and Trevor -- we can take a look at improving INCLUDING > CONSTRAINTS for 8.4. > > On Mon, 2007-16-07 at 15:50 +0530, NikhilS wrote: > > I agree, since its a LIKE operation and not inheritance as such, how about > > "src_idxoptions", just to make the reference to the source index > > clearer? > > I decided to keep it as "src_options", but we can always change it > later. > > -Neil > > > > ---(end of broadcast)--- > TIP 4: Have you searched our list archives? > >http://archives.postgresql.org ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
I've applied the latest version of the patch to HEAD. Thanks for the patches, Nikhil and Trevor -- we can take a look at improving INCLUDING CONSTRAINTS for 8.4. On Mon, 2007-16-07 at 15:50 +0530, NikhilS wrote: > I agree, since its a LIKE operation and not inheritance as such, how about > "src_idxoptions", just to make the reference to the source index > clearer? I decided to keep it as "src_options", but we can always change it later. -Neil ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Hi, > You suppose "src_options" is much more readable than "inhreloptions" > or "inh_idxoptions"? Your call :). Yeah, I'm not necessarily sure that it is. "inhreloptions" made me think of table inheritance, whereas LIKE in general is more of a "source" => "target" copying operation. But I'm not convinced about src_options, either ... suggestions/comments welcome. I agree, since its a LIKE operation and not inheritance as such, how about "src_idxoptions", just to make the reference to the source index clearer? To reiterate its a minor nitpick really from my side and can be ignored too. Regards, Nikhils -- EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
On Mon, 2007-16-07 at 14:28 +0530, NikhilS wrote: > Guess you want to use "src_options" here to be uniform with usages > elsewhere that you have replaced. Thanks, good catch. > You suppose "src_options" is much more readable than "inhreloptions" > or "inh_idxoptions"? Your call :). Yeah, I'm not necessarily sure that it is. "inhreloptions" made me think of table inheritance, whereas LIKE in general is more of a "source" => "target" copying operation. But I'm not convinced about src_options, either ... suggestions/comments welcome. -Neil ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Hi, Attached is a revised patch that does that. Barring any objections, I'll apply this to HEAD tomorrow. A minor correction below in your patch: --- src/include/commands/defrem.h 16 Jul 2007 05:19:43 - *** extern void DefineIndex(RangeVar *heapRe *** 26,31 --- 26,32 List *options, + char *inhreloptions, Guess you want to use "src_options" here to be uniform with usages elsewhere that you have replaced. You suppose "src_options" is much more readable than "inhreloptions" or "inh_idxoptions"? Your call :). Regards, Nikhils -- EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
On Tue, 2007-10-07 at 12:53 +0530, NikhilS wrote:
> Yes, in the CREATE..LIKE case, options will be NULL and in the normal
> CREATE..INDEX case inhreloptions will be NULL. Note that options is a
> List of DefElem entries, whereas inhreloptions is a char pointer.
Hmm, right -- ugly. I'll just stick with your approach.
BTW, I notice a problem with the patch as implemented:
# create table abc (a int, b int);
CREATE TABLE
# create index abc_a_idx on abc using hash (a);
CREATE INDEX
# create index abc_a_idx2 on abc (a);
CREATE INDEX
# create table abc2 (like abc including indexes);
CREATE TABLE
# \d abc2
Table "public.abc2"
Column | Type | Modifiers
+-+---
a | integer |
b | integer |
Indexes:
"abc2_a_key" hash (a)
This occurs because transformIndexConstraints() eliminates "duplicate"
indexes from the index list by looking for two indexes with equal()
column lists. This makes some sense for the normal CREATE TABLE case,
but the above behavior is certainly objectionable -- when the access
method differs it is merely surprising, but when partial indexes are
involved it is surely a bug.
One way to fix this would be to check for duplicates by comparing all
the fields of the two IndexStmts, *except* the index name and "is PK?"
status. But that's ugly -- it seems much cleaner to keep index
definitions arising from LIKE ... INCLUDING INDEXES in a separate list
from the indexes derived from constraints.
Attached is a revised patch that does that. Barring any objections, I'll
apply this to HEAD tomorrow.
-Neil
Index: doc/src/sgml/ref/create_table.sgml
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/doc/src/sgml/ref/create_table.sgml,v
retrieving revision 1.108
diff -p -c -r1.108 create_table.sgml
*** doc/src/sgml/ref/create_table.sgml 3 Jun 2007 17:06:03 - 1.108
--- doc/src/sgml/ref/create_table.sgml 16 Jul 2007 05:19:43 -
*** PostgreSQL documentation
*** 23,29
CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } ] TABLE table_name ( [
{ column_name data_type [ DEFAULT default_expr ] [ column_constraint [ ... ] ]
| table_constraint
! | LIKE parent_table [ { INCLUDING | EXCLUDING } { DEFAULTS | CONSTRAINTS } ] ... }
[, ... ]
] )
[ INHERITS ( parent_table [, ... ] ) ]
--- 23,29
CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } ] TABLE table_name ( [
{ column_name data_type [ DEFAULT default_expr ] [ column_constraint [ ... ] ]
| table_constraint
! | LIKE parent_table [ { INCLUDING | EXCLUDING } { DEFAULTS | CONSTRAINTS | INDEXES } ] ... }
[, ... ]
] )
[ INHERITS ( parent_table [, ... ] ) ]
*** and table
*** 237,243
! LIKE parent_table [ { INCLUDING | EXCLUDING } { DEFAULTS | CONSTRAINTS } ]
The LIKE clause specifies a table from which
--- 237,243
! LIKE parent_table [ { INCLUDING | EXCLUDING } { DEFAULTS | CONSTRAINTS | INDEXES } ]
The LIKE clause specifies a table from which
*** and table
*** 266,275
requested, all check constraints are copied.
Note also that unlike INHERITS, copied columns and
constraints are not merged with similarly named columns and constraints.
If the same name is specified explicitly or in another
! LIKE clause an error is signalled.
--- 266,280
requested, all check constraints are copied.
+ Any indexes on the original table will not be created on the new
+ table, unless the INCLUDING INDEXES clause is
+ specified.
+
+
Note also that unlike INHERITS, copied columns and
constraints are not merged with similarly named columns and constraints.
If the same name is specified explicitly or in another
! LIKE clause, an error is signalled.
Index: src/backend/bootstrap/bootparse.y
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/backend/bootstrap/bootparse.y,v
retrieving revision 1.88
diff -p -c -r1.88 bootparse.y
*** src/backend/bootstrap/bootparse.y 13 Mar 2007 00:33:39 - 1.88
--- src/backend/bootstrap/bootparse.y 16 Jul 2007 05:19:43 -
*** Boot_DeclareIndexStmt:
*** 252,258
LexIDStr($8),
NULL,
$10,
! NULL, NIL,
false, false, false,
false, false, true, false, false);
do_end();
--- 252,258
LexIDStr($8),
NULL,
$10,
! NULL, NIL, NULL,
false, false, false,
false, false, true, false, false);
do_end();
*** Boot_DeclareUniqueIndexStmt:
*** 270,276
LexIDStr($9),
NULL,
$11,
! NULL, NIL,
true, false, false,
false, fa
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
On 7/10/07, Bruce Momjian <[EMAIL PROTECTED]> wrote: Neil Conway wrote: > Attached is a revised version of this patch. Sorry for taking so long to > make progress on this: my new job been keeping my busy, and I've > recently been ill. Illness only counts as an excuse if you _don't_ recover. If you recover, you weren't sick enough. ;-) LOL uh! that sounds like my boss talking! -- regards, Jaime Casanova "Programming today is a race between software engineers striving to build bigger and better idiot-proof programs and the universe trying to produce bigger and better idiots. So far, the universe is winning." Richard Cook ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Neil Conway wrote: > On Tue, 2007-05-06 at 14:50 +0530, NikhilS wrote: > > PFA, a patch which provides the "CREATE .. INCLUDING INDEXES" > > functionality. This patch uses the same functionality introduced by > > the earlier patches in this thread albeit for the "INCLUDING INDEXES" > > case. > > Attached is a revised version of this patch. Sorry for taking so long to > make progress on this: my new job been keeping my busy, and I've > recently been ill. Illness only counts as an excuse if you _don't_ recover. If you recover, you weren't sick enough. ;-) LOL -- Bruce Momjian <[EMAIL PROTECTED]> http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Hi, This version updates the patch to CVS HEAD and has various fixes and refactoring, including proper docs. I refactored get_opclass_name() into lsyscache.c, but then realized that this means that lsyscache.c will depend on commands/indexcmds.c (for GetDefaultOpClass()), which is arguably improper, so I'm tempted to revert and just duplicate the syscache lookups in both ruleutils.c and parse_utilcmd.c Yes, I too would vote for duplicating the lookups for the sake of simplicity. Nikhil: why are both "options" and "inhreloptions" necessary in IndexStmt? Won't at least one be NULL? Yes, in the CREATE..LIKE case, options will be NULL and in the normal CREATE..INDEX case inhreloptions will be NULL. Note that options is a List of DefElem entries, whereas inhreloptions is a char pointer. The challenge was with converting the stored reloptions belonging to the parent index into some form which could be consumed by transformRelOptions(). It was difficult to convert it into a list of DefElem entries and hence I had to introduce inhreloptions. Regards, Nikhils -- EnterpriseDB http://www.enterprisedb.com
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
On Tue, 2007-05-06 at 14:50 +0530, NikhilS wrote:
> PFA, a patch which provides the "CREATE .. INCLUDING INDEXES"
> functionality. This patch uses the same functionality introduced by
> the earlier patches in this thread albeit for the "INCLUDING INDEXES"
> case.
Attached is a revised version of this patch. Sorry for taking so long to
make progress on this: my new job been keeping my busy, and I've
recently been ill.
This version updates the patch to CVS HEAD and has various fixes and
refactoring, including proper docs. I refactored get_opclass_name() into
lsyscache.c, but then realized that this means that lsyscache.c will
depend on commands/indexcmds.c (for GetDefaultOpClass()), which is
arguably improper, so I'm tempted to revert and just duplicate the
syscache lookups in both ruleutils.c and parse_utilcmd.c
Nikhil: why are both "options" and "inhreloptions" necessary in
IndexStmt? Won't at least one be NULL?
BTW, I notice that include/defrem.h contains declarations for several
different, marginally-related .c files (indexcmds.c, functioncmds.c,
operatorcmds.c, aggregatecmds.c, opclasscmds.c, define.c). I'm inclined
to separate these declarations into separate header files; any
objections to doing that?
-Neil
Index: doc/src/sgml/ref/create_table.sgml
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/doc/src/sgml/ref/create_table.sgml,v
retrieving revision 1.108
diff -p -c -r1.108 create_table.sgml
*** doc/src/sgml/ref/create_table.sgml 3 Jun 2007 17:06:03 - 1.108
--- doc/src/sgml/ref/create_table.sgml 9 Jul 2007 04:34:40 -
*** PostgreSQL documentation
*** 23,29
CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } ] TABLE table_name ( [
{ column_name data_type [ DEFAULT default_expr ] [ column_constraint [ ... ] ]
| table_constraint
! | LIKE parent_table [ { INCLUDING | EXCLUDING } { DEFAULTS | CONSTRAINTS } ] ... }
[, ... ]
] )
[ INHERITS ( parent_table [, ... ] ) ]
--- 23,29
CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } ] TABLE table_name ( [
{ column_name data_type [ DEFAULT default_expr ] [ column_constraint [ ... ] ]
| table_constraint
! | LIKE parent_table [ { INCLUDING | EXCLUDING } { DEFAULTS | CONSTRAINTS | INDEXES } ] ... }
[, ... ]
] )
[ INHERITS ( parent_table [, ... ] ) ]
*** and table
*** 237,243
! LIKE parent_table [ { INCLUDING | EXCLUDING } { DEFAULTS | CONSTRAINTS } ]
The LIKE clause specifies a table from which
--- 237,243
! LIKE parent_table [ { INCLUDING | EXCLUDING } { DEFAULTS | CONSTRAINTS | INDEXES } ]
The LIKE clause specifies a table from which
*** and table
*** 266,275
requested, all check constraints are copied.
Note also that unlike INHERITS, copied columns and
constraints are not merged with similarly named columns and constraints.
If the same name is specified explicitly or in another
! LIKE clause an error is signalled.
--- 266,280
requested, all check constraints are copied.
+ Any indexes on the original table will not be created on the new
+ table, unless the INCLUDING INDEXES clause is
+ specified.
+
+
Note also that unlike INHERITS, copied columns and
constraints are not merged with similarly named columns and constraints.
If the same name is specified explicitly or in another
! LIKE clause, an error is signalled.
Index: src/backend/bootstrap/bootparse.y
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/backend/bootstrap/bootparse.y,v
retrieving revision 1.88
diff -p -c -r1.88 bootparse.y
*** src/backend/bootstrap/bootparse.y 13 Mar 2007 00:33:39 - 1.88
--- src/backend/bootstrap/bootparse.y 8 Jul 2007 00:49:29 -
*** Boot_DeclareIndexStmt:
*** 252,258
LexIDStr($8),
NULL,
$10,
! NULL, NIL,
false, false, false,
false, false, true, false, false);
do_end();
--- 252,258
LexIDStr($8),
NULL,
$10,
! NULL, NIL, NULL,
false, false, false,
false, false, true, false, false);
do_end();
*** Boot_DeclareUniqueIndexStmt:
*** 270,276
LexIDStr($9),
NULL,
$11,
! NULL, NIL,
true, false, false,
false, false, true, false, false);
do_end();
--- 270,276
LexIDStr($9),
NULL,
$11,
! NULL, NIL, NULL,
true, false, false,
false, false, true, false, false);
do_end();
Index: src/backend/commands/indexcmds.c
===
RC
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Hi, 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. PFA, a patch which provides the "CREATE .. INCLUDING INDEXES" functionality. This patch uses the same functionality introduced by the earlier patches in this thread albeit for the "INCLUDING INDEXES" case. For ease of review, this patch is based on the latest version which was posted by Neil. Please let me know if there are any issues. Regards, Nikhils -- EnterpriseDB http://www.enterprisedb.com Index: doc/src/sgml/ref/create_table.sgml === RCS file: /repositories/postgreshome/cvs/pgsql/doc/src/sgml/ref/create_table.sgml,v retrieving revision 1.108 diff -c -r1.108 create_table.sgml *** doc/src/sgml/ref/create_table.sgml 3 Jun 2007 17:06:03 - 1.108 --- doc/src/sgml/ref/create_table.sgml 5 Jun 2007 09:20:14 - *** *** 263,275 INCLUDING CONSTRAINTS is specified; other types of constraints will never be copied. Also, no distinction is made between column constraints and table constraints — when constraints are ! requested, all check constraints are copied. Note also that unlike INHERITS, copied columns and constraints are not merged with similarly named columns and constraints. If the same name is specified explicitly or in another ! LIKE clause an error is signalled. --- 263,278 INCLUDING CONSTRAINTS is specified; other types of constraints will never be copied. Also, no distinction is made between column constraints and table constraints — when constraints are ! requested, all check constraints are copied. UNIQUE, ! and PRIMARY KEY constraints along with other index ! entries will be copied if INCLUDING INDEXES is ! specified. Note also that unlike INHERITS, copied columns and constraints are not merged with similarly named columns and constraints. If the same name is specified explicitly or in another ! LIKE clause, an error is signalled. Index: src/backend/bootstrap/bootparse.y === RCS file: /repositories/postgreshome/cvs/pgsql/src/backend/bootstrap/bootparse.y,v retrieving revision 1.88 diff -c -r1.88 bootparse.y *** src/backend/bootstrap/bootparse.y 13 Mar 2007 00:33:39 - 1.88 --- src/backend/bootstrap/bootparse.y 5 Jun 2007 09:20:14 - *** *** 252,258 LexIDStr($8), NULL, $10, ! NULL, NIL, false, false, false, false, false, true, false, false); do_end(); --- 252,258 LexIDStr($8), NULL, $10, ! NULL, NIL, NULL, false, false, false, false, false, true, false, false); do_end(); *** *** 270,276 LexIDStr($9), NULL, $11, ! NULL, NIL, true, false, false, false, false, true, false, false); do_end(); --- 270,276 LexIDStr($9), NULL, $11, ! NULL, NIL, NULL, true, false, false, false, false, true, false, false); do_end(); Index: src/backend/commands/indexcmds.c === RCS file: /repositories/postgreshome/cvs/pgsql/src/backend/commands/indexcmds.c,v retrieving revision 1.159 diff -c -r1.159 indexcmds.c *** src/backend/commands/indexcmds.c 3 Jun 2007 17:06:16 - 1.159 --- src/backend/commands/indexcmds.c 5 Jun 2007 09:20:14 - *** *** 100,105 --- 100,106 List *attributeList, Expr *predicate, List *options, + char *inhreloptions, bool unique, bool primary, bool isconstraint, *** *** 393,400 /* * Parse AM-specific options, convert to text array form, validate. */ ! reloptions = transformRelOptions((Datum) 0, options, false, false); (void) index_reloptions(amoptions, reloptions, true); --- 394,408 /* * Parse AM-specific options, convert to text array form, validate. + * The inh reloptions introduced due to using indexes via + * the "CREATE LIKE INCLUDING INDEXES" statement also need to be merged here */ ! if (inhreloptions) ! reloptions = deflatten_reloptions(inhreloptions); ! else ! reloptions = (Datum) 0; ! ! reloptions = transformRelOptions(reloptions, options, false, false); (void) index_reloptions(amoptions, reloptions, true); Index: src/backend/commands/tablecmds.c === RCS file: /repos
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
NikhilS <[EMAIL PROTECTED]> writes: > On 6/3/07, Tom Lane <[EMAIL PROTECTED]> 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. regards, tom lane ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Hi, On 6/3/07, Tom Lane <[EMAIL PROTECTED]> wrote: Neil Conway <[EMAIL PROTECTED]> 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
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Hi, On 6/3/07, Neil Conway <[EMAIL PROTECTED]> 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
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Neil Conway <[EMAIL PROTECTED]> 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 ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
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 Index: doc/src/sgml/ref/create_table.sgml === RCS file: /home/neilc/postgres/cvs_root/pgsql/doc/src/sgml/ref/create_table.sgml,v retrieving revision 1.107 diff -p -c -r1.107 create_table.sgml *** doc/src/sgml/ref/create_table.sgml 1 Feb 2007 00:28:18 - 1.107 --- doc/src/sgml/ref/create_table.sgml 2 Jun 2007 19:52:22 - *** and table *** 259,275 Not-null constraints are always copied to the new table. ! CHECK constraints will only be copied if ! INCLUDING CONSTRAINTS is specified; other types of ! constraints will never be copied. Also, no distinction is made between ! column constraints and table constraints — when constraints are ! requested, all check constraints are copied. Note also that unlike INHERITS, copied columns and constraints are not merged with similarly named columns and constraints. If the same name is specified explicitly or in another ! LIKE clause an error is signalled. --- 259,276 Not-null constraints are always copied to the new table. ! CHECK, UNIQUE, and ! PRIMARY KEY constraints will only be copied ! if INCLUDING CONSTRAINTS is specified. Also, ! no distinction is made between column constraints and table ! constraints — when constraints are requested, all eligible ! constraints are copied. Note also that unlike INHERITS, copied columns and constraints are not merged with similarly named columns and constraints. If the same name is specified explicitly or in another ! LIKE clause, an error is signalled. Index: src/backend/bootstrap/bootparse.y === RCS file: /home/neilc/postgres/cvs_root/pgsql/src/backend/bootstrap/bootparse.y,v retrieving revision 1.88 diff -p -c -r1.88 bootparse.y *** src/backend/bootstrap/bootparse.y 13 Mar 2007 00:33:39 - 1.88 --- src/backend/bootstrap/bootparse.y 2 Jun 2007 19:46:07 - *** Boot_DeclareIndexStmt: *** 252,258 LexIDStr($8), NULL, $10, ! NULL, NIL, false, false, false, false, false, true, false, false); do_end(); --- 252,258 LexIDStr($8), NULL, $10, ! NULL, NIL, NULL, false, false, false, false, false, true, false, false); do_end(); *** Boot_DeclareUniqueIndexStmt: *** 270,276 LexIDStr($9), NULL, $11, ! NULL, NIL, true, false, false, false, false, true, false, false); do_end(); --- 270,276 LexIDStr($9), NULL, $11, ! NULL, NIL, NULL, true, false, false, false, false, true, false, false); do_end(); Index: src/backend/commands/indexcmds.c === RCS file: /home/neilc/postgres/cvs_root/pgsql/src/backend/commands/indexcmds.c,v retrieving revision 1.158 diff -p -c -r1.158 indexcmds.c *** src/backend/commands/indexcmds.c 2 May 2007 21:08:45 - 1.158 --- src/backend/commands/indexcmds.c 2 Jun 2007 19:46:07 - *** DefineIndex(RangeVar *heapRelation, *** 100,1
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Bruce Momjian <[EMAIL PROTECTED]> writes: > OK, so the patch is ready to be applied? Neil's still reviewing it, last I heard. regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
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 <[EMAIL PROTECTED]> http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
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
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Alvaro Herrera <[EMAIL PROTECTED]> 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 ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
NikhilS escribió: > On 5/23/07, Tom Lane <[EMAIL PROTECTED]> wrote: > > > >NikhilS <[EMAIL PROTECTED]> 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 Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Hi, On 5/23/07, Tom Lane <[EMAIL PROTECTED]> wrote: NikhilS <[EMAIL PROTECTED]> 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
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
NikhilS <[EMAIL PROTECTED]> 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 ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
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 Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Hi, On 5/23/07, NikhilS <[EMAIL PROTECTED]> 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
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
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
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
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
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
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 Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Hi Neil, On 5/18/07, Neil Conway <[EMAIL PROTECTED]> 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 Index: doc/src/sgml/ref/create_table.sgml === RCS file: /repositories/postgreshome/cvs/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 - 1.107 --- doc/src/sgml/ref/create_table.sgml 21 May 2007 06:50:34 - *** *** 259,269 Not-null constraints are always copied to the new table. ! CHECK constraints will only be copied if ! INCLUDING CONSTRAINTS is specified; other types of ! constraints will never be copied. Also, no distinction is made between ! column constraints and table constraints — when constraints are ! requested, all check constraints are copied. Note also that unlike INHERITS, copied columns and --- 259,268 Not-null constraints are always copied to the new table. ! CHECK, UNIQUE, and PRIMARY KEY constraints will only ! be copied if INCLUDING CONSTRAINTS is specified. Also, ! no distinction is made between column constraints and table constraints ! — when constraints are requested, all check constraints are copied. Note also that unlike INHERITS, copied columns and Index: src/backend/bootstrap/bootparse.y === RCS file: /repositories/postgreshome/cvs/pgsql/src/backend/bootstrap/bootparse.y,v retrieving revision 1.88 diff -c -r1.88 bootparse.y *** src/backend/bootstrap/bootparse.y 13 Mar 2007 00:33:39 - 1.88 --- src/backend/bootstrap/bootparse.y 21 May 2007 06:50:34 - *** *** 252,258 LexIDStr($8), NULL, $10, ! NULL, NIL, false, false, false, false, false, true, false, false); do_end(); --- 252,258 LexIDStr($8), NULL, $10, ! NULL, NIL, NULL, false, false, false, false, false, true, false, false); do_end(); *** *** 270,276 LexIDStr($9), NULL, $11, ! NULL, NIL, true, false, false, false, false, true, false, false); do_end(); --- 270,276 LexIDStr($9), NULL, $11, ! NULL, NIL, NULL, true, false, false, false, false, true, false, false); do_end(); Index: src/backend/commands/indexcmds.c === RCS file: /repositories/postgreshome/cvs/pgsql/src/backend/commands/indexcmds.c,v retrieving revision 1.158 diff -c -r1.158 indexcmds.c *** src/backend/commands/indexcmds.c 2 May 2007 21:08:45 - 1.158 --- src/backend/commands/indexcmds.c 21 May 2007 06:50:34 - *** *** 100,105 --- 100,106 List *attributeList, Expr *predicate, List *options, + char *inhreloptions, bool unique, bool primary, bool isconstraint, *** *** 393,400 /* * Parse AM-specific options, convert to text array form, validate. */ ! reloptions = transformRelOptions((Datum) 0, options, false, false); (void) index_reloptions(amoptions, reloptions, true); --- 394,408 /* * Parse AM-specific options, convert to text array form, validate. + * The inh reloptions introduced due to using unique/primary indexes via + * the "CREATE LIKE INCLUDING CONSTRAINTS" statement also need to be merged here */ ! if (inhreloptions) ! reloptions = deflatten_reloptions(inhreloptions); ! else ! reloptions = (Datum) 0; ! ! reloptions = transformRelOptions(reloptions, options, false, false); (void) index_reloptions(amoptions, reloptions, true); Index: src/backend/commands/tablecmds.c === RCS file: /repositories/postgreshome/cvs/pgsql/src/backend/commands/tablecmds.c,v retrieving revision 1.225 diff -c -r1.225 tablecmds.c
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
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
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
NikhilS <[EMAIL PROTECTED]> 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 ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
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
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
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 <[EMAIL PROTECTED]> http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
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 ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
I wrote: > Gregory Stark <[EMAIL PROTECTED]> 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 ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Gregory Stark <[EMAIL PROTECTED]> writes: > "Tom Lane" <[EMAIL PROTECTED]> 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 ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
"Tom Lane" <[EMAIL PROTECTED]> writes: > NikhilS <[EMAIL PROTECTED]> 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 ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
NikhilS <[EMAIL PROTECTED]> 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 ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
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
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
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 ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
NikhilS <[EMAIL PROTECTED]> writes: > But this patch is not for the "INCLUDING INDEXES" case. Pardon me for being misled by the thread title :-( regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Hi, On 5/3/07, Tom Lane <[EMAIL PROTECTED]> wrote: NikhilS <[EMAIL PROTECTED]> 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
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
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 ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
NikhilS <[EMAIL PROTECTED]> 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 ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
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
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
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
Index: doc/src/sgml/ref/create_table.sgml
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/doc/src/sgml/ref/create_table.sgml,v
retrieving revision 1.107
diff -c -p -r1.107 create_table.sgml
*** doc/src/sgml/ref/create_table.sgml 1 Feb 2007 00:28:18 - 1.107
--- doc/src/sgml/ref/create_table.sgml 27 Apr 2007 21:49:20 -
*** and table
*** 259,269
Not-null constraints are always copied to the new table.
! CHECK constraints will only be copied if
! INCLUDING CONSTRAINTS is specified; other types of
! constraints will never be copied. Also, no distinction is made between
! column constraints and table constraints — when constraints are
! requested, all check constraints are copied.
Note also that unlike INHERITS, copied columns and
--- 259,268
Not-null constraints are always copied to the new table.
! CHECK, UNIQUE, and PRIMARY KEY constraints will only
! be copied if INCLUDING CONSTRAINTS is specified. Also,
! no distinction is made between column constraints and table constraints
! — when constraints are requested, all check constraints are copied.
Note also that unlike INHERITS, copied columns and
Index: src/backend/parser/analyze.c
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/backend/parser/analyze.c,v
retrieving revision 1.362
diff -c -p -r1.362 analyze.c
*** src/backend/parser/analyze.c 13 Mar 2007 00:33:41 - 1.362
--- src/backend/parser/analyze.c 27 Apr 2007 22:50:32 -
***
*** 27,32
--- 27,33
#include "postgres.h"
+ #include "access/genam.h"
#include "access/heapam.h"
#include "catalog/heap.h"
#include "catalog/index.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"
*** transformInhRelation(ParseState *pstate,
*** 1325,1359
* If default expr could contain any vars, we'd need to fix 'em,
* but it can't; so default is ready to apply to child.
*/
-
def->cooked_default = pstrdup(this_default);
}
}
! /*
! * Copy CHECK constraints if requested, being careful to adjust
! * attribute numbers
! */
! if (including_constraints && tupleDesc->constr)
{
! AttrNumber *attmap = varattnos_map_schema(tupleDesc, cxt->columns);
! int ccnum;
! for (ccnum = 0; ccnum < tupleDesc->constr->num_check; ccnum++)
{
! char *ccname = tupleDesc->constr->check[ccnum].ccname;
! char *ccbin = tupleDesc->constr->check[ccnum].ccbin;
! Node *ccbin_node = stringToNode(ccbin);
! Constraint *n = makeNode(Constraint);
! change_varattnos_of_a_node(ccbin_node, attmap);
! n->contype = CONSTR_CHECK;
! n->name = pstrdup(ccname);
! n->raw_expr = NULL;
! n->cooked_expr = nodeToString(ccbin_node);
! n->indexspace = NULL;
! cxt->ckconstraints = lappend(cxt->ckconstraints, (Node *) n);
}
}
--- 1327,1424
* If default expr could contain any vars, we'd need to fix 'em,
* but it can't; so default is ready to apply to child.
*/
def->cooked_default = pstrdup(this_default);
}
}
! if (including_constraints)
{
! /*
! * Copy CHECK constraints, being careful to adjust attribute
! * numbers.
! */
! if (tupleDesc->constr)
! {
! AttrNumber *attmap;
! int ccnum;
!
! attmap = varattnos_map_schema(tupleDesc, cxt->columns);
! for (ccnum = 0; ccnum < tupleDesc-
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
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 - 1.362
> --- src/backend/parser/analyze.c 13 Apr 2007 16:41:46 -
> ***
> *** 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)
> + {
> + Oidparent_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
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
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.c13 Mar 2007 00:33:41 - 1.362
--- src/backend/parser/analyze.c13 Apr 2007 16:41:46 -
***
*** 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)
+ {
+ Oidparent_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
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
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
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Bruce Momjian wrote:
NikhilS wrote:
Hi,
On 4/10/07, Bruce Momjian <[EMAIL PROTECTED]> 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
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.c13 Mar 2007 00:33:41 - 1.362
--- src/backend/parser/analyze.c12 Apr 2007 17:54:49 -
***
*** 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,1431
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)
+ {
+ Oidparent_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
+
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
NikhilS wrote: > Hi, > > On 4/10/07, Bruce Momjian <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Hi, On 4/10/07, Bruce Momjian <[EMAIL PROTECTED]> 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
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
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" <[EMAIL PROTECTED]> 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 - 1.361
> --- src/backend/parser/analyze.c 7 Mar 2007 01:43:12 -
> ***
> *** 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)
> + {
> + Oidparent_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;
> +
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Gregory Stark wrote: > "Bruce Momjian" <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
"Bruce Momjian" <[EMAIL PROTECTED]> 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 ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
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" <[EMAIL PROTECTED]> 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 - 1.361
> --- src/backend/parser/analyze.c 7 Mar 2007 01:43:12 -
> ***
> *** 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)
> + {
> + Oidparent_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;
> +
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
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" <[EMAIL PROTECTED]> 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 - 1.361
> --- src/backend/parser/analyze.c 7 Mar 2007 01:43:12 -
> ***
> *** 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)
> + {
> + Oidparent_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 */
> +
[PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
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" <[EMAIL PROTECTED]> 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.c20 Feb 2007 17:32:16 - 1.361
--- src/backend/parser/analyze.c7 Mar 2007 01:43:12 -
***
*** 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)
+ {
+ Oidparent_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;
+
