[#22200] - [mysql] Correct position of column "state" in table "#__privacy_consents" in file "3.9.0-2018-08-12.sql"
- Fixed in Code Base
- 22 Sep 2018
- Medium
- Build: staging
- # 22200
- Diff
- richard67:patch-2
User tests: Successful: Unsuccessful:
Pull Request for new issue.
Summary of Changes
Add new column state to table #__privacy_consents at the same place as with a new installation in schema update for mysql.
Testing Instructions
Code review: Check the position of column state in the CREATE TABLE statement for table #__privacy_consents in the installation sql for mysql. Check that the changes of this PR correct the schema update so it adds the column at the same place.
See here:
https://github.com/joomla/joomla-cms/blob/staging/installation/sql/mysql/joomla.sql#L1702.
Verify also that there are already schema updates for mysql which use the "AFTER" clause when adding new columns, e.g. 2.5.0-2011-12-06.sql, 2.5.0-2012-01-10.sql, ..., 3.8.0-2017-07-28.sql, so you can see this PR adds nothing new regarding that.
Or real life test: Using MySQL database, install a new 3.9-beta2-dev (nightly build from today) on one site, install a 3.8.12 on another site and then update that site to nightly build from today. Then export both databases in sql and compare the sql files regarding table structures.
Expected result
Code review: Column state of table #__privacy_consents will be added at the right place by the schema update with this PR.
Real life test: Column state of table #__privacy_consents is at the same place on both sites.
Actual result
On a new installation, the column state of table #__privacy_consents comes after column user_id, while on an updated 3.8.12 it comes at the end of the table.
Documentation Changes Required
None.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | SQL Administration com_admin |
| Title |
|
||||||
The order of columns is not relevant in a relational database,
the AFTER clause is mysql only iirc, therefore maybe is better to change the install order instead
in this way we have a mysql table DDL for that table different from the DDL of the same table in mssql & postgresql, again column order is not important in relational databases, but having the same column ordering
for the same table cross different databases is more consistent
@alikon I only did it in the same way as it is done already with all the other schema updates in mysql: They make order in updated mysql consistent with order in new installed mysql. There never was consistency between updated mysql and updated postgresql. If it was not already this way, I would never have made this PR here. Please check that and you will see I am right.
| Title |
|
||||||
this not mean, that we are doing it in a consistent/correct way, imho we were wrong even in the past
@alikon It seems we are both right: If you change directory to folder "administrator/components/com_admin/sql/updates/mysql" and then on a Windows command shell do a findstr /i /c:"add column" *.sql or on a Linux shell do a grep -i "add column" *.sql, you will see that some 30 or 40 % of the statements use the "AFTER" clause and the others not. And so we are not really doing it in a consistent way, so or so.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/22200.
I leave this PR open for a while to see if there are other opinions about what is desired:
- Consistency between updated mysql and new installed mysql by always using "BEFORE" or "AFTER" with every "ADD COLUMN" statement in mysql schema updates, and not having consitency between updated mysql and updated other databases, or
- consistency between updated mysql and updated other database types by never using "BEFORE" or "AFTER" with any "ADD COLUMN" statement in mysql schema updates and keeping schema updates for all databases consistent regarding the order of "ADD COLUMN" statements over all schema updates (I think I remember this was never 100 % fulfilled in past and so this consistency can not be 100% granted from my point of view), and not having consistency between updated database and new installed database of same type
AFTER is always used in MySQL files unless column is being at the end of the table. I'm not particularly familiar with other databases but quick search tells me they don't support this. Their loss.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/22200.
Thanks @SharkyKZ . That confirms my finding.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/22200.
...we have always done this way....
never fight against windmills
Windmills? I always thought it was giants ;-)
https://en.wikipedia.org/wiki/Tilting_at_windmills
sorry i'm italian
so it's quite hard to translate from italian -> to english -> to whatever ... maybe klingon?
Don Quixote was fighting against imaginary giants which in fact were windmills, that's what I referred to, too.
Maybe I should not try to make jokes in English.
no problem at all, i'm always open to understand different sense of humor
This is purely about consistency.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/22200.
Finally.
so we have 2 tests now RTC
p.s.
i still think that this is not so consistent/correct by with 0.001 % of no-mysql users....
ubi major minor cessat
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/22200.
so or so, this pr is correct regarding the current way how things are done. If you want things to be done differently, feel free to make an own PR but not hijack other people'S PRs for that purpose.
mbabker
- | Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2018-09-22 15:12:51 |
| Closed_By | ⇒ | mbabker | |
| Labels |
Added:
?
|
||
@alikon @SharkyKZ Can you check this PR here, too?