From a4f5bc9dce8fb4065879db02559ef044d31ea888 Mon Sep 17 00:00:00 2001 From: Thomas Berezansky Date: Tue, 11 Sep 2012 15:02:59 -0400 Subject: [PATCH] QueryParser Driver: Protect against NULLs mrd.attrs->'value' can return NULL. If this happens: Checking that the value is within a range or list will work fine. NEGATING that will not. This is because: AND NULL returns NULL AND NOT (NULL) also returns NULL The solution? Adjust things so we can wrap all the offending checks in a COALESCE to false. Then if mrd.attrs->'value' is null we get a false. In the process we move any and all negations to outside the COALESCE. Also apply the same logic to the bib_source filter, not to mention making it support being negated. Signed-off-by: Thomas Berezansky Signed-off-by: Lebbeous Fogle-Weekley --- .../Application/Storage/Driver/Pg/QueryParser.pm | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Driver/Pg/QueryParser.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Driver/Pg/QueryParser.pm index 9ddb169b48..c0e345b88c 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Driver/Pg/QueryParser.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Driver/Pg/QueryParser.pm @@ -984,6 +984,7 @@ sub flatten { my $joiner = sprintf(" %s ", ($self->joiner eq '&' ? 'AND' : 'OR')); # for each dynamic filter, build more of the WHERE clause for my $filter (@{$self->filters}) { + my $NOT = $filter->negate ? 'NOT ' : ''; if (grep { $_ eq $filter->name } @{ $self->QueryParser->dynamic_filters }) { warn "flatten(): processing dynamic filter ". $filter->name ."\n" @@ -993,42 +994,40 @@ sub flatten { $where .= $joiner if $where ne '('; my @fargs = @{$filter->args}; - my $NOT = $filter->negate ? ' NOT' : ''; my $fname = $filter->name; $fname = 'item_lang' if $fname eq 'language'; #XXX filter aliases $where .= sprintf( - "attrs->'%s'$NOT IN (%s)", $fname, + "${NOT}COALESCE((mrd.attrs->'%s') IN (%s), false)", $fname, join(',', map { $self->QueryParser->quote_value($_) } @fargs) ); warn "flatten(): filter where => $where\n" if $self->QueryParser->debug; } else { - my $NOT = $filter->negate ? 'NOT ' : ''; switch ($filter->name) { case 'before' { if (@{$filter->args} == 1) { $where .= $joiner if $where ne '('; - $where .= "$NOT(mrd.attrs->'date1') <= " . $self->QueryParser->quote_value($filter->args->[0]); + $where .= "${NOT}COALESCE((mrd.attrs->'date1') <= " . $self->QueryParser->quote_value($filter->args->[0]) . ", false)"; } } case 'after' { if (@{$filter->args} == 1) { $where .= $joiner if $where ne '('; - $where .= "$NOT(mrd.attrs->'date1') >= " . $self->QueryParser->quote_value($filter->args->[0]); + $where .= "${NOT}COALESCE((mrd.attrs->'date1') >= " . $self->QueryParser->quote_value($filter->args->[0]) . ", false)"; } } case 'during' { if (@{$filter->args} == 1) { $where .= $joiner if $where ne '('; - $where .= $self->QueryParser->quote_value($filter->args->[0]) . " ${NOT}BETWEEN (mrd.attrs->'date1') AND (mrd.attrs->'date2')"; + $where .= "${NOT}COALESCE(" . $self->QueryParser->quote_value($filter->args->[0]) . " BETWEEN (mrd.attrs->'date1') AND (mrd.attrs->'date2'), false)"; } } case 'between' { if (@{$filter->args} == 2) { $where .= $joiner if $where ne '('; - $where .= "(mrd.attrs->'date1') ${NOT}BETWEEN " . $self->QueryParser->quote_value($filter->args->[0]) . " AND " . $self->QueryParser->quote_value($filter->args->[1]); + $where .= "${NOT}COALESCE((mrd.attrs->'date1') BETWEEN " . $self->QueryParser->quote_value($filter->args->[0]) . " AND " . $self->QueryParser->quote_value($filter->args->[1]) . ", false)"; } } case 'container' { @@ -1096,7 +1095,7 @@ sub flatten { case 'bib_source' { if (@{$filter->args} > 0) { $where .= $joiner if $where ne '('; - $where .= "bre.source IN (" . join(',', map { $self->QueryParser->quote_value($_) } @{ $filter->args }) . ")"; + $where .= "${NOT}COALESCE(bre.source IN (" . join(',', map { $self->QueryParser->quote_value($_) } @{ $filter->args }) . "), false)"; } } } -- 2.43.2