Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix install languages and discover extension for mssql and small fixes for other databases #14321

Merged
merged 2 commits into from Mar 19, 2017

Conversation

csthomas
Copy link
Contributor

@csthomas csthomas commented Mar 3, 2017

Pull Request for Issue #9457 and related to #11763 which is incomplete.

Summary of Changes

Mysql strict mode - install languages fix.
Postgresql - reinstall languages fix, prevent double installation in db.
Mssql - install and reinstall and discover extension fix.

Additional changes:

  • column xreference across all databases has default value ''
  • asset_id on mysql has to be unsigned.

Mysql column type TEXT can not have default value ''. It is ignored.
Source: https://dev.mysql.com/doc/refman/5.7/en/blob.html

BLOB and TEXT columns cannot have DEFAULT values.

So from others db I also removed default value for:

  • data (#__updates)
  • custom_data (#__extensions)
  • system_data (#__extensions)

Testing Instructions

[MYSQL - strict mode]

  1. Install Joomla on mysql and enable strict mode, for ex change one line:
diff --git a/libraries/joomla/database/driver/mysqli.php b/libraries/joomla/database/driver/mysqli.php
index 37c9ae0f44..9c9e10206d 100644
--- a/libraries/joomla/database/driver/mysqli.php
+++ b/libraries/joomla/database/driver/mysqli.php
@@ -184,7 +184,8 @@ class JDatabaseDriverMysqli extends JDatabaseDriver
                }
 
                // Set sql_mode to non_strict mode
-               mysqli_query($this->connection, "SET @@SESSION.sql_mode = '';");
+               mysqli_query($this->connection, "SET @@SESSION.sql_mode = 'ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION';");
  1. Try to install languages, it should failed
  2. apply patch
  3. Now should be OK.

[POSTGRESQL]

  1. Install joomla on postgresql.
  2. Install some language pack, reinstall it. before patch language will be installed twice.
  3. After patch it is installed only once

[SQLSRV]

  1. Install joomla on sqlsrv
  2. Try to install some language - it fails
  3. Try to discover some extension - it fails (you should have one not installed, but extracted in correct place)
  4. Apply patch
  5. Install language
  6. Try to discover extension

Expected result

All above things works

Documentation Changes Required

None

@infograf768
Copy link
Member

infograf768 commented Mar 4, 2017

Tested on mysqli, using the JDatabaseDriverMysqli modification posted above.

Before patch, when installing Joomla, I can't install languages.
I get
Field 'data' doesn't have a default value

After patch, I indeed can install languages but I get in CPanel 2 errors

I can install languages but I get in CPanel

( ! ) Warning: Invalid argument supplied for foreach() in /Applications/MAMP/htdocs/testnew/trunkgitnew/administrator/modules/mod_popular/tmpl/default.php on line 16
( ! ) Warning: Invalid argument supplied for foreach() in /Applications/MAMP/htdocs/testnew/trunkgitnew/administrator/modules/mod_latest/tmpl/default.php on line 16

and, if I go to administrator/index.php?option=com_content
I get an error:

Error
'strictmode.a.title' isn't in GROUP BY 'strictmode.a.title' isn't in GROUP BY
An error has occurred.

Same for Newsfeeds Manager

Error also for other managers

Error
An error has occurred.

@infograf768
Copy link
Member

Forgot to say:
My db version is 5.5.25

@csthomas
Copy link
Contributor Author

csthomas commented Mar 4, 2017

Which version of joomla do you use?
I suppose you use some older version where such errors has not been fixed yet.

Besides that, this errors are unrelated. This PR only fix stuff for install language queries/database.

@infograf768
Copy link
Member

infograf768 commented Mar 4, 2017

I used a staging clean install (multilang) and the errors only display when I modify the driver with
mysqli_query($this->connection, "SET @@SESSION.sql_mode = 'ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION';");

As soon as I take off this modification, no more errors, even when your patch is applied.

@csthomas
Copy link
Contributor Author

csthomas commented Mar 4, 2017

Hmm, I have mysql 5.7 and I do not see any errors.

Please enable debug log in global configuration.
Change setting in System debug plugin:
Log Categories: database-error

(Logging Tab)
Log Almost Everything: Yes

Delete file if exist: administrator/logs/everything.php

Then try to generate above errors.
Next check above files. You should see some log errors.

@infograf768
Copy link
Member

Forgot to say that my database name is "strictmode"

#
#<?php die('Forbidden.'); ?>
#Date: 2017-03-04 10:46:15 UTC
#Software: Joomla Platform 13.1.0 Stable [ Curiosity ] 24-Apr-2013 00:00 GMT

#Fields: datetime	priority clientip	category	message
2017-03-04T10:46:15+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.title' isn't in GROUP BY
2017-03-04T10:46:15+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.title' isn't in GROUP BY
2017-03-04T10:46:15+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.title' isn't in GROUP BY
2017-03-04T10:46:15+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.title' isn't in GROUP BY
2017-03-04T10:46:15+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.asset_id' isn't in GROUP BY
2017-03-04T10:46:15+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.asset_id' isn't in GROUP BY
2017-03-04T10:46:16+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.title' isn't in GROUP BY
2017-03-04T10:46:16+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.title' isn't in GROUP BY
2017-03-04T10:46:16+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.title' isn't in GROUP BY
2017-03-04T10:46:16+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.title' isn't in GROUP BY
2017-03-04T10:46:16+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.asset_id' isn't in GROUP BY
2017-03-04T10:46:16+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.asset_id' isn't in GROUP BY
2017-03-04T10:46:16+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.title' isn't in GROUP BY
2017-03-04T10:46:16+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.title' isn't in GROUP BY
2017-03-04T10:46:16+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.asset_id' isn't in GROUP BY
2017-03-04T10:46:16+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.title' isn't in GROUP BY
2017-03-04T10:46:16+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.title' isn't in GROUP BY
2017-03-04T10:46:16+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.asset_id' isn't in GROUP BY
2017-03-04T10:48:10+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.title' isn't in GROUP BY
2017-03-04T10:48:10+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.title' isn't in GROUP BY
2017-03-04T10:48:10+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.asset_id' isn't in GROUP BY
2017-03-04T10:48:10+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.title' isn't in GROUP BY
2017-03-04T10:48:10+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.title' isn't in GROUP BY
2017-03-04T10:48:10+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.asset_id' isn't in GROUP BY
2017-03-04T10:48:11+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.title' isn't in GROUP BY
2017-03-04T10:48:11+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.title' isn't in GROUP BY
2017-03-04T10:48:11+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.asset_id' isn't in GROUP BY
2017-03-04T10:48:12+00:00	INFO ::1	updater	Loading information from update site #1 with name "Joomla! Core" and URL https://update.joomla.org/core/list.xml took 0.20 seconds
2017-03-04T10:48:12+00:00	INFO ::1	updater	Loading information from update site #2 with name "Accredited Joomla! Translations" and URL https://update.joomla.org/language/translationlist_3.xml took 0.14 seconds
2017-03-04T10:48:13+00:00	INFO ::1	updater	Loading information from update site #3 with name "Joomla! Update Component Update Site" and URL https://update.joomla.org/core/extensions/com_joomlaupdate.xml took 0.13 seconds
2017-03-04T10:48:14+00:00	INFO ::1	updater	Loading information from update site #1 with name "Joomla! Core" and URL https://update.joomla.org/core/list.xml took 0.17 seconds
2017-03-04T10:48:50+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.name' isn't in GROUP BY
2017-03-04T10:48:50+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.name' isn't in GROUP BY
2017-03-04T10:48:50+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.name' isn't in GROUP BY
2017-03-04T10:48:50+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.name' isn't in GROUP BY
2017-03-04T10:48:50+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.asset_id' isn't in GROUP BY
2017-03-04T10:48:50+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.asset_id' isn't in GROUP BY
2017-03-04T10:48:51+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.name' isn't in GROUP BY
2017-03-04T10:48:51+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.name' isn't in GROUP BY
2017-03-04T10:48:51+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.asset_id' isn't in GROUP BY
2017-03-04T10:48:51+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.name' isn't in GROUP BY
2017-03-04T10:48:51+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.name' isn't in GROUP BY
2017-03-04T10:48:51+00:00	ERROR ::1	database-error	Database query failed (error # 1055): 'strictmode.a.asset_id' isn't in GROUP BY

@infograf768
Copy link
Member

Changing the database name has no effect.

@infograf768
Copy link
Member

I think there are important differences between msql 5.5 and 5.7
Saw this on the net:
in MySQL 5.7 sql mode is set by default to ONLY_FULL_GROUP_BY

@infograf768
Copy link
Member

That was it!

If I take off ONLY_FULL_GROUP_BY from the code above, I do not have any more errors.
I.e. using
mysqli_query($this->connection, "SET @@SESSION.sql_mode = 'STRICT_TRANS_TABLES,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION';");

@infograf768
Copy link
Member

Shall I therefore consider this PR as OK?

@csthomas
Copy link
Contributor Author

csthomas commented Mar 4, 2017

Yes. This PR is OK, your errors is unrelated.

Which sample data do you use on instalation?

@infograf768
Copy link
Member

None. I have set up the site as multilingual.

@infograf768
Copy link
Member

I have tested this item ✅ successfully on d231e82

OK here on mysql.


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

@waader
Copy link
Contributor

waader commented Mar 4, 2017

Works with postgresql, but couldn´t test with mssql as installation could not be finalised. After choosing the Install Sample Data and pressing the Install-Button the program returns to the Overview tab.

@csthomas
Copy link
Contributor Author

csthomas commented Mar 4, 2017

If you does not have latest version then samples data is broken. Try to install without samples.

@waader
Copy link
Contributor

waader commented Mar 4, 2017

@csthomas I also tried without sample data and I am always using the current staging.

@csthomas
Copy link
Contributor Author

csthomas commented Mar 4, 2017

Please edit 691 line in libraries/joomla/database/driver/sqlsrv.php
and replace:

$this->errorMsg = $errorMsg;

to

$this->errorMsg = $errorMsg;JFactory::getApplication()->enqueueMessage($this->errorMsg . $query);

and in installation/model/database.php line 1144
replace (in method setDatabaseCharset)

{

to

{return false;

Then you will see what query generate error.

@waader
Copy link
Contributor

waader commented Mar 4, 2017

Then I get: Incorrect syntax near the keyword 'SET' ALTER DATABASE [joomla37] CHARACTER SET 'utf8'

@csthomas
Copy link
Contributor Author

csthomas commented Mar 4, 2017

I know that error but it not stop installation. Normally it is hidden and installation go forward.
Did you change the second file?

@waader
Copy link
Contributor

waader commented Mar 4, 2017

The method looks like this:

public function setDatabaseCharset($db, $name)
{
	// Run the create database query.
	$db->setQuery($db->getAlterDbCharacterSet($name));

	try
	{
		$db->execute();
	}
	catch (RuntimeException $e)
	{
		return false;
	}

	return false;
}

@csthomas
Copy link
Contributor Author

csthomas commented Mar 4, 2017

Yes and it generates error which not stop installation. I want you to do not run this query. Type return false; on the top of method.

@waader
Copy link
Contributor

waader commented Mar 4, 2017

Ok, now I have

public function setDatabaseCharset($db, $name)
{	
		return false;
}

and the installation goes back to Tab Overview with no message.

@csthomas
Copy link
Contributor Author

csthomas commented Mar 4, 2017

If you not try with this zip then may be this help:
https://github.com/csthomas/joomla-cms/archive/sqlupdate_strict.zip

I do not know what going wrong.
Please check your php error_log. Or ajax call results on installation, ex firebug, tab net or connections.

@csthomas
Copy link
Contributor Author

csthomas commented Mar 4, 2017

IIRC some days I had some php warning or notice which was displayed. It broke ajax results and installation stop.

may be turn off display errors may temporary help.

@waader
Copy link
Contributor

waader commented Mar 4, 2017

PHP Warning: simplexml_load_file() [function.simplexml-load-file]: I/O warning : failed to load external entity "file:///C:/Program%20Files%20(x86)/Ampps/www/joomla37/plugins/fields/gallery/gallery.xml" in C:\Program Files (x86)\Ampps\www\joomla37\libraries\cms\installer\installer.php on line 1997, referer: http://localhost/joomla37/installation/index.php?view=site

When I delete the respective line in joomla.sql installation works. So next I tried to install another language, but I get this error message:
Fatal error: Using $this when not in object context in C:\Program Files (x86)\Ampps\www\joomla37\administrator\components\com_installer\models\languages.php on line 151

@csthomas
Copy link
Contributor Author

csthomas commented Mar 4, 2017

This is php 5.3 only. We can not use $this in closure. Before closure create a $ins = $this; and replace $this to $ins inner closure. Oh, also add use ($ins)

@csthomas
Copy link
Contributor Author

csthomas commented Mar 4, 2017

		$ins = $this;

		// Sort the array by value of subarray
		usort(
			$languages,
			function($a, $b) use ($ins)
			{
				$ordering = $ins->getState('list.ordering');

				if (strtolower($ins->getState('list.direction')) === 'asc')
				{
					return StringHelper::strcmp($a->$ordering, $b->$ordering);
				}
				else
				{
					return StringHelper::strcmp($b->$ordering, $a->$ordering);
				}
			}
		);

@waader
Copy link
Contributor

waader commented Mar 4, 2017

That works!

@csthomas
Copy link
Contributor Author

csthomas commented Mar 4, 2017

OK, I'm adding that changes to PR.

@waader
Copy link
Contributor

waader commented Mar 4, 2017

The gallery plugin should also be removed from the mysql and postgres installation files.

@csthomas
Copy link
Contributor Author

csthomas commented Mar 4, 2017

I think it was removed in #14309

@waader
Copy link
Contributor

waader commented Mar 4, 2017

But the are still included in your patch.

@csthomas
Copy link
Contributor Author

csthomas commented Mar 4, 2017

Yes because you used Patch tester which replace all joomla.sql file instead add only changes.

If you download https://github.com/csthomas/joomla-cms/archive/sqlupdate_strict.zip then everything should be ok (this is a version of joomla where gallery has not been deleted yet)

Tomasz Narloch added 2 commits March 4, 2017 19:12
Mysql strict mode - install languages fix
Postgresql - reinstall languages fix
Mssql - install and reinstall and discover extension fix
@csthomas
Copy link
Contributor Author

csthomas commented Mar 4, 2017

I have rebased PR. Now it will work with Patch Tester too without mentioned problem.

@waader
Copy link
Contributor

waader commented Mar 4, 2017

I have tested this item ✅ successfully on 26cccca

I did a retest with all 3 databases and using php 5.3 and 5.6.


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

@ghost
Copy link

ghost commented Mar 5, 2017

@infograf768 can you please test again so we have 2 successfully tests?

@@ -537,7 +537,7 @@ public function update()

// Update an entry to the extension table
$row = JTable::getInstance('extension');
$eid = $row->find(array('element' => strtolower($this->get('tag')), 'type' => 'language', 'client_id' => $clientId));
$eid = $row->find(array('element' => $this->get('tag'), 'type' => 'language', 'client_id' => $clientId));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason we had strtolower and you take it off?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mysql compares string case-insensitive and strtolower() do nothing, but on postgersql and mssql string is compared case-sensitive, so en-GB = en-gb returns false. Text in table #__extensions in element column (language package) has upper case letters too.

This is fix for duplication after reinstall language.

@infograf768
Copy link
Member

I have tested this item ✅ successfully on 26cccca

ok on mysqli


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

@ghost
Copy link

ghost commented Mar 5, 2017

RTC as there are 2 successfully Tests?

@wilsonge wilsonge merged commit 558b138 into joomla:staging Mar 19, 2017
@wilsonge wilsonge added this to the Joomla 3.7.0 milestone Mar 19, 2017
@csthomas csthomas deleted the sqlupdate_strict branch March 19, 2017 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants