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

Revert result inference for scalar method #588

Merged
merged 1 commit into from
Jun 28, 2024

Conversation

VincentLanglet
Copy link
Contributor

As recommended here: #520 (comment)

@ondrejmirtes
Copy link
Member

Please explain in your own words why this has to be reverted.

@janedbal
Copy link
Contributor

I believe it is summarized here: #520 (comment)

@ondrejmirtes
Copy link
Member

I'm on a phone and I can't load the comment.

I'd like a brief explanation and also some tests that fail when this is not reverted.

@VincentLanglet
Copy link
Contributor Author

Please explain in your own words why this has to be reverted.

When we're computing the template values of Query<Tkey, TValue>, TValue represent the result of the Query for the hydration mode HYDRATE_OBJECT but this hydration mode does not behave always like others.

For instance, SELECT o.bigIntColumn from ... will return

  • An array of array of one string with HYDRATE_OBJECT
  • An array of array of one string with HYDRATE_ARRAY
  • An array of array of one int with HYDRATE_SCALAR
  • An int with HYDRATE_SINGLE_SCALAR
  • An array of int with HYDRATE_SCALAR_COLUM

I didn't revert HYDRATE_ARRAY, dunno if @janedbal has a counter example for this case.

I would say that this dev is working something like 90% of the time but such false-positive is non acceptable so we should keep mixed until it's fixed.

For non-regression test, we need to directly test hydration mode on different driver so I made #587 but I think @janedbal had something else in mind.

@ondrejmirtes
Copy link
Member

Please add a test case that would fail if the changes weren't reverted.

@janedbal janedbal mentioned this pull request Jun 28, 2024
@ondrejmirtes ondrejmirtes merged commit c54ce9b into phpstan:1.4.x Jun 28, 2024
36 checks passed
@ondrejmirtes
Copy link
Member

Thank you.

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.

3 participants