You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@spamassassin.apache.org by bu...@issues.apache.org on 2010/06/16 01:40:48 UTC
[Bug 6454] New: wrong status test on $sth->rows in BayesStore::PgSQL
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6454
Summary: wrong status test on $sth->rows in BayesStore::PgSQL
Product: Spamassassin
Version: 3.3.1
Platform: All
OS/Version: All
Status: NEW
Severity: normal
Priority: P2
Component: Libraries
AssignedTo: dev@spamassassin.apache.org
ReportedBy: Mark.Martinec@ijs.si
The return value from $sth->rows is inappropriately tested
for string equality to '0E0', instead of testing its numeric
value, as required by the DBI module documentation.
A change like the following:
my $rows = $sth->rows;
- if ($rows eq '0E0') {
+ if (!$rows) {
is needed in two places in BayesStore::PgSQL.
(the MySQL counterpart seems fine).
The mistake probably originates from the fact that a call to
$dbh->do() does return '0E0' for no rows affected, while a
call to $sth->rows is different and returns a numeric value,
and a -1 on error. This was probably a cut/paste mistake
when making BayesStore::PgSQL out of BayesStore::MySQL,
or an unwarranted assumption without reading the DBI docs
and proper testing.
The mistake causes unnecessary updating of bayes_vars
with each call to tok_touch_all, instead of doing it only
when the number of rows affected is greater than zero.
Functionality is unaffected, just some redundant work is
performed.
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6454] [review] wrong status test on $sth->rows in
BayesStore::PgSQL
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6454
Mark Martinec <Ma...@ijs.si> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution| |FIXED
--- Comment #6 from Mark Martinec <Ma...@ijs.si> 2011-05-10 17:31:05 UTC ---
3.3:
Bug 6454: wrong status test on $sth->rows in BayesStore::PgSQL
Sending lib/Mail/SpamAssassin/BayesStore/PgSQL.pm
Committed revision 1101556.
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6454] [review] wrong status test on $sth->rows in
BayesStore::PgSQL
Posted by bu...@issues.apache.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6454
Mark Martinec <Ma...@ijs.si> changed:
What |Removed |Added
----------------------------------------------------------------------------
Summary|review] wrong status test |[review] wrong status test
|on $sth->rows in |on $sth->rows in
|BayesStore::PgSQL |BayesStore::PgSQL
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6454] [review] wrong status test on $sth->rows in
BayesStore::PgSQL
Posted by bu...@issues.apache.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6454
Kevin A. McGrail <km...@pccc.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status Whiteboard|needs 2 votes |needs 1 vote
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6454] [review] wrong status test on $sth->rows in
BayesStore::PgSQL
Posted by bu...@issues.apache.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6454
--- Comment #3 from Mark Martinec <Ma...@ijs.si> 2010-06-16 12:55:05 EDT ---
(In reply to comment #2)
> I'm not voting on this issue but worried the patch could have unanticipated
> consequences.
> DBI can return 0E0 "If no rows were affected, then "execute" returns "0E0",
> which Perl will treat as 0 but will regard as true."
> 0E0 will eval to true because it's a string.
> I am concerned when I saw you remove the check on '0E0' as being a possible
> problem and wondered if you new about this silly return code issue and why it
> is/isn't an issue here.
I think you misunderstood the issue. The calls to ->do and ->execute
do behave as you describe, and according to their documentation.
Unlike a call to ->rows, which does NOT behave this way,
nor in the docs, nor in reality. I have only changed a test
on return value from ->rows. Other calls to do and execute
are untouched, they still test for '0E0'.
This only affects the PgSQL module. The MySQL module does not call
the rows method, but uses a return value from ->do, which is why
a test for '0E0' there is still appropriate and correct there.
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6454] [review] wrong status test on $sth->rows in
BayesStore::PgSQL
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6454
--- Comment #5 from Michael Parker <pa...@pobox.com> 2011-05-10 17:02:00 UTC ---
+1
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6454] review] wrong status test on $sth->rows in
BayesStore::PgSQL
Posted by bu...@issues.apache.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6454
Mark Martinec <Ma...@ijs.si> changed:
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|Undefined |3.3.2
Summary|wrong status test on |review] wrong status test
|$sth->rows in |on $sth->rows in
|BayesStore::PgSQL |BayesStore::PgSQL
Status Whiteboard| |needs 2 votes
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6454] wrong status test on $sth->rows in BayesStore::PgSQL
Posted by bu...@issues.apache.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6454
--- Comment #1 from Mark Martinec <Ma...@ijs.si> 2010-06-15 19:44:44 EDT ---
Created an attachment (id=4777)
--> (https://issues.apache.org/SpamAssassin/attachment.cgi?id=4777)
suggested fix
trunk:
Bug 6454: wrong status test on $sth->rows in BayesStore::PgSQL
Sending lib/Mail/SpamAssassin/BayesStore/PgSQL.pm
Committed revision 955092.
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6454] [review] wrong status test on $sth->rows in
BayesStore::PgSQL
Posted by bu...@issues.apache.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6454
--- Comment #4 from Kevin A. McGrail <km...@pccc.com> 2010-06-16 13:01:51 EDT ---
(In reply to comment #3)
> (In reply to comment #2)
> > I'm not voting on this issue but worried the patch could have unanticipated
> > consequences.
> > DBI can return 0E0 "If no rows were affected, then "execute" returns "0E0",
> > which Perl will treat as 0 but will regard as true."
> > 0E0 will eval to true because it's a string.
> > I am concerned when I saw you remove the check on '0E0' as being a possible
> > problem and wondered if you new about this silly return code issue and why it
> > is/isn't an issue here.
>
> I think you misunderstood the issue. The calls to ->do and ->execute
> do behave as you describe, and according to their documentation.
>
> Unlike a call to ->rows, which does NOT behave this way,
> nor in the docs, nor in reality. I have only changed a test
> on return value from ->rows. Other calls to do and execute
> are untouched, they still test for '0E0'.
>
> This only affects the PgSQL module. The MySQL module does not call
> the rows method, but uses a return value from ->do, which is why
> a test for '0E0' there is still appropriate and correct there.
Understood. Then it looks good and I see what you mean that the test for 0e0
was a complete error previously. +1
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6454] [review] wrong status test on $sth->rows in
BayesStore::PgSQL
Posted by bu...@issues.apache.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6454
Kevin A. McGrail <km...@pccc.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |kmcgrail@pccc.com
--- Comment #2 from Kevin A. McGrail <km...@pccc.com> 2010-06-16 12:39:42 EDT ---
(In reply to comment #1)
> Created an attachment (id=4777)
--> (https://issues.apache.org/SpamAssassin/attachment.cgi?id=4777) [details]
> suggested fix
>
> trunk:
> Bug 6454: wrong status test on $sth->rows in BayesStore::PgSQL
> Sending lib/Mail/SpamAssassin/BayesStore/PgSQL.pm
> Committed revision 955092.
I'm not voting on this issue but worried the patch could have unanticipated
consequences.
DBI can return 0E0 "If no rows were affected, then "execute" returns "0E0",
which Perl will treat as 0 but will regard as true."
0E0 will eval to true because it's a string.
I am concerned when I saw you remove the check on '0E0' as being a possible
problem and wondered if you new about this silly return code issue and why it
is/isn't an issue here.
Regards,
KAM
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.