Tom Lane wrote:
Tom Dunstan [EMAIL PROTECTED] writes:
Here's the current version of the enums patch.
Applied with revisions --- some cosmetic, some not so much.
Attached is the patch-as-applied if you care to compare; feel free
to ask questions about anything not obvious.
There's a little
Heikki Linnakangas wrote:
Tom Lane wrote:
Tom Dunstan [EMAIL PROTECTED] writes:
Here's the current version of the enums patch.
Applied with revisions --- some cosmetic, some not so much.
Attached is the patch-as-applied if you care to compare; feel free
to ask questions about anything not
Looks like we need to check the length on type creation
too.
I'll fix later if not beaten to it.
It works for me (ie fails with an appropriate error) locally
on Linux FC6 x86-64. Perhaps this platform initializes
memory to zero on allocation? I dunno. Anyway, if you can
reproduce it, please
Tom Dunstan [EMAIL PROTECTED] writes:
Looks like we need to check the length on type creation
too.
It works for me (ie fails with an appropriate error) locally
on Linux FC6 x86-64. Perhaps this platform initializes
memory to zero on allocation?
Sounds more like you're testing without
Heikki Linnakangas [EMAIL PROTECTED] writes:
There's a little bug:
postgres=# CREATE TYPE aenum AS ENUM ('a','b','c'); CREATE TYPE
postgres=# CREATE TABLE t (e aenum); CREATE TABLE postgres=# INSERT
INTO t VALUES
Tom Lane wrote:
Heikki Linnakangas [EMAIL PROTECTED] writes:
There's a little bug:
postgres=# CREATE TYPE aenum AS ENUM ('a','b','c'); CREATE TYPE
postgres=# CREATE TABLE t (e aenum); CREATE TABLE postgres=# INSERT
INTO t VALUES
Hm, I suppose we should apply truncate_identifier rather than letting
the strings be blindly truncated (perhaps in mid-character). Should we
have it throw the truncation NOTICE, or not? First thought is to do so
during CREATE TYPE but not during plain enum_in().
I don't see much point in
Tom Dunstan [EMAIL PROTECTED] writes:
I was about to suggest that we just truncate the value in the input
function and look it up on the basis that if it's too long, the lookup
will fail and we can just tell the user that it wasn't a valid value.
But if there's a valid value that has the
Tom Lane wrote:
While all this reasoning is perfectly OK on its own terms, it ignores
the precedents of SQL identifier handling. Maybe we should revisit the
question of whether the labels are identifiers.
If we do that can we still cache the values in the syscache? My
impression
Tom Lane wrote:
While all this reasoning is perfectly OK on its own terms, it ignores
the precedents of SQL identifier handling. Maybe we should revisit the
question of whether the labels are identifiers.
Implying that they shouldn't be quoted like text (or should be
double-quoted if
Andrew Dunstan [EMAIL PROTECTED] writes:
Tom Lane wrote:
While all this reasoning is perfectly OK on its own terms, it ignores
the precedents of SQL identifier handling. Maybe we should revisit the
question of whether the labels are identifiers.
If we do that can we still cache the values
Tom Lane wrote:
But probably making them act like identifiers is not a good idea,
because I doubt we want automatic downcasing in enum_in. People
wouldn't be happy if they had to write WHERE color = 'Red' or
something like that to get at a mixed-case enum label.
Ah, that's what you had in
Tom Dunstan wrote:
Let's just throw the error instead. (I agree that enum_in can just fail
with no such label, but CREATE TYPE ought to give a specific
error about it.)
Agreed.
Andrew, you said you had a mostly-working patch already?
Working patch attached. If everyone's happy I'll apply
Andrew Dunstan [EMAIL PROTECTED] writes:
Working patch attached. If everyone's happy I'll apply it.
Why not put the create-time length test into EnumValuesCreate's loop,
which has to grovel through the list already?
I'm wondering why bother with the temp variable in cstring_enum,
as opposed to
Tom Lane wrote:
Andrew Dunstan [EMAIL PROTECTED] writes:
Working patch attached. If everyone's happy I'll apply it.
Why not put the create-time length test into EnumValuesCreate's loop,
which has to grovel through the list already?
My idea was to do the error check before calling
Andrew Dunstan [EMAIL PROTECTED] writes:
Tom Lane wrote:
Why not put the create-time length test into EnumValuesCreate's loop,
which has to grovel through the list already?
My idea was to do the error check before calling TypeCreate. If that
doesn't matter I can move it - it just seemed a
Tom Lane wrote:
enum_first, enum_last: these return ANYENUM but violate the rule that
a polymorphic-result function must have a polymorphic input argument
from which the parser may deduce the actual output type.
Is this a tragedy when the supplied parameter gives the result type
directly?
Andrew Dunstan wrote:
Tom Lane wrote:
enum_first, enum_last: these return ANYENUM but violate the rule that
a polymorphic-result function must have a polymorphic input argument
from which the parser may deduce the actual output type.
Is this a tragedy when the supplied parameter gives the
text_enum: same problem as above, plus not acceptable to assume that it
gets the right enum OID, plus very definitely not acceptable to assume
that OID and typmod are the same size, plus I don't like the
undocumented kluge in getBaseTypeAndTypmod that is pretending to supply
the type OID for some
Tom Dunstan [EMAIL PROTECTED] writes:
Andrew Dunstan wrote:
Tom Lane wrote:
enum_first, enum_last: these return ANYENUM but violate the rule that
a polymorphic-result function must have a polymorphic input argument
from which the parser may deduce the actual output type.
Is this a tragedy
Tom Lane wrote:
The null Datum itself obviously doesn't carry that info, but the
expression tree does, and there are provisions for letting functions
retrieve that info --- see get_fn_expr_rettype and get_fn_expr_argtype.
Hmm. I vaguely remember that there was some feeling that the PLs
Tom Dunstan [EMAIL PROTECTED] writes:
Tom Lane wrote:
... could we
have a special rule that would look for e.g. a regtype as the first
parameter if the return type is generic and there are no generic parameters?
I thought about that too but don't like it much. The problem is mainly
that
Tom Lane wrote:
OK, I give up. :) Why?
Because the whole point is that the type has to be known at parse time.
Oh, duh. :)
I've got it working with get_fn_expr_argtype and it seems fairly
reasonable.
Cool!
Thanks
Tom
---(end of
Tom Dunstan [EMAIL PROTECTED] writes:
Here's the current version of the enums patch.
Applied with revisions --- some cosmetic, some not so much.
Attached is the patch-as-applied if you care to compare; feel free
to ask questions about anything not obvious.
regards, tom
Peter Eisentraut wrote:
Am Dienstag, 27. März 2007 03:36 schrieb Tom Dunstan:
Here's the current version of the enums patch. Not much change from last
time, the only thought-inducing stuff was fixing up some macros that
changed with the VARLENA changes, and adding a regression test to do
Andrew Dunstan [EMAIL PROTECTED] writes:
If you want to review or test the feature, the attached patch can be
used as a replacement for the portion that affects parse_coerce.c, and
with this it compiles and passes regression. I think it's correct but it
should still be OKed by at least one
Here's the current version of the enums patch.
[ sounds of reviewing... ] Is there a specific reason for
pg_enum.enumname to be type name and not type text? ISTM that type name
wastes space (because most labels will probably be a lot shorter than 63
bytes) and at the same time imposes an
Tom Lane wrote:
Here's the current version of the enums patch.
[ sounds of reviewing... ]
(What are those? It's a bit hard to imagine you singing doo di doo doo
a la Homer while reviewing )
Is there a specific reason for
pg_enum.enumname to be type name and not type text?
Andrew Dunstan [EMAIL PROTECTED] writes:
Tom Lane wrote:
Is there a specific reason for
pg_enum.enumname to be type name and not type text?
IIRC at one stage Tom wanted to try to make these identifiers, but that
was quickly abandoned. This might be a hangover from that.
Actually I think I
Tom Lane wrote:
Andrew Dunstan [EMAIL PROTECTED] writes:
Tom Lane wrote:
Is there a specific reason for
pg_enum.enumname to be type name and not type text?
IIRC at one stage Tom wanted to try to make these identifiers, but that
was quickly abandoned. This might be a
Hi all! Thanks for reviewing the patch!
Tom Lane wrote:
Andrew Dunstan [EMAIL PROTECTED] writes:
Tom Lane wrote:
Is there a specific reason for
pg_enum.enumname to be type name and not type text?
IIRC at one stage Tom wanted to try to make these identifiers, but that
was quickly abandoned.
Tom Lane wrote:
Unless someone objects, I'll change this and also revert to the
enumlabel name that seems to have been used originally (it was still
used in the docs). It seems more readable somehow (I guess it's the
lack of either ascenders or descenders in enumname).
The name/text thing
Tom Dunstan [EMAIL PROTECTED] writes:
If we ditched the second syscache, we'd want some other way to convert a
type OID and name into the enum value oid efficiently.
True. OK, never mind that then. I'll still rename it as per your
other comment.
regards, tom lane
Tom Lane wrote:
Andrew Dunstan [EMAIL PROTECTED] writes:
If you want to review or test the feature, the attached patch can be
used as a replacement for the portion that affects parse_coerce.c, and
with this it compiles and passes regression. I think it's correct but it
should still be OKed by
Tom Lane wrote:
Unless someone objects, I'll change this and also revert to the
enumlabel name that seems to have been used originally (it was still
used in the docs). It seems more readable somehow (I guess it's the
lack of either ascenders or descenders in enumname).
Wow, I wasn't aware
BTW, I've got fairly serious problems with a few of the cuter parts of
the patch:
enum_first, enum_last: these return ANYENUM but violate the rule that
a polymorphic-result function must have a polymorphic input argument
from which the parser may deduce the actual output type.
enum_range_all:
Am Dienstag, 27. März 2007 03:36 schrieb Tom Dunstan:
Here's the current version of the enums patch. Not much change from last
time, the only thought-inducing stuff was fixing up some macros that
changed with the VARLENA changes, and adding a regression test to do
basic checking of RI
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.
---
Hi all
Here's the current version of the enums patch. Not much change from last
time, the only thought-inducing stuff was fixing up some macros that
changed with the VARLENA changes, and adding a regression test to do
basic checking of RI behavior, after the discussions that we had
recently
39 matches
Mail list logo
