? Failure

User tests: Successful: Unsuccessful:

avatar eXsiLe95
eXsiLe95
29 Aug 2017

Summary of Changes

Edited 'content' field on table '#__modules' to be NULL by default.

Testing Instructions

  1. Get a fresh installation of Joomla! 4
  2. Go to Extensions > Modules
  3. Create a new module for Administrator (e.g. Administrator Menu)
  4. Give it a name and try to save it

Expected result

Module gets saved and can be added to a custom menu

Actual result

image

Additional information

After I got the said error (see above), I tried to figure out why there is no value in the SQL request for 'content' I figured out that the type TEXT doesn't allow anything else but NULL as a default value.

This refers to 792c3d5#diff-135c6f58583408312a709459b19594c7R1451

Votes

# of Users Experiencing Issue
2/2
Average Importance Score
5.00

avatar joomla-cms-bot joomla-cms-bot - change - 29 Aug 2017
Category SQL Installation Postgresql
avatar eXsiLe95 eXsiLe95 - open - 29 Aug 2017
avatar eXsiLe95 eXsiLe95 - change - 29 Aug 2017
Status New Pending
avatar brianteeman
brianteeman - comment - 29 Aug 2017

I cannot replicate the bug

avatar eXsiLe95
eXsiLe95 - comment - 29 Aug 2017

I cannot replicate the bug

Can you give me additional information to your system and setup?

avatar brianteeman
brianteeman - comment - 29 Aug 2017

Joomla 4 as of the time of this post
php 7.1.1
mysql

screenshotr14-38-18

avatar roland-d
roland-d - comment - 29 Aug 2017

@brianteeman Can you check the field configuration in the database? This is the modules table and it concerns the field content.

In the current codebase, as referred to in the commit link, we have set ````content` text NOT NULL DEFAULT '',``` but MySQL doesn't support default values for text except for NULL.

BLOB and TEXT columns cannot have DEFAULT values. Source: https://dev.mysql.com/doc/refman/5.6/en/blob.html

I am not sure how the content fields is sometimes not given an error and sometimes is. However when the field is set as defined in the SQL file with not having a default value, the error should appear.

avatar brianteeman
brianteeman - comment - 29 Aug 2017

Am i misunderstanding something? the modules table doesnt have a field "content"

avatar brianteeman
brianteeman - comment - 29 Aug 2017

Sorry ignore that comment

avatar brianteeman
brianteeman - comment - 29 Aug 2017

Extensions database said it was up to date but clearly it wasnt - reinstalling and rechecking

avatar brianteeman
brianteeman - comment - 29 Aug 2017

reinstalled - still dont have a bug

avatar brianteeman
brianteeman - comment - 29 Aug 2017

screenshotr15-01-38

avatar roland-d
roland-d - comment - 29 Aug 2017

@brianteeman This is weird right? In your screenshot I can see there is no default value nor is this module saving any data for a content field because it doesn't have this field. I wonder if this is something related to the new nulldate in 5.7, which doesn't stop Joomla from functioning but you can't update the fields using PhpMyAdmin for example.

I was able to reproduce this by renaming the modules table and create it again using the SQL command from the SQL file. If I then tried to save the module I see the error. Of course I had to change the SQL query to not include the DEFAULT '' because otherwise it would not execute the query.

Regardless, looking at the MySQL documentation, the SQL code must be fixed.

avatar brianteeman
brianteeman - comment - 29 Aug 2017

I am running 5.7 so maybe

avatar eXsiLe95
eXsiLe95 - comment - 29 Aug 2017

@brianteeman Thanks for your testing! As your screenshot shows, there is no default. The install script creates a default of '', so that's a difference, too, isn't it? Anyway, like @roland-d said, it's not valid SQL Code, so a fix would be fine

avatar nibra
nibra - comment - 30 Aug 2017

The different behaviour is most likely caused by different MySQL/MariaDB versions.

avatar roland-d
roland-d - comment - 30 Aug 2017

@nibra I wouldn't be surprised. Do you agree that the SQLs should be fixed regardless because it is invalid SQL syntax?

avatar mbabker
mbabker - comment - 30 Aug 2017

To be honest I lean so heavily on ORMs and DBALs to build my schemas that I don't know if this is "right" or "wrong". So I defer to those who do, but if this change does result in using valid SQL syntax then it looks fine to me.

avatar roland-d
roland-d - comment - 30 Aug 2017

I am just going by the MySQL documentation in this case. Going to poke Eli.

avatar nibra
nibra - comment - 30 Aug 2017

Yes, using NULL as default for 'no value' is the right thing to do.

avatar aschkenasy
aschkenasy - comment - 30 Aug 2017

@nibra It might be lost in translation but default NULL isn't correct. It should be TEXT (allow null) and no default.
@eXsiLe95 content text,

avatar alikon
alikon - comment - 30 Aug 2017

Null or Not Null this is the question !

?

avatar alikon
alikon - comment - 30 Aug 2017

