Обсуждение: A small bug in gram.y
Hi,<br /> In the gram.y, under a_expr rule<br /> under the subrule "a_expr NOT SIMILAR TO a_expr %prec
SIMILAR"<br/> the action is as follows<br /> {<br /> FuncCall *n = makeNode(FuncCall);<br />
n->funcname = SystemFuncName("similar_escape");<br /> n->args = list_make2($5,
makeNullAConst(-1));<br/> n->agg_star = FALSE;<br /> n->agg_distinct =
FALSE;<br/> n->func_variadic = FALSE;<br /> n->over = NULL;<br />
n->location = @5;<br /> $$ = (Node *) makeSimpleA_Expr(AEXPR_OP, "!~", $1, (Node *) n,
@2);<br/> }<br /><br /> I think the n->location should be @3. <br /><br />Thanks,<br />Gokul.<br />
Hmmm.... no-one else feels this as a bug????<br /><br />The logic is that a function call is made for "similar" and the
positionwhere SIMILAR occurs is at the third position, but it has been coded that it is at fifth position. <br /><br
/>Thanks,<br/>Gokul.<br /><br /><div class="gmail_quote">On Tue, Oct 27, 2009 at 6:51 AM, Gokulakannan Somasundaram
<spandir="ltr"><<a href="mailto:gokul007@gmail.com">gokul007@gmail.com</a>></span> wrote:<br /><blockquote
class="gmail_quote"style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
Hi,<br/> In the gram.y, under a_expr rule<br /> under the subrule "a_expr NOT SIMILAR TO a_expr %prec
SIMILAR"<br/> the action is as follows<br /> {<br /> FuncCall *n = makeNode(FuncCall);<br />
n->funcname = SystemFuncName("similar_escape");<br /> n->args = list_make2($5,
makeNullAConst(-1));<br/> n->agg_star = FALSE;<br /> n->agg_distinct =
FALSE;<br/> n->func_variadic = FALSE;<br /> n->over = NULL;<br />
n->location = @5;<br /> $$ = (Node *) makeSimpleA_Expr(AEXPR_OP, "!~", $1, (Node *) n,
@2);<br/> }<br /><br /> I think the n->location should be @3. <br /><br />Thanks,<br />Gokul.<br
/></blockquote></div><br/>
Gokulakannan Somasundaram wrote: > Hmmm.... no-one else feels this as a bug???? > > The logic is that a function call is made for "similar" and the position > where SIMILAR occurs is at the third position, but it has been coded that it > is at fifth position. The function call is constructed for the similar_escape function, to construct a regular expression equivalent to the right operand of the SIMILAR TO. So setting the error location to the right operand seems OK to me. However, I note that for the "a_expr SIMILAR TO a_expr" rule we're doing what you expected and the error location points to SIMILAR. I think we should change that to behave like NOT SIMILAR TO. Here's an example that exercises those paths: postgres=# SELECT 'aa' NOT SIMILAR TO 123; ERROR: function pg_catalog.similar_escape(integer, unknown) does not exist LINE 1: SELECT 'aa' NOT SIMILAR TO 123; ^ HINT: No function matches the given name and argument types. You might need to add explicit type casts. postgres=# SELECT 'aa' SIMILAR TO 123; ERROR: function pg_catalog.similar_escape(integer, unknown) does not exist LINE 1: SELECT 'aa' SIMILAR TO 123; ^ HINT: No function matches the given name and argument types. You might need to add explicit type casts. postgres=# I think the former error location is better. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
> Here's an example that exercises those paths:
> postgres=# SELECT 'aa' NOT SIMILAR TO 123;
> ERROR: function pg_catalog.similar_escape(integer, unknown) does not exist
> LINE 1: SELECT 'aa' NOT SIMILAR TO 123;
> ^
> HINT: No function matches the given name and argument types. You might
> need to add explicit type casts.
> postgres=# SELECT 'aa' SIMILAR TO 123;
> ERROR: function pg_catalog.similar_escape(integer, unknown) does not exist
> LINE 1: SELECT 'aa' SIMILAR TO 123;
> ^
> HINT: No function matches the given name and argument types. You might
> need to add explicit type casts.
> postgres=#
> I think the former error location is better.
FWIW, I like the second one better, and if you check around you'll find
that it matches most other similar stuff, eg
regression=# select 12 like 34;
ERROR: operator does not exist: integer ~~ integer
LINE 1: select 12 like 34; ^
HINT: No operator matches the given name and argument type(s). You might need to add explicit type casts.
I think the current coding probably is just a typo, but hadn't gotten
around to doing anything about it.
regards, tom lane
Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> wrote: > LINE 1: SELECT 'aa' NOT SIMILAR TO 123; > ^ > LINE 1: SELECT 'aa' SIMILAR TO 123; > ^ > I think the former error location is better. So do I. -Kevin
"Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
> Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> wrote:
>> LINE 1: SELECT 'aa' NOT SIMILAR TO 123;
>> ^
>> LINE 1: SELECT 'aa' SIMILAR TO 123;
>> ^
>> I think the former error location is better.
> So do I.
Uh, why? It looks like it's complaining about the constant 123,
not about the operator.
regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> wrote: > "Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes: >> Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> wrote: >>> LINE 1: SELECT 'aa' NOT SIMILAR TO 123; >>> ^ > >>> LINE 1: SELECT 'aa' SIMILAR TO 123; >>> ^ > >>> I think the former error location is better. > >> So do I. > > Uh, why? It looks like it's complaining about the constant 123, > not about the operator. I wrote that before I saw your post, which left me ambivalent. My thinking was that it seems clearest to me when it points to the token at which things become untenable. At this point there it is still possible for the statement to be completed with a valid query: SELECT 'aa' SIMILAR TO At this point it is not: SELECT 'aa' SIMILAR TO 123 If you had something like '123' instead of 123, it would be OK, so I'd be good with it complaining about the constant 123. SELECT 'aa' SIMILAR TO '123';?column? ----------f (1 row) But if we normally point to the operator when it isn't in the set allowed with the given operands, then consistency trumps the above. I yield the point. -Kevin
Tom Lane wrote: > "Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes: >> Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> wrote: >>> LINE 1: SELECT 'aa' NOT SIMILAR TO 123; >>> ^ > >>> LINE 1: SELECT 'aa' SIMILAR TO 123; >>> ^ > >>> I think the former error location is better. > >> So do I. > > Uh, why? It looks like it's complaining about the constant 123, > not about the operator. The problem *is* in the constant 123. It's of wrong type for SIMILAR TO operator. I guess your viewpoint is that the operator isn't correct for the operands. Fair enough. BTW, the corresponding error in the "SIMILAR TO ... ESCAPE ..." syntax is: postgres=# SELECT 'aa' SIMILAR TO 123 ESCAPE 'f'; ERROR: function pg_catalog.similar_escape(integer, unknown) does not exist LINE 1: SELECT 'aa' SIMILAR TO 123 ESCAPE 'f'; ^ HINT: No function matches the given name and argument types. You might need to add explicit type casts. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
> BTW, the corresponding error in the "SIMILAR TO ... ESCAPE ..." syntax is:
> postgres=# SELECT 'aa' SIMILAR TO 123 ESCAPE 'f';
> ERROR: function pg_catalog.similar_escape(integer, unknown) does not exist
> LINE 1: SELECT 'aa' SIMILAR TO 123 ESCAPE 'f';
> ^
Well, that's complaining specifically about the ESCAPE part of it.
This does expose the implementation detail that we try to build the
similar_escape() call before the overall similar() function call.
regards, tom lane
Tom Lane wrote: > Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes: >> BTW, the corresponding error in the "SIMILAR TO ... ESCAPE ..." syntax is: > >> postgres=# SELECT 'aa' SIMILAR TO 123 ESCAPE 'f'; >> ERROR: function pg_catalog.similar_escape(integer, unknown) does not exist >> LINE 1: SELECT 'aa' SIMILAR TO 123 ESCAPE 'f'; >> ^ > > Well, that's complaining specifically about the ESCAPE part of it. > This does expose the implementation detail that we try to build the > similar_escape() call before the overall similar() function call. Yeah. The "ESCAPE 'f'" part is OK. It's still the 123 that's of the wrong type. I'd say that message should point to 123 as well. Or to SIMILAR, if we take your stance that the error is with the SIMILAR TO operator in general. But pointing to ESCAPE is just weird. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
"Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
> Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Uh, why? It looks like it's complaining about the constant 123,
>> not about the operator.
> I wrote that before I saw your post, which left me ambivalent. My
> thinking was that it seems clearest to me when it points to the token
> at which things become untenable.
Our error pointers are *not* about how far to the right did parsing
get, they're about which part of the construct seems to be most
directly related to the problem. Otherwise most of them would point
at the ending semicolon ;-). A possibly less flippant example is
select nosuchfunction(1,2,3,avalidfunction(4)); ^
select nosuchfunction(1,2,3,avalidfunction(4)); ^
Which of these is less likely to be misread about which function is
being complained of?
regards, tom lane
Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
> ... But pointing to ESCAPE is just weird.
Maybe. The implementation's-eye view of this is that the construct is
data SIMILAR-operator (pattern ESCAPE-operator escape-char)
but I agree that isn't the way a user will think of it. I could see
making the location be that of the SIMILAR keyword for both of the
operators.
regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> wrote: > select nosuchfunction(1,2,3,avalidfunction(4)); > ^ > > select nosuchfunction(1,2,3,avalidfunction(4)); > ^ > > Which of these is less likely to be misread about which function is > being complained of? Actually, I much prefer what PostgreSQL does. :-) ERROR: function nosuchfunction(integer, integer, integer, integer) does not exist LINE 1: select nosuchfunction(1,2,3,avalidfunction(4)); ^ HINT: No function matches the given name and argument types. You might need to add explicit type casts. But, anyway, point taken. -Kevin
Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
> ... But pointing to ESCAPE is just weird.
I've changed these all to @2 (LIKE, ILIKE, SIMILAR TO).
regards, tom lane