byterock

Its little bird day here in the Moose-Pen

How does that 'The Be Good Tanyas' song go?

And I sing, the littlest birds sing the prettiest songs …

For us Perl types it should be

And I swear, the littlest typos make the ugliest bugs ...

You will remember that I left off yesterday with this little bug;

Expected SQL--> SELECT people.first_name, people.last_name, people.bonus * abs(?) FROM people

# Generated SQL-> SELECT people.first_name, people.last_name, people.bonus * abs(?),1 FROM people

I was getting ',1' tacked onto the end of some of my generated SQL.

This one proved rather frustrating to track down, and it took much more time than one would expect. I suspected it has something to do with the 'function' part of the '_field_sql' code so I started there but despite my best efforts of adding warnings on each line I could not find it.

I eventually found out that at some point during the recourse on '_field_sql' the $use_alias value was not being passed in as I had this waring at the top of the sub;

sub _field_sql {

  my $self = shift;

  my ($element,$use_view) = @_;

  warn("element =".Dumper($element).",use_view=$use_view");

...

I eventually had to go to my old standby and use 'caller' to see where that '$use_view' was being called in an undef state. I used this call to save me scanning though hundreds of warnings;

sub _field_sql {

  my $self = shift;

  my ($element,$use_view) = @_;

  my ($package, $filename, $line) = caller;

  warn("called by line=$line")

     if (!$use_view);

and the problem finally showed itself

called by line=303 at GitHub\database-accessor-driver-dbi\lib/Database/Accessor/Driver/DBI.pm line 287.

and here it is;

foreach my $param (@{$element->right()}){

--        push(@right_sql,$self->_field_sql($param),$use_view);

++        push(@right_sql,$self->_field_sql($param,$use_view));

      }     

so just a simple typo on my part. It is doing exactly what I see as the error line will add the value of '$use_view”, which is '1' to the '@right' so in that array I will have

@right = (“abs(?)”,  “1” )

and when I processing it a little later on with this join;

 my $right_sql = join(',',@right_sql);

I get the error result;

 $right_sql = ' abs(?),1'

Well after I fixed that typo I now only get one error

…

ok 36 - 2 Fields and a function within a function create params correct

not ok 37 - 2 Fields and a function within a function retrieve SQL correct

ok 38 - 2 Fields and a function within a function retrieve params correct

…

and the expected SQL is;

SELECT people.first_name, 

               people.last_name, 

               substr(people.username,?,left(people.address,?)) 

  FROM people

and the generated SQL is

SELECT people.first_name, 

                people.last_name, 

               substr(people.username,?,left(.address,?)) 

   FROM people

so I am missing that alias on the second embedded function;

To debug this one I did this little trick.

$utils->sql_param_ok( $in_hash, [$tests->[5]] );

in my test case so I would only have to deal with the test that is in error. I quickly discovered, as I suspected, the bug is in Database::Accessor as I am not getting the default view set on that second function’s left element;

$element = bless( {

                 'left' => bless( {

                                    'name' => 'address'

                                  }, 'Database::Accessor::Element' ),

                 ...

               }, 'Database::Accessor::Function' );

Now what I did was take the offending element from the test and dropped it into the '57_dad_elements.t' test case in Database::Accessor which now fails

...

ok 53 - Gather condition index 2 right inherits view

not ok 54 - function in function inherits view

Now the key here is in this case I have an array-ref for the 'right' value of that function;

right    => [

                    { param => 3 },

                    {

                        function => 'left',

                        left     => { name => 'address' },

                        right    => { param => 4 }

                    }

                ]

and I did not take that into account in Database::Accessor. The patch was simple enough and took less time that that silly typo. All I had to do was extend the if statement found in the '_check_element' sub

...

   }

++ elsif (ref($element) eq 'ARRAY'){

++    foreach my $sub_element (@{$element}){

++       $self->_check_element($sub_element);

++     }

++ }

   else {

       return 

          unless(does_role($element,"Database::Accessor::Roles::Comparators"));

...

   

to account for an 'array-ref' and now I get a full pass on both Database::Accessor's '57_dad_elements.t' test case and Driver::DBI's '20_fields.t' test case.

Progress!