On Thu, Oct 21, 2010 at 2:44 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
> On tor, 2010-10-14 at 22:54 -0400, Robert Haas wrote:
>> and maybe not that bad, but I wonder if there is some preparatory
>> refactoring that could be done to trim it down a bit. I notice, for
>> example, that a lot of places that looked at <asc/desc, nulls
>> first/last> now look at <asc/desc, nulls first/last, collation>. In
>> particular, all the pathkey stuff is like this. And similarly places
>> that used to care about <type, typmod> now have to care about <type,
>> tymod, collation>. There might be ways to restructure some of this
>> code so that these things can be changed without having to touch quite
>> so many places.
>
> Yeah, I think that's what I'll try to do next.
>
> We already have TypeName as a structure that contains type and typmod
> (and collation, in my patch). We could make that a primnode instead of
> a parsenode, and use it in more places, or we could make a new leaner
> structure that only contains the numeric info.
>
> We could then, for example, change things like this:
>
> typedef struct Var
> {
> Expr xpr;
> ...
> Oid vartype;
> int32 vartypmod;
> ...
> }
>
> into this
>
> typedef struct Var
> {
> Expr xpr;
> ...
> TypeName/TypeFoo vartype;
> ...
> }
>
> This would save boatloads of duplicate code.
I think that the idea of having a node that represents "a type in all
its glory" is a very good one. I'm somewhat inclined not to reuse
TypeName, because I think we'll end up wanting to use this in places
where "names" and "location" are not available. In fact, judging by
some of the logic in LookupTypeNames(), we have some cases like that
already, which might be worth trying to clean up.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company