Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
offsides opened this issue Jan 5, 2023 · 9 comments · Fixed by #411
Closed
Assignees
Labels
Milestone

Comments

@offsides
Copy link

offsides commented Jan 5, 2023

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.

@offsides
Copy link
Author

offsides commented Jan 5, 2023

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

@offsides
Copy link
Author

offsides commented Jan 5, 2023

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.

@williamdes williamdes transferred this issue from phpmyadmin/phpmyadmin Jan 6, 2023
@williamdes
Copy link
Member

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?

@williamdes williamdes added the bug label Jan 6, 2023
@offsides
Copy link
Author

offsides commented Jan 6, 2023

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.

@offsides offsides closed this as completed Jan 6, 2023
@williamdes
Copy link
Member

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

@williamdes
Copy link
Member

But no, it's not fixed in there.

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

@offsides
Copy link
Author

offsides commented Jan 6, 2023

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.

@williamdes
Copy link
Member

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 :)

iifawzi added a commit to iifawzi/sql-parser that referenced this issue Jan 15, 2023
Fix phpmyadmin#406

Signed-off-by: Fawzi Abdulfattah <iifawzie@gmail.com>
@williamdes williamdes added this to the 5.7.0 milestone Jan 15, 2023
@williamdes williamdes self-assigned this Jan 17, 2023
williamdes added a commit that referenced this issue Jan 17, 2023
Pull-request: #411

Signed-off-by: William Desportes <williamdes@wdes.fr>
@williamdes
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants