[Phpmyadmin-devel] BugFix question - Bug #641765
Yo, I'm busy fixing bug #641765 And i've come across a strange bit in sql.php3 @ line 88 $parsed_sql = PMA_SQP_parse((get_magic_quotes_gpc() ? stripslashes($sql_query) : $sql_query)); //86 $analyzed_sql = PMA_SQP_analyze($parsed_sql); //87 $sql_query = PMA_SQP_formatHtml($parsed_sql, 'query_only'); //88 These lines are from: "Committed Marc's patches to the SQL parser and pretty printer from bugs #605030 and #631421" I'm trying to look for the reasoning behind line 88. It is, for all-practical purposes, useless, since it just puts the same query back into the $sql_query variable. It is also part of the problem with #641765, since the SQL parser was giving an empty $parsed_sql return value in specific cases. I'm commenting it out for now, since that seems to have no ill side effects. Any objections can get it put back in. -- Robin Hugh Johnson E-Mail : robbat2@orbis-terrarum.net Home Page : http://www.orbis-terrarum.net/?l=people.robbat2 ICQ# : 30269588 or 41961639 GnuPG FP : 11AC BA4F 4778 E3F6 E4ED F38E B27B 944E 3488 4E85
Robin Johnson wrote:
Yo,
I'm busy fixing bug #641765 And i've come across a strange bit in sql.php3 @ line 88 $parsed_sql = PMA_SQP_parse((get_magic_quotes_gpc() ? stripslashes($sql_query) : $sql_query)); //86 $analyzed_sql = PMA_SQP_analyze($parsed_sql); //87 $sql_query = PMA_SQP_formatHtml($parsed_sql, 'query_only'); //88
These lines are from: "Committed Marc's patches to the SQL parser and pretty printer from bugs #605030 and #631421"
I'm trying to look for the reasoning behind line 88. It is, for all-practical purposes, useless, since it just puts the same query back into the $sql_query variable. It is also part of the problem with #641765, since the SQL parser was giving an empty $parsed_sql return value in specific cases.
I'm commenting it out for now, since that seems to have no ill side effects. Any objections can get it put back in.
Robin, I understand that line 88 might be the cause of a bug, but commenting out will cause other troubles. This line is not useless since it puts back a reformatted query, with reserved words upcased. There was a function that I removed: http://cvs.sourceforge.net/cgi-bin/viewcvs.cgi/phpmyadmin/phpMyAdmin/sql.php... that was modifying sql_query (see the comments in the function), because later in sql.php3 (current lines 244+) there are several tests made. But this function had problems in some cases. Maybe those tests could be made with the analyzer (some more queryflags) so we would not need to modify $sql_query. Marc
On Mon, Jan 13, 2003 at 09:16:29AM -0500, Marc Delisle wrote: > There was a function that I removed: > http://cvs.sourceforge.net/cgi-bin/viewcvs.cgi/phpmyadmin/phpMyAdmin/sql.php3.diff?r1=1.134&r2=1.135 http://cvs.sourceforge.net/cgi-bin/viewcvs.cgi/phpmyadmin/phpMyAdmin/sql.php3.diff?r1=1.133&r2=1.135 Shows the function quite nicely. > This line is not useless since it puts back a reformatted query, > with reserved words upcased. The things that function is listed as doing are: 1. Separates a backquoted expression with spaces 2. Separates a parenthesized expression with spaces 3. Removes repeated spaces 4. Uppercase reserved words why do we need to uppercase the words anyway (see further below)? MySQL has never been case-sensitive for reserved AFAIK, that is part of the ANSI SQL standard (which mysql strives to follow). The SQL parser definetly does #1, #3, #4 and is a lot pickier about using #2. > that was modifying sql_query (see the comments in the function), > because later in sql.php3 (current lines 244+) there are several tests > made. But this function had problems in some cases. > Maybe those tests could be made with the analyzer (some more queryflags) > so we would not need to modify $sql_query. Looking at that code, it looks like a major code-cleaning is needed there, because I see MySQL4 and even later parts of MySQL3.23 subetly failing parts of that code. It uses case-insensative regex for searching negating #4 above. Furthermore that stuff can use the results of the SQL parser for a nice speed improvement (through replacing the slow eregi), and have the same results. I'll go and rewrite that code to use the SQP results instead. It seems to be lines 48, 92, 222, 239-296, 379. Any more than you can point out ? -- Robin Hugh Johnson E-Mail : robbat2@orbis-terrarum.net Home Page : http://www.orbis-terrarum.net/?l=people.robbat2 ICQ# : 30269588 or 41961639 GnuPG FP : 11AC BA4F 4778 E3F6 E4ED F38E B27B 944E 3488 4E85
Robin Johnson wrote:
Looking at that code, it looks like a major code-cleaning is needed there, because I see MySQL4 and even later parts of MySQL3.23 subetly failing parts of that code. It uses case-insensative regex for searching negating #4 above. Furthermore that stuff can use the results of the SQL parser for a nice speed improvement (through replacing the slow eregi), and have the same results.
I'll go and rewrite that code to use the SQP results instead. It seems to be lines 48, 92, 222, 239-296, 379. Any more than you can point out ?
For sql.php3, this is complete. But *sigh* there is also libraries/display_tbl.lib.php3 (and others, I am sure). Marc
participants (2)
-
Marc Delisle -
Robin Johnson