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 Component + Library installation for PDO driver with strict mode #9461

Merged
merged 5 commits into from Mar 18, 2016

Conversation

wilsonge
Copy link
Contributor

This fixes some of the issues described in #9447 and the issue described in #9455

To test try and install a component like weblinks through Extension Manager with the PDO Driver (note that you must apply joomla-extensions/weblinks@ae8e62c to the weblinks 3.4.1 release for this to work). You must have strict mode enabled to replicate this problem. If you don't have MySql 5.6.6+ (and even in some other cases it's worth doing this) you can emulate the error by setting the sql_mode in pdomydriver file. For that, add, after this line https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/database/driver/pdomysql.php#L149 the following code:

static $sqlMode = null;
if (is_null($sqlMode))
{
    $this->connection->query("SET @@SESSION.sql_mode = 'STRICT_TRANS_TABLES,NO_ENGINE_SUBSTITUTION';");
    $sqlMode = true;
}

@wilsonge
Copy link
Contributor Author

@nikosdion Can you try testing this please? I can't reproduce it 100% of the time so I'm not sure if this solves it or not - it definitely seems to solve the weblinks/general component install issue though

@andrepereiradasilva
Copy link
Contributor

@wilsonge shouldn't also be needed to set the sql_mode in pdomysql to an empty value to match with mysql and mysqli db drivers current sql_mode?

$this->connection->query("SET @@SESSION.sql_mode = '';");

Will remove all warnings and is not a solution itself (since IMHO it should use strict mode in all drivers) but, for now, the 3 mysql db drivers (mysql, mysqli and pdomysql) should have the same sql_mode for consistency.

See:

Or the idea is to had strict sql mode in all mysql db drivers in 3.5.0+?

@richard67
Copy link
Member

@wilsonge I just tested with the modified weblinks package. The errors I had before have disappeared now, but now I run into following:

Warning
SQLSTATE[HY000]: General error: 1364 Field 'system_data' doesn't have a default value
Package Install: There was an error installing an extension: mod_weblinks.zip
Error
Error installing package


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

@richard67
Copy link
Member

@wilsonge In addition I still have the problem I mentioned yesterday on Skype:

The extensions update status is shown as unknown on the control panel.

When I go to "Extensions -> Manage -> Update" and click the "Find Updates" button, it ends with this sql error:

SQLSTATE[HY000]: General error: 1364 Field 'data' doesn't have a default value

Shall I open a new issue for that? Or will you handle it with this PR here, too?


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

@wilsonge
Copy link
Contributor Author

I can't replicate that :( Like the PR adds a line to deal with exactly that issue for modules: https://github.com/joomla/joomla-cms/pull/9461/files#diff-6544c81f9f6cf3dad62bf0e394ed9792R377 - are you sure it's applied properly :/

@richard67
Copy link
Member

Ahhh just as you say it: No, you PR was not applied with latest changes (shame). I'll do and test again soon.

@richard67
Copy link
Member

@wilsonge Ok, tested with success the stuff from issues mentioned here.

But again my question: What to do with the error on "Find Updates"? Wanna handle this with this PR here?

In this case I wait with marking test result.

Or with another issue?

In this case I mark test result positive here.

But I would prefer you to handle the new issue then for this "Find Updates" problem, because you now know better than me where to look and can immediately make a PR with the issue maybe.


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

@wilsonge
Copy link
Contributor Author

It just needs a change here https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/updater/updater.php#L182 to ensure that the update property is set - but I can't work out what data the column is supposed to contain from here - will look up on it tonight

@richard67
Copy link
Member

@wilsonge "will look up on it tonight" means in a new PR/issue so I shall mark test result of this one here with success, or means do this thing also with this PR so I shall wait?

@nikosdion
Copy link
Contributor

I applied the patch to staging but it doesn't fix the issue for me. I keep getting the same error :(

@andrepereiradasilva
Copy link
Contributor

@wilsonge @nikosdion i can't reproduce the problem, to check if this works. For what i understand, the issue is:

HY000 SQLSTATE[HY000]: General error: 2014 Cannot execute queries while other unbuffered queries are active. Consider using PDOStatement::fetchAll().
Alternatively, if your code is only ever going to run against mysql, you may enable query buffering by setting the PDO::MYSQL_ATTR_USE_BUFFERED_QUERY attribute.

Adding a PDO::MYSQL_ATTR_USE_BUFFERED_QUERY flag to the beginning of the pdomysql.php connect method so it can be used on PDO connect probably works.

    public function connect()
    {
        $this->options['driverOptions'][PDO::MYSQL_ATTR_USE_BUFFERED_QUERY] = true;
        // [...]
    }

In https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/database/driver/pdomysql.php#L103-L104

UPDATE: Has able to reproduce the problem and the fix i proposed doesn't work.
To reproduce:

  1. Install latest staging (with mysqli driver)
  2. Install Akeeba Backup 4.5.4 or lower
  3. Change the database driver to "PDO (Mysql)" in global configuration
  4. Upgrade akeeba backup (with upload) to version 4.5.5

UPDATE 2: Just tested and this problem also exists in 3.4.8

@wilsonge
Copy link
Contributor Author

@andrepereiradasilva does my fix work for weblinks?

@andrepereiradasilva
Copy link
Contributor

@wilsonge is there a weblinks zip to test?

@richard67
Copy link
Member

@andrepereiradasilva
Copy link
Contributor

@wilsonge installed @richard67 zip from URL with pdo (mysql) in strict sql mode and worked fine.

@andrepereiradasilva
Copy link
Contributor

The extensions update status is shown as unknown on the control panel.
When I go to "Extensions -> Manage -> Update" and click the "Find Updates" button, it ends with this sql error:
SQLSTATE[HY000]: General error: 1364 Field 'data' doesn't have a default value
Shall I open a new issue for that? Or will you handle it with this PR here, too?

I have this problem too. Of course only in strict sql_mode

@nikosdion
Copy link
Contributor

@andrepereiradasilva In the case of Akeeba Backup installation the problem I tracked down the issue. In order to work around the infamous "MySQL server has gone away" errors I have my own database driver which piggybacks on Joomla!'s db driver object. Trying to make it work on older Joomla! versions I am overriding connected() with my own. I stupidly did a $this->connection->exec('SELECT 1') and only checking for exceptions. However, even though it returns an integer, PDO internally creates a cursor, like it does with all SELECT queries, but which does not make available to you. However, if you do not close the cursor that you don't have access to you get the error about having open queries. Ugh.

So, it was my fault. It only happens on the second update / reinstallation. However, it seems that I must have uncorked some kind of genie's bottle with this issue report to George because here we are with a bunch of other issues regarding PDOMySQL. Sorry.

@wilsonge
Copy link
Contributor Author

OK I'm going to merge this as we have tests here and Nic's solved his Akeeba Backup Issue. I am going to try and fix the language issue and find updates issues before I PR turning off strict mode for PDO :)

wilsonge added a commit that referenced this pull request Mar 18, 2016
Fix Component + Library installation for PDO driver with strict mode
@wilsonge wilsonge merged commit 425084c into joomla:staging Mar 18, 2016
@wilsonge wilsonge deleted the pdo_mysql_strict branch March 18, 2016 17:46
@wilsonge wilsonge added this to the Joomla! 3.5.0 milestone Mar 18, 2016
@andrepereiradasilva
Copy link
Contributor

So, it was my fault. It only happens on the second update / reinstallation. However, it seems that I must have uncorked some kind of genie's bottle with this issue report to George because here we are with a bunch of other issues regarding PDOMySQL. Sorry.

no problem. actually the first one open the PDO (Mysql) strict mode "genie's bottle" has @mbabker with is mysql 5.6.29 server :)

@mbabker
Copy link
Contributor

mbabker commented Mar 18, 2016

Just because I run the default configuration from MySQL doesn't mean nothin' 😝

@andrepereiradasilva
Copy link
Contributor

I am going to try and fix the language issue and find updates issues before I PR turning off strict mode for PDO :)

Yeah, and IMHO any other strict sql mode issues should go to 3.5.1+, since this issues are all also in 3.4.8 too.

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

6 participants