Skip to content

Conversation

@hemberger
Copy link
Contributor

Remove the restriction that placeholder validation is performed only for SELECT queries. As a result, write queries will now validate their placeholders if write query analysis is enabled.

Previously, placeholder errors in a write query would result in all errors in the query being suppressed (even those unrelated to the placeholders).

Fixes #503.

Remove the restriction that placeholder validation is performed only for SELECT queries. As a result, write queries will now validate their placeholders if write query analysis is enabled. Previously, placeholder errors in a write query would result in all errors in the query being suppressed (even those unrelated to the placeholders). Fixes staabm#503.
319,
],
[
'Query expects placeholder :freigabe1u1, but it is missing from values given.',
Copy link
Owner

@staabm staabm Feb 14, 2023

Choose a reason for hiding this comment

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

please check the example in this file (which is the one tested here).

$connection->preparedQuery('INSERT into ada(adaid, gesperrt, email,freigabe1u1) values(:adaid, :gesperrt, :email, :freigabe1u1)', ['adaid' => 1, 'gesperrt' => 0, 'email' => 'test@github.com', 0]); 

I guess this error is not correct, since the last value of the array will be handled by PDO as the last value of the column list.


please also add new test-cases (at the very end of the file) which show that the rule works for writable queries as you expect it (and also cover cases which it should not error about)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this query is invalid (because it names a parameter :freigabe1u1 that it does not define), and the new warning is correct. In my testing, I get the following error if I actually try to execute this query:

PDOException: SQLSTATE[HY093]: Invalid parameter number: parameter was not defined

The write queries that are already part of the test suite seem to cover all the cases I can think of. I'm happy to add more if you feel that some category has been omitted. Please let me know!

Copy link
Owner

Choose a reason for hiding this comment

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

I see, thx for testing. we can merge after conflicts are resolved then.

@staabm staabm enabled auto-merge (squash) February 15, 2023 19:05
@staabm staabm merged commit ff4eac0 into staabm:main Feb 15, 2023
@staabm
Copy link
Owner

staabm commented Feb 15, 2023

thank you

@hemberger hemberger deleted the issue-503 branch February 16, 2023 11:08
@staabm
Copy link
Owner

staabm commented Feb 16, 2023

your first feature is avaible within the new release:

https://twitter.com/markusstaab/status/1626234424059908096

thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants