byterock

It back to coding here in the Moose-Pen

About a week ago I had this rather obtuse SQL expression;

(abs((people.salary + .05) * 1.5))*people.overtime+(abs(people.salary+.05) *2)*people.doubletime)

for testing parentheses. I figured I might as well re-use that little snippet of code else where as it is valid SQL anywhere you would use a 'field'.

One very odd place to use it would be in a sort if you want to order a list by how much over time and double time a person earned. So I added a new test to my '60_order_by.t' case;

...

    sql     => "SELECT people.first_name, people.last_name, people.user_id FROM people ORDER BY people.last_name, people.first_name",

},{

    key  =>'sorts',

    sorts => [$expression],

    caption => "Expression in Order by ",

       sql     => "SELECT people.first_name, people.last_name, people.user_id FROM people ORDER BY (abs((people.salary + ?) * ?))*people.overtime+(abs(people.salary+.?) *?)*people.doubletime)",

    params  => ['0.5','1.5','0.5','2']

}];

use Test::More  tests =>3;

my $utils =  Test::Utils->new();

$utils->sql_param_ok($in_hash,$tests);

Now in the above the $expression is that hash-ref from my earlier post so no need to repeat that here. Lets see what the result of this test is;

ok 1 - Simple Order by  SQL correct

not ok 2 - Complex Expression in Order by  SQL correct

ok 3 - Complex Expression in Order by  params correct

and the report on fail 2 is;

# Expected SQL--> SELECT people.first_name, people.last_name, people.user_id 

FROM people ORDER BY (abs((people.salary + ?) * ?))*people.overtime+

(abs(people.salary+.?) *?)*people.doubletime)

# Generated SQL-> SELECT people.first_name, people.last_name, people.user_id 

FROM people ORDER BY (((abs((people.salary + ?)) * ?) * people.overtime) + 

((abs((people.salary + ?)) * ?) * people.doubletime))

so it looks like I have a few extra parentheses in there. Though checking the expression it is valid as all the parentheses are balanced and the exrta set at the start or each expression have no effect as

((abs((people.salary + ?)) * ?) * people.overtime) 

and

abs((people.salary + ?) * ?))*people.overtime

would come out to the same value, but I am not liking all those extra ones.

To start debugging this I took all the

        open_parentheses  => 1,

        close_parentheses => 1,

out of the $expression and I still got

ORDER BY (((abs((people.salary + ?)) * ?) * people.overtime) + ((abs((people.salary + ?)) * ?) * people.doubletime))

That tells me I am not even paying attention to the open and close keys as without them I would of expected to get

ORDER BY abs(people.salary + ?) * ? * people.overtime + abs(people.salary + ?) * ? * people.doubletime

The first thing I noticed was the only place I check for either the 'open' and 'close' is in the '_predicate_sql' sub. Make sense that taking out those keys does nothing as I have no predicates in the expression.

Looking deeper I see that I do this

   return  Database::Accessor::Driver::DBI::SQL::OPEN_PARENS

             .join(" "

             ,$left_sql

             ,$element->expression

             ,$right_sql)

             .Database::Accessor::Driver::DBI::SQL::CLOSE_PARENS;

in the '_field_sql' sub when the field is an 'expression' and very similar;

 return $element->function

             .Database::Accessor::Driver::DBI::SQL::OPEN_PARENS

             .$left_sql

             .$comma

             .$right_sql

             .Database::Accessor::Driver::DBI::SQL::CLOSE_PARENS;

when the field is an function.

I need the open and close around each function so they stay I will give this a go my taking the open and close out of the expression part.

return  join(" "

             ,$left_sql

             ,$element->expression

             ,$right_sql);

and now I get;

ORDER BY abs(people.salary + ?) * ? * people.overtime + abs(people.salary + ?) * ? *  people.doubletime

which I think is close to what I want. I just have to account for those open and close so in they go again to the $expression hash-ref. Next I have to add a little code to the '_field_SQL' and my first crak at that is

--     my $left_sql = $self->_field_sql($element->left());

++      my ($left_sql,$right_sql);

++      $left_sql = Database::Accessor::Driver::DBI::SQL::OPEN_PARENS

++              ." "

++         if ( $element->open_parentheses() );

++      $right_sql = Database::Accessor::Driver::DBI::SQL::CLOSE_PARENS

++              ." "

++         if ( $element->close_parentheses() );

++      $left_sql .= $self->_field_sql($element->left());

…

--      return  Database::Accessor::Driver::DBI::SQL::OPEN_PARENS

--             .join(" "

–                    ,$left_sql 

–                    ,$element->expression

--                  ,$right_sql)

-–                  .Database::Accessor::Driver::DBI::SQL::CLOSE_PARENS;

--             );

++      return join(" "

++             ,$left_sql

++             ,$element->expression

++             ,$right_sql);

  

and from that I got

 ORDER BY ( ( abs(people.salary + ?) * ) ? * ) people.overtime + ( ( abs(people.salary + ?) * ) ? * ) 

still a little of and I see what my mistake was I want that 'Close' at the end not the beginning of my '$right_sql. Now I have this as the full call;

 my $left_sql;

      $left_sql = Database::Accessor::Driver::DBI::SQL::OPEN_PARENS

              ." "

         if ( $element->open_parentheses() );

      $left_sql .= $self->_field_sql($element->left());

      my @right_sql;

      if (ref($element->right()) ne "Array"){

         my $param = $element->right();

         $element->right([$param])

         if ($param);

      }

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

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

      }        

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

      $right_sql   .= Database::Accessor::Driver::DBI::SQL::CLOSE_PARENS

              ." "

         if ( $element->close_parentheses() );      

      return join(" "

             ,$left_sql

             ,$element->expression

             ,$right_sql);

and I get;

ORDER BY ( ( abs(people.salary + ?) * ?)  * people.overtime)  + ( ( abs(people.salary + ?) * ?)  * people.doubletime) 

which is very close to what I want. I think by playing with my $expression a little I may get it;

I ended up add in open and close parentheses in both the 'salary' expressions;

...

 function => 'abs',

                left     => {

++                  open_parentheses  => 1,

++                  close_parentheses => 1,

                    expression => '+',

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

                    right      => { value => '0.5' }

                },

…

 function => 'abs',

                left     => {

++                  open_parentheses  => 1,

++                  close_parentheses => 1,

                    expression => '+',

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

                    right      => { value => '0.5' }

                },

and I got;

ORDER BY ( ( abs(( people.salary + ?) ) * ?)  * people.overtime)  + ( ( abs(( people.salary + ?) ) * ?)  * people.doubletime) 

and after cleaning up some extra spaces I get

 ((abs((people.salary + ?)) * ?) * people.overtime) + ((abs((people.salary + ?)) * ?) * people.doubletime)

which is what I really want so a quick change to the test and I get a full pass.