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

Fix #339 - Fixing the issue of not handling the procedures's options #346

Merged
merged 1 commit into from
Aug 20, 2021

Conversation

iifawzi
Copy link
Contributor

@iifawzi iifawzi commented Aug 20, 2021

Hi, This PR should fix the issues relating to #339
The issue was in

public static $FUNC_OPTIONS = [
'COMMENT' => [
1,
'var=',
],
'LANGUAGE SQL' => 2,
'DETERMINISTIC' => 3,
'NOT DETERMINISTIC' => 3,
'CONTAINS SQL' => 4,
'NO SQL' => 4,
'READS SQL DATA' => 4,
'MODIFIES SQL DATA' => 4,
'SQL SECURITY DEFINER' => [
5,
'var',
],
];

it had some wrong tokens, and wasn't have many, which was leading to break the for loop that parses the options tokens
if (! isset($options[$upper])) {
// There is no option to be processed.
break;
}

I've fixed and added the correct options tokens as much as I could. Ref

Signed-off-by: Fawzi E. Abdulfattah iifawzie@gmail.com

@codecov
Copy link

codecov bot commented Aug 20, 2021

Codecov Report

Merging #346 (58f852a) into QA (c2ba926) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##                 QA     #346   +/-   ##
=========================================
  Coverage     99.80%   99.80%           
  Complexity     1916     1916           
=========================================
  Files            63       63           
  Lines          4604     4605    +1     
=========================================
+ Hits           4595     4596    +1     
  Misses            9        9           
Impacted Files Coverage Δ
src/Statements/CreateStatement.php 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c2ba926...58f852a. Read the comment docs.

@iifawzi
Copy link
Contributor Author

iifawzi commented Aug 20, 2021

I've made a fake integration with PMA, and all the tests passed too :')

Signed-off-by: Fawzi E. Abdulfattah <iifawzie@gmail.com>
Copy link
Member

@williamdes williamdes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect

@williamdes williamdes self-assigned this Aug 20, 2021
@williamdes williamdes added this to the 4.7.3 milestone Aug 20, 2021
@@ -52,6 +52,7 @@ public function alterProvider()
array('parser/parseAlterUser5'),
array('parser/parseAlterUser6'),
array('parser/parseAlterUser7'),
array('parser/parseAlterUser8'),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, I forgot to let you know I had patched it in: 1acb64c

@williamdes williamdes merged commit 7654273 into phpmyadmin:QA Aug 20, 2021
@williamdes williamdes linked an issue Aug 20, 2021 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bad procedure body
2 participants