[4.0] Invalid SQL prevents saving of modules for Administrator

[eXsiLe95]

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

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

[brianteeman]

I cannot replicate the bug

[eXsiLe95]

I cannot replicate the bug

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

[brianteeman]

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

[roland-d]

@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.

[brianteeman]

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

[brianteeman]

Sorry ignore that comment

[brianteeman]

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

[brianteeman]

reinstalled - still dont have a bug

[brianteeman]

[roland-d]

@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.

[brianteeman]

I am running 5.7 so maybe

[eXsiLe95]

@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

[nibra]

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

[roland-d]

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

[mbabker]

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.

[roland-d]

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

[nibra]

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

[aschkenasy]

@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,

[alikon]

Null or Not Null this is the question !

😆

[alikon]

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

[eXsiLe95]

@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

[aschkenasy]

'''
'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)

[roland-d]

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

[ggppdk]

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

[eXsiLe95]

@ggppdk: Exactly my point!

[roland-d]

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

[ggppdk]

@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 ?

[eXsiLe95]

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)

[roland-d]

@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.

[aschkenasy]

@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

[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.
Ooops, I was a little fast on that one!

@aschkenasy I'll take care of it tomorrow!

[aschkenasy]

@eXsiLe95 awesome. thank you.

[brianteeman]

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

[joomla-cms-bot]

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