Обсуждение: Re: [PATCHES] Bundle of patches

Поиск
Список
Период
Сортировка

Re: [PATCHES] Bundle of patches

От
Tom Lane
Дата:
Teodor Sigaev <teodor@sigaev.ru> writes:
> 1) Typmod for user-defined types
>    http://www.sigaev.ru/misc/user_defined_typmod-0.7.gz
>    Patch is based on ideas from
>    http://archives.postgresql.org/pgsql-hackers/2004-06/msg00932.php
>    http://archives.postgresql.org/pgsql-hackers/2005-08/msg01007.php

This one seems generally workable, but I really dislike the approach
that's been used for passing typmod arguments to the typmod_in function.
Representing them with an "internal" parameter means it'll be forever
impossible to write typmod functions in anything but C, which seems an
ugly restriction.  Perhaps an array of int4 would be better?  How much
flexibility do we really want to provide for typmod arguments?  Allowing
full "expr_list" in the grammar seems less than sane, considering the
result is still going to have to pack into 32 bits.

The patch needs more cleanup before applying, too, eg make comments
match code, get rid of unused keywords added to gram.y.

            regards, tom lane

Re: [PATCHES] Bundle of patches

От
Teodor Sigaev
Дата:

> This one seems generally workable, but I really dislike the approach
> that's been used for passing typmod arguments to the typmod_in function.
> Representing them with an "internal" parameter means it'll be forever
> impossible to write typmod functions in anything but C, which seems an
> ugly restriction.  Perhaps an array of int4 would be better?  How much
I don't think that is a problem - I'll change that

> flexibility do we really want to provide for typmod arguments?  Allowing
> full "expr_list" in the grammar seems less than sane, considering the
> result is still going to have to pack into 32 bits.
As I remember, I tried to use some thing else but, I've got a lot conflicts
with
AexprConst:
    func_name '(' expr_list ')' Sconst

>
> The patch needs more cleanup before applying, too, eg make comments
> match code, get rid of unused keywords added to gram.y.
Ok.

--
Teodor Sigaev                                   E-mail: teodor@sigaev.ru
                                                    WWW: http://www.sigaev.ru/

Re: [PATCHES] Bundle of patches

От
Martijn van Oosterhout
Дата:
On Mon, Dec 04, 2006 at 02:04:26PM -0500, Tom Lane wrote:
> Teodor Sigaev <teodor@sigaev.ru> writes:
> > 1) Typmod for user-defined types
> >    http://www.sigaev.ru/misc/user_defined_typmod-0.7.gz
> >    Patch is based on ideas from
> >    http://archives.postgresql.org/pgsql-hackers/2004-06/msg00932.php
> >    http://archives.postgresql.org/pgsql-hackers/2005-08/msg01007.php
>
> This one seems generally workable, but I really dislike the approach
> that's been used for passing typmod arguments to the typmod_in function.
> Representing them with an "internal" parameter means it'll be forever
> impossible to write typmod functions in anything but C, which seems an
> ugly restriction.  Perhaps an array of int4 would be better?  How much
> flexibility do we really want to provide for typmod arguments?  Allowing
> full "expr_list" in the grammar seems less than sane, considering the
> result is still going to have to pack into 32 bits.

People have been discussion passing character set names as typmod
parameters, so restricting them to int4 seems too tight.

I'd favour the approach where the arguments to the typmod_in function
determine the types required. This allows the system to do proper
checking and casting and most important of all, good error messages,
eg:

ERROR: Invalid argument to type: must be one of
    numeric(), numeric(integer), numeric(integer, integer)

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.

Вложения

Re: [PATCHES] Bundle of patches

От
Teodor Sigaev
Дата:
 >>   Perhaps an array of int4 would be better?  How much

Done
http://www.sigaev.ru/misc/user_defined_typmod-0.9.gz

>> The patch needs more cleanup before applying, too, eg make comments
>> match code, get rid of unused keywords added to gram.y.

Cleaned.


--
Teodor Sigaev                                   E-mail: teodor@sigaev.ru
                                                    WWW: http://www.sigaev.ru/

Re: [PATCHES] Bundle of patches

От
Tom Lane
Дата:
Teodor Sigaev <teodor@sigaev.ru> writes:
>>> Perhaps an array of int4 would be better?  How much

> Done
> http://www.sigaev.ru/misc/user_defined_typmod-0.9.gz

>>> The patch needs more cleanup before applying, too, eg make comments
>>> match code, get rid of unused keywords added to gram.y.

> Cleaned.

OK.  I'm up to my rear in the opclass/opfamily rewrite, but will take
another look at the typmod code as soon as I have a working HEAD again ;-)

            regards, tom lane

Re: [PATCHES] Bundle of patches

От
Teodor Sigaev
Дата:
0.9 doesn't apply cleanly after Peter's changes, so, new version

http://www.sigaev.ru/misc/user_defined_typmod-0.10.gz

Teodor Sigaev wrote:
>  >>   Perhaps an array of int4 would be better?  How much
>
> Done
> http://www.sigaev.ru/misc/user_defined_typmod-0.9.gz
>
>>> The patch needs more cleanup before applying, too, eg make comments
>>> match code, get rid of unused keywords added to gram.y.
>
> Cleaned.
>
>

--
Teodor Sigaev                                   E-mail: teodor@sigaev.ru
                                                    WWW: http://www.sigaev.ru/

Re: [PATCHES] Bundle of patches

От
Teodor Sigaev
Дата:
Just a freshing for clean applying..
http://www.sigaev.ru/misc/user_defined_typmod-0.11.gz

Is any objections to commit?
--
Teodor Sigaev                                   E-mail: teodor@sigaev.ru
                                                    WWW: http://www.sigaev.ru/

Re: [PATCHES] Bundle of patches

От
Tom Lane
Дата:
Teodor Sigaev <teodor@sigaev.ru> writes:
> Just a freshing for clean applying..
> http://www.sigaev.ru/misc/user_defined_typmod-0.11.gz
> Is any objections to commit?

There's still a lot I don't particularly care for here (lack of
documentation being the biggest), but I'll make a pass at cleaning it up.

One thought I had after a quick reading of the patch is that there
doesn't seem to be a whole lot of percentage in trying to move the
typmod handling for time/timestamp/interval types into callable
functions, because we still have to special-case the darn things
thanks to the SQL spec's oddball syntax requirements.  For instance
in format_type_be() you've got

          case TIMETZOID:
              if (with_typemod)
!                 buf = psnprintf(50, "time(%d) with time zone",
!                                 typemod);
              else
                  buf = pstrdup("time with time zone");
              break;

changed to

          case TIMETZOID:
              if (with_typemod)
!                 buf = printTypmod("time", typemod, typeform->typmodoutput);
              else
                  buf = pstrdup("time with time zone");
              break;

which hardly seems like much of an improvement.  If we could have gotten
rid of the switch() branch for TIMETZOID entirely, then we'd be
somewhere, but there's no chance because the typmod-less case still has
to be special.  So I'm sort of inclined to leave the processing of these
datatypes as-is, and only use the typmodin/typmodout infrastructure for
datatypes that follow the "canonical" syntax of type_name(int[,int ...]).
Thoughts?

            regards, tom lane

Re: [PATCHES] Bundle of patches

От
Tom Lane
Дата:
Teodor Sigaev <teodor@sigaev.ru> writes:
> Just a freshing for clean applying..
> http://www.sigaev.ru/misc/user_defined_typmod-0.11.gz

Applied with some revisions, and pg_dump support and regression tests
added.

            regards, tom lane

Re: [PATCHES] Bundle of patches

От
Teodor Sigaev
Дата:
Nice, thanks a lot.

Tom Lane wrote:
> Teodor Sigaev <teodor@sigaev.ru> writes:
>> Just a freshing for clean applying..
>> http://www.sigaev.ru/misc/user_defined_typmod-0.11.gz
>
> Applied with some revisions, and pg_dump support and regression tests
> added.
>
>             regards, tom lane
>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: Have you checked our extensive FAQ?
>
>                http://www.postgresql.org/docs/faq

--
Teodor Sigaev                                   E-mail: teodor@sigaev.ru
                                                    WWW: http://www.sigaev.ru/