Обсуждение: parse_coerce question

Hi Thomas,

I have been noticing that if I write something like... WHERE float8column < 33;
the system is not very smart about it, whereas... WHERE float8column < 33.0;
works fine.  The reason is that what the optimizer gets handed in the
first case is actually... WHERE float8column < float8(33);
ie, the parse tree reflects a run-time type coercion function call;
and the optimizer doesn't recognize that as a potential indexqual
restriction --- it wants "var op constant".

Of course the long-run answer for problems of this ilk is to insert a
constant-expression-folding stage, but for this particular case it seems
to me that the parser is wasting a lot of cycles by not outputting a
constant node of the right type in the first place.  Especially since it
does convert constants to the desired type in other cases.

Looking into this, I find that the reason for the difference is that
parse_coerce() only performs parse-time coercion of constants if they
are of type UNKNOWNOID --- ie, the constant is of string type.  And
indeed... WHERE float8column < '33';
produces a pre-coerced float8 constant!  But make_const produces type
INT4OID or INT8OID for integer or float constants.

It seems to me that parse_coerce ought to do parse-time coercion if
the input tree is a constant of either UNKNOWNOID, INT4OID, or FLOAT8OID
type, and only fall back to inserting a function call if it's unable
to do the coercion.  Am I missing anything?

It also looks like parser_typecast2() could be dispensed with, or more
accurately folded into parse_coerce().  Is there a reason not to?
        regards, tom lane

> Looking into this, I find that the reason for the difference is that
> parse_coerce() only performs parse-time coercion of constants if they
> are of type UNKNOWNOID --- ie, the constant is of string type.  And
> indeed
>     ... WHERE float8column < '33';
> produces a pre-coerced float8 constant!  But make_const produces type
> INT4OID or INT8OID for integer or float constants.
> 
> It seems to me that parse_coerce ought to do parse-time coercion if
> the input tree is a constant of either UNKNOWNOID, INT4OID, or FLOAT8OID
> type, and only fall back to inserting a function call if it's unable
> to do the coercion.  Am I missing anything?

You are right.  The textin/out trick is an old one, and one we only did
because we _had_ to make some conversion at that point.  No problem
making it more general.


--  Bruce Momjian                        |  http://www.op.net/~candle maillist@candle.pha.pa.us            |  (610)
853-3000+  If your life is a hard drive,     |  830 Blythe Avenue +  Christ can be your backup.        |  Drexel Hill,
Pennsylvania19026
 

> > It seems to me that parse_coerce ought to do parse-time coercion if
> > the input tree is a constant of either UNKNOWNOID, INT4OID, or FLOAT8OID
> > type, and only fall back to inserting a function call if it's unable
> > to do the coercion.  Am I missing anything?
> You are right.  The textin/out trick is an old one, and one we only did
> because we _had_ to make some conversion at that point.  No problem
> making it more general.

Sure, as long as we don't use textin/out to do it. It's an old trick
with more limitations than benefits. The Right Way to approach it is
to use type-specific conversion functions, so that real conversions
can take place. textin/out relies on the fact that the printed format
of a type is *precisely* the same as the format for the target type,
which is true for only a very limited number of cases.

There is already code for doing type coersion. As Tom points out, it
currently wraps a type conversion function around the constant, to be
evaluated later. It should be easy to pre-evaluate that function,
which btw should happen anyway. afaik it does, but not until after the
optimizer has had its look at the query, and by then it is too late to
select an index properly, for example.

For the index selection problem, I was thinking to move some of the
parse_coerce techniques to that part of the code, so that functions on
constants are allowed to be considered as candidate constants in a
query.

In any case, you'll need to make sure that you only promote types one
direction, so that (for example)
 select intcol from table where intcol < 33.5;

gets evaluated correctly.
                       - Thomas

-- 
Thomas Lockhart                lockhart@alumni.caltech.edu
South Pasadena, California

Thomas Lockhart <lockhart@alumni.caltech.edu> writes:
>>>> It seems to me that parse_coerce ought to do parse-time coercion if
>>>> the input tree is a constant of either UNKNOWNOID, INT4OID, or FLOAT8OID
>>>> type, and only fall back to inserting a function call if it's unable
>>>> to do the coercion.  Am I missing anything?
>> You are right.  The textin/out trick is an old one, and one we only did
>> because we _had_ to make some conversion at that point.  No problem
>> making it more general.

> Sure, as long as we don't use textin/out to do it. It's an old trick
> with more limitations than benefits. The Right Way to approach it is
> to use type-specific conversion functions, so that real conversions
> can take place.

Right --- the revision I committed last night looks up the
type-conversion function the same as before, but then applies it
immediately if the input is a constant.

> It should be easy to pre-evaluate that function,
> which btw should happen anyway. afaik it does, but not until after the
> optimizer has had its look at the query,

I'm not aware of any post-optimizer place where that might happen.
In any case, the optimizer would be much happier if constant-expression
reduction happened before it rather than after.

> For the index selection problem, I was thinking to move some of the
> parse_coerce techniques to that part of the code, so that functions on
> constants are allowed to be considered as candidate constants in a
> query.

I still think we want a generalized constant-expression folder, applied
after rule rewrite and before the optimizer.  This particular case was
just something I thought the parser should handle, since it was already
handling closely related cases...

> In any case, you'll need to make sure that you only promote types one
> direction, so that (for example)
>   select intcol from table where intcol < 33.5;
> gets evaluated correctly.

That is not parse_coerce()'s problem --- it just does what it's told.
        regards, tom lane

> > In any case, you'll need to make sure that you only promote types one
> > direction, so that (for example)
> >   select intcol from table where intcol < 33.5;
> > gets evaluated correctly.
> That is not parse_coerce()'s problem --- it just does what it's told.

Right. I wasn't sure how you were going to implement it. If you are
doing everything the same, but just pre-evaluating the result, we
should be OK.
                      - Thomas

-- 
Thomas Lockhart                lockhart@alumni.caltech.edu
South Pasadena, California