Re: [PATCHES] [HACKERS] Arrays of Complex Types
Awhile back I wrote: I did some tests just now to determine the total number of catalog entries associated with a simple table definition. Assuming it has N user columns of built-in types (hence not requiring pg_depend entries for the datatypes), I count 1 pg_class entry for the table itself 1 pg_type entry for the rowtype N + 6 pg_attribute entries for the user and system columns 2 pg_depend entries (type - table and table - namespace) 2 pg_shdepend entries (ownership of table and type) Of course this goes up *fast* if you need a toast table, indexes, constraints, etc, but that's the irreducible minimum. Generating an array rowtype would add three more catalog entries to this (the array pg_type entry, a pg_depend arraytype-rowtype link, and another pg_shdepend entry), which isn't a huge percentage overhead. Obviously if we wanted to trim some fat here, getting rid of the redundant pg_attribute entries for system columns would be the first place to look. BTW, in the array patch as just committed, I was able to get rid of the pg_shdepend entries for a table rowtype (when it's not a free-standing composite type) and for an array type; instead they indirectly depend on the owner of the parent table or element type respectively. So the net increase from 8.2 is only one catalog entry (we save one existing pg_shdepend entry for the rowtype, and then add a pg_type entry for the array type and a pg_depend entry to link it to the rowtype). regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] [HACKERS] Arrays of Complex Types
On 5/11/07, Tom Lane [EMAIL PROTECTED] wrote: BTW, in the array patch as just committed, I was able to get rid of the I am testing this feature, no problem so far. It's fast, and works exactly as advertised! Great work! (aiui, no domain arrays for 8.3?) merlin ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [HACKERS] Arrays of Complex Types
I wrote: OK, summarising what looks to me like a consensus position, ISTM the plan could be: . fix makeArrayTypeName() or similar to make it try harder to generate a unique non-clashing name . remove the existing 62 instead of 63 name length restrictions . autogenerate array types for all explicitly or implicitly created composite types other than for system catalog objects. . defer for the present any consideration of a CREATE TYPE foo AS ARRAY ... command. Regarding catalog objects, we might have to try a little harder than just not generating in bootstrap mode - IIRC we generate system views (including pg_stats) in non-bootstrap mode. Maybe we just need to exempt anything in the pg_catalog namespace. What would happen if a user created a view over pg_statistic? Should the test be to avoid arrays for things that depend on the catalogs? Or maybe we should go to the heart of the problem and simply check for pseudo-types directly. I've been working on David's patch and done the following: . inhibit creation of array types for composites during initdb . some bug fixes . have CheckAttributeType recurse into composite types, so you can no longer create a table/type with a composite field which contains a pseudo-type column (like pg_statistic) However, there are still some oddities. For example, a change to or removal of the base type affects the array type, but the array type can be directly operated on (e.g. alter type _aa set schema foo ). I'm inclined to say we should prevent direct operations on array types, and they should live or die by their parent types. Thoughts? cheers andrew ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [HACKERS] Arrays of Complex Types
On Sun, May 06, 2007 at 01:33:47PM -0400, Andrew Dunstan wrote: However, there are still some oddities. For example, a change to or removal of the base type affects the array type, but the array type can be directly operated on (e.g. alter type _aa set schema foo ). I'm inclined to say we should prevent direct operations on array types, and they should live or die by their parent types. Thoughts? +1 on binding the array types tightly to the parent types. Cheers, D -- David Fetter [EMAIL PROTECTED] http://fetter.org/ phone: +1 415 235 3778AIM: dfetter666 Skype: davidfetter Remember to vote! Consider donating to PostgreSQL: http://www.postgresql.org/about/donate ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [HACKERS] Arrays of Complex Types
Tom Lane wrote: Andrew Dunstan [EMAIL PROTECTED] writes: How would we do that? Not create the array types in bootstrap mode? Or just special-case pg_statistic? Not generate them in bootstrap mode works for me. IIRC, there's code somewhere in there that allows anyarray to pass as a column type in bootstrap mode, so that seems to fit ... OK, summarising what looks to me like a consensus position, ISTM the plan could be: . fix makeArrayTypeName() or similar to make it try harder to generate a unique non-clashing name . remove the existing 62 instead of 63 name length restrictions . autogenerate array types for all explicitly or implicitly created composite types other than for system catalog objects. . defer for the present any consideration of a CREATE TYPE foo AS ARRAY ... command. Regarding catalog objects, we might have to try a little harder than just not generating in bootstrap mode - IIRC we generate system views (including pg_stats) in non-bootstrap mode. Maybe we just need to exempt anything in the pg_catalog namespace. What would happen if a user created a view over pg_statistic? Should the test be to avoid arrays for things that depend on the catalogs? Or maybe we should go to the heart of the problem and simply check for pseudo-types directly. cheers andrew ---(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] [HACKERS] Arrays of Complex Types
Andrew Dunstan [EMAIL PROTECTED] writes: Regarding catalog objects, we might have to try a little harder than just not generating in bootstrap mode - IIRC we generate system views (including pg_stats) in non-bootstrap mode. Maybe we just need to exempt anything in the pg_catalog namespace. What would happen if a user created a view over pg_statistic? Nothing: regression=# create view vvv as select * from pg_statistic; ERROR: column stavalues1 has pseudo-type anyarray which means we do have an issue for the pg_stats view. Now that I look instead of guessing, the existing test in CheckAttributeType is not on bootstrap mode but standalone mode: /* Special hack for pg_statistic: allow ANYARRAY during initdb */ if (atttypid != ANYARRAYOID || IsUnderPostmaster) ereport(ERROR, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), errmsg(column \%s\ has pseudo-type %s, attname, format_type_be(atttypid; so for consistency we should use the same condition to suppress types for system catalogs. Or maybe we should go to the heart of the problem and simply check for pseudo-types directly. Actually we may have an issue already: regression=# create table zzz (f1 pg_statistic); CREATE TABLE I couldn't make it misbehave in a short amount of trying: regression=# insert into zzz values(row(0,0,0,0,0,0,0,0,0,0,0,0,0,array[1,2],null,null,null,array[12,13],null,null,null)); ERROR: ROW() column has type integer[] instead of type anyarray but I don't feel comfortable about this at all. Maybe CheckAttributeType should be made to recurse into composite columns. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [HACKERS] Arrays of Complex Types
On Mon, Apr 09, 2007 at 10:40:49AM -0400, Tom Lane wrote: Andrew Dunstan [EMAIL PROTECTED] writes: Regarding catalog objects, we might have to try a little harder than just not generating in bootstrap mode - IIRC we generate system views (including pg_stats) in non-bootstrap mode. Maybe we just need to exempt anything in the pg_catalog namespace. What would happen if a user created a view over pg_statistic? Nothing: regression=# create view vvv as select * from pg_statistic; ERROR: column stavalues1 has pseudo-type anyarray which means we do have an issue for the pg_stats view. Now that I look instead of guessing, the existing test in CheckAttributeType is not on bootstrap mode but standalone mode: /* Special hack for pg_statistic: allow ANYARRAY during initdb */ if (atttypid != ANYARRAYOID || IsUnderPostmaster) ereport(ERROR, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), errmsg(column \%s\ has pseudo-type %s, attname, format_type_be(atttypid; so for consistency we should use the same condition to suppress types for system catalogs. Groovy :) Or maybe we should go to the heart of the problem and simply check for pseudo-types directly. Actually we may have an issue already: regression=# create table zzz (f1 pg_statistic); CREATE TABLE I couldn't make it misbehave in a short amount of trying: regression=# insert into zzz values(row(0,0,0,0,0,0,0,0,0,0,0,0,0,array[1,2],null,null,null,array[12,13],null,null,null)); ERROR: ROW() column has type integer[] instead of type anyarray but I don't feel comfortable about this at all. Maybe CheckAttributeType should be made to recurse into composite columns. That'd be great :) Cheers, D -- David Fetter [EMAIL PROTECTED] http://fetter.org/ phone: +1 415 235 3778AIM: dfetter666 Skype: davidfetter Remember to vote! Consider donating to PostgreSQL: http://www.postgresql.org/about/donate ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [HACKERS] Arrays of Complex Types
On Mon, Apr 09, 2007 at 10:14:41AM -0400, Andrew Dunstan wrote: . defer for the present any consideration of a CREATE TYPE foo AS ARRAY ... command. What is the rationale for allowing people to name the array type. When I originally proposed the syntax I presumed that the array name would be kept internal and hidden from the user, just that it would exist after that command. What possible reason is there for allowing the user to give the array type a name? Have a nice day, -- Martijn van Oosterhout kleptog@svana.org http://svana.org/kleptog/ From each according to his ability. To each according to his ability to litigate. signature.asc Description: Digital signature
Re: [PATCHES] [HACKERS] Arrays of Complex Types
Martijn van Oosterhout wrote: On Mon, Apr 09, 2007 at 10:14:41AM -0400, Andrew Dunstan wrote: . defer for the present any consideration of a CREATE TYPE foo AS ARRAY ... command. What is the rationale for allowing people to name the array type. When I originally proposed the syntax I presumed that the array name would be kept internal and hidden from the user, just that it would exist after that command. What possible reason is there for allowing the user to give the array type a name? Have a nice day, Some type systems have named array types, some don't. I can live happily with either. Are array types anonymous in the standard? At any rate, the point of the remark was to take this off the table for now. cheers andrew ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] [HACKERS] Arrays of Complex Types
On Mon, Apr 09, 2007 at 04:07:16PM -0400, Andrew Dunstan wrote: Some type systems have named array types, some don't. I can live happily with either. Are array types anonymous in the standard? Yes, they're anonymous in the standard. That doesn't mean we can't give them names if we wanted... At any rate, the point of the remark was to take this off the table for now. Sure, once the array types are created automatically the command becomes completely redundant. Have a nice day, -- Martijn van Oosterhout kleptog@svana.org http://svana.org/kleptog/ From each according to his ability. To each according to his ability to litigate. signature.asc Description: Digital signature
Re: [PATCHES] [HACKERS] Arrays of Complex Types
Tom Lane wrote: David Fetter [EMAIL PROTECTED] writes: On Mon, Apr 02, 2007 at 10:01:44PM -0400, Alvaro Herrera wrote: So, hum, what happened to the idea of creating the array types only on demand? Scotched, as far as I could tell, More like you submitted a patch that entirely ignores multiple people's opinion on what is needed. Bruce may have put this into the patch queue, but do not labor under the delusion that that means it'll get applied as-is. The queue is currently operating as a list of open issues. One of the things that's been bothering me about this proposal is that it leaves untouched and indeed greatly expands the scope of the typename mangling we do. (i.e. adding an entry to pg_type with _ prepended). Up to now we've only used this gadget in a way that might matter a lot on user defined non-composite types, I think, and now we have expanded that to include enums, which are really a special case of user defined non-composites which don't require an extra C module. That's a comparatively small window, but this proposal will extend it to all composites, which is quite a large expansion in scope. And since _ is a perfectly legal initial char for an identifier, if type _foo exists then any attempt to create a table or view or composite called foo will fail. Is it possible to fix this, or am I trying to shut the stable door after the horse has well and truly bolted? If it can be fixed, I'd like to see it fixed before we fix the problem David is trying to address here. It's been suggested to me that this is an insignificant corner case. But I have often seen coding standards that actually require certain classes of identifier to being with _, so it's very far from a merely theoretical point. I'm slightly inclined to agree with David that the danger of catalog bloat isn't that great, and might not justify the extra work that some sort of explicit array creation would involve (e.g. changes in grammar, pg_dump), as long as we are agreed that we don't want array types ever to have their own user definable names or settable namespace. cheers andrew ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] [HACKERS] Arrays of Complex Types
Andrew Dunstan [EMAIL PROTECTED] writes: One of the things that's been bothering me about this proposal is that it leaves untouched and indeed greatly expands the scope of the typename mangling we do. (i.e. adding an entry to pg_type with _ prepended). Yeah, that's been bothering me too. One of the problems with the patch as-is is that it extends the 62-instead-of-63-char limit to table names as well as type names. I've been thinking of proposing that we add a column to pg_type that points from a type to its array type (if any), ie the reverse link from typelem. If we had that then the parser could follow that to determine which type is foo[], instead of relying on the _foo naming convention. I don't suggest that we stop using the naming convention, but it would no longer be a hard-and-fast rule, just a convention. In particular we could rejigger things around the edges to reduce the name conflict problem. For instance the rule for forming array type names could be prepend _, truncate to less than 64 bytes if necessary, then substitute numbers at the end if needed to get something unique. This is not all that different from what we do now to get unique serial sequence names, for example. This would also open the door to supporting CREATE TYPE foo AS ARRAY OF bar without having to have any restrictions about the name of foo. I'd still much rather do things that way for arrays of composites than invent a ton of pg_type entries that are mostly going to go unused. regards, tom lane PS: Has anyone looked at what it will take to make the entries in an array-of-composite be something smaller than full tuples? It's not going to be anything but a toy unless you can get the per-entry overhead down to something sane. Perhaps the MinimalTuple representation would work. ---(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] [HACKERS] Arrays of Complex Types
On Sun, Apr 08, 2007 at 07:08:38PM -0400, Tom Lane wrote: Andrew Dunstan [EMAIL PROTECTED] writes: One of the things that's been bothering me about this proposal is that it leaves untouched and indeed greatly expands the scope of the typename mangling we do. (i.e. adding an entry to pg_type with _ prepended). Yeah, that's been bothering me too. One of the problems with the patch as-is is that it extends the 62-instead-of-63-char limit to table names as well as type names. I did this by copying some code which already creates array names, so should that code change to do something different, the 62-instead-of-63-char thing would go away along with it. I agree that the prepended _s are far from optimal. I've been thinking of proposing that we add a column to pg_type that points from a type to its array type (if any), ie the reverse link from typelem. If we had that then the parser could follow that to determine which type is foo[], instead of relying on the _foo naming convention. I don't suggest that we stop using the naming convention, but it would no longer be a hard-and-fast rule, just a convention. That'd be neat :) In particular we could rejigger things around the edges to reduce the name conflict problem. For instance the rule for forming array type names could be prepend _, truncate to less than 64 bytes if necessary, then substitute numbers at the end if needed to get something unique. This is not all that different from what we do now to get unique serial sequence names, for example. This would also open the door to supporting CREATE TYPE foo AS ARRAY OF bar I'm sorry to keep harping on this, but I really don't see a use case and do see foot guns both with making the array types optional and with decoupling their names from those of their respective compound types. When they're optional, we get all kinds of stepping on a step that isn't there issues, and when they're decoupled, operations like, ALTER TABLE foo RENAME TO bar have either surprising or undefined behavior, or both. without having to have any restrictions about the name of foo. I'd still much rather do things that way for arrays of composites than invent a ton of pg_type entries that are mostly going to go unused. I'm sure there's a better way than my first attempt. PS: Has anyone looked at what it will take to make the entries in an array-of-composite be something smaller than full tuples? It's not going to be anything but a toy unless you can get the per-entry overhead down to something sane. Perhaps the MinimalTuple representation would work. Sounds neat, too :) Cheers, D -- David Fetter [EMAIL PROTECTED] http://fetter.org/ phone: +1 415 235 3778AIM: dfetter666 Skype: davidfetter Remember to vote! Consider donating to PostgreSQL: http://www.postgresql.org/about/donate ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] [HACKERS] Arrays of Complex Types
Tom Lane wrote: I've been thinking of proposing that we add a column to pg_type that points from a type to its array type (if any), ie the reverse link from typelem. If we had that then the parser could follow that to determine which type is foo[], instead of relying on the _foo naming convention. good. I don't suggest that we stop using the naming convention, but it would no longer be a hard-and-fast rule, just a convention. In particular we could rejigger things around the edges to reduce the name conflict problem. For instance the rule for forming array type names could be prepend _, truncate to less than 64 bytes if necessary, then substitute numbers at the end if needed to get something unique. This is not all that different from what we do now to get unique serial sequence names, for example. Sounds OK but I'd add something that might make it even more unlikely to generate a name clash. This would also open the door to supporting CREATE TYPE foo AS ARRAY OF bar without having to have any restrictions about the name of foo. I'd still much rather do things that way for arrays of composites than invent a ton of pg_type entries that are mostly going to go unused. ISTM we should either do it all automatically or all manually. If you want user defined names for array types then we can forget name mangling for user defined types and do everything manually. cheers andrew ---(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] [HACKERS] Arrays of Complex Types
Andrew Dunstan [EMAIL PROTECTED] writes: Tom Lane wrote: I don't suggest that we stop using the naming convention, but it would no longer be a hard-and-fast rule, just a convention. In particular we could rejigger things around the edges to reduce the name conflict problem. For instance the rule for forming array type names could be prepend _, truncate to less than 64 bytes if necessary, then substitute numbers at the end if needed to get something unique. This is not all that different from what we do now to get unique serial sequence names, for example. Sounds OK but I'd add something that might make it even more unlikely to generate a name clash. Like what? I don't want to stray far from _foo when we don't have to, because I'm sure there is user code out there that'll still rely on that naming convention; we shouldn't break it if we don't have to. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [HACKERS] Arrays of Complex Types
Tom Lane wrote: Andrew Dunstan [EMAIL PROTECTED] writes: Tom Lane wrote: I don't suggest that we stop using the naming convention, but it would no longer be a hard-and-fast rule, just a convention. In particular we could rejigger things around the edges to reduce the name conflict problem. For instance the rule for forming array type names could be prepend _, truncate to less than 64 bytes if necessary, then substitute numbers at the end if needed to get something unique. This is not all that different from what we do now to get unique serial sequence names, for example. Sounds OK but I'd add something that might make it even more unlikely to generate a name clash. Like what? I don't want to stray far from _foo when we don't have to, because I'm sure there is user code out there that'll still rely on that naming convention; we shouldn't break it if we don't have to. Oh, in that case maybe we'd better live with it :-( I certainly think we should deprecate relying on it. cheers andrew ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [HACKERS] Arrays of Complex Types
Andrew Dunstan [EMAIL PROTECTED] writes:
I'm slightly inclined to agree with David that the danger of catalog
bloat isn't that great, and might not justify the extra work that some
sort of explicit array creation would involve (e.g. changes in grammar,
pg_dump), as long as we are agreed that we don't want array types ever
to have their own user definable names or settable namespace.
I did some tests just now to determine the total number of catalog
entries associated with a simple table definition. Assuming it has
N user columns of built-in types (hence not requiring pg_depend entries
for the datatypes), I count
1 pg_class entry for the table itself
1 pg_type entry for the rowtype
N + 6 pg_attribute entries for the user and system columns
2 pg_depend entries (type - table and table - namespace)
2 pg_shdepend entries (ownership of table and type)
Of course this goes up *fast* if you need a toast table, indexes,
constraints, etc, but that's the irreducible minimum.
Generating an array rowtype would add three more catalog entries to this
(the array pg_type entry, a pg_depend arraytype-rowtype link, and
another pg_shdepend entry), which isn't a huge percentage overhead.
Obviously if we wanted to trim some fat here, getting rid of the
redundant pg_attribute entries for system columns would be the first
place to look.
Based on this, I withdraw my efficiency concern about generating
rowtypes for all user tables. I do, however, still object to generating
them for system tables. In particular an array type for pg_statistic
will actively Not Work and probably constitute a security hole, because
of the anyarray hack we use there.
BTW, I just noticed that we currently create array types with AUTO
dependencies on their element type, meaning that you can drop them
separately:
regression=# create type fooey as enum ('a','b');
CREATE TYPE
regression=# drop type _fooey;
DROP TYPE
Is this a bad idea? If we made the dependency INTERNAL then the
system would refuse the drop above. I think we would have to do
that if we wanted to add the base-array link I suggested, because
otherwise this drop would leave a dangling pointer in pg_type.
regards, tom lane
---(end of broadcast)---
TIP 6: explain analyze is your friend
Re: [PATCHES] [HACKERS] Arrays of Complex Types
Tom Lane wrote: Based on this, I withdraw my efficiency concern about generating rowtypes for all user tables. I do, however, still object to generating them for system tables. In particular an array type for pg_statistic will actively Not Work and probably constitute a security hole, because of the anyarray hack we use there. How would we do that? Not create the array types in bootstrap mode? Or just special-case pg_statistic? cheers andrew ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [HACKERS] Arrays of Complex Types
Andrew Dunstan [EMAIL PROTECTED] writes: How would we do that? Not create the array types in bootstrap mode? Or just special-case pg_statistic? Not generate them in bootstrap mode works for me. IIRC, there's code somewhere in there that allows anyarray to pass as a column type in bootstrap mode, so that seems to fit ... regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] Arrays of Complex Types
On Mon, Apr 02, 2007 at 10:01:44PM -0400, Alvaro Herrera wrote: Bruce Momjian wrote: 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. So, hum, what happened to the idea of creating the array types only on demand? Scotched, as far as I could tell, partly due to nobody's having actually done work toward such a thing, and partly because the closest thing I've heard to an objection is pretty nebulous. :) It's a lot simpler to have them always, and it fits in with the larger picture of making arrays fully composable with other operations like DOMAIN, ENUM and TYPE. Cheers, D -- David Fetter [EMAIL PROTECTED] http://fetter.org/ phone: +1 415 235 3778AIM: dfetter666 Skype: davidfetter Remember to vote! Consider donating to PostgreSQL: http://www.postgresql.org/about/donate ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [HACKERS] Arrays of Complex Types
David Fetter [EMAIL PROTECTED] writes: On Mon, Apr 02, 2007 at 10:01:44PM -0400, Alvaro Herrera wrote: So, hum, what happened to the idea of creating the array types only on demand? Scotched, as far as I could tell, More like you submitted a patch that entirely ignores multiple people's opinion on what is needed. Bruce may have put this into the patch queue, but do not labor under the delusion that that means it'll get applied as-is. The queue is currently operating as a list of open issues. regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [HACKERS] Arrays of Complex Types
On Tue, Apr 03, 2007 at 02:30:07AM -0400, Tom Lane wrote: David Fetter [EMAIL PROTECTED] writes: On Mon, Apr 02, 2007 at 10:01:44PM -0400, Alvaro Herrera wrote: So, hum, what happened to the idea of creating the array types only on demand? Scotched, as far as I could tell, More like you submitted a patch that entirely ignores multiple people's opinion on what is needed. Bruce may have put this into the patch queue, but do not labor under the delusion that that means it'll get applied as-is. I assure you I'm not. Two glaring things it's missing are regression tests and documentation. I should have those in this week. Cheers, D -- David Fetter [EMAIL PROTECTED] http://fetter.org/ phone: +1 415 235 3778AIM: dfetter666 Skype: davidfetter Remember to vote! Consider donating to PostgreSQL: http://www.postgresql.org/about/donate ---(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] [HACKERS] Arrays of Complex Types
Tom Lane wrote: David Fetter [EMAIL PROTECTED] writes: On Mon, Apr 02, 2007 at 10:01:44PM -0400, Alvaro Herrera wrote: So, hum, what happened to the idea of creating the array types only on demand? Scotched, as far as I could tell, More like you submitted a patch that entirely ignores multiple people's opinion on what is needed. Bruce may have put this into the patch queue, but do not labor under the delusion that that means it'll get applied as-is. The queue is currently operating as a list of open issues. Correct. -- 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: [HACKERS] Arrays of Complex Types
On Fri, Mar 30, 2007 at 05:08:42PM -0400, Tom Lane wrote:
David Fetter [EMAIL PROTECTED] writes:
After several rounds of patches, it appears that it might be easier to
create a new typtype entry, which I'll tentatively call 'a' because it
seems a little fragile and a lot inelegant and hard to maintain to
have typtype='c' and typrelid=InvalidOid mean, this is an array of
complex types.
Uh, wouldn't it be typtype = 'c' and typelem != 0 ?
Right. The attached patch passes the current regression tests and at
least to a smoke test level does what it's supposed to do. I'd
really like to help refactor the whole array system to use 'a', tho.
Cheers,
D
--
David Fetter [EMAIL PROTECTED] http://fetter.org/
phone: +1 415 235 3778AIM: dfetter666
Skype: davidfetter
Remember to vote!
Consider donating to PostgreSQL: http://www.postgresql.org/about/donate
Index: src/backend/catalog/heap.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/catalog/heap.c,v
retrieving revision 1.318
diff -c -r1.318 heap.c
*** src/backend/catalog/heap.c 2 Apr 2007 03:49:37 - 1.318
--- src/backend/catalog/heap.c 2 Apr 2007 20:09:16 -
***
*** 45,50
--- 45,51
#include catalog/pg_statistic.h
#include catalog/pg_type.h
#include commands/tablecmds.h
+ #include commands/typecmds.h
#include miscadmin.h
#include optimizer/clauses.h
#include optimizer/var.h
***
*** 763,768
--- 764,770
Relationpg_class_desc;
Relationnew_rel_desc;
Oid new_type_oid;
+ char *relarrayname;
pg_class_desc = heap_open(RelationRelationId, RowExclusiveLock);
***
*** 815,820
--- 817,856
relnamespace,
relid,
relkind);
+ /*
+* Add in the corresponding array types if appropriate.
+*/
+ if (relkind == RELKIND_RELATION ||
+ relkind == RELKIND_VIEW ||
+ relkind == RELKIND_COMPOSITE_TYPE)
+ {
+ relarrayname = makeArrayTypeName(relname);
+ TypeCreate(relarrayname,/* Array type name */
+ relnamespace,/* Same
namespace as parent */
+ InvalidOid, /* relation's
type oid, set here to InvalidOid to make dependency work right */
+ 0, /*
relkind, also N/A here */
+ -1, /*
Internal size, unlimited */
+ 'c', /* It's
a complex type */
+ DEFAULT_TYPDELIM,/* Use the default */
+ F_ARRAY_IN, /* Macro for
array input procedure */
+ F_ARRAY_OUT, /* Macro for
array output procedure */
+ F_ARRAY_RECV,/* Macro for
array receive (binary input) procedure */
+ F_ARRAY_SEND,/* Macro for
array send (binary output) procedure */
+ InvalidOid, /* No input
typmod */
+ InvalidOid, /* No output
typmod */
+ InvalidOid, /* Default
ANALYZE procedure */
+ new_type_oid,/* The OID just
created */
+ InvalidOid, /* No base
type--this isn't a DOMAIN */
+ NULL,/* No
default type value */
+ NULL,/*
Don't send binary */
+ false, /*
Never passed by value */
+ 'd', /* Type
alignment. Should this be something else? */
+ 'x', /*
Always TOASTable */
+ -1, /* No
typMod for regular composite types. */
+ 0, /*
Array diminsions of typbasetype */
+ false); /* Type
NOT NULL */
+ pfree(relarrayname);/* Seems like the right thing to do
here. */
+ }
/*
* now create an entry in
Re: [HACKERS] Arrays of Complex Types
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. --- David Fetter wrote: On Fri, Mar 30, 2007 at 05:08:42PM -0400, Tom Lane wrote: David Fetter [EMAIL PROTECTED] writes: After several rounds of patches, it appears that it might be easier to create a new typtype entry, which I'll tentatively call 'a' because it seems a little fragile and a lot inelegant and hard to maintain to have typtype='c' and typrelid=InvalidOid mean, this is an array of complex types. Uh, wouldn't it be typtype = 'c' and typelem != 0 ? Right. The attached patch passes the current regression tests and at least to a smoke test level does what it's supposed to do. I'd really like to help refactor the whole array system to use 'a', tho. Cheers, D -- David Fetter [EMAIL PROTECTED] http://fetter.org/ phone: +1 415 235 3778AIM: dfetter666 Skype: davidfetter Remember to vote! Consider donating to PostgreSQL: http://www.postgresql.org/about/donate [ Attachment, skipping... ] ---(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 -- 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 2: Don't 'kill -9' the postmaster
Re: [HACKERS] Arrays of Complex Types
Bruce Momjian wrote: 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. So, hum, what happened to the idea of creating the array types only on demand? David Fetter wrote: On Fri, Mar 30, 2007 at 05:08:42PM -0400, Tom Lane wrote: David Fetter [EMAIL PROTECTED] writes: After several rounds of patches, it appears that it might be easier to create a new typtype entry, which I'll tentatively call 'a' because it seems a little fragile and a lot inelegant and hard to maintain to have typtype='c' and typrelid=InvalidOid mean, this is an array of complex types. Uh, wouldn't it be typtype = 'c' and typelem != 0 ? Right. The attached patch passes the current regression tests and at least to a smoke test level does what it's supposed to do. I'd really like to help refactor the whole array system to use 'a', tho. Cheers, -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: Macros for typtype (was Re: [HACKERS] Arrays of Complex Types)
Tom Lane wrote:
Why not enums? ;-)
Well, macros is how we do it elsewhere for char system columns.
I'm not sure we could rely on the behavior if we declared
pg_type.typtype as an enum type ... and if we don't, there's not
much point.
I was thinking C enums:
enum typtype_type {
TYPTYPE_BASE = 'b',
TYPTYPE_COMPOSITE = 'c',
TYPTYPE_DOMAIN = 'd',
TYPTYPE_ENUM = 'e',
TYPTYPE_PSEUDO = 'p'
};
I'm not sure if this is better.
--
Peter Eisentraut
http://developer.postgresql.org/~petere/
---(end of broadcast)---
TIP 4: Have you searched our list archives?
http://archives.postgresql.org
Re: Macros for typtype (was Re: [HACKERS] Arrays of Complex Types)
Peter Eisentraut [EMAIL PROTECTED] writes:
Tom Lane wrote:
I'm not sure we could rely on the behavior if we declared
pg_type.typtype as an enum type ... and if we don't, there's not
much point.
I was thinking C enums:
enum typtype_type {
TYPTYPE_BASE = 'b',
TYPTYPE_COMPOSITE = 'c',
TYPTYPE_DOMAIN = 'd',
TYPTYPE_ENUM = 'e',
TYPTYPE_PSEUDO = 'p'
};
I'm not sure if this is better.
What bothers me about that is I don't think the C spec mandates the
representation width. If we could guarantee that enum typtype_type
was 1 byte I'd be all for it.
regards, tom lane
---(end of broadcast)---
TIP 4: Have you searched our list archives?
http://archives.postgresql.org
Re: Macros for typtype (was Re: [HACKERS] Arrays of Complex Types)
Tom Lane wrote:
What bothers me about that is I don't think the C spec mandates the
representation width. If we could guarantee that enum typtype_type
was 1 byte I'd be all for it.
The width is 4 both for the macro and the enum case. Both
#define TYPTYPE_BASE 'b'
and
enum ... {
TYPTYPE_BASE = 'b',
effectively generate int constants named TYPTYPE_BASE with decimal value
98. So there are no storage advantages either way.
--
Peter Eisentraut
http://developer.postgresql.org/~petere/
---(end of broadcast)---
TIP 4: Have you searched our list archives?
http://archives.postgresql.org
Re: Macros for typtype (was Re: [HACKERS] Arrays of Complex Types)
Peter Eisentraut [EMAIL PROTECTED] writes:
Tom Lane wrote:
What bothers me about that is I don't think the C spec mandates the
representation width. If we could guarantee that enum typtype_type
was 1 byte I'd be all for it.
The width is 4 both for the macro and the enum case. Both
#define TYPTYPE_BASE 'b'
and
enum ... {
TYPTYPE_BASE = 'b',
effectively generate int constants named TYPTYPE_BASE with decimal value
98. So there are no storage advantages either way.
That's not accurate at all. The macro case gives you a constant you can only
use to initialize integer variables and members that are explicitly declared
with some integral type. If we consistently declare them char then they'll
be predictably 1 byte long.
The enum case does two things. It defines a syntactic meaning for the label,
*and* it defines a thing enum typtype which can be used to define variables
and members. If the latter is used then Tom is saying the standard doesn't
specify what width the variable or member will be.
We could use enum {} to define the labels and then make a rule that all actual
variables should be declared using char rather than declaring them as enum
typtype. But I fear somebody would get that wrong some day. The worst thing
is that it might work on their compiler or it might even work on all compilers
and just silently be causing things to be aligned differently than they
expect.
On the other hand it I don't really think it would cause any problems if
people stored their typtypes in integers. Except for the actual FormData_pg_*
structures the precise alignment doesn't actually matter for anything does it?
--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
---(end of broadcast)---
TIP 6: explain analyze is your friend
Re: Macros for typtype (was Re: [HACKERS] Arrays of Complex Types)
Gregory Stark wrote:
The width is 4 both for the macro and the enum case. Both
#define TYPTYPE_BASE 'b'
and
enum ... {
TYPTYPE_BASE = 'b',
effectively generate int constants named TYPTYPE_BASE with decimal
value 98. So there are no storage advantages either way.
That's not accurate at all.
How so?
The macro case gives you a constant you
can only use to initialize integer variables and members that are
explicitly declared with some integral type. If we consistently
declare them char then they'll be predictably 1 byte long.
But character constants are actually ints, so when you do what you
describe then the compiler has to generate code to copy a four-byte
integer into a single byte. (Of course that can be optimized away,
probably.)
The enum case does two things. It defines a syntactic meaning for the
label, *and* it defines a thing enum typtype which can be used to
define variables and members. If the latter is used then Tom is
saying the standard doesn't specify what width the variable or member
will be.
The standard says that enums are the same as ints. So when you assign
an enum label to a char variable, then compiler has to generate code to
copy a four-byte integer into a single byte. (Of course that can be
optimized away, probably.)
The fact that you can also declare variables of the enum type is not
under consideration here.
QED
--
Peter Eisentraut
http://developer.postgresql.org/~petere/
---(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: Macros for typtype (was Re: [HACKERS] Arrays of Complex Types)
Gregory Stark [EMAIL PROTECTED] writes:
We could use enum {} to define the labels and then make a rule that
all actual variables should be declared using char rather than
declaring them as enum typtype. But I fear somebody would get that
wrong some day.
Yeah, that seems to me to be just asking for trouble.
On the other hand it I don't really think it would cause any problems
if people stored their typtypes in integers. Except for the actual
FormData_pg_* structures the precise alignment doesn't actually matter
for anything does it?
The layout of the FormData struct is exactly the sticking point. If the
compiler makes the size or alignment of a struct field different from
what the tuple packing/unpacking code does for the corresponding column
type, we've got big trouble.
As for Peter's claim that the storage of an enum field is always int,
I think the C spec says otherwise. In 6.7.2.2 of C99 I see
[#4] Each enumerated type shall be compatible with an
integer type. The choice of type is
implementation-defined, 97) but shall be capable of
representing the values of all the members of the
enumeration. The enumerated type is incomplete until after
the } that terminates the list of enumerator declarations.
97) An implementation may delay the choice of which integer
type until all enumeration constants have been seen.
It seems clear to me that this authorizes, but *does not require*,
the compiler to store an enum field in a byte or short instead of
an int when all the declared values will fit. So if we tried to
do this, we'd have the problem of needing compiler-specific data
type information entered in pg_type.
Perhaps all C compilers do this alike, but how would we know?
Anyway the possible gain seems not worth the risk to me.
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: Macros for typtype (was Re: [HACKERS] Arrays of Complex Types)
Tom Lane wrote: It seems clear to me that this authorizes, but *does not require*, the compiler to store an enum field in a byte or short instead of an int when all the declared values will fit. So if we tried to do this, we'd have the problem of needing compiler-specific data type information entered in pg_type. FWIW, I never meant to suggest using enums tuple structures. I did, however, stumble over a case that appears to be handled similar to what I had in mind: see enum CoercionCodes in primnodes.h. Again, it's not really important, but it's interesting to see that there is precedent. -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: Macros for typtype (was Re: [HACKERS] Arrays of Complex Types)
Peter Eisentraut [EMAIL PROTECTED] writes: Tom Lane wrote: It seems clear to me that this authorizes, but *does not require*, the compiler to store an enum field in a byte or short instead of an int when all the declared values will fit. FWIW, I never meant to suggest using enums tuple structures. I did, however, stumble over a case that appears to be handled similar to what I had in mind: see enum CoercionCodes in primnodes.h. Again, it's not really important, but it's interesting to see that there is precedent. AFAIK, we don't store CoercionCodes in any system catalog columns. But now that you mention it there is at least one place where we do it like that: pg_depend.deptype is a char but its values are defined by enum DependencyType. I have some recollection of doing it that way because I was concerned that most places that cared about dependency types should be switch statements that covered all the possible values (which is pretty much the only benefit you get from doing it that way). Having just gone over the typtype uses, there are only a couple of places where we'd win from having that sort of compile-time check for typtype. regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Macros for typtype (was Re: [HACKERS] Arrays of Complex Types)
I wrote: David Fetter [EMAIL PROTECTED] writes: What parts of the code would need a once-over? A lot :-( ... probably every place that touches typtype or typelem would need at least a look. It'd be a good idea to take the opportunity to start using macros for the values of typtype, as we do for relkind but for some reason never adopted for typtype. I just realized that I need to check every usage of typtype to be sure that the enums patch is sane. So, barring objection, I intend to take this opportunity to make the code stop referring directly to 'b', 'c' etc whereever possible. Any objections to these names? #define TYPTYPE_BASE'b' #define TYPTYPE_COMPOSITE 'c' #define TYPTYPE_DOMAIN 'd' #define TYPTYPE_ENUM'e' #define TYPTYPE_PSEUDO 'p' regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: Macros for typtype (was Re: [HACKERS] Arrays of Complex Types)
Tom Lane wrote: I wrote: David Fetter [EMAIL PROTECTED] writes: What parts of the code would need a once-over? A lot :-( ... probably every place that touches typtype or typelem would need at least a look. It'd be a good idea to take the opportunity to start using macros for the values of typtype, as we do for relkind but for some reason never adopted for typtype. I just realized that I need to check every usage of typtype to be sure that the enums patch is sane. So, barring objection, I intend to take this opportunity to make the code stop referring directly to 'b', 'c' etc whereever possible. Any objections to these names? #define TYPTYPE_BASE'b' #define TYPTYPE_COMPOSITE 'c' #define TYPTYPE_DOMAIN 'd' #define TYPTYPE_ENUM'e' #define TYPTYPE_PSEUDO 'p' I like macros. ;-) -- 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 2: Don't 'kill -9' the postmaster
Re: Macros for typtype (was Re: [HACKERS] Arrays of Complex Types)
On Sat, Mar 31, 2007 at 07:13:20PM -0400, Bruce Momjian wrote: Tom Lane wrote: I wrote: David Fetter [EMAIL PROTECTED] writes: What parts of the code would need a once-over? A lot :-( ... probably every place that touches typtype or typelem would need at least a look. It'd be a good idea to take the opportunity to start using macros for the values of typtype, as we do for relkind but for some reason never adopted for typtype. I just realized that I need to check every usage of typtype to be sure that the enums patch is sane. So, barring objection, I intend to take this opportunity to make the code stop referring directly to 'b', 'c' etc whereever possible. Any objections to these names? #define TYPTYPE_BASE'b' #define TYPTYPE_COMPOSITE 'c' #define TYPTYPE_DOMAIN 'd' #define TYPTYPE_ENUM'e' #define TYPTYPE_PSEUDO 'p' I like macros. ;-) Macros are great. :) What say we put one in pre-emptively for TYPTYPE_ARRAY? Cheers, D -- David Fetter [EMAIL PROTECTED] http://fetter.org/ phone: +1 415 235 3778AIM: dfetter666 Skype: davidfetter Remember to vote! Consider donating to PostgreSQL: http://www.postgresql.org/about/donate ---(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: Macros for typtype (was Re: [HACKERS] Arrays of Complex Types)
David Fetter [EMAIL PROTECTED] writes: What say we put one in pre-emptively for TYPTYPE_ARRAY? When and if the patch appears, you can add it ;-). I'm just intending a search-and-replace at the moment. 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: Macros for typtype (was Re: [HACKERS] Arrays of Complex Types)
On Sat, Mar 31, 2007 at 07:58:34PM -0400, Tom Lane wrote:
David Fetter [EMAIL PROTECTED] writes:
What say we put one in pre-emptively for TYPTYPE_ARRAY?
When and if the patch appears, you can add it ;-). I'm just intending a
search-and-replace at the moment.
Like this?
Cheers,
D
--
David Fetter [EMAIL PROTECTED] http://fetter.org/
phone: +1 415 235 3778AIM: dfetter666
Skype: davidfetter
Remember to vote!
Consider donating to PostgreSQL: http://www.postgresql.org/about/donate
Index: src/backend/catalog/heap.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/catalog/heap.c,v
retrieving revision 1.317
diff -c -r1.317 heap.c
*** src/backend/catalog/heap.c 14 Feb 2007 01:58:56 - 1.317
--- src/backend/catalog/heap.c 1 Apr 2007 00:17:20 -
***
*** 412,418
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
errmsg(column \%s\ has type \unknown\,
attname),
errdetail(Proceeding with relation creation
anyway.)));
! else if (att_typtype == 'p')
{
/* Special hack for pg_statistic: allow ANYARRAY during initdb
*/
if (atttypid != ANYARRAYOID || IsUnderPostmaster)
--- 412,418
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
errmsg(column \%s\ has type \unknown\,
attname),
errdetail(Proceeding with relation creation
anyway.)));
! else if (att_typtype == TYPTYPE_PSEUDO)
{
/* Special hack for pg_statistic: allow ANYARRAY during initdb
*/
if (atttypid != ANYARRAYOID || IsUnderPostmaster)
Index: src/backend/commands/typecmds.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/commands/typecmds.c,v
retrieving revision 1.100
diff -c -r1.100 typecmds.c
*** src/backend/commands/typecmds.c 14 Feb 2007 01:58:57 - 1.100
--- src/backend/commands/typecmds.c 1 Apr 2007 00:17:20 -
***
*** 625,631
* might be made to work in the future, but not today.
*/
typtype = baseType-typtype;
! if (typtype != 'b' typtype != 'd')
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg(\%s\ is not a valid base type for a
domain,
--- 625,631
* might be made to work in the future, but not today.
*/
typtype = baseType-typtype;
! if (typtype != TYPTYPE_BASE typtype != TYPTYPE_DOMAIN)
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg(\%s\ is not a valid base type for a
domain,
***
*** 907,913
/* Check that this is actually a domain */
typtype = ((Form_pg_type) GETSTRUCT(tup))-typtype;
! if (typtype != 'd')
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg(\%s\ is not a domain,
--- 907,913
/* Check that this is actually a domain */
typtype = ((Form_pg_type) GETSTRUCT(tup))-typtype;
! if (typtype != TYPTYPE_DOMAIN)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg(\%s\ is not a domain,
***
*** 1835,1841
Form_pg_type typTup = (Form_pg_type) GETSTRUCT(tup);
/* Check that this is actually a domain */
! if (typTup-typtype != 'd')
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg(\%s\ is not a domain,
--- 1835,1841
Form_pg_type typTup = (Form_pg_type) GETSTRUCT(tup);
/* Check that this is actually a domain */
! if (typTup-typtype != TYPTYPE_DOMAIN)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg(\%s\ is not a domain,
***
*** 2021,2027
elog(ERROR, cache lookup failed for type %u, typeOid);
typTup = (Form_pg_type) GETSTRUCT(tup);
! if (typTup-typtype != 'd')
{
/* Not a domain, so done */
ReleaseSysCache(tup);
--- 2021,2027
elog(ERROR, cache lookup failed for type %u, typeOid);
typTup = (Form_pg_type) GETSTRUCT(tup);
! if (typTup-typtype != TYPTYPE_DOMAIN)
{
/* Not a domain, so done */
ReleaseSysCache(tup);
***
*** 2148,2154
Re: Macros for typtype (was Re: [HACKERS] Arrays of Complex Types)
Bruce Momjian wrote: I just realized that I need to check every usage of typtype to be sure that the enums patch is sane. So, barring objection, I intend to take this opportunity to make the code stop referring directly to 'b', 'c' etc whereever possible. Any objections to these names? #define TYPTYPE_BASE'b' #define TYPTYPE_COMPOSITE 'c' #define TYPTYPE_DOMAIN 'd' #define TYPTYPE_ENUM'e' #define TYPTYPE_PSEUDO 'p' I like macros. ;-) Why not enums? ;-) -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(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: Macros for typtype (was Re: [HACKERS] Arrays of Complex Types)
Peter Eisentraut [EMAIL PROTECTED] writes: I just realized that I need to check every usage of typtype to be sure that the enums patch is sane. So, barring objection, I intend to take this opportunity to make the code stop referring directly to 'b', 'c' etc whereever possible. Any objections to these names? Why not enums? ;-) Well, macros is how we do it elsewhere for char system columns. I'm not sure we could rely on the behavior if we declared pg_type.typtype as an enum type ... and if we don't, there's not much point. regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] Arrays of Complex Types
On Fri, Mar 02, 2007 at 03:40:16PM -0800, David Fetter wrote: Folks, I'd like to take the TODO item that reads, Add support for arrays of complex types, but before I start patching, I'd like to see whether what I'm about to do makes any sense: After several rounds of patches, it appears that it might be easier to create a new typtype entry, which I'll tentatively call 'a' because it seems a little fragile and a lot inelegant and hard to maintain to have typtype='c' and typrelid=InvalidOid mean, this is an array of complex types. I'd like to see about making this new typtype available for arrays of DOMAINs eventually, but that's not a requirement right this instant. What parts of the code would need a once-over? Cheers, D -- David Fetter [EMAIL PROTECTED] http://fetter.org/ phone: +1 415 235 3778AIM: dfetter666 Skype: davidfetter Remember to vote! Consider donating to PostgreSQL: http://www.postgresql.org/about/donate ---(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: [HACKERS] Arrays of Complex Types
David Fetter [EMAIL PROTECTED] writes: After several rounds of patches, it appears that it might be easier to create a new typtype entry, which I'll tentatively call 'a' because it seems a little fragile and a lot inelegant and hard to maintain to have typtype='c' and typrelid=InvalidOid mean, this is an array of complex types. Uh, wouldn't it be typtype = 'c' and typelem != 0 ? I'd like to see about making this new typtype available for arrays of DOMAINs eventually, but that's not a requirement right this instant. Hmm. It might not be a bad idea to switch to 'a' for arrays over regular scalar types too. Right now we have some klugy rules involving looking at typlen to decide whether an array type is a normal array. (There are also some special subscriptable types like name and point, which should continue to not use 'a' because they are not general purpose arrays. So the 'a' marker wouldn't be entirely redundant with typelem being nonzero, rather checking for 'a' would replace the places where we test both typelem and typlen.) OTOH this is a lot of hacking for something that I'm not convinced is really needed. Anyway, the point is that I dislike the idea of doing arrays for complex types differently from those for scalars --- either both should use a new typtype, or neither. If you try to do it differently then you'll have more complexity, not less, since there are a lot of places that shouldn't need to care. get_element_type() is an example. What parts of the code would need a once-over? A lot :-( ... probably every place that touches typtype or typelem would need at least a look. It'd be a good idea to take the opportunity to start using macros for the values of typtype, as we do for relkind but for some reason never adopted for typtype. regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] Arrays of Complex Types
On 2007-03-27, David Fetter [EMAIL PROTECTED] wrote: Per further discussion with Andrew of Supernews and Merlin Moncure, I've added a check for compound types and moved the creation of the array type from DefineRelation in backend/commands/tablecmds.c to heap_create_with_catalog in backend/catalog/heap.c. You've still got the usage of the relation OID and the relation _type_ OID reversed. The array element type that you pass to TypeCreate must be the _type_ OID. -- Andrew, Supernews http://www.supernews.com - individual and corporate NNTP services ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] Arrays of Complex Types
On Wed, Mar 28, 2007 at 07:05:24AM -, Andrew - Supernews wrote: On 2007-03-27, David Fetter [EMAIL PROTECTED] wrote: Per further discussion with Andrew of Supernews and Merlin Moncure, I've added a check for compound types and moved the creation of the array type from DefineRelation in backend/commands/tablecmds.c to heap_create_with_catalog in backend/catalog/heap.c. You've still got the usage of the relation OID and the relation _type_ OID reversed. The array element type that you pass to TypeCreate must be the _type_ OID. The attached patch takes it down to two regression test failures, also attached: The first is in type_sanity, which basically doesn't understand that complex types now have array types associated with them and thinks they're orphan array types, so it's actually the test that's not right. The second is in alter_table where ALTER TABLE ... SET SCHEMA doesn't pick up the array types associated with the tables. Andrew at Supernews also noticed that in general, the array type doesn't change schemas when its base type does. Is this the intended behavior? If not, should we change it globally? Cheers, D -- David Fetter [EMAIL PROTECTED] http://fetter.org/ phone: +1 415 235 3778AIM: dfetter666 Skype: davidfetter Remember to vote! Consider donating to PostgreSQL: http://www.postgresql.org/about/donate ? GNUmakefile ? array_of_complex.diff ? config.log ? config.status ? contrib/spi/.deps ? src/Makefile.global ? src/backend/postgres ? src/backend/access/common/.deps ? src/backend/access/gin/.deps ? src/backend/access/gist/.deps ? src/backend/access/hash/.deps ? src/backend/access/heap/.deps ? src/backend/access/index/.deps ? src/backend/access/nbtree/.deps ? src/backend/access/transam/.deps ? src/backend/bootstrap/.deps ? src/backend/catalog/.deps ? src/backend/catalog/postgres.bki ? src/backend/catalog/postgres.description ? src/backend/catalog/postgres.shdescription ? src/backend/commands/.deps ? src/backend/executor/.deps ? src/backend/lib/.deps ? src/backend/libpq/.deps ? src/backend/main/.deps ? src/backend/nodes/.deps ? src/backend/optimizer/geqo/.deps ? src/backend/optimizer/path/.deps ? src/backend/optimizer/plan/.deps ? src/backend/optimizer/prep/.deps ? src/backend/optimizer/util/.deps ? src/backend/parser/.deps ? src/backend/port/.deps ? src/backend/postmaster/.deps ? src/backend/regex/.deps ? src/backend/rewrite/.deps ? src/backend/storage/buffer/.deps ? src/backend/storage/file/.deps ? src/backend/storage/freespace/.deps ? src/backend/storage/ipc/.deps ? src/backend/storage/large_object/.deps ? src/backend/storage/lmgr/.deps ? src/backend/storage/page/.deps ? src/backend/storage/smgr/.deps ? src/backend/tcop/.deps ? src/backend/utils/.deps ? src/backend/utils/adt/.deps ? src/backend/utils/cache/.deps ? src/backend/utils/error/.deps ? src/backend/utils/fmgr/.deps ? src/backend/utils/hash/.deps ? src/backend/utils/init/.deps ? src/backend/utils/mb/.deps ? src/backend/utils/mb/conversion_procs/conversion_create.sql ? src/backend/utils/mb/conversion_procs/ascii_and_mic/.deps ? src/backend/utils/mb/conversion_procs/ascii_and_mic/libascii_and_mic.so.0.0 ? src/backend/utils/mb/conversion_procs/cyrillic_and_mic/.deps ? src/backend/utils/mb/conversion_procs/cyrillic_and_mic/libcyrillic_and_mic.so.0.0 ? src/backend/utils/mb/conversion_procs/euc_cn_and_mic/.deps ? src/backend/utils/mb/conversion_procs/euc_cn_and_mic/libeuc_cn_and_mic.so.0.0 ? src/backend/utils/mb/conversion_procs/euc_jis_2004_and_shift_jis_2004/.deps ? src/backend/utils/mb/conversion_procs/euc_jis_2004_and_shift_jis_2004/libeuc_jis_2004_and_shift_jis_2004.so.0.0 ? src/backend/utils/mb/conversion_procs/euc_jp_and_sjis/.deps ? src/backend/utils/mb/conversion_procs/euc_jp_and_sjis/libeuc_jp_and_sjis.so.0.0 ? src/backend/utils/mb/conversion_procs/euc_kr_and_mic/.deps ? src/backend/utils/mb/conversion_procs/euc_kr_and_mic/libeuc_kr_and_mic.so.0.0 ? src/backend/utils/mb/conversion_procs/euc_tw_and_big5/.deps ? src/backend/utils/mb/conversion_procs/euc_tw_and_big5/libeuc_tw_and_big5.so.0.0 ? src/backend/utils/mb/conversion_procs/latin2_and_win1250/.deps ? src/backend/utils/mb/conversion_procs/latin2_and_win1250/liblatin2_and_win1250.so.0.0 ? src/backend/utils/mb/conversion_procs/latin_and_mic/.deps ? src/backend/utils/mb/conversion_procs/latin_and_mic/liblatin_and_mic.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_ascii/.deps ? src/backend/utils/mb/conversion_procs/utf8_and_ascii/libutf8_and_ascii.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_big5/.deps ? src/backend/utils/mb/conversion_procs/utf8_and_big5/libutf8_and_big5.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_cyrillic/.deps ? src/backend/utils/mb/conversion_procs/utf8_and_cyrillic/libutf8_and_cyrillic.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_euc_cn/.deps ? src/backend/utils/mb/conversion_procs/utf8_and_euc_cn/libutf8_and_euc_cn.so.0.0 ?
Re: [HACKERS] Arrays of Complex Types
David Fetter wrote: The first is in type_sanity, which basically doesn't understand that complex types now have array types associated with them and thinks they're orphan array types, so it's actually the test that's not right. Hmm, I question the usefulness of automatically creating array types for all relation types that are created -- the catalog bloat seems a bit too much. An array of pg_autovacuum for example, does that make sense? I'm not sure what was the reaction to having an CREATE TYPE foo ARRAY OF bar command of some kind? I think this was discussed but not explicitely rejected, or was it? -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support ---(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: [HACKERS] Arrays of Complex Types
Alvaro Herrera wrote: David Fetter wrote: The first is in type_sanity, which basically doesn't understand that complex types now have array types associated with them and thinks they're orphan array types, so it's actually the test that's not right. Hmm, I question the usefulness of automatically creating array types for all relation types that are created -- the catalog bloat seems a bit too much. An array of pg_autovacuum for example, does that make sense? I'm not sure what was the reaction to having an CREATE TYPE foo ARRAY OF bar command of some kind? I think this was discussed but not explicitely rejected, or was it? It certainly seems rather inconsistent to have array types autocreated for some types but not others. But unless we create them for all types then I think we need a command such as you suggest. How much bloat will this really be? If it's not used it won't get into the type cache. I find it hard to believe there will be any very significant performance effect. cheers andrew ---(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: [HACKERS] Arrays of Complex Types
Alvaro Herrera [EMAIL PROTECTED] writes: Hmm, I question the usefulness of automatically creating array types for all relation types that are created -- the catalog bloat seems a bit too much. An array of pg_autovacuum for example, does that make sense? Not only that, it won't even work for pg_statistic, which has got a special kluge to allow anyarray in a place where it usually mustn't go. I'm not sure what was the reaction to having an CREATE TYPE foo ARRAY OF bar command of some kind? I think this was discussed but not explicitely rejected, or was it? I think this is a much better idea than automatically creating a pile of usually-useless types. In the long run maybe we should even migrate to the assumption that array types aren't automatically created? If we think this way, it changes the ground rules for Andrew's question about whether an array type ought to be affected by ALTER TYPE SET SCHEMA on its base type --- it starts to look more like an independent entity than an auxiliary component. I'm not really sure which answer I like better. One point here is that currently the system depends on the naming convention foo[] is named _foo to be able to find the array type from the base type. The syntax you suggest would break that. We could fix it by adding More Stuff to pg_type, but I wonder whether it's worth it, compared to say CREATE ARRAY TYPE FOR foo Also, at the moment ALTER TYPE SET SCHEMA is certainly broken because it destroys this naming convention ... we either abandon the convention or fix SET SCHEMA. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] Arrays of Complex Types
On Wed, Mar 28, 2007 at 01:33:56PM -0400, Andrew Dunstan wrote: Alvaro Herrera wrote: David Fetter wrote: The first is in type_sanity, which basically doesn't understand that complex types now have array types associated with them and thinks they're orphan array types, so it's actually the test that's not right. Hmm, I question the usefulness of automatically creating array types for all relation types that are created -- the catalog bloat seems a bit too much. An array of pg_autovacuum for example, does that make sense? I'm not sure what was the reaction to having an CREATE TYPE foo ARRAY OF bar command of some kind? I think this was discussed but not explicitely rejected, or was it? It certainly seems rather inconsistent to have array types autocreated for some types but not others. This was my thought in the latest version of the patch. But unless we create them for all types then I think we need a command such as you suggest. How much bloat will this really be? If it's not used it won't get into the type cache. I find it hard to believe there will be any very significant performance effect. So do I, but how would we check this? Cheers, D -- David Fetter [EMAIL PROTECTED] http://fetter.org/ phone: +1 415 235 3778AIM: dfetter666 Skype: davidfetter Remember to vote! Consider donating to PostgreSQL: http://www.postgresql.org/about/donate ---(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: [HACKERS] Arrays of Complex Types
Tom Lane [EMAIL PROTECTED] writes: CREATE ARRAY TYPE FOR foo I also made a suggestion along the way that we never create array types automatically except for domains. Ie, we don't need a new command, we just document that what you do if you want to create an array of something is create a domain for it then use arrays of that domain. I'm not sure whether having to create a new command is cleaner or less clean than overloading an existing command with two purposes. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com ---(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: [HACKERS] Arrays of Complex Types
Gregory Stark [EMAIL PROTECTED] writes: Tom Lane [EMAIL PROTECTED] writes: CREATE ARRAY TYPE FOR foo I also made a suggestion along the way that we never create array types automatically except for domains. That seems awfully strange, not to mention very non-backwards-compatible since it exactly reverses what happens now. I'd be willing to consider it if a domain were a zero-cost addition to the equation, but it is not --- every operation on a domain has to check to see if there are constraints to enforce. You shouldn't have to buy into that overhead to have an array. regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] Arrays of Complex Types
On Wed, Mar 28, 2007 at 03:24:26PM -0400, Tom Lane wrote: Gregory Stark [EMAIL PROTECTED] writes: Tom Lane [EMAIL PROTECTED] writes: CREATE ARRAY TYPE FOR foo I also made a suggestion along the way that we never create array types automatically except for domains. That seems awfully strange, not to mention very non-backwards-compatible since it exactly reverses what happens now. I'd be willing to consider it if a domain were a zero-cost addition to the equation, but it is not --- every operation on a domain has to check to see if there are constraints to enforce. You shouldn't have to buy into that overhead to have an array. The way I see the big picture, complex types, arrays and domains should all compose without limit, as in arrays of domains of complex types, etc. The SQL standard even has something like our SETOF (which should probably be called BAGOF, but let's not go there just now ;) in the form of MULTISET, and that, too, should eventually be in the above mix. I'm not advocating the idea that people should *store* those compositions--if it were just up to me, I'd disallow it--but they're very handy for input and output :) Cheers, D -- David Fetter [EMAIL PROTECTED] http://fetter.org/ phone: +1 415 235 3778AIM: dfetter666 Skype: davidfetter Remember to vote! Consider donating to PostgreSQL: http://www.postgresql.org/about/donate ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] Arrays of Complex Types
On Tue, Mar 27, 2007 at 11:08:44AM -0700, David Fetter wrote:
On Sun, Mar 25, 2007 at 10:18:14PM -0400, Tom Lane wrote:
David Fetter [EMAIL PROTECTED] writes:
I've written up a patch intended to implement this on the
non-pg_catalog tables and VIEWs, but while it builds, it doesn't
initdb. Enclosed are the patch and the error log.
Any hints as to what I might look at?
creating template1 database in
/var/lib/pgsql/pgsql/src/test/regress/./tmp_check/data/base/1 ... ok
initializing pg_authid ... ok
initializing dependencies ... ok
creating system views ... ok
loading system objects' descriptions ... FATAL: cache lookup failed for
type 11096
child process exited with exit code 1
That step of initdb creates a TEMP table ... maybe your patch
doesn't work for temp tables? Anyway, you're certainly far enough
along there that you could fire up the postmaster and reproduce
the error in a normal debugging environment.
I've done that, and thanks to Andrew of Supernews, I've got a
slightly better patch, albeit one that bombs out at the same spot.
In the patch attached, it appears that TypeCreate is not doing the
right thing in pg_depend, either because I'm not invoking it right
or because it needs more machinery.
Any ideas?
Pardon the self-follow-up.
Per further discussion with Andrew of Supernews and Merlin Moncure,
I've added a check for compound types and moved the creation of the
array type from DefineRelation in backend/commands/tablecmds.c to
heap_create_with_catalog in backend/catalog/heap.c.
It now initdb's successfully, but fails on a lot of regression tests.
Please find attached the new patch vs. CVS TIP and the regression test
output.
Am I on the right track here?
Cheers,
D
--
David Fetter [EMAIL PROTECTED] http://fetter.org/
phone: +1 415 235 3778AIM: dfetter666
Skype: davidfetter
Remember to vote!
Consider donating to PostgreSQL: http://www.postgresql.org/about/donate
Index: src/backend/catalog/heap.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/catalog/heap.c,v
retrieving revision 1.317
diff -c -r1.317 heap.c
*** src/backend/catalog/heap.c 14 Feb 2007 01:58:56 - 1.317
--- src/backend/catalog/heap.c 27 Mar 2007 19:33:52 -
***
*** 45,50
--- 45,51
#include catalog/pg_statistic.h
#include catalog/pg_type.h
#include commands/tablecmds.h
+ #include commands/typecmds.h
#include miscadmin.h
#include optimizer/clauses.h
#include optimizer/var.h
***
*** 763,768
--- 764,770
Relationpg_class_desc;
Relationnew_rel_desc;
Oid new_type_oid;
+ char *relarrayname;
pg_class_desc = heap_open(RelationRelationId, RowExclusiveLock);
***
*** 815,820
--- 817,857
relnamespace,
relid,
relkind);
+ /*
+* Add in the corresponding array types if appropriate.
+*/
+ if (
+ relnamespace != 11 /* pg_catalog's namespace */
+ (relkind == 'r' || relkind == 'v' || relkind == 'c')
+ )
+ {
+ relarrayname = makeArrayTypeName(relname);
+ TypeCreate(relarrayname,/* Array type name */
+ relnamespace,/* Same
namespace as parent */
+ new_type_oid,/* relation's
type oid */
+ 0, /*
relkind, also N/A here */
+ -1, /*
Internal size, unlimited */
+ 'c', /* It's
a complex type */
+ DEFAULT_TYPDELIM,/* Use the default */
+ F_ARRAY_IN, /* Macro for
array input procedure */
+ F_ARRAY_OUT, /* Macro for
array output procedure */
+ F_ARRAY_RECV,/* Macro for
array receive (binary input) procedure */
+ F_ARRAY_SEND,/* Macro for
array send (binary output) procedure */
+ -1, /* No
input typmod */
+ -1, /* No
output typmod */
+ InvalidOid, /* Default
ANALYZE procedure */
+ relid, /* The
OID just created */
+
Re: [HACKERS] Arrays of Complex Types
David Fetter wrote: It now initdb's successfully, but fails on a lot of regression tests. Please find attached the new patch vs. CVS TIP and the regression test output. The regression test output is useless without seeing the regression diffs. cheers andrew ---(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: [HACKERS] Arrays of Complex Types
On Sun, Mar 25, 2007 at 10:18:14PM -0400, Tom Lane wrote:
David Fetter [EMAIL PROTECTED] writes:
I've written up a patch intended to implement this on the
non-pg_catalog tables and VIEWs, but while it builds, it doesn't
initdb. Enclosed are the patch and the error log.
Any hints as to what I might look at?
creating template1 database in
/var/lib/pgsql/pgsql/src/test/regress/./tmp_check/data/base/1 ... ok
initializing pg_authid ... ok
initializing dependencies ... ok
creating system views ... ok
loading system objects' descriptions ... FATAL: cache lookup failed for
type 11096
child process exited with exit code 1
That step of initdb creates a TEMP table ... maybe your patch doesn't
work for temp tables? Anyway, you're certainly far enough along there
that you could fire up the postmaster and reproduce the error in a
normal debugging environment.
I've done that, and thanks to Andrew of Supernews, I've got a slightly
better patch, albeit one that bombs out at the same spot. In the
patch attached, it appears that TypeCreate is not doing the right
thing in pg_depend, either because I'm not invoking it right or
because it needs more machinery.
Any ideas?
Cheers,
David.
--
David Fetter [EMAIL PROTECTED] http://fetter.org/
phone: +1 415 235 3778AIM: dfetter666
Skype: davidfetter
Remember to vote!
Consider donating to PostgreSQL: http://www.postgresql.org/about/donate
Index: src/backend/commands/tablecmds.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/commands/tablecmds.c,v
retrieving revision 1.218
diff -c -r1.218 tablecmds.c
*** src/backend/commands/tablecmds.c19 Mar 2007 23:38:29 - 1.218
--- src/backend/commands/tablecmds.c27 Mar 2007 17:59:36 -
***
*** 287,298
Datum reloptions;
ListCell *listptr;
AttrNumber attnum;
/*
!* Truncate relname to appropriate length (probably a waste of time, as
!* parser should have done this already).
*/
! StrNCpy(relname, stmt-relation-relname, NAMEDATALEN);
/*
* Check consistency of arguments
--- 287,310
Datum reloptions;
ListCell *listptr;
AttrNumber attnum;
+ char *relarrayname;
/*
!* Truncate relname to appropriate length (probably a waste of time, as
*
!* parser should have done this already). Because tables and views now
get
!* an array type, this depends on the relkind.
*/
! if (
! namespaceId != 11 /* pg_catalog's namespace */
! (relkind == 'r' || relkind == 'v')
! )
! {
! StrNCpy(relname, stmt-relation-relname, NAMEDATALEN-2);
! }
! else
! {
! StrNCpy(relname, stmt-relation-relname, NAMEDATALEN);
! }
/*
* Check consistency of arguments
***
*** 496,501
--- 508,549
*/
relation_close(rel, NoLock);
+ /*
+* Add the array type if appropriate.
+*/
+ if (
+ namespaceId != 11 /* pg_catalog's namespace */
+ (relkind == 'r' || relkind == 'v')
+ )
+ {
+ relarrayname = makeArrayTypeName(relname);
+ TypeCreate(relarrayname,/* Array type name */
+ namespaceId, /* Same
namespace as parent */
+ relationId, /* relation
oid, N/A here */
+ 0, /*
relkind, also N/A here */
+ -1, /*
Internal size, unlimited */
+ 'c', /* It's
a complex type */
+ DEFAULT_TYPDELIM,/* Use the default */
+ F_ARRAY_IN, /* Macro for
array input procedure */
+ F_ARRAY_OUT, /* Macro for
array output procedure */
+ F_ARRAY_RECV,/* Macro for
array receive (binary input) procedure */
+ F_ARRAY_SEND,/* Macro for
array send (binary output) procedure */
+ -1, /* No
input typmod */
+ -1, /* No
output typmod */
+ InvalidOid, /* Default
ANALYZE procedure */
+ relationId, /* The OID just
created */
+ InvalidOid, /* No base
type--this isn't a DOMAIN */
+
Re: [HACKERS] Arrays of Complex Types
I wrote:
I'd like to take the TODO item that reads, Add support for arrays of
complex types, but before I start patching, I'd like to see whether
what I'm about to do makes any sense:
I've written up a patch intended to implement this on the
non-pg_catalog tables and VIEWs, but while it builds, it doesn't
initdb. Enclosed are the patch and the error log.
Any hints as to what I might look at?
Cheers,
David.
1. In src/backend/commands/tablecmds.c, change DefineRelation as
follows:
* After the first call to heap_create_with_catalog, construct and
do another call to for the array type.
* Add an appropriate pg_depend entry.
2. Change RemoveRelation to reflect the above.
3. Change TypeRename appropriately, whatever that turns out to be.
Does the above make sense? Have I missed anything critical?
Cheers,
D
--
David Fetter [EMAIL PROTECTED] http://fetter.org/
phone: +1 415 235 3778AIM: dfetter666
Skype: davidfetter
Remember to vote!
Consider donating to PostgreSQL: http://www.postgresql.org/about/donate
---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster
--
David Fetter [EMAIL PROTECTED] http://fetter.org/
phone: +1 415 235 3778AIM: dfetter666
Skype: davidfetter
Remember to vote!
Consider donating to PostgreSQL: http://www.postgresql.org/about/donate
Index: src/backend/commands/tablecmds.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/commands/tablecmds.c,v
retrieving revision 1.218
diff -c -r1.218 tablecmds.c
*** src/backend/commands/tablecmds.c19 Mar 2007 23:38:29 - 1.218
--- src/backend/commands/tablecmds.c26 Mar 2007 00:30:08 -
***
*** 287,298
Datum reloptions;
ListCell *listptr;
AttrNumber attnum;
/*
!* Truncate relname to appropriate length (probably a waste of time, as
!* parser should have done this already).
*/
! StrNCpy(relname, stmt-relation-relname, NAMEDATALEN);
/*
* Check consistency of arguments
--- 287,307
Datum reloptions;
ListCell *listptr;
AttrNumber attnum;
+ char *relarrayname;
/*
!* Truncate relname to appropriate length (probably a waste of time, as
*
!* parser should have done this already). Because tables and views now
get
!* an array type, this depends on the relkind.
*/
! if (relkind == 'r' || relkind == 'v' !IsBootstrapProcessingMode())
! {
! StrNCpy(relname, stmt-relation-relname, NAMEDATALEN-2);
! }
! else
! {
! StrNCpy(relname, stmt-relation-relname, NAMEDATALEN);
! }
/*
* Check consistency of arguments
***
*** 496,501
--- 505,543
*/
relation_close(rel, NoLock);
+ /*
+* Add the array type if appropriate.
+*/
+ if (relkind == 'r' || relkind == 'v' !IsBootstrapProcessingMode())
+ {
+ relarrayname = makeArrayTypeName(relname);
+ TypeCreate(relarrayname,/* Array type name */
+ namespaceId, /* Same
namespace as parent */
+ InvalidOid, /* relation
oid, N/A here */
+ 0, /*
relkind, also N/A here */
+ -1, /*
Internal size, unlimited */
+ 'c', /* It's
a complex type */
+ DEFAULT_TYPDELIM,/* Use the default */
+ F_ARRAY_IN, /* Macro for
array input procedure */
+ F_ARRAY_OUT, /* Macro for
array output procedure */
+ F_ARRAY_RECV,/* Macro for
array receive (binary input) procedure */
+ F_ARRAY_SEND,/* Macro for
array send (binary output) procedure */
+ -1, /* No
input typmod */
+ -1, /* No
output typmod */
+ InvalidOid, /* Default
ANALYZE procedure */
+ relationId, /* The OID just
created */
+ InvalidOid, /* No base
type--this isn't a DOMAIN */
+ NULL,/* No
default type value */
+
Re: [HACKERS] Arrays of Complex Types
David Fetter [EMAIL PROTECTED] writes: I've written up a patch intended to implement this on the non-pg_catalog tables and VIEWs, but while it builds, it doesn't initdb. Enclosed are the patch and the error log. Any hints as to what I might look at? creating template1 database in /var/lib/pgsql/pgsql/src/test/regress/./tmp_check/data/base/1 ... ok initializing pg_authid ... ok initializing dependencies ... ok creating system views ... ok loading system objects' descriptions ... FATAL: cache lookup failed for type 11096 child process exited with exit code 1 That step of initdb creates a TEMP table ... maybe your patch doesn't work for temp tables? Anyway, you're certainly far enough along there that you could fire up the postmaster and reproduce the error in a normal debugging environment. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] Arrays of Complex Types
On Fri, Mar 02, 2007 at 06:59:50PM -0500, Tom Lane wrote: David Fetter [EMAIL PROTECTED] writes: 1. In src/backend/commands/tablecmds.c, change DefineRelation as follows: * After the first call to heap_create_with_catalog, construct and do another call to for the array type. I'm still not happy about the idea of doing this for every relation (and doing it for sequences and indexes would be the height of wastefulness). How about we only do it for composite types? How about doing it for user-defined tables, views and composite types, and skipping ? * Add an appropriate pg_depend entry. 2. Change RemoveRelation to reflect the above. You only need one of those two: either you drop by hand or you let the dependency machinery deal with it. Not both. pg_depend it is, then :) Does the above make sense? Have I missed anything critical? Ummm ... making it actually work? Possibly that just falls out, but I'm not sure. If it turns out that it does Just Work, you might take a stab at arrays of domains too. OK. I noticed something in src/backend/commands/tablecmds.c which worries me, namely that it ignores functions and views. It should at least be checking that the typeoid isn't in pg_proc.prorettype or pg_proc.proargtypes, and if possible, the DECLARE section of pl/pgsql functions. Is there a way to do SQL at that place in the back-end, or is there some different kind of Magick(TM) needed to access these kinds of things at that level? Cheers, D -- David Fetter [EMAIL PROTECTED] http://fetter.org/ phone: +1 415 235 3778AIM: dfetter666 Skype: davidfetter Remember to vote! Consider donating to PostgreSQL: http://www.postgresql.org/about/donate ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] Arrays of Complex Types
David Fetter [EMAIL PROTECTED] writes: I noticed something in src/backend/commands/tablecmds.c which worries me, namely that it ignores functions and views. What? regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] Arrays of Complex Types
On Tue, Mar 06, 2007 at 04:14:07PM -0500, Tom Lane wrote: David Fetter [EMAIL PROTECTED] writes: I noticed something in src/backend/commands/tablecmds.c which worries me, namely that it ignores functions and views. What? The it in question is, find_composite_type_dependencies() Cheers, D -- David Fetter [EMAIL PROTECTED] http://fetter.org/ phone: +1 415 235 3778AIM: dfetter666 Skype: davidfetter Remember to vote! Consider donating to PostgreSQL: http://www.postgresql.org/about/donate ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] Arrays of Complex Types
David Fetter [EMAIL PROTECTED] writes: On Tue, Mar 06, 2007 at 04:14:07PM -0500, Tom Lane wrote: David Fetter [EMAIL PROTECTED] writes: I noticed something in src/backend/commands/tablecmds.c which worries me, namely that it ignores functions and views. What? The it in question is, find_composite_type_dependencies() All that that's interested in is whether there are stored values of the datatype somewhere. Views don't have any storage, and a function definition doesn't either. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] Arrays of Complex Types
On Tue, Mar 06, 2007 at 04:24:36PM -0500, Tom Lane wrote: David Fetter [EMAIL PROTECTED] writes: On Tue, Mar 06, 2007 at 04:14:07PM -0500, Tom Lane wrote: David Fetter [EMAIL PROTECTED] writes: I noticed something in src/backend/commands/tablecmds.c which worries me, namely that it ignores functions and views. What? The it in question is, find_composite_type_dependencies() All that that's interested in is whether there are stored values of the datatype somewhere. Views don't have any storage, and a function definition doesn't either. I see. Perhaps I've misunderstood what this thing was for, then. What is it that checks whether it's OK to change a composite type, then? Cheers, D -- David Fetter [EMAIL PROTECTED] http://fetter.org/ phone: +1 415 235 3778AIM: dfetter666 Skype: davidfetter Remember to vote! Consider donating to PostgreSQL: http://www.postgresql.org/about/donate ---(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: [HACKERS] Arrays of Complex Types
On Fri, Mar 02, 2007 at 06:42:14PM -0600, Andrew Dunstan wrote: I'm still not happy about the idea of doing this for every relation (and doing it for sequences and indexes would be the height of wastefulness). How about we only do it for composite types? I'm not happy about that. I agree that indexes and sequences should not be done, but can we please do plain table types? I would be OK if we skipped catalog tables, if that would make you happier. Two thoughts: 1. Make the array types only when someone actually uses them (create a table with it or something). 2. Make a command: CREATE TYPE ARRAY OF foo; The latter has the benefit of not restricting it to an arbitrary choice of types, you could accept both domains and composite types here. I don't think it's unreasonable to require people to predeclare their usage of array-of-compostite-type. Perhaps change the word CREATE to DECLARE. I'm thinking of the explicit declaration of shell types as precedent here. Have a nice day, -- Martijn van Oosterhout kleptog@svana.org http://svana.org/kleptog/ From each according to his ability. To each according to his ability to litigate. signature.asc Description: Digital signature
Re: [HACKERS] Arrays of Complex Types
Martijn van Oosterhout wrote: On Fri, Mar 02, 2007 at 06:42:14PM -0600, Andrew Dunstan wrote: I'm still not happy about the idea of doing this for every relation (and doing it for sequences and indexes would be the height of wastefulness). How about we only do it for composite types? I'm not happy about that. I agree that indexes and sequences should not be done, but can we please do plain table types? I would be OK if we skipped catalog tables, if that would make you happier. Two thoughts: 1. Make the array types only when someone actually uses them (create a table with it or something). 2. Make a command: CREATE TYPE ARRAY OF foo; The latter has the benefit of not restricting it to an arbitrary choice of types, you could accept both domains and composite types here. I don't think it's unreasonable to require people to predeclare their usage of array-of-compostite-type. Perhaps change the word CREATE to DECLARE. I'm thinking of the explicit declaration of shell types as precedent here. #2 would be fine with me - not keen on #1. cheers andrew ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] Arrays of Complex Types
On Sat, Mar 03, 2007 at 09:06:11AM -0500, Andrew Dunstan wrote: Martijn van Oosterhout wrote: On Fri, Mar 02, 2007 at 06:42:14PM -0600, Andrew Dunstan wrote: I'm still not happy about the idea of doing this for every relation (and doing it for sequences and indexes would be the height of wastefulness). How about we only do it for composite types? I'm not happy about that. I agree that indexes and sequences should not be done, but can we please do plain table types? I would be OK if we skipped catalog tables, if that would make you happier. Two thoughts: 1. Make the array types only when someone actually uses them (create a table with it or something). This doesn't sound so great. 2. Make a command: CREATE TYPE ARRAY OF foo; The latter has the benefit of not restricting it to an arbitrary choice of types, you could accept both domains and composite types here. I'm thinking that DOMAINs over simple types should just automatically get corresponding array types. We don't yet support DOMAINs over complex types, but when we do, whatever we do with arrays of regular complex types should apply the same way. I don't think it's unreasonable to require people to predeclare their usage of array-of-compostite-type. Perhaps change the word CREATE to DECLARE. I'm thinking of the explicit declaration of shell types as precedent here. #2 would be fine with me - not keen on #1. Per your earlier suggestion in IRC, I'm picturing a polymorphic function like pg_catalog.array_for(typepoid OID) pg_catalog.array_for(typename NAME) pg_catalog.array_for(typenamespace NAME, typename NAME) I don't see a good reason to allow putting array types in a different schema from their base types. Cheers, D -- David Fetter [EMAIL PROTECTED] http://fetter.org/ phone: +1 415 235 3778AIM: dfetter666 Skype: davidfetter Remember to vote! Consider donating to PostgreSQL: http://www.postgresql.org/about/donate ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [HACKERS] Arrays of Complex Types
David Fetter [EMAIL PROTECTED] writes: 2. Make a command: CREATE TYPE ARRAY OF foo; The latter has the benefit of not restricting it to an arbitrary choice of types, you could accept both domains and composite types here. I'm thinking that DOMAINs over simple types should just automatically get corresponding array types. We don't yet support DOMAINs over complex types, but when we do, whatever we do with arrays of regular complex types should apply the same way. Just a thought, but if we supported domains over complex types we could kill two birds with one stone and say when you create a domain the array type is created and that's how you have to refer to arrays over complex types. Probably doesn't make anything easier though. Just a thought. -- 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: [HACKERS] Arrays of Complex Types
David Fetter wrote: Per your earlier suggestion in IRC, I'm picturing a polymorphic function like pg_catalog.array_for(typepoid OID) pg_catalog.array_for(typename NAME) pg_catalog.array_for(typenamespace NAME, typename NAME) I don't see a good reason to allow putting array types in a different schema from their base types. Actually, I think I prefer Martijns simple SQL extension suggestion better. cheers andrew ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
[HACKERS] Arrays of Complex Types
Folks, I'd like to take the TODO item that reads, Add support for arrays of complex types, but before I start patching, I'd like to see whether what I'm about to do makes any sense: 1. In src/backend/commands/tablecmds.c, change DefineRelation as follows: * After the first call to heap_create_with_catalog, construct and do another call to for the array type. * Add an appropriate pg_depend entry. 2. Change RemoveRelation to reflect the above. 3. Change TypeRename appropriately, whatever that turns out to be. Does the above make sense? Have I missed anything critical? Cheers, D -- David Fetter [EMAIL PROTECTED] http://fetter.org/ phone: +1 415 235 3778AIM: dfetter666 Skype: davidfetter Remember to vote! Consider donating to PostgreSQL: http://www.postgresql.org/about/donate ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] Arrays of Complex Types
David Fetter [EMAIL PROTECTED] writes: 1. In src/backend/commands/tablecmds.c, change DefineRelation as follows: * After the first call to heap_create_with_catalog, construct and do another call to for the array type. I'm still not happy about the idea of doing this for every relation (and doing it for sequences and indexes would be the height of wastefulness). How about we only do it for composite types? * Add an appropriate pg_depend entry. 2. Change RemoveRelation to reflect the above. You only need one of those two: either you drop by hand or you let the dependency machinery deal with it. Not both. Does the above make sense? Have I missed anything critical? Ummm ... making it actually work? Possibly that just falls out, but I'm not sure. If it turns out that it does Just Work, you might take a stab at arrays of domains too. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] Arrays of Complex Types
Tom Lane wrote: David Fetter [EMAIL PROTECTED] writes: 1. In src/backend/commands/tablecmds.c, change DefineRelation as follows: * After the first call to heap_create_with_catalog, construct and do another call to for the array type. I'm still not happy about the idea of doing this for every relation (and doing it for sequences and indexes would be the height of wastefulness). How about we only do it for composite types? I'm not happy about that. I agree that indexes and sequences should not be done, but can we please do plain table types? I would be OK if we skipped catalog tables, if that would make you happier. cheers andrew ---(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
