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 #17482 - default to "Full texts" when running explain statements #17906

Merged
merged 5 commits into from
Nov 19, 2022

Conversation

iifawzi
Copy link
Contributor

@iifawzi iifawzi commented Nov 18, 2022

Description

Hi, this should fix #17482.
Fixes #17482

Signed-off-by: iifawzi <iifawzie@gmail.com>
@iifawzi
Copy link
Contributor Author

iifawzi commented Nov 18, 2022

I'm not very sure that this's the correct way of testing this.

Signed-off-by: iifawzi <iifawzie@gmail.com>
@iifawzi
Copy link
Contributor Author

iifawzi commented Nov 18, 2022

I think it's not testing against the SQL-Parser#dev-master. I've hard-coded the SQL-Parser changes on my machine and it passed.

@williamdes williamdes changed the title Fixing #17482: default to "Full texts" when running explain statements Fix #17482: default to "Full texts" when running explain statements Nov 18, 2022
@williamdes williamdes changed the title Fix #17482: default to "Full texts" when running explain statements Fix #17482 - default to "Full texts" when running explain statements Nov 18, 2022
@williamdes
Copy link
Member

I think it's not testing against the SQL-Parser#dev-master. I've hard-coded the SQL-Parser changes on my machine and it passed.

yeah, you should update in this PR the hash I put in the composer.json file ;)

@iifawzi
Copy link
Contributor Author

iifawzi commented Nov 18, 2022

what do you mean by updating the hash @williamdes?

@williamdes
Copy link
Member

what do you mean by updating the hash @williamdes?

"phpmyadmin/sql-parser": "dev-master#a9cd167be77829ec2dd4496ce1010ed6abde92f8",

Signed-off-by: iifawzi <iifawzie@gmail.com>
@codecov
Copy link

codecov bot commented Nov 18, 2022

Codecov Report

Base: 49.71% // Head: 49.77% // Increases project coverage by +0.05% 🎉

Coverage data is based on head (706c08a) compared to base (1bf31e5).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff              @@
##             QA_5_2   #17906      +/-   ##
============================================
+ Coverage     49.71%   49.77%   +0.05%     
- Complexity    16742    16743       +1     
============================================
  Files           602      602              
  Lines         59784    59858      +74     
============================================
+ Hits          29722    29792      +70     
- Misses        30062    30066       +4     
Flag Coverage Δ
dbase-extension 49.56% <100.00%> (+0.64%) ⬆️
recode-extension 49.53% <100.00%> (+0.04%) ⬆️
unit-7.2-ubuntu-latest 48.41% <100.00%> (+0.06%) ⬆️
unit-7.3-ubuntu-latest 50.24% <100.00%> (+0.67%) ⬆️
unit-7.4-ubuntu-latest 49.76% <100.00%> (+0.21%) ⬆️
unit-8.0-ubuntu-latest 49.86% <100.00%> (+0.17%) ⬆️
unit-8.1-ubuntu-latest 49.85% <100.00%> (+0.12%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
libraries/classes/Display/Results.php 60.58% <100.00%> (+0.04%) ⬆️
libraries/classes/Sql.php 47.19% <100.00%> (+0.14%) ⬆️
libraries/classes/Partitioning/Partition.php 17.20% <0.00%> (-8.61%) ⬇️
libraries/classes/BrowseForeigners.php 60.52% <0.00%> (-1.22%) ⬇️
libraries/classes/Transformations.php 60.91% <0.00%> (-0.51%) ⬇️
libraries/classes/Navigation/Nodes/NodeColumn.php 80.00% <0.00%> (-0.49%) ⬇️
libraries/classes/CheckUserPrivileges.php 34.45% <0.00%> (-0.24%) ⬇️
libraries/classes/Config/FormDisplay.php 24.41% <0.00%> (-0.20%) ⬇️
...s/classes/Controllers/Table/RelationController.php 22.75% <0.00%> (-0.16%) ⬇️
libraries/classes/InsertEdit.php 87.64% <0.00%> (-0.08%) ⬇️
... and 26 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Signed-off-by: iifawzi <iifawzie@gmail.com>
Signed-off-by: iifawzi <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.

Looks good, will test it
Thank you for your work on this subject :)

@williamdes williamdes self-assigned this Nov 19, 2022
@williamdes williamdes added this to the 5.2.1 milestone Nov 19, 2022
@williamdes williamdes merged commit 7fb122a into phpmyadmin:QA_5_2 Nov 19, 2022
@nunoperalta
Copy link

Thank you very much for this, guys. You're amazing! 🤩🥳🎉

williamdes added a commit that referenced this pull request Nov 19, 2022
Pull-request: #17906

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

The (phpMyAdmin 5.2+snapshot) also available as a non official docker image is updated with this PR 🚀

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.

None yet

4 participants