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
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)
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)
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)
"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)
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: >> 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: > > 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: >>> 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: 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)
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 */
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: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)
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