From: Andrew Gierth <andrew@tao11.riddles.org.uk>
To: pgsql-hackers@postgresql.org
Subject: Re: A really subtle lexer bug
In-Reply-To: <87va851ppl.fsf@news-spur.riddles.org.uk> (Andrew Gierth's
	message of "Mon, 20 Aug 2018 08:38:52 +0100")
Message-ID: <87ftz90yma.fsf@news-spur.riddles.org.uk>
References: <87va851ppl.fsf@news-spur.riddles.org.uk>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (berkeley-unix)
Date: Mon, 20 Aug 2018 17:58:49 +0100
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="=-=-="
Precedence: bulk

--=-=-=
Content-Type: text/plain

>>>>> "Andrew" == Andrew Gierth <andrew@tao11.riddles.org.uk> writes:

 Andrew> select f(a =>-1);  -- ERROR:  column "a" does not exist

 Andrew> I guess the fix is to extend the existing special case code
 Andrew> that checks for one character left after removing trailing [+-]
 Andrew> and also check for the two-character ops "<>" ">=" "<=" "=>"
 Andrew> "!=".

Patch attached.

This fixes two bugs: first the mis-lexing of two-char ops as mentioned
originally; second, the O(N^3) lexing time of strings of - or +
characters is reduced to O(N^2) (in practice it's better than O(N^2)
once N gets large because the bison stack gets blown out, ending the
loop early).

-- 
Andrew (irc:RhodiumToad)


--=-=-=
Content-Type: text/x-patch
Content-Disposition: inline; filename=lex.patch
Content-Description: lexer patch

diff --git a/src/backend/parser/scan.l b/src/backend/parser/scan.l
index 0cd782827a..cc855ffb1c 100644
--- a/src/backend/parser/scan.l
+++ b/src/backend/parser/scan.l
@@ -885,20 +885,34 @@ other			.
 					 * to forbid operator names like '?-' that could not be
 					 * sequences of SQL operators.
 					 */
-					while (nchars > 1 &&
-						   (yytext[nchars - 1] == '+' ||
-							yytext[nchars - 1] == '-'))
+					if (nchars > 1 &&
+						(yytext[nchars - 1] == '+' ||
+						 yytext[nchars - 1] == '-'))
 					{
 						int			ic;
 
 						for (ic = nchars - 2; ic >= 0; ic--)
 						{
-							if (strchr("~!@#^&|`?%", yytext[ic]))
+							char c = yytext[ic];
+							if (c == '~' || c == '!' || c == '@' ||
+								c == '#' || c == '^' || c == '&' ||
+								c == '|' || c == '`' || c == '?' ||
+								c == '%')
 								break;
 						}
-						if (ic >= 0)
-							break; /* found a char that makes it OK */
-						nchars--; /* else remove the +/-, and check again */
+						if (ic < 0)
+						{
+							/*
+							 * didn't find a qualifying character, so remove
+							 * all trailing [+-]
+							 */
+							for (nchars--;
+								 nchars > 1 &&
+									 (yytext[nchars - 1] == '+' ||
+									  yytext[nchars - 1] == '-');
+								 nchars--)
+								continue;
+						}
 					}
 
 					SET_YYLLOC();
@@ -916,6 +930,25 @@ other			.
 						if (nchars == 1 &&
 							strchr(",()[].;:+-*/%^<>=", yytext[0]))
 							return yytext[0];
+						/*
+						 * Likewise, if what we have left is two chars, and
+						 * those match the tokens ">=", "<=", "=>", "<>" or
+						 * "!=", then we must return the sppropriate token
+						 * rather than the generic Op.
+						 */
+						if (nchars == 2)
+						{
+							if (yytext[0] == '=' && yytext[1] == '>')
+								return EQUALS_GREATER;
+							if (yytext[0] == '>' && yytext[1] == '=')
+								return GREATER_EQUALS;
+							if (yytext[0] == '<' && yytext[1] == '=')
+								return LESS_EQUALS;
+							if (yytext[0] == '<' && yytext[1] == '>')
+								return NOT_EQUALS;
+							if (yytext[0] == '!' && yytext[1] == '=')
+								return NOT_EQUALS;
+						}
 					}
 
 					/*

--=-=-=--