Skip to content

Remove from #__extensions useless columns: system_data and custom_data#14750

Merged
wilsonge merged 2 commits intojoomla:4.0-devfrom
csthomas:extension_del_columns
Mar 19, 2017
Merged

Remove from #__extensions useless columns: system_data and custom_data#14750
wilsonge merged 2 commits intojoomla:4.0-devfrom
csthomas:extension_del_columns

Conversation

@csthomas
Copy link
Contributor

Pull Request for Issue #14409

Summary of Changes

Remove 2 columns from #__extensions table and all files.

  1. Fix template copy functionality.
  2. Fix postgresql installation, some plugins in extension table has missing column value.
  3. Set correct PK value for template style: protostar in mysql
  4. Set correct auto increment value for postgresql - copy template error

Testing Instructions

  1. Try to copy template style for protostar - before patch it does not work.

  2. a. Install dev-4.0 joomla branch and then apply patch and go to database fix button. Mentioned columns should be deleted from databases.
    b. Test installation of patched Joomla on mysql and/or postgresql.

  3. Try to copy template style again - now it should work

Expected result

Template copy works.

Actual result

Error.

Documentation Changes Required

Maybe. Two columns has been deleted from #__extensions table.

@ghost
Copy link

ghost commented Mar 19, 2017

I have tested this item ✅ successfully on eb86c05


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/14750.

@wilsonge wilsonge merged commit 5ec7333 into joomla:4.0-dev Mar 19, 2017
@wilsonge wilsonge added this to the Joomla 4.0 milestone Mar 19, 2017
@csthomas csthomas deleted the extension_del_columns branch March 19, 2017 10:48
@Bakual
Copy link
Contributor

Bakual commented Mar 19, 2017

As I wrote in the issue, custom_data and system_data aren't useless columns. While they are currently not used by core, the system_data one was used store compatibility informations in J2.5 and probably will be used again in J3 again.
The custom_data one may be used by 3rd parties (eg a module or plugin wants to store something and an own table would be to much overhead).

So imho this PR is a wrong approach to fix a valid issue. We should instead seriously rethink the "NOT NULL" constraint we put on every field. We could save us some trouble by removing that on some of them (and it would fix this issue as well).

@csthomas
Copy link
Contributor Author

If the columns is really needed then we can revert it, but ...

all (or almost all) TEXT/BLOB columns should be NULL DEFAULT NULL.

@mbabker
Copy link
Contributor

mbabker commented Mar 19, 2017

+1 for revert.

We should make it clear what the field purposes are but just dropping them out of the blue seems a little drastic.

@Bakual
Copy link
Contributor

Bakual commented Mar 19, 2017

We would have to ask Samuel Moffatt for the real reason they were initially added. It was part of the #__extension table when it was created :)

I am just judging from the name of those fields, and as said I have used one of them in core itself in the past. Imho they are of situational use. We could add some comment to it of course 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

Comments