OGR SQL expression parser overflow

[tamas]

OGR SQL limits the size of the expression token stack to 200 and a "memory exhausted" error is generated for large expressions. This is related to the YYINITDEPTH setting in swq_parser.cpp. Currently the built in stack extension mechanism is not enabled.

Attached a fix for the parser definition file.

[rouault]

Tamas, I've updated your patch by just adding a few line of comments, because it took me quite a bit to understand how this worked. Looks fine to apply in trunk and 1.8 branch too

[tamas]

Even, The documentation you refer to is quite confusing. It talks about YYMAXDEPTH which is strictly related to the feature you mean "undocumented".

I also have an impression that YYSTYPE_IS_TRIVIAL may depend on how YYSTYPE is defined actually. for an example see the mapserver parser in  mapparser.c where YYSTYPE_IS_TRIVIAL is defined by default.

[rouault]

What I meant to be undocumented is YYSTYPE_IS_TRIVIAL itself. But perhaps I didn't look at the right place ? I found out why YYSTYPE_IS_TRIVIAL was necessary by grepping for it in swq_parser.cpp

#if (! defined yyoverflow \
     && (! defined __cplusplus \
	 || (defined YYSTYPE_IS_TRIVIAL && YYSTYPE_IS_TRIVIAL)))

From the  http://www.gnu.org/s/bison/manual/html_node/Memory-Management.html page, my feeling is that the stack extension mechanism isn't enabled by default when generated the parser as a C++ file (which is OGR case, to be opposed to the C mapparser.c). So you then have to define YYSTYPE_IS_TRIVIAL yourself after ensuring that the YYSTYPE isn't a C++ object, but something that can be safely memcpy'ed for example. Well, this is my interpretation of a quick look at the generated code.

I tested a bit your patch with GCC on Linux and it worked fine. I suppose you tested it on Windows with MSVC ? Enhancing the test suite might be good too.

As the patch isn't invasive, so I think it is OK to apply it to trunk even if there's some uncertainty about its correctness, and in case of issues, the revert will be easy. If you wish, you could wait for it to be a bit more tested in trunk before backporting to 1.8. Just my 2 cents.

[tamas]

Applied in trunk (r23206)

[rouault]

r23211 /trunk/autotest/ogr/ogr_sql_test.py: Test huge SQL queries (#4262)

[rouault]

r23311 /branches/1.8/ (3 files in 2 dirs): Allow parser to deal with huge SQL queries (#4262)

[rouault]

Milestone 1.8.2 deleted