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

[4.x] Implemented fallback for PDO::quote method on QueryWatcher #1066

Merged
merged 6 commits into from
May 31, 2021
Merged

[4.x] Implemented fallback for PDO::quote method on QueryWatcher #1066

merged 6 commits into from
May 31, 2021

Conversation

juanparati
Copy link
Contributor

This pull request will solve #1065.

Note: The fallback method may not constraint with all the target databases that ODBC is connected, It means that there is no way to know which charset, and quote symbol is the default used by the database, however the fallback method should represent an accurate query string for the most popular databases like mSQL, BigQuery or Snowflake.

* Add quotes to string bindings.
*
* @param \Illuminate\Database\Events\QueryExecuted $event
* @param string $binding
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* @param string $binding
* @param string $binding

{
try {
return $event->connection->getPdo()->quote($binding);
} catch (\PDOException $e) {
Copy link
Member

Choose a reason for hiding this comment

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

Import this object type.

@juanparati
Copy link
Contributor Author

@driesvints : Sorry driesvints, I did a last change.

@driesvints driesvints changed the title Implemented fallback for PDO::quote method on QueryWatcher [4.x] Implemented fallback for PDO::quote method on QueryWatcher May 27, 2021
@juanparati
Copy link
Contributor Author

juanparati commented May 27, 2021

😅 I hope that last commit is accepted.

I replaced $event->quoteStringBinding by $this->quoteStringBinding

In other way the release is buggy.

@driesvints
Copy link
Member

@juanparati can you please do the changes I asked for above? Thanks

@juanparati
Copy link
Contributor Author

Done.

@taylorotwell taylorotwell merged commit e098dd1 into laravel:4.x May 31, 2021
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