Re: [HACKERS] Arrays of Complex Types

2007-04-03 Thread David Fetter
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: [HACKERS] Arrays of Complex Types

2007-04-02 Thread David Fetter
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

2007-04-02 Thread Bruce Momjian

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

2007-04-02 Thread Alvaro Herrera
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)

2007-04-01 Thread Peter Eisentraut
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)

2007-04-01 Thread Tom Lane
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)

2007-04-01 Thread Peter Eisentraut
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)

2007-04-01 Thread Gregory Stark
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)

2007-04-01 Thread Peter Eisentraut
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)

2007-04-01 Thread Tom Lane
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)

2007-04-01 Thread Peter Eisentraut
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)

2007-04-01 Thread Tom Lane
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)

2007-03-31 Thread Tom Lane
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)

2007-03-31 Thread Bruce Momjian
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)

2007-03-31 Thread David Fetter
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)

2007-03-31 Thread Tom Lane
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)

2007-03-31 Thread David Fetter
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)

2007-03-31 Thread Peter Eisentraut
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)

2007-03-31 Thread Tom Lane
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

2007-03-30 Thread David Fetter
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

2007-03-30 Thread Tom Lane
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

2007-03-28 Thread Andrew - Supernews
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

2007-03-28 Thread David Fetter
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

2007-03-28 Thread Alvaro Herrera
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

2007-03-28 Thread Andrew Dunstan

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

2007-03-28 Thread Tom Lane
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

2007-03-28 Thread David Fetter
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

2007-03-28 Thread Gregory Stark

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

2007-03-28 Thread Tom Lane
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

2007-03-28 Thread David Fetter
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

2007-03-27 Thread David Fetter
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

2007-03-27 Thread Andrew Dunstan

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

2007-03-27 Thread David Fetter
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

2007-03-25 Thread David Fetter
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

2007-03-25 Thread Tom Lane
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

2007-03-06 Thread David Fetter
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

2007-03-06 Thread Tom Lane
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

2007-03-06 Thread David Fetter
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

2007-03-06 Thread Tom Lane
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

2007-03-06 Thread David Fetter
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

2007-03-03 Thread Martijn van Oosterhout
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

2007-03-03 Thread Andrew Dunstan



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

2007-03-03 Thread David Fetter
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

2007-03-03 Thread Gregory Stark
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

2007-03-03 Thread Andrew Dunstan

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


Re: [HACKERS] Arrays of Complex Types

2007-03-02 Thread Tom Lane
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

2007-03-02 Thread Andrew Dunstan
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