Fix #15994 - Attribute bug when default current_timestamp used with on update current_timestamp

[iifawzi]

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

Description

It seems like the bug in #15994 is occurred ONLY in the following case:

• when the default attribute is set to CURRENT_TIMESTAMP and an on update CURRENT_TIMESTAMP is used, this leads to generate the following Extra field: DEFAULT_GENERATED ON UPDATE CURRENT_TIMESTAMP as shown below (tested on the PMA demo with version 5.2.0-dev)
  

which fails this check:

phpmyadmin/templates/columns_definitions/column_attribute.twig

Lines 4 to 5 in c1a66ad

	and column_meta['Extra'] == 'on update CURRENT_TIMESTAMP' %}
	{% set attribute = 'on update CURRENT_TIMESTAMP' %}

i've changed it to check if the extra field contains on update CURRENT_TIMESTAMP or not.

Fixes #15994

[codecov]

Codecov Report

Merging #16653 (787a820) into QA_5_1 (88752d6) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             QA_5_1   #16653   +/-   ##
=========================================
  Coverage     53.03%   53.03%           
  Complexity    15168    15168           
=========================================
  Files           470      470           
  Lines         63112    63112           
=========================================
  Hits          33469    33469           
  Misses        29643    29643           

Flag	Coverage Δ	Complexity Δ
dbase-extension	52.66% <ø> (ø)	0.00 <ø> (ø)
recode-extension	52.62% <ø> (ø)	0.00 <ø> (ø)
unit-7.1-ubuntu-latest	52.62% <ø> (ø)	0.00 <ø> (ø)
unit-7.2-ubuntu-latest	52.80% <ø> (ø)	0.00 <ø> (ø)
unit-7.3-ubuntu-latest	57.61% <ø> (ø)	0.00 <ø> (ø)
unit-7.4-ubuntu-latest	57.62% <ø> (ø)	0.00 <ø> (ø)
unit-8.0-ubuntu-latest	57.78% <ø> (ø)	0.00 <ø> (ø)

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

---

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 88752d6...9503151. Read the comment docs.

[williamdes]

This looks like a good fix !

[williamdes]

@sudo-robot deploy

[sudo-robot]

Deployed (phpmyadmin-pr-16653): fix-extra-attribute-bug commit: c5f7735cce63b454e94b56853953e89e62c66529

url: https://phpmyadmin-pr-16653.wdes.eu.org
user: public
user: public

---

Deploy finished

[williamdes]

Hi @iifawzi

I could confirm that your fix works only for MySQL, could you make it MariaDB compatible ?

tested using the deployed server

[iifawzi]

Hi @iifawzi

I could confirm that your fix works only for MySQL, could you make it MariaDB compatible ?

tested using the deployed server

Hmm, it seems like that's because MariaDB generate the Extra field with additional Parentheses:
ON UPDATE CURRENT_TIMESTAMP()

I will work on it

[iifawzi]

i've added an or statement to check the case if it was with parentheses, I was thinking about using regex to delete the parentheses from the Extra column, but this would maybe cause an unexpected bugs/errors if there was other parentheses.

[williamdes]

i've added an or statement to check the case if it was with parentheses, I was thinking about using regex to delete the parentheses from the Extra column, but this would maybe cause an unexpected bugs/errors if there was other parentheses.

Maybe you could add a starts with or ends with https://twig.symfony.com/doc/3.x/templates.html#comparisons

And add a comment {# MariaDB has additional parentheses #}

What do you think ?
(could you squash your commits and force push ?)

[iifawzi]

i've added an or statement to check the case if it was with parentheses, I was thinking about using regex to delete the parentheses from the Extra column, but this would maybe cause an unexpected bugs/errors if there was other parentheses.

Maybe you could add a starts with or ends with https://twig.symfony.com/doc/3.x/templates.html#comparisons

And add a comment {# MariaDB has additional parentheses #}

What do you think ?
(could you squash your commits and force push ?)

I think starts with wouldn't work with MySql, since in MySql, the Extra field is generated as following (when default is used): DEFAULT_GENERATED ON UPDATE CURRENT_TIMESTAMP, am i missing something?

sure, i will squash them and add the comment.

[williamdes]

Indeed, very accurate, I seem to have missed that detail
I will merge your work tomorrow (16/02) after your squash ;)

[iifawzi]

Indeed, very accurate, I seem to have missed that detail
I will merge your work tomorrow (16/02) after your squash ;)

Awesome, thanks for your time reviewing this.

[williamdes]

I added 5e5733d because it did not work on my MariaDB 10.4 instance, using a lowercase compare is safer