Skip to content

Conversation

@vigneshgurusamy
Copy link

@vigneshgurusamy vigneshgurusamy commented Jun 20, 2023

To enhance the support for named query bindings with multiple occurrences, I have modified the preg_replace() function. Currently, it only replaces the first occurrence of the named query binding, which is limiting for our requirements.

So I have updated the function to replace all occurrences of the named query binding.

Query

SELECT user_id FROM user_credit_log
WHERE points > :minimum_points_user
    AND event = :event_user
    AND created_at BETWEEN :start_date AND :end_date
UNION
SELECT user_id FROM user_credit_log
WHERE points > :minimum_points_admin
    AND event = :event_admin
    AND created_at BETWEEN :start_date AND :end_date

Bindings

[
    'minimum_points_user' => 10,
    'event_user' => 'user',
    'minimum_points_admin' => 100,
    'event_admin' => 'admin',
    'start_date' => '2020-01-01 00:00:00',
    'end_date' => '2022-01-01 00:00:00',
]

Output

telescope-named-bindings

@taylorotwell
Copy link
Member

I'm a bit concerned about this breaking other use cases. That 1 was placed in the preg_replace call for a reason. I'd like to know what that reason was before merging this.

@taylorotwell
Copy link
Member

Please mark as ready for review once you respond with more information.

@taylorotwell taylorotwell marked this pull request as draft June 21, 2023 23:21
@vigneshgurusamy
Copy link
Author

@taylorotwell I have reviewed the code, but I'm unsure about the reasoning behind its implementation. The code was originally written by barryvdh (#706), and I have examined his Laravel Debugbar package, which exhibits the same issue.

DB::select('select * from users where user_id = :id and user_id > :id', ['id' => 100]);

Screenshot 2023-06-26 101803

@barryvdh, could you assist us in understanding why there is a restriction on the limit?

@barryvdh
Copy link
Contributor

I guess you could use the new functions from Laravel 10 now? laravel/framework#47507

@driesvints
Copy link
Member

Closing because of inactivity.

@driesvints driesvints closed this Mar 5, 2024
@vigneshgurusamy vigneshgurusamy deleted the fix/query-named-bindings branch August 22, 2024 05:12
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