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

Make ParamValues follow expectations #449

Merged
merged 1 commit into from
Nov 4, 2024
Merged

Conversation

dveeden
Copy link
Collaborator

@dveeden dveeden commented Oct 30, 2024

Closes #447

  1. The keys are expected (not guaranteed) to be integers starting at 1. They previously started at 0.
  2. The values should be undef if not yet bound

@dveeden dveeden changed the title Make PramValues follow expectations Make ParamValues follow expectations Oct 30, 2024
1. The keys are expected (not guaranteed) to be integers starting at 1. They previously started at 0.
2. The values should be undef if not yet bound
@myrrhlin
Copy link
Contributor

myrrhlin commented Oct 31, 2024

I am not an experienced XS developer, sorry I am perhaps not sufficiently qualified to review this change.
It looks plausible to handle the keys-starting-at-1 problem.

The other you mention "The values should be undef if not yet bound", and my understanding of the code in this commit, suggests to me there is a misunderstanding. My reading of DBI docs are that, before any values are bound or given to execute, the ParamValues attribute should be undef. It should not be a hashref containing undefined values.
The only time it should be a hashref with undef values, is if someone actually bound undef values to the placeholders for NULL (either using bind_param or execute with args).

As for the third problem in issue #447 -- the segfault -- we need a test to reproduce it so that you can find a root cause. I'll try to provide something useful in this regard.

keylen, newSVsv(imp_sth->params[n].value), 0);
} else {
(void)hv_store(pvhv, key, keylen, &PL_sv_undef, 0);
}
}
}
retsv= sv_2mortal(newRV_noinc((SV*)pvhv));
Copy link
Contributor

@myrrhlin myrrhlin Oct 31, 2024

Choose a reason for hiding this comment

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

If I read this correctly, the return value here is always a hashref. When there are no params saved yet, the return should simply be undef -- if I understand the DBI docs correctly 😛

@dveeden
Copy link
Collaborator Author

dveeden commented Oct 31, 2024

I am not an experienced XS developer, sorry I am perhaps not sufficiently qualified to review this change. It looks plausible to handle the keys-starting-at-1 problem.

Feel free to consider the XS stuff a black box and just verify if the behavior from it matches what you want it to do.

The other you mention "The values should be undef if not yet bound", and my understanding of the code in this commit, suggests to me there is a misunderstanding. My reading of DBI docs are that, before any values are bound or given to execute, the ParamValues attribute should be undef. It should not be a hashref containing undefined values. The only time it should be a hashref with undef values, is if someone actually bound undef values to the placeholders for NULL (either using bind_param or execute with args).

"If the driver supports ParamValues but no values have been bound yet then the driver should return a hash with placeholders names in the keys but all the values undef, but some drivers may return a ref to an empty hash because they can't pre-determine the names."
(source: https://metacpan.org/pod/DBI#ParamValues)

How I read this, the driver is expected to return a hash with the keys, but values set to undef as soon as the statement is prepared.

This seems to match DBD::SQLite:

#!/bin/perl
use v5.36;
use DBI;
use Data::Dumper;

my $dbh = DBI->connect('dbi:SQLite:dbname=:memory:','','');

my $sth = $dbh->prepare('SELECT ?');
say "after prepare:\n" . Dumper($sth->{ParamValues});
$sth->execute(1);
say "after execute:\n" . Dumper($sth->{ParamValues});

while (my @row = $sth->fetchrow_array()) {
	say "result: " . $row[0];
}
after prepare:
$VAR1 = {
          '1' => undef
        };

after execute:
$VAR1 = {
          '1' => 1
        };

result: 1

As for the third problem in issue #447 -- the segfault -- we need a test to reproduce it so that you can find a root cause. I'll try to provide something useful in this regard.

Thanks

@myrrhlin
Copy link
Contributor

You are correct, I was mistaken about the expected behavior. I was misled by

Some drivers cannot provide valid values for some or all of these attributes until after $sth->execute has been successfully called. Typically the attribute will be undef in these situations

And DBD::mysql has similar language:

Note, that most attributes are valid only after a successful execute. An undef value will returned otherwise

but that is the default expectation for all attributes, and this one is specially described!.

Sorry for my confusion. Thanks for your patience. I will update my tests.

@dveeden dveeden merged commit 2870fe0 into perl5-dbi:master Nov 4, 2024
7 of 8 checks passed
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.

segfault using ParamValues from $sth
2 participants