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 for insertid not returning what it's supposed to #9610

Closed
wants to merge 1 commit into from

Conversation

chrisdavenport
Copy link
Contributor

Pull Request for Issues #9534, #9572 and #9582.

Summary of Changes

Same as #9601 but code has been moved to the MySQL-specific PDO driver.

Testing Instructions

You need a site which fails under the circumstances described in the issues above. The easiest is probably #9572. Apply the PR then check that the issue is resolved.

@Fedik
Copy link
Member

Fedik commented Mar 26, 2016

I have tested this item ✅ successfully on e5a9742


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

{
$this->connect();

return $this->setQuery('SELECT LAST_INSERT_ID() AS id')->execute()->fetchObject()->id;
Copy link
Contributor

Choose a reason for hiding this comment

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

Two comments:

  • Perhaps here you can use JQuery (for a cleaner approach):
    $query = $this->getQuery(true)->select('LAST_INSERT_ID()');
  • I guess a shorter call would be:
    (int) $this->setQuery($query)->loadResult();
    so:
public function insertid()
{
    $query = $this->getQuery(true)->select('LAST_INSERT_ID()');
    return (int) $this->setQuery($query)->loadResult();
}

@wilsonge
Copy link
Contributor

I found the root cause of this and committed the fix after getting it approved my @mbabker

@wilsonge wilsonge closed this Mar 26, 2016
wilsonge added a commit that referenced this pull request Mar 26, 2016
@chrisdavenport
Copy link
Contributor Author

Well done. :-)

@chrisdavenport chrisdavenport deleted the pdo-fixes branch March 26, 2016 21:27
@Fedik
Copy link
Member

Fedik commented Mar 27, 2016

@wilsonge 👍

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