Mailing List Archive: Limit with IN operator and empty VALUES results in invalid SQL queries

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

Dec 3, 2014, 12:39 AM

Post #1 of 4 (1804 views)

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]
https://github.com/bestpractical/rt/blob/stable/lib/RT/Report/Tickets.pm#L490
[2] https://github.com/bestpractical/rt/commit/b14df0e

Re: Limit with IN operator and empty VALUES results in invalid SQL queries [ In reply to ]

cloos at netcologne

Dec 10, 2014, 1:46 AM

Post #2 of 4 (1770 views)

Permalink

Am 03.12.2014 um 09:39 schrieb Christian Loos:
> 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]
> https://github.com/bestpractical/rt/blob/stable/lib/RT/Report/Tickets.pm#L490
> [2] https://github.com/bestpractical/rt/commit/b14df0e
>

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

Re: Limit with IN operator and empty VALUES results in invalid SQL queries [ In reply to ]

alexmv at bestpractical

Dec 10, 2014, 10:49 AM

Post #3 of 4 (1768 views)

Permalink

On 12/10/2014 04:46 AM, Christian Loos wrote:
>> 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

Re: Limit with IN operator and empty VALUES results in invalid SQL queries [ In reply to ]

cloos at netcologne

Dec 11, 2014, 7:02 AM

Post #4 of 4 (1765 views)

Permalink

Am 10.12.2014 um 19:49 schrieb Alex Vandiver:
> On 12/10/2014 04:46 AM, Christian Loos wrote:
>>> 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.

>
>> 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.