Re: Macros for typtype (was Re: [HACKERS] Arrays of Complex Types)
Tom Lane wrote:
Why not enums? ;-)
Well, macros is how we do it elsewhere for char system columns.
I'm not sure we could rely on the behavior if we declared
pg_type.typtype as an enum type ... and if we don't, there's not
much point.
I was thinking C enums:
enum typtype_type {
TYPTYPE_BASE = 'b',
TYPTYPE_COMPOSITE = 'c',
TYPTYPE_DOMAIN = 'd',
TYPTYPE_ENUM = 'e',
TYPTYPE_PSEUDO = 'p'
};
I'm not sure if this is better.
--
Peter Eisentraut
http://developer.postgresql.org/~petere/
---(end of broadcast)---
TIP 4: Have you searched our list archives?
http://archives.postgresql.org
Re: Macros for typtype (was Re: [HACKERS] Arrays of Complex Types)
Peter Eisentraut [EMAIL PROTECTED] writes:
Tom Lane wrote:
I'm not sure we could rely on the behavior if we declared
pg_type.typtype as an enum type ... and if we don't, there's not
much point.
I was thinking C enums:
enum typtype_type {
TYPTYPE_BASE = 'b',
TYPTYPE_COMPOSITE = 'c',
TYPTYPE_DOMAIN = 'd',
TYPTYPE_ENUM = 'e',
TYPTYPE_PSEUDO = 'p'
};
I'm not sure if this is better.
What bothers me about that is I don't think the C spec mandates the
representation width. If we could guarantee that enum typtype_type
was 1 byte I'd be all for it.
regards, tom lane
---(end of broadcast)---
TIP 4: Have you searched our list archives?
http://archives.postgresql.org
Re: Macros for typtype (was Re: [HACKERS] Arrays of Complex Types)
Tom Lane wrote:
What bothers me about that is I don't think the C spec mandates the
representation width. If we could guarantee that enum typtype_type
was 1 byte I'd be all for it.
The width is 4 both for the macro and the enum case. Both
#define TYPTYPE_BASE 'b'
and
enum ... {
TYPTYPE_BASE = 'b',
effectively generate int constants named TYPTYPE_BASE with decimal value
98. So there are no storage advantages either way.
--
Peter Eisentraut
http://developer.postgresql.org/~petere/
---(end of broadcast)---
TIP 4: Have you searched our list archives?
http://archives.postgresql.org
Re: Macros for typtype (was Re: [HACKERS] Arrays of Complex Types)
Peter Eisentraut [EMAIL PROTECTED] writes:
Tom Lane wrote:
What bothers me about that is I don't think the C spec mandates the
representation width. If we could guarantee that enum typtype_type
was 1 byte I'd be all for it.
The width is 4 both for the macro and the enum case. Both
#define TYPTYPE_BASE 'b'
and
enum ... {
TYPTYPE_BASE = 'b',
effectively generate int constants named TYPTYPE_BASE with decimal value
98. So there are no storage advantages either way.
That's not accurate at all. The macro case gives you a constant you can only
use to initialize integer variables and members that are explicitly declared
with some integral type. If we consistently declare them char then they'll
be predictably 1 byte long.
The enum case does two things. It defines a syntactic meaning for the label,
*and* it defines a thing enum typtype which can be used to define variables
and members. If the latter is used then Tom is saying the standard doesn't
specify what width the variable or member will be.
We could use enum {} to define the labels and then make a rule that all actual
variables should be declared using char rather than declaring them as enum
typtype. But I fear somebody would get that wrong some day. The worst thing
is that it might work on their compiler or it might even work on all compilers
and just silently be causing things to be aligned differently than they
expect.
On the other hand it I don't really think it would cause any problems if
people stored their typtypes in integers. Except for the actual FormData_pg_*
structures the precise alignment doesn't actually matter for anything does it?
--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
---(end of broadcast)---
TIP 6: explain analyze is your friend
Re: Macros for typtype (was Re: [HACKERS] Arrays of Complex Types)
Gregory Stark wrote:
The width is 4 both for the macro and the enum case. Both
#define TYPTYPE_BASE 'b'
and
enum ... {
TYPTYPE_BASE = 'b',
effectively generate int constants named TYPTYPE_BASE with decimal
value 98. So there are no storage advantages either way.
That's not accurate at all.
How so?
The macro case gives you a constant you
can only use to initialize integer variables and members that are
explicitly declared with some integral type. If we consistently
declare them char then they'll be predictably 1 byte long.
But character constants are actually ints, so when you do what you
describe then the compiler has to generate code to copy a four-byte
integer into a single byte. (Of course that can be optimized away,
probably.)
The enum case does two things. It defines a syntactic meaning for the
label, *and* it defines a thing enum typtype which can be used to
define variables and members. If the latter is used then Tom is
saying the standard doesn't specify what width the variable or member
will be.
The standard says that enums are the same as ints. So when you assign
an enum label to a char variable, then compiler has to generate code to
copy a four-byte integer into a single byte. (Of course that can be
optimized away, probably.)
The fact that you can also declare variables of the enum type is not
under consideration here.
QED
--
Peter Eisentraut
http://developer.postgresql.org/~petere/
---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to [EMAIL PROTECTED] so that your
message can get through to the mailing list cleanly
Re: Macros for typtype (was Re: [HACKERS] Arrays of Complex Types)
Gregory Stark [EMAIL PROTECTED] writes:
We could use enum {} to define the labels and then make a rule that
all actual variables should be declared using char rather than
declaring them as enum typtype. But I fear somebody would get that
wrong some day.
Yeah, that seems to me to be just asking for trouble.
On the other hand it I don't really think it would cause any problems
if people stored their typtypes in integers. Except for the actual
FormData_pg_* structures the precise alignment doesn't actually matter
for anything does it?
The layout of the FormData struct is exactly the sticking point. If the
compiler makes the size or alignment of a struct field different from
what the tuple packing/unpacking code does for the corresponding column
type, we've got big trouble.
As for Peter's claim that the storage of an enum field is always int,
I think the C spec says otherwise. In 6.7.2.2 of C99 I see
[#4] Each enumerated type shall be compatible with an
integer type. The choice of type is
implementation-defined, 97) but shall be capable of
representing the values of all the members of the
enumeration. The enumerated type is incomplete until after
the } that terminates the list of enumerator declarations.
97) An implementation may delay the choice of which integer
type until all enumeration constants have been seen.
It seems clear to me that this authorizes, but *does not require*,
the compiler to store an enum field in a byte or short instead of
an int when all the declared values will fit. So if we tried to
do this, we'd have the problem of needing compiler-specific data
type information entered in pg_type.
Perhaps all C compilers do this alike, but how would we know?
Anyway the possible gain seems not worth the risk to me.
regards, tom lane
---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to [EMAIL PROTECTED] so that your
message can get through to the mailing list cleanly
Re: Macros for typtype (was Re: [HACKERS] Arrays of Complex Types)
Tom Lane wrote: It seems clear to me that this authorizes, but *does not require*, the compiler to store an enum field in a byte or short instead of an int when all the declared values will fit. So if we tried to do this, we'd have the problem of needing compiler-specific data type information entered in pg_type. FWIW, I never meant to suggest using enums tuple structures. I did, however, stumble over a case that appears to be handled similar to what I had in mind: see enum CoercionCodes in primnodes.h. Again, it's not really important, but it's interesting to see that there is precedent. -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: Macros for typtype (was Re: [HACKERS] Arrays of Complex Types)
Peter Eisentraut [EMAIL PROTECTED] writes: Tom Lane wrote: It seems clear to me that this authorizes, but *does not require*, the compiler to store an enum field in a byte or short instead of an int when all the declared values will fit. FWIW, I never meant to suggest using enums tuple structures. I did, however, stumble over a case that appears to be handled similar to what I had in mind: see enum CoercionCodes in primnodes.h. Again, it's not really important, but it's interesting to see that there is precedent. AFAIK, we don't store CoercionCodes in any system catalog columns. But now that you mention it there is at least one place where we do it like that: pg_depend.deptype is a char but its values are defined by enum DependencyType. I have some recollection of doing it that way because I was concerned that most places that cared about dependency types should be switch statements that covered all the possible values (which is pretty much the only benefit you get from doing it that way). Having just gone over the typtype uses, there are only a couple of places where we'd win from having that sort of compile-time check for typtype. regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Macros for typtype (was Re: [HACKERS] Arrays of Complex Types)
I wrote: David Fetter [EMAIL PROTECTED] writes: What parts of the code would need a once-over? A lot :-( ... probably every place that touches typtype or typelem would need at least a look. It'd be a good idea to take the opportunity to start using macros for the values of typtype, as we do for relkind but for some reason never adopted for typtype. I just realized that I need to check every usage of typtype to be sure that the enums patch is sane. So, barring objection, I intend to take this opportunity to make the code stop referring directly to 'b', 'c' etc whereever possible. Any objections to these names? #define TYPTYPE_BASE'b' #define TYPTYPE_COMPOSITE 'c' #define TYPTYPE_DOMAIN 'd' #define TYPTYPE_ENUM'e' #define TYPTYPE_PSEUDO 'p' regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: Macros for typtype (was Re: [HACKERS] Arrays of Complex Types)
Tom Lane wrote: I wrote: David Fetter [EMAIL PROTECTED] writes: What parts of the code would need a once-over? A lot :-( ... probably every place that touches typtype or typelem would need at least a look. It'd be a good idea to take the opportunity to start using macros for the values of typtype, as we do for relkind but for some reason never adopted for typtype. I just realized that I need to check every usage of typtype to be sure that the enums patch is sane. So, barring objection, I intend to take this opportunity to make the code stop referring directly to 'b', 'c' etc whereever possible. Any objections to these names? #define TYPTYPE_BASE'b' #define TYPTYPE_COMPOSITE 'c' #define TYPTYPE_DOMAIN 'd' #define TYPTYPE_ENUM'e' #define TYPTYPE_PSEUDO 'p' I like macros. ;-) -- Bruce Momjian [EMAIL PROTECTED] http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: Macros for typtype (was Re: [HACKERS] Arrays of Complex Types)
On Sat, Mar 31, 2007 at 07:13:20PM -0400, Bruce Momjian wrote: Tom Lane wrote: I wrote: David Fetter [EMAIL PROTECTED] writes: What parts of the code would need a once-over? A lot :-( ... probably every place that touches typtype or typelem would need at least a look. It'd be a good idea to take the opportunity to start using macros for the values of typtype, as we do for relkind but for some reason never adopted for typtype. I just realized that I need to check every usage of typtype to be sure that the enums patch is sane. So, barring objection, I intend to take this opportunity to make the code stop referring directly to 'b', 'c' etc whereever possible. Any objections to these names? #define TYPTYPE_BASE'b' #define TYPTYPE_COMPOSITE 'c' #define TYPTYPE_DOMAIN 'd' #define TYPTYPE_ENUM'e' #define TYPTYPE_PSEUDO 'p' I like macros. ;-) Macros are great. :) What say we put one in pre-emptively for TYPTYPE_ARRAY? Cheers, D -- David Fetter [EMAIL PROTECTED] http://fetter.org/ phone: +1 415 235 3778AIM: dfetter666 Skype: davidfetter Remember to vote! Consider donating to PostgreSQL: http://www.postgresql.org/about/donate ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: Macros for typtype (was Re: [HACKERS] Arrays of Complex Types)
David Fetter [EMAIL PROTECTED] writes: What say we put one in pre-emptively for TYPTYPE_ARRAY? When and if the patch appears, you can add it ;-). I'm just intending a search-and-replace at the moment. regards, tom lane ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: Macros for typtype (was Re: [HACKERS] Arrays of Complex Types)
On Sat, Mar 31, 2007 at 07:58:34PM -0400, Tom Lane wrote:
David Fetter [EMAIL PROTECTED] writes:
What say we put one in pre-emptively for TYPTYPE_ARRAY?
When and if the patch appears, you can add it ;-). I'm just intending a
search-and-replace at the moment.
Like this?
Cheers,
D
--
David Fetter [EMAIL PROTECTED] http://fetter.org/
phone: +1 415 235 3778AIM: dfetter666
Skype: davidfetter
Remember to vote!
Consider donating to PostgreSQL: http://www.postgresql.org/about/donate
Index: src/backend/catalog/heap.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/catalog/heap.c,v
retrieving revision 1.317
diff -c -r1.317 heap.c
*** src/backend/catalog/heap.c 14 Feb 2007 01:58:56 - 1.317
--- src/backend/catalog/heap.c 1 Apr 2007 00:17:20 -
***
*** 412,418
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
errmsg(column \%s\ has type \unknown\,
attname),
errdetail(Proceeding with relation creation
anyway.)));
! else if (att_typtype == 'p')
{
/* Special hack for pg_statistic: allow ANYARRAY during initdb
*/
if (atttypid != ANYARRAYOID || IsUnderPostmaster)
--- 412,418
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
errmsg(column \%s\ has type \unknown\,
attname),
errdetail(Proceeding with relation creation
anyway.)));
! else if (att_typtype == TYPTYPE_PSEUDO)
{
/* Special hack for pg_statistic: allow ANYARRAY during initdb
*/
if (atttypid != ANYARRAYOID || IsUnderPostmaster)
Index: src/backend/commands/typecmds.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/commands/typecmds.c,v
retrieving revision 1.100
diff -c -r1.100 typecmds.c
*** src/backend/commands/typecmds.c 14 Feb 2007 01:58:57 - 1.100
--- src/backend/commands/typecmds.c 1 Apr 2007 00:17:20 -
***
*** 625,631
* might be made to work in the future, but not today.
*/
typtype = baseType-typtype;
! if (typtype != 'b' typtype != 'd')
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg(\%s\ is not a valid base type for a
domain,
--- 625,631
* might be made to work in the future, but not today.
*/
typtype = baseType-typtype;
! if (typtype != TYPTYPE_BASE typtype != TYPTYPE_DOMAIN)
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg(\%s\ is not a valid base type for a
domain,
***
*** 907,913
/* Check that this is actually a domain */
typtype = ((Form_pg_type) GETSTRUCT(tup))-typtype;
! if (typtype != 'd')
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg(\%s\ is not a domain,
--- 907,913
/* Check that this is actually a domain */
typtype = ((Form_pg_type) GETSTRUCT(tup))-typtype;
! if (typtype != TYPTYPE_DOMAIN)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg(\%s\ is not a domain,
***
*** 1835,1841
Form_pg_type typTup = (Form_pg_type) GETSTRUCT(tup);
/* Check that this is actually a domain */
! if (typTup-typtype != 'd')
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg(\%s\ is not a domain,
--- 1835,1841
Form_pg_type typTup = (Form_pg_type) GETSTRUCT(tup);
/* Check that this is actually a domain */
! if (typTup-typtype != TYPTYPE_DOMAIN)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg(\%s\ is not a domain,
***
*** 2021,2027
elog(ERROR, cache lookup failed for type %u, typeOid);
typTup = (Form_pg_type) GETSTRUCT(tup);
! if (typTup-typtype != 'd')
{
/* Not a domain, so done */
ReleaseSysCache(tup);
--- 2021,2027
elog(ERROR, cache lookup failed for type %u, typeOid);
typTup = (Form_pg_type) GETSTRUCT(tup);
! if (typTup-typtype != TYPTYPE_DOMAIN)
{
/* Not a domain, so done */
ReleaseSysCache(tup);
***
*** 2148,2154
Re: Macros for typtype (was Re: [HACKERS] Arrays of Complex Types)
Bruce Momjian wrote: I just realized that I need to check every usage of typtype to be sure that the enums patch is sane. So, barring objection, I intend to take this opportunity to make the code stop referring directly to 'b', 'c' etc whereever possible. Any objections to these names? #define TYPTYPE_BASE'b' #define TYPTYPE_COMPOSITE 'c' #define TYPTYPE_DOMAIN 'd' #define TYPTYPE_ENUM'e' #define TYPTYPE_PSEUDO 'p' I like macros. ;-) Why not enums? ;-) -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: Macros for typtype (was Re: [HACKERS] Arrays of Complex Types)
Peter Eisentraut [EMAIL PROTECTED] writes: I just realized that I need to check every usage of typtype to be sure that the enums patch is sane. So, barring objection, I intend to take this opportunity to make the code stop referring directly to 'b', 'c' etc whereever possible. Any objections to these names? Why not enums? ;-) Well, macros is how we do it elsewhere for char system columns. I'm not sure we could rely on the behavior if we declared pg_type.typtype as an enum type ... and if we don't, there's not much point. regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
