Skip to content

#475: Improve error message for unsupported named parameters #476

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

JKDingwall
Copy link

Previously just 'Illegal parameter number' was logged, make this a more helpful message.

[25-05-06 20:46:04.2629] Slim::Schema::Storage::throw_exception (122) Error: DBI Exception: DBD::mysql::st bind_param failed: named parameters are unsupported: :album [for Statement "
                                        SELECT contributor_album.role AS role, contributors.name AS name, contributors.id AS id
                                        FROM contributor_album
                                        JOIN contributors ON contributors.id = contributor_album.contributor
                                        WHERE contributor_album.album = :album
                                        AND contributor_album.role IN (5,1)
                                        ORDER BY contributor_album.role, contributors.namesort
                                "]

@dveeden
Copy link
Collaborator

dveeden commented May 9, 2025

This probably need a few doc/pod updates and one or more tests.

@dveeden dveeden linked an issue May 9, 2025 that may be closed by this pull request
@JKDingwall JKDingwall force-pushed the JKDingwall/475-named-parameter-error-message branch from c782330 to cc09167 Compare May 9, 2025 08:40
@JKDingwall
Copy link
Author

This probably need a few doc/pod updates and one or more tests.

@dveeden This is my first attempt working on a perl module so if you don't mind a couple of questions...

  1. doc/pod: would you just like a mention that named parameters are not supported and indicate what error message would be reported if they are used?

  2. tests: would one new test that asserts the expected message is emitted if a named parameter is used be sufficient? would it be best in a new test file or as an addition to an existing one?

I couldn't understand why "looks_like_number(1)" failed here https://github.com/perl5-dbi/DBD-mysql/actions/runs/14923375602/job/41923094232#step:10:49 but I changed the code to put the new check under the failure which was originally being triggered. When I dpkg-buildpackage -b all tests pass with MySQL.

@dveeden
Copy link
Collaborator

dveeden commented May 9, 2025

@JKDingwall

  1. Yes just check the current docs and update if needed. Preferably with some suggestion about what to use instead. If you think no updates are needed than that might be ok.
  2. Yes. Probably a new test file to test that this does work as intended. If there is a file that already has something very similar that including this in that file would also be fine.

Also note that MySQL might make it easier to add named attributes in the future:

Previously just 'Illegal parameter number' was logged, make this a more
helpful message.

```
[25-05-06 20:46:04.2629] Slim::Schema::Storage::throw_exception (122) Error: DBI Exception: DBD::mysql::st bind_param failed: named parameters are unsupported: :album [for Statement "
                                        SELECT contributor_album.role AS role, contributors.name AS name, contributors.id AS id
                                        FROM contributor_album
                                        JOIN contributors ON contributors.id = contributor_album.contributor
                                        WHERE contributor_album.album = :album
                                        AND contributor_album.role IN (5,1)
                                        ORDER BY contributor_album.role, contributors.namesort
                                "]
```
@JKDingwall JKDingwall force-pushed the JKDingwall/475-named-parameter-error-message branch from cc09167 to 32b7fed Compare June 27, 2025 14:34
If a named parameter is passed to bind_param() ensure the expected error
message is reported.
@JKDingwall
Copy link
Author

JKDingwall commented Jun 27, 2025

@dveeden I've added a new test that the expected error message is reported if bind_param(':name', 1) is called. I've also added a couple of sentences to the documentation that named parameters are not supported. Please let me know you'd like any changes made, thanks!

This is the output from the test:

t/45bindnamedparam_error.t ....................
1..11
ok 1
ok 2
ok 3 - create table dbd_mysql_t45bindnamedparam
ok 4 - insert into dbd_mysql_t45bindnamedparam (null, 1)
ok 5
ok 6 - $rows->[0][1] == 1
ok 7
ok 8
ok 9 - bind_param reports exepcted error with named parameter
ok 10
ok 11
Argument ":num" isn't numeric in subroutine entry at t/45bindnamedparam.t line 50.
DBD::mysql::st bind_param failed: named parameters are unsupported: :num at t/45bindnamedparam.t line 50.
ok

I had seen the documentation about named attributes but after some research I realised that its actually a bit misleading. The feature provides a way to annotate a query with, for example, the name of script which invoked the query.

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.

Improve error message for (unsupported) named parameter usage
2 participants