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

[5.2] Properly support PDO::FETCH_CLASS in cursor() #14052

Merged
merged 4 commits into from
Jun 27, 2016
Merged

[5.2] Properly support PDO::FETCH_CLASS in cursor() #14052

merged 4 commits into from
Jun 27, 2016

Conversation

vlakoff
Copy link
Contributor

@vlakoff vlakoff commented Jun 19, 2016

Fixes the issue I have pointed in these comments.

@vlakoff
Copy link
Contributor Author

vlakoff commented Jun 19, 2016

I have refined the code:

  • Consistent across select() and cursor().
  • Makes it clear we are handling the erroneous use of PDO::FETCH_CLASS without class name ([5.2] Use proper PDO fetch style laravel#3814).
  • Supports as well modes such as PDO::FETCH_CLASS|PDO::FETCH_PROPS_LATE (of course the user is expected to provide a class name).

ping @jdpanderson as this might interest you.

@vlakoff
Copy link
Contributor Author

vlakoff commented Jun 21, 2016

Slightly adjusted testAlternateFetchModes (which btw doesn't test the modes are actually working).

Because setFetchMode(PDO::FETCH_CLASS, 'stdClass', [1, 2, 3]); would actually produce this error:

PDOStatement::setFetchMode(): SQLSTATE[HY000]: General error: user-supplied class does not have a constructor, use NULL for the ctor_params parameter, or simply omit it

With PDO::FETCH_CLASS and 'stdClass', constructor argument (3rd argument) can be: omitted, null, empty array. Edit: HHVM doesn't support empty array to skip the argument, it has to be omitted or null.

@aaronhuisinga
Copy link

Hey @vlakoff, since this has been merged into the v5.2.40 release, I and others (https://laracasts.com/discuss/channels/forge/sqlstatehy000-general-error-user-supplied-class-does-not-have-a-constructor) have been seeing intermittent SQL errors, and this commit seems to be what is causing them.

For some select queries, I'm getting:
SQLSTATE[HY000]: General error: user-supplied class does not have a constructor, use NULL for the ctor_params parameter, or simply omit it
The stack trace leads to line 345 of the Connection file as the culprit:
return isset($fetchArgument) ? $statement->fetchAll($fetchMode, $fetchArgument, $me->getFetchConstructorArgument()) : $statement->fetchAll($fetchMode);

To be perfectly honest, I'm not exactly sure what is happening here, but I hadn't had any issues before this was merged. Any idea on what could be happening here?

@taylorotwell
Copy link
Member

Sigh. What needs to happen to fix this @vlakoff

@vlakoff
Copy link
Contributor Author

vlakoff commented Jul 22, 2016

Fixed by #14429.

@dbr0
Copy link

dbr0 commented Jul 31, 2016

Hey guys, I'm still having problem on my DO server deployed by Forge.

On my local machine there are no problems.

Locally I'm using mysql and on forge I selected Maria and HHVM (though I don't know anything about HHVM I just Googled it when I was deploying the server and it seemed as something that I should investigate and use in the future).

General error: user-supplied class does not have a constructor, use NULL for the ctor_params parameter, or simply omit it (SQL: select exists(select * from users where email = my.email@gmail.com and users.deleted_at is null) as exists)

I'm sending an email to the App and checking if there is a user with this email in it (using softdeletes).

@vlakoff
Copy link
Contributor Author

vlakoff commented Jul 31, 2016

That's in the above linked PR.

I had fixed cursor() but introduced an incompatibility of both select() and cursor() with HHVM. So basically Laravel 5.2.40 and 5.2.41 are broken on HHVM.

I had submitted a PR less than two hours after the bug report. It has been merged, but we still need a new Laravel 5.2 release to be tagged.

For now you have to hold on Laravel 5.2.39.

@raphcadiz
Copy link

Hey guys, I'm still getting this the same error on 5.2.42

@dbr0
Copy link

dbr0 commented Aug 1, 2016

In your composer.json replace

"laravelcollective/html": "5.2.*",

with

"laravelcollective/html": "5.2.39",

This will force your app to use the .39 version where the bug doesn't exist.

@vlakoff
Copy link
Contributor Author

vlakoff commented Aug 8, 2016

Laravel 5.2.42 just released :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants