Skip Menu |
 

This queue is for tickets about the SQL-Statement CPAN distribution.

Report information
The Basics
Id: 100551
Status: resolved
Priority: 0/
Queue: SQL-Statement

People
Owner: Nobody in particular
Requestors: wyant [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: 1.406_002



Subject: The output of the conv() function does not contain significant trailing zeros
An example of this is the fact that select conv('16', 10, 16) produces '1', not '10' The problem seems to be that SQL::Statement::Functions->SQL_FUNCTION_CONV() never produces trailing zeros when converting integers, execpt maybe for the degenerate case where the input and output radix are the same. A patch against commit 08b12032b34126d00e4246e1ce595257e3e3bbd1 (the latest as of about 2014-11-25 16:00 GMT) is attached, because I am not Git-savvy enough to generate a pull request, and don't have a GITHUB account anyway, I'm afraid. Sorry. This behavior of the conv() function appears to have a number of causes: 1) The "Final cleanup" code at about line 594 removes all leading zeros, even if it leaves an empty string. 2) The short-circuit code at or about line 542 of the module is not executed, because line 489 (or thereabouts) forces $use_big to be true. So the generic output conversion code is always used. 3) The output conversion code at and after about line 564 (the 'while' loop) works left-to-right through the number to be converted. As coded, this loop exits whenever the unconverted number is found to be 0, without provision for appending any needed trailing zeroes. The patch contains proposed fixes as follows: 1) Alter the regular expressions to leave a zero before the end of the string or a decimal point. 2) No action. I ran 'git blame' on the module, and the code that forces $use_big true is the same commit as the rest of the method. Since I did not have relevant history on why this line is there, I left it alone. 3) Alter the condition of the 'while' loop so that it requires both $i >= 0 and $dnum == 0 for loop exit. I have added a couple tests to t/06virtual.t to test these changes. All of t/06virtual.t passes under my builds of Perl 5.8.9 and 5.20.1. There is actually something else going on in this method which I have been unable to run to earth. If I understand the intent of the conv() function, "select conv('16.1', 10, 16)" should produce '10.1'. In fact, it produces 'A1'. The patches do not affect this behavior.
Subject: SQL-Statement-Functions.patch
diff --git a/lib/SQL/Statement/Functions.pm b/lib/SQL/Statement/Functions.pm index 813c1a5..9c7c8c9 100644 --- a/lib/SQL/Statement/Functions.pm +++ b/lib/SQL/Statement/Functions.pm @@ -561,7 +561,7 @@ sub SQL_FUNCTION_CONV )->bfloor()->bstr() : int( log($dnum) / log($ebase) ); - while ( $dnum != 0 && length($new) < 255 ) + while ( ( $dnum != 0 || $i >= 0 ) && length($new) < 255 ) { if ( $i == -1 ) { # time to go pro... @@ -591,8 +591,8 @@ sub SQL_FUNCTION_CONV } # Final cleanup - $new =~ s/^(-?)0+/$1/ if ( $ebase <= 62 ); - $new =~ s/^(-?)A+/$1/ if ( $ebase > 62 ); + $new =~ s/^(-?)(?:0(?!(?:\.|\z)))+/$1/ if ( $ebase <= 62 ); + $new =~ s/^(-?)(?:A(?!(?:\.|\z)))+/$1/ if ( $ebase > 62 ); $new =~ s/0+$// if ( $ebase <= 62 && $is_dec ); $new =~ s/A+$// if ( $ebase > 62 && $is_dec ); diff --git a/t/06virtual.t b/t/06virtual.t index 72c6c87..d285ab2 100644 --- a/t/06virtual.t +++ b/t/06virtual.t @@ -528,6 +528,21 @@ foreach my $test_dbd (@test_dbds) sql => "SELECT CONV('1AF.EQO0000000000000000000000000000', 32, 8)", result => [ [2517.3553] ], }, + { + test => 'conv 10->16 integer with trailing 0', + sql => "select conv('16', 10, 16)", + result => [ ['10'] ], + }, + { + test => 'conv 10->16 float with trailing 0 in integer part', + sql => "select conv('16.1', 10, 16)", + result => [ ['10.1'] ], + }, + { + test => 'conv 10->16 integer 0', + sql => "select conv('0', 10, 16)", + result => [ ['0'] ], + }, { test => 'decode', sql =>
Download (untitled) / with headers
text/plain 110b
Correction to the patch, which contained one test too many. The corrected patch is attached. Sorry about that.
Subject: SQL-Statement-Functions.patch
diff --git a/lib/SQL/Statement/Functions.pm b/lib/SQL/Statement/Functions.pm index 813c1a5..9c7c8c9 100644 --- a/lib/SQL/Statement/Functions.pm +++ b/lib/SQL/Statement/Functions.pm @@ -561,7 +561,7 @@ sub SQL_FUNCTION_CONV )->bfloor()->bstr() : int( log($dnum) / log($ebase) ); - while ( $dnum != 0 && length($new) < 255 ) + while ( ( $dnum != 0 || $i >= 0 ) && length($new) < 255 ) { if ( $i == -1 ) { # time to go pro... @@ -591,8 +591,8 @@ sub SQL_FUNCTION_CONV } # Final cleanup - $new =~ s/^(-?)0+/$1/ if ( $ebase <= 62 ); - $new =~ s/^(-?)A+/$1/ if ( $ebase > 62 ); + $new =~ s/^(-?)(?:0(?!(?:\.|\z)))+/$1/ if ( $ebase <= 62 ); + $new =~ s/^(-?)(?:A(?!(?:\.|\z)))+/$1/ if ( $ebase > 62 ); $new =~ s/0+$// if ( $ebase <= 62 && $is_dec ); $new =~ s/A+$// if ( $ebase > 62 && $is_dec ); diff --git a/t/06virtual.t b/t/06virtual.t index 72c6c87..9b93f73 100644 --- a/t/06virtual.t +++ b/t/06virtual.t @@ -528,6 +528,16 @@ foreach my $test_dbd (@test_dbds) sql => "SELECT CONV('1AF.EQO0000000000000000000000000000', 32, 8)", result => [ [2517.3553] ], }, + { + test => 'conv 10->16 integer with trailing 0', + sql => "select conv('16', 10, 16)", + result => [ ['10'] ], + }, + { + test => 'conv 10->16 integer 0', + sql => "select conv('0', 10, 16)", + result => [ ['0'] ], + }, { test => 'decode', sql =>
Download (untitled) / with headers
text/plain 433b
Hi Tom, first: thanks for the hint. At the moment I have no clue what ANSI SQL defines - and I fear I don't have time next few weeks to prove (missing trailing zeros is a bug, however - no questions about that). You're patch looks as a very good fundament to continuesly work on. If you want be credited for, feel free to send the patch also as Pull Request via GitHub. Otherwise I will commit it using your mail address as author.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.