we should seriously start thinking about "schema" re-design

avatar eXsiLe95
eXsiLe95 - comment - 30 Aug 2017

@aschkenasy

@eXsiLe95 content text,

I don't think I got your message right... What is it supposed to look like?

The thing is, I experienced an error with the SQL installation script as it is right now, so for people with my combination of software running, we need a fix for that.

I forgot to post that information, I guess:
PHP Built on: Windows NT 10.0 build 15063
Database Version: 5.5.5-10.1.25-MariaDB
Database Collation: utf8_general_ci
Database Connection Collation: utf8_general_ci
PHP Version: 7.1.7
Web Server: Apache/2.4.26 (Win32) OpenSSL/1.0.2l PHP/7.1.7
WebServer to PHP Interface: apache2handler
Joomla! Version; Joomla! 4.0.0-dev Development [ Amani ] 31-March-2017 23:59 GMT

In the MySQL documentation, I found this for MySQL 5.5-5.7:

BLOB and TEXT columns cannot have DEFAULT values.
See:
https://dev.mysql.com/doc/refman/5.7/en/blob.html
https://dev.mysql.com/doc/refman/5.6/en/blob.html
https://dev.mysql.com/doc/refman/5.5/en/blob.html

avatar aschkenasy
aschkenasy - comment - 1 Sep 2017

'''
'content' TEXT
'''
No default value at all. DEFAULT NULL is incorrect. Just to clarify; do not use NOT NULL or DEFAULT NULL

@alikon I'll ping you directly. We are in the process of full schema revisit (including FKs)

avatar roland-d
roland-d - comment - 3 Sep 2017

@aschkenasy Is this PR still relevant if a new schema is coming?

avatar ggppdk
ggppdk - comment - 3 Sep 2017

In any case the current installation SQL file is wrong

  • we ask database CREATE stratement to make a default value for Text which is not allowed / not possible

This default value is ignored by MySql / Maria-DB during table creation, but this lack of default value results in some DB versions with this error message

so in any case this and all the other default values for text / mediumtext / etc columns,

  • need to be removed and either allow NULL in them or make sure that the saving code saves an empty string
avatar eXsiLe95
eXsiLe95 - comment - 3 Sep 2017

@ggppdk: Exactly my point!

avatar roland-d
roland-d - comment - 3 Sep 2017

If we get a new schema that will take care of all these issues as well I would think.

avatar ggppdk
ggppdk - comment - 3 Sep 2017

@roland-d

If we get a new schema that will take care of all these issues as well I would think.

I was not aware of this, can you like to information about "new schema" ?
is there a PR ?
a link to some discussions about this ?

avatar eXsiLe95
eXsiLe95 - comment - 3 Sep 2017

It will, for sure, solve this problem. How far is the development of a new database scheme? I think until that's released, this quick bugfix is okay for now (when I remove DEFAULT NULL like @aschkenasy said)

avatar roland-d
roland-d - comment - 3 Sep 2017

@ggppdk I was not aware of a new schema either until Eli mentioned it in this comment .

@eXsiLe95 Since this PR is for 4.0 it won't be a quick fix unless the new schema is not going to be implemented in 4.0. If it is broken in Joomla 3 as well, we should have a separate PR for Joomla 3.

avatar aschkenasy
aschkenasy - comment - 3 Sep 2017

@eXsiLe95 @roland-d it needs to be changed and an additional PR for 3 needs to be created.

all of this assuming the PR changes to be correctly not assigning a default value as otherwise it doesn't fix the problem of OP

avatar eXsiLe95
eXsiLe95 - comment - 3 Sep 2017

Since this PR is for 4.0 it won't be a quick fix unless the new schema is not going to be implemented in 4.0. If it is broken in Joomla 3 as well, we should have a separate PR for Joomla 3.
Ooops, I was a little fast on that one!

@aschkenasy I'll take care of it tomorrow!

avatar aschkenasy
aschkenasy - comment - 4 Sep 2017

@eXsiLe95 awesome. thank you.

avatar eXsiLe95 eXsiLe95 - change - 4 Sep 2017
Labels Added: ?
avatar brianteeman
brianteeman - comment - 22 Mar 2019

Looks like this has been resolved elsewhere and can be closed.

avatar joomla-cms-bot joomla-cms-bot - close - 22 Mar 2019
avatar Quy Quy - change - 22 Mar 2019
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2019-03-22 16:49:19
Closed_By Quy
avatar joomla-cms-bot
joomla-cms-bot - comment - 22 Mar 2019

Set to "closed" on behalf of @Quy by The JTracker Application at issues.joomla.org/joomla-cms/17752

avatar joomla-cms-bot joomla-cms-bot - change - 22 Mar 2019
Status Fixed in Code Base Closed
Closed_Date 2019-03-22 16:49:19 2019-03-22 16:49:20
Closed_By Quy joomla-cms-bot

Add a Comment

Login with GitHub to post a comment