Attached is my rework of David Fetter's array of composites patch. It
has all the agreed modifications and checks, except altering the name
mangling.

If people prefer I can apply this as it stands and then get working on
the name mangling piece. Or I can hold off.

I'm still wondering if we can get away without a catalog change on that
- e.g. could we look up an array type by looking for a pg_type entry
containing the base type's oid in the typelem field? Or would that be
too slow?

cheers

andrew


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

Andrew Dunstan <andrew@dunslane.net> writes:
> I'm still wondering if we can get away without a catalog change on that
> - e.g. could we look up an array type by looking for a pg_type entry
> containing the base type's oid in the typelem field? Or would that be
> too slow?

(a) too slow, (b) not unique.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match

Andrew Dunstan <andrew@dunslane.net> writes:
> The attached version removes all the places we had NAMEDATALEN - 2
> restrictions on object names. If there's no objection I will commit this
> shortly, as I think it now does all the previously agreed things.

It needs some work yet, but I'll go over it and commit it.

BTW, just idly looking at this, I'm wondering whether we couldn't
now support arrays of domains too. If record_in/record_out work for
array elements, why wouldn't domain_in/domain_out?

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 2: Don't 'kill -9' the postmaster

Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
>
>> The attached version removes all the places we had NAMEDATALEN - 2
>> restrictions on object names. If there's no objection I will commit this
>> shortly, as I think it now does all the previously agreed things.
>>
>
> It needs some work yet, but I'll go over it and commit it.
>
> BTW, just idly looking at this, I'm wondering whether we couldn't
> now support arrays of domains too. If record_in/record_out work for
> array elements, why wouldn't domain_in/domain_out?
>
>
>

Good question. I'm all in favor of doing that. Presumably we would not
in the case of a domain of an array type?

cheers

andrew

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

Andrew Dunstan <andrew@dunslane.net> writes:
>>> Attached is my rework of David Fetter's array of composites patch. It
>>> has all the agreed modifications and checks, except altering the name
>>> mangling.

Applied with revisions. There are some loose ends yet:

* I didn't do anything about arrays of domains. Although I think they'd
basically work, there's one nasty fly in the ointment, which is ALTER
DOMAIN ADD CONSTRAINT. get_rels_with_domain() is not smart enough to
detect arrays of domains, and its callers are not nearly smart enough to
apply their checks to arrays. So I think this had better wait for 8.4.

BTW, I realized there's an existing bug here as of 8.2: when I enabled
domains over domains I didn't do anything with get_rels_with_domain().
Fortunately this is a relatively easy thing to deal with, we can just
recurse to find columns of derived domain types, which the callers don't
really need to treat any differently than they do now. I'll go fix
that part.

* The feature leaves something to be desired in terms of usability,
because array[row()] doesn't work:

regression=# create type foo as (f1 int, f2 int);
CREATE TYPE
regression=# create table bar (ff1 foo[]);
CREATE TABLE
regression=# insert into bar values(array[row(1,2),row(3,4)]);
ERROR: could not find array type for data type record
regression=#

You can only get it to work if you plaster ::foo onto *each* row()
construct. Ugh. This didn't seem trivial to improve.

* I'm a bit concerned about dump order. If a user wants to create
types named "foo" and "_foo", he can, but it will only work if he
makes "_foo" first --- else the derived type for foo is in the way.
Since pg_dump has no clue about that constraint, it might easily
dump "foo" first leading to an unrestorable dump. The most usable
solution would be to auto-rename previously created array types,
but I dunno how implementable that would be.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 2: Don't 'kill -9' the postmaster

"Tom Lane" <tgl@sss.pgh.pa.us> writes:

> * I'm a bit concerned about dump order. If a user wants to create
> types named "foo" and "_foo", he can, but it will only work if he
> makes "_foo" first --- else the derived type for foo is in the way.
> Since pg_dump has no clue about that constraint, it might easily
> dump "foo" first leading to an unrestorable dump. The most usable
> solution would be to auto-rename previously created array types,
> but I dunno how implementable that would be.

BTW, why exactly do we need array types to have names at all? The only
user-visible way to refer to these types is always by foo[] isn't it?

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


---------------------------(end of broadcast)---------------------------
TIP 2: Don't 'kill -9' the postmaster

Andrew Dunstan <andrew@dunslane.net> writes:
> Tom Lane wrote:
>> That's not really the most preferable solution, I think, seeing that it
>> still leaves the user with the problem of having to create the types in
>> the right order to start with.

> I'm not sure we can keep the _foo convention and avoid that.

Auto-rename. I'm working on a patch now, and it doesn't look like it'll
be too awful. Will post it for comments when it's working.

> ... I'd vote to revert the new name
> mangling piece (but keep the typarray mapping column), deprecate the use
> of the _foo convention, and abandon it next release.

I came across a comment in the source that says PG has been using _foo
for arrays since 3.1 (!). I don't think we can get away with changing
it, certainly not with only one release cycle's notice.

The current code is OK from a compatibility point of view, since it only
changes _foo to something else in situations where the old way would've
failed outright. I think we need to preserve that property ...

regards, tom lane

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

Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
>
>> Tom Lane wrote:
>>
>>> That's not really the most preferable solution, I think, seeing that it
>>> still leaves the user with the problem of having to create the types in
>>> the right order to start with.
>>>
>
>
>> I'm not sure we can keep the _foo convention and avoid that.
>>
>
> Auto-rename. I'm working on a patch now, and it doesn't look like it'll
> be too awful. Will post it for comments when it's working.
>


Ok, cool. I look forward to it.

cheers

andrew

---------------------------(end of broadcast)---------------------------
TIP 3: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faq

Tom Lane wrote:
>
> There is *tons* of legacy code that uses _foo, mainly because there was
> a time when we didn't support the [] notation in a lot of places where
> types can be named. There still are some places, in fact:
>
> regression=# alter type widget[] set schema public;
> ERROR: syntax error at or near "["
> LINE 1: alter type widget[] set schema public;
> ^
> regression=# alter type _widget set schema public;
> ERROR: cannot alter array type widget[]
> HINT: You can alter type widget, which will alter the array type as well.
> regression=#
>
> That particular one may not need fixed (anymore) but the real problem is
> the torches-and-pitchforks session that will ensue if we break legacy
> code for no reason beyond cosmetics.
>
> IIRC some of the contrib modules still have instances of _foo in
> their SQL scripts.
>

Then I think we need to work out a way to make pg_dump smart enough to
dump things in the right order.

Can we perhaps explicitly deprecate using the type name to refer to
array types?

cheers

andrew


---------------------------(end of broadcast)---------------------------
TIP 3: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faq

Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
>
>> Then I think we need to work out a way to make pg_dump smart enough to
>> dump things in the right order.
>>
>
> That's not really the most preferable solution, I think, seeing that it
> still leaves the user with the problem of having to create the types in
> the right order to start with.
>
>
>

I'm not sure we can keep the _foo convention and avoid that. If we can't
find something sensible pretty quickly, I'd vote to revert the new name
mangling piece (but keep the typarray mapping column), deprecate the use
of the _foo convention, and abandon it next release. It would be a pity
though.

cheers

andrew

---------------------------(end of broadcast)---------------------------
TIP 3: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faq