BUG #2164: Very minor, very low priority SQL parser 'bug' with /* */ comments - Mailing list pgsql-bugs

The following bug has been logged online:

Bug reference:      2164
Logged by:          John Engelhart
Email address:      johne_ganz@yahoo.com
PostgreSQL version: 8.1
Operating system:   FreeBSD 5.4
Description:        Very minor, very low priority SQL parser 'bug' with /*
*/ comments
Details:

This is very minor bug, and doesn't necessarily require fixing.

This bug is kind of a gray area on what the right thing to do is.  It's easy
to work around, so IMHO whether or not it's 'fixed' should depend on
consensus of what the right thing to do is and how much parser/lexer code
needs to be changed to accommodate a fix.

It looks like there's a minor problem with the SQL parser where it tokenizes
any */ to mean 'end of comment block'.  I ran in to this while commenting
out a block of plpgsql code only to have it spit back an error when
reloading the file with all the sql functions in it.

Bug reproduction:

CREATE OR REPLACE FUNCTION comment_bug_example(string TEXT)



RETURNS INT AS



$PROC$



DECLARE



        matched TEXT;



BEGIN



        SELECT INTO matched substring(string, '.*/something/([0-9]+)/.*');



        IF matched IS NOT NULL THEN



                RETURN 1;



        ELSE



                RETURN 0;



        END IF;



END;



$PROC$ LANGUAGE 'plpgsql';

-- end example

When this piece of code is wrapped fully inside a /* */ style comment block,
the parser interprets the */ inside the substring() function as the end of
comment.  'Expected' behavior (principle of least astonishment, at least) is
that the .*/ part inside the substring function not trigger the end of
comments.

As read in from psql, before /* */ comment:

bugexample=# \i bug.sql
CREATE FUNCTION
Time: 0.846 ms
bugexample =#

And after wrapping in /* */ comment

bugexample =# \i bug.sql
psql:bug.sql:16: ERROR:  syntax error at or near "something" at character
158
psql:bug.sql:16: LINE 8:  SELECT INTO matched substring(string,
'.*/something/([0-9]+...
psql:bug.sql:16:                                                    ^
bugexample =#

Arguably the current behavior is the correct behavior, but it's also just
enough of a corner case to maybe argue for an exception.  Can't say that I
even have an opinion on which way it should be.  I'm mostly reporting this
as one of those oddball corner cases for discussion.  IMHO, if it requires
anything more than a very, very minor tweak to the parser then code
stability and simplicity trumps 'fixing' this hands down.  Bolting on a lot
of code to track grammar state while inside /* */ comments makes me cringe,
and is not worth the effort.  Also there may be code out there in which the
current behavior is the 'right thing' and 'fixing' it might break that
code.

All things being equal, I can understand if the decision is to just close
the bug without action if that's the consensus after weighing
effort/impact/ease of workaround issues.  I've got no problem with that, and
again, this is being reported just as one of those odd little corner
cases/side effects that you occasionally run in to but there's no clear
'right thing' to do about it.

Priority: very low

Work arounds:

The very obvious is to, say, insert a space at the offending */ comment
matcher.  If it's production code and the */ is in something important (like
a regex expression), then put comments near where the change was done and at
the start/end markers of /* */ so that anyone one else modifying the code
knows they need to make an additional change besides just removing comments.