Limit with IN operator and empty VALUES results in invalid SQL queries

Loos_Christian · December 3, 2014, 8:39am

Hi,

yesterday I saw some invalid MySQL queries in my logs:
SELECT ObjectCustomFieldValues_1.Content AS col0, COUNT(main.id) AS id
FROM Tickets main
LEFT JOIN ObjectCustomFieldValues ObjectCustomFieldValues_1
ON ( ObjectCustomFieldValues_1.CustomField = ‘90’ )
AND ( ObjectCustomFieldValues_1.Disabled = ‘0’ )
AND ( ObjectCustomFieldValues_1.ObjectType = ‘RT::Ticket’ )
AND ( ObjectCustomFieldValues_1.ObjectId = main.id )
WHERE (main.Id IN ())
GROUP BY ObjectCustomFieldValues_1.Content

I think this comes from [1].
Recently there was also a branch merged [2] where you switched from
looping over value lists to using the IN operator.

May there need to be a check that the VALUE list is not empty to avoid
invalid SQL queries?

Chris

[1]

github.com

bestpractical/rt/blob/stable/lib/RT/Report/Tickets.pm#L490


      
                  my $self = shift;
                  my ($function, $field) = @_;
                  return (FUNCTION => "$function(?)*60", FIELD => $field);
              },
              Display => 'DurationAsString',
          },
          TimeAll => {
              SubValues => sub { return ('Minimum', 'Average', 'Maximum', 'Total') },
              Function => sub {
                  my $self = shift;
                  my $field = shift;
                  return (
                      Minimum => { FUNCTION => "MIN(?)*60", FIELD => $field },
                      Average => { FUNCTION => "AVG(?)*60", FIELD => $field },
                      Maximum => { FUNCTION => "MAX(?)*60", FIELD => $field },
                      Total   => { FUNCTION => "SUM(?)*60", FIELD => $field },
                  );
              },
              Display => 'DurationAsString',
          },
          DateTimeInterval => {

[2] https://github.com/bestpractical/rt/commit/b14df0e

Loos_Christian · December 10, 2014, 9:46am

Hi,

yesterday I saw some invalid MySQL queries in my logs:
SELECT ObjectCustomFieldValues_1.Content AS col0, COUNT(main.id) AS id
FROM Tickets main
LEFT JOIN ObjectCustomFieldValues ObjectCustomFieldValues_1
ON ( ObjectCustomFieldValues_1.CustomField = ‘90’ )
AND ( ObjectCustomFieldValues_1.Disabled = ‘0’ )
AND ( ObjectCustomFieldValues_1.ObjectType = ‘RT::Ticket’ )
AND ( ObjectCustomFieldValues_1.ObjectId = main.id )
WHERE (main.Id IN ())
GROUP BY ObjectCustomFieldValues_1.Content

I think this comes from [1].
Recently there was also a branch merged [2] where you switched from
looping over value lists to using the IN operator.

May there need to be a check that the VALUE list is not empty to avoid
invalid SQL queries?

Chris

[1]
rt/lib/RT/Report/Tickets.pm at stable · bestpractical/rt · GitHub
[2] When possible, switch to IN or NOT IN rather than multiple OR'd clauses · bestpractical/rt@b14df0e · GitHub

I changed
$self->Limit( FIELD => ‘Id’, OPERATOR => ‘IN’, VALUE => @match );
to
$self->Limit( FIELD => ‘Id’, OPERATOR => ‘IN’, VALUE => @match ? @match
: [0] );
which fixes the bug for me.

It would be great if one of the developers can review this and give some
feedback.

I’m not sure if this should only be fixed in RT or if this should be
fixed in DBIx::SearchBuilder (always generate valid SQL even if the
provided VALUE is an empty list).

Chris

Alex_Vandiver · December 10, 2014, 6:49pm

yesterday I saw some invalid MySQL queries in my logs:
[snip]

I changed
$self->Limit( FIELD => ‘Id’, OPERATOR => ‘IN’, VALUE => @match );
to
$self->Limit( FIELD => ‘Id’, OPERATOR => ‘IN’, VALUE => @match ? @match
: [0] );
which fixes the bug for me.

It would be great if one of the developers can review this and give some
feedback.

Sorry – this was flagged on my “should look at” but I’d not gotten to
it. I’ve pushed 4.2/empty-in-clause with a similar-looking change.

I’m not sure if this should only be fixed in RT or if this should be
fixed in DBIx::SearchBuilder (always generate valid SQL even if the
provided VALUE is an empty list).

It’s hard to decide if DBIx::SearchBuilder should apply no limit, or
apply a never-match limit in such cases. RT has cases of each.

Alex

Loos_Christian · December 11, 2014, 3:02pm

yesterday I saw some invalid MySQL queries in my logs:
[snip]

I changed
$self->Limit( FIELD => ‘Id’, OPERATOR => ‘IN’, VALUE => @match );
to
$self->Limit( FIELD => ‘Id’, OPERATOR => ‘IN’, VALUE => @match ? @match
: [0] );
which fixes the bug for me.

It would be great if one of the developers can review this and give some
feedback.

Sorry – this was flagged on my “should look at” but I’d not gotten to
it. I’ve pushed 4.2/empty-in-clause with a similar-looking change.
I applied you change for RT::Report::Tickets on my 4.2.9 and the hundred
of error messages are gone. Thanks.