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: Add the bind value that is initialized in reorder() #14456

Merged
merged 6 commits into from
Oct 8, 2024

Conversation

bzy107
Copy link
Contributor

@bzy107 bzy107 commented Oct 7, 2024

Description

In the case of PostgreSQL, we are performing reorder during individual processing at the time of export. However, when the query contains bindings, the bindings are initialized, leading to an SQL error.

In this fix, we search for the initialized bind attributes before and after the reorder, and re-bind the initialized values to prevent SQL errors.

Visual changes

Functional changes

  • Code style has been fixed by running the composer cs command.
  • Changes have been tested to not break existing functionality.
  • Documentation is up-to-date.

@bzy107
Copy link
Contributor Author

bzy107 commented Oct 7, 2024

There is an error in PHPStan, but the type of the target is an array.

Copy link
Member

@danharrin danharrin left a comment

Choose a reason for hiding this comment

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

Please move the logic back into the main function as it is only called once

Please fix the PHPStan issue, you may need to add a Docblock @var

@danharrin danharrin added the bug Something isn't working label Oct 8, 2024
@danharrin danharrin added this to the v3 milestone Oct 8, 2024
@danharrin danharrin merged commit f8e7688 into filamentphp:3.x Oct 8, 2024
10 checks passed
@bzy107 bzy107 deleted the export_sort_bind_pgsql branch October 8, 2024 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants