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

Do not call closeCursor() in prepared statements on TYPO3 8.7 #955

Merged
merged 4 commits into from
Sep 1, 2020
Merged

Do not call closeCursor() in prepared statements on TYPO3 8.7 #955

merged 4 commits into from
Sep 1, 2020

Conversation

sgrossberndt
Copy link
Contributor

Calling closeCursor() in prepared statements on TYPO3 8.7 leads to always the first result being returned. In TYPO3 >= 9.5 the calls to closeCursor are correct.

Calling `closeCursor()` in prepared statements on TYPO3 8.7 leads to always the first result being returned. In TYPO3 >= 9.5 the calls to closeCursor are correct.
@liayn
Copy link
Contributor

liayn commented Jul 23, 2020

I suppose this is a doctrine/dbal error then in the older versions.

@sgrossberndt
Copy link
Contributor Author

Yes, suppose so too. Which is definitely not going to be fixed.

@sypets
Copy link
Contributor

sypets commented Jul 24, 2020

Should we add a comment to the code snippet? As this is different per version (and probably a bug?) this might confuse people in the future.

@sypets
Copy link
Contributor

sypets commented Jul 26, 2020

Stephan, can you give more information: Could you reproduce this? With which database? Can you link to a resource?

This looks like it might be related: doctrine/dbal#2546

As per the source comment, it should be possible to reuse statements after calling closeCursor: https://github.com/doctrine/dbal/blob/v2.5.4/lib/Doctrine/DBAL/Driver/ResultStatement.php#L30

My suggestion would be to make the change as Stephan suggested but add a note with explanation.

@sgrossberndt
Copy link
Contributor Author

Discussion leading to this pull request: https://typo3.slack.com/archives/C03AM9R17/p1595509409408400
Added a note as requested, feel free to improve.
Is it possible to preview the rendered documentation for the pull request before merge?

@sypets
Copy link
Contributor

sypets commented Aug 4, 2020

@sgrossberndt You can preview the rendered documentation either by:

  1. Render it locally: Get the repo with git clone and render with Docker container, see https://docs.typo3.org/m/typo3/docs-how-to-document/master/en-us/RenderingDocs/Index.html
  2. or in the pull request download "artefact"
  • In the PR, see the block "All checks have passed"
  • click on "Details" for "CI / use-docker-container (online) (pull_request) "
  • find "Artefacts", click on triangle and download

download-artefact2

In this PR, the checks have not run, I do not not why. You should usually see something like this:

pr-checks2


I will look at your changes.

@sypets
Copy link
Contributor

sypets commented Aug 4, 2020

  1. When you render locally, the file you want to open is (usually): Documentation-GENERATED-temp/Result/project/0.0.0/Index.html

  2. When you download and unpack the artefact zip, open Index.html in the browser.

If you render locally, the first setup may take some time (e.g. clone GitHub repo, pull Docker container, render etc.), but once you have it setup and do rerendering of changes, it is pretty fast because the results are cached and only the changes are rerendered.

@sgrossberndt
Copy link
Contributor Author

Thanks for the detailed explanation and for looking into the issue, why there are no running checks for this PR. Maybe because its 8.7 and thus no longer LTS?

@sypets
Copy link
Contributor

sypets commented Aug 4, 2020

You are right. Thanks for pointing that out, I forgot. The checks run via GitHub actions. The 8.7. branch probably does not contain a .github/workflows/CI.yml file. Can't really say, but it was added quite recently and might have only been added for "master"

@lolli42 lolli42 merged commit fced62c into TYPO3-Documentation:8.7 Sep 1, 2020
@sgrossberndt sgrossberndt deleted the patch-2 branch September 2, 2020 07:21
linawolf added a commit that referenced this pull request Jul 28, 2024
* [TASK] Provide PSR-7 Request in PolicyMutatedEvent #955

Releases: main
Resolves: TYPO3-Documentation/Changelog-To-Doc#955

* Update Documentation/ApiOverview/Events/Events/Core/Security/PolicyMutatedEvent.rst

Co-authored-by: Chris Müller <2566282+brotkrueml@users.noreply.github.com>

* Update Documentation/ApiOverview/Events/Events/Core/Security/PolicyMutatedEvent.rst

Co-authored-by: Chris Müller <2566282+brotkrueml@users.noreply.github.com>

---------

Co-authored-by: Chris Müller <2566282+brotkrueml@users.noreply.github.com>
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.

4 participants