phpmyadmin/sql-parser

Exporting procedures with 'MODIFIES SQL DATA' generates a warning in OptionsArray.php

offsides opened this issue · 9 comments

Describe the bug

When you export a database with the 'Add CREATE PROCEDURE / FUNCTION / EVENT statement' option enabled, and the procedure definition includes 'MODIFIES SQL DATA', the parser generates the following message:

Warning in ./vendor/phpmyadmin/sql-parser/src/Components/OptionsArray.php#306
Undefined array key "value"

Dumping the value of $option just before that shows the following as the cause of the message:

Array
(   
    [name] => MODIFIES
    [equals] =>
    [expr] =>
)

To Reproduce

Create a procedure with 'MODIFIES SQL DATA' as the SQL data access option.
Click the Export tab for the database with said procedure.
Perform a custom export with the 'Add CREATE PROCEDURE / FUNCTION / EVENT statement'
See the warning and backtrace.

Expected behavior

No warning messages during the export.

Screenshots

Here's the full backtrace:

Warning in ./vendor/phpmyadmin/sql-parser/src/Components/OptionsArray.php#306
Undefined array key "value"

Backtrace

CreateStatement.php#491: PhpMyAdmin\SqlParser\Components\OptionsArray::build()
ExportSql.php#2814: PhpMyAdmin\SqlParser\Statements\CreateStatement->build()
ExportSql.php#558: PhpMyAdmin\Plugins\Export\ExportSql->replaceWithAliases(
string 'CREATE DEFINER=`root`@`localhost` PROCEDURE `split_string`(IN `x` VARCHAR(255), IN `delim` VARCHAR(12), IN `id` CHAR(36)) MODIFIES SQL DATA DETERMINISTIC BEGIN SET @valcount = substrCount(x,delim)+1; SET @vl = 0; CREATE TEMPORARY TABLE IF NOT EXISTS splitResultsTemp (uuid char(36), split_value varchar(255)) ENGINE=MEMORY; WHILE (@vl < @valcount) DO SET @val = stringSplit(x,delim,@vl+1); INSERT INTO splitResultsTemp (uuid, split_value) VALUES (id, @val); SET @vl = @vl + 1; END WHILE; END',
array,
string 'ftp_accounts',
string '',
boolean false,
)
ExportSql.php#620: PhpMyAdmin\Plugins\Export\ExportSql->exportRoutineSQL(
string 'ftp_accounts',
array,
string 'PROCEDURE',
string 'Procedures',
array,
string '$$',
)
Export.php#742: PhpMyAdmin\Plugins\Export\ExportSql->exportRoutines(
string 'ftp_accounts',
array,
)
ExportController.php#548: PhpMyAdmin\Export->exportDatabase(
string 'ftp_accounts',
array,
string 'structure_and_data',
array,
array,
,
string ' ',
string 'index.php?route=/database/export&db=ftp_accounts',
string 'database',
boolean true,
boolean true,
boolean true,
boolean true,
array,
string '',
)
Routing.php#192: PhpMyAdmin\Controllers\Export\ExportController->__invoke(
,
array,
)
index.php#43: PhpMyAdmin\Routing::callControllerForRoute(
,
string '/export',
,
,
)

Server configuration

  • Operating system: RHEL 9
  • Web server: Apache httpd 2.4.53-7.el9
  • Database version: mariadb-server 10.5.16-2.el9_0
  • PHP version: 8.1.8-1module+el91.0+15877+c3862210
  • phpMyAdmin version: 5.2.0-1.el9.remi

Client configuration

  • Browser: Firefox 108.0.1
  • Operating system: Windows 10 Enterprise 20H2

Additional context

This happens with multiple databases, each of witch have a stored procedure that modifies SQL data.

One other important piece I just noticed. The export of the CREATE PROCEDURE statement leaves out the word 'SQL' between 'MODIFIES' and 'DATA'. Here's the line from the export:

CREATE DEFINER=`root`@`localhost` PROCEDURE `split_string` (IN `x` VARCHAR(255), IN `delim` VARCHAR(12), IN `id` CHAR(36))  DETERMINISTIC MODIFIES  DATA BEGIN

And that gave me the hint to find the error - in vendor/phpmyadmin/sql-parser/src/Statements/CreateStatement.php line 248 is 'expr', when it should be 'var' (to match the 'NO' and 'READS' function options). I don't know if I'll have time to put together a PR anytime soon, but there's the fix.

This may already be fixed, maybe you can let us know by testing the latest 5.2 version in development (phpMyAdmin 5.2+snapshot) also available as a non official docker image?

OK, it looks like this actually belongs in phpmyadmin/sql-parser, rather than here. I'll refile in the correct project (and not it's not fixed there). I didn't realize that multiple projects had been packaged in a single RPM for EL9. But no, it's not fixed in there.

I am re-opening this one as I move it to the right project earlier

But no, it's not fixed in there.

How did you test this?
Did you try the snapshot?

I downloaded the code and looked at it. 'MODIFIES' is still set to 'expr' in the file src/Statements/CreateStatement.php on line 257. And sorry about the dup, I hadn't realized you moved it.

I downloaded the code and looked at it. 'MODIFIES' is still set to 'expr' in the file src/Statements/CreateStatement.php on line 257. And sorry about the dup, I hadn't realized you moved it.

thanks for taking the time to report back :)

I downloaded the code and looked at it. 'MODIFIES' is still set to 'expr' in the file src/Statements/CreateStatement.php on line 257. And sorry about the dup, I hadn't realized you moved it.

I think you can download the code again now that I merged #411
It should be fixed