Skip to content

Commit

Permalink
update filtering logic as per convo with Sean
Browse files Browse the repository at this point in the history
use composition rather than prewrapped functions
add more parentheses to avoid ambiguity
avoid ILIKE, stopping bugs due to inadvertent wildcards
  • Loading branch information
mathemancer committed Jul 25, 2024
1 parent 45490f9 commit bb78a2e
Show file tree
Hide file tree
Showing 2 changed files with 115 additions and 65 deletions.
36 changes: 16 additions & 20 deletions db/sql/00_msar.sql
Original file line number Diff line number Diff line change
Expand Up @@ -3317,30 +3317,26 @@ INSERT INTO msar.filter_templates VALUES
('and', '(%s) AND (%s)'),
('or', '(%s) OR (%s)'),
-- general comparison operators
('equal', '%s = %s'),
('lesser', '%s < %s'),
('greater', '%s > %s'),
('lesser_or_equal', '%s <= %s'),
('greater_or_equal', '%s >= %s'),
('null', '%s IS NULL'),
('not_null', '%s IS NOT NULL'),
('equal', '(%s) = (%s)'),
('lesser', '(%s) < (%s)'),
('greater', '(%s) > (%s)'),
('lesser_or_equal', '(%s) <= (%s)'),
('greater_or_equal', '(%s) >= (%s)'),
('null', '(%s) IS NULL'),
('not_null', '(%s) IS NOT NULL'),
-- string specific filters
('contains_case_insensitive', '%s ILIKE ''%%'' || %s || ''%%'''),
('starts_with_case_insensitive', '%s ILIKE %s || ''%%'''),
('contains_case_insensitive', 'strpos(lower(%s), lower(%s))::boolean'),
('starts_with_case_insensitive', 'starts_with(lower(%s), lower(%s))'),
('contains', 'strpos((%s), (%s))::boolean'),
('starts_with', 'starts_with((%s), (%s))'),
-- json(b) filters
('json_array_length_equals', 'jsonb_array_length(%s::jsonb) = %s'),
('json_array_length_greater_than', 'jsonb_array_length(%s::jsonb) > %s'),
('json_array_length_greater_or_equal', 'jsonb_array_length(%s::jsonb) >= %s'),
('json_array_length_less_than', 'jsonb_array_length(%s::jsonb) < %s'),
('json_array_length_less_or_equal', 'jsonb_array_length(%s::jsonb) <= %s'),
('json_array_not_empty', 'jsonb_array_length(%s::jsonb) > 0'),
('json_array_contains', '%s @> %s'),
('json_array_length', 'jsonb_array_length((%s)::jsonb)'),
('json_array_contains', '(%s) @> (%s)'),
-- URI filters
('uri_scheme_equals', 'mathesar_types.uri_scheme(%s) = %s'),
('uri_authority_contains', 'mathesar_types.uri_authority(%s) LIKE ''%%'' || %s || ''%%'''),
('uri_scheme', 'mathesar_types.uri_scheme(%s)'),
('uri_authority', 'mathesar_types.uri_authority(%s)'),
-- Email filters
('email_domain_equals', 'mathesar_types.email_domain_name(%s) = %s'),
('email_domain_contains', 'mathesar_types.email_domain_name(%s) LIKE ''%%'' || %s || ''%%''')
('email_domain', 'mathesar_types.email_domain_name(%s)')
;

CREATE OR REPLACE FUNCTION msar.build_filter_expr(rel_id oid, tree jsonb) RETURNS text AS $$
Expand Down
144 changes: 99 additions & 45 deletions db/sql/test_00_msar.sql
Original file line number Diff line number Diff line change
Expand Up @@ -3027,7 +3027,7 @@ BEGIN
'type', 'equal', 'args', jsonb_build_array(
jsonb_build_object('type', 'attnum', 'value', 2),
jsonb_build_object('type', 'literal', 'value', 500)))),
'col1 = ''500'''
'(col1) = (''500'')'
);
RETURN NEXT is(
msar.build_filter_expr(
Expand All @@ -3036,7 +3036,7 @@ BEGIN
'type', 'lesser', 'args', jsonb_build_array(
jsonb_build_object('type', 'attnum', 'value', 2),
jsonb_build_object('type', 'literal', 'value', 500)))),
'col1 < ''500'''
'(col1) < (''500'')'
);
RETURN NEXT is(
msar.build_filter_expr(
Expand All @@ -3045,7 +3045,7 @@ BEGIN
'type', 'greater', 'args', jsonb_build_array(
jsonb_build_object('type', 'attnum', 'value', 2),
jsonb_build_object('type', 'literal', 'value', 500)))),
'col1 > ''500'''
'(col1) > (''500'')'
);
RETURN NEXT is(
msar.build_filter_expr(
Expand All @@ -3054,7 +3054,7 @@ BEGIN
'type', 'lesser_or_equal', 'args', jsonb_build_array(
jsonb_build_object('type', 'attnum', 'value', 2),
jsonb_build_object('type', 'literal', 'value', 500)))),
'col1 <= ''500'''
'(col1) <= (''500'')'
);
RETURN NEXT is(
msar.build_filter_expr(
Expand All @@ -3063,23 +3063,23 @@ BEGIN
'type', 'greater_or_equal', 'args', jsonb_build_array(
jsonb_build_object('type', 'attnum', 'value', 2),
jsonb_build_object('type', 'literal', 'value', 500)))),
'col1 >= ''500'''
'(col1) >= (''500'')'
);
RETURN NEXT is(
msar.build_filter_expr(
rel_id,
jsonb_build_object(
'type', 'null', 'args', jsonb_build_array(
jsonb_build_object('type', 'attnum', 'value', 2)))),
'col1 IS NULL'
'(col1) IS NULL'
);
RETURN NEXT is(
msar.build_filter_expr(
rel_id,
jsonb_build_object(
'type', 'not_null', 'args', jsonb_build_array(
jsonb_build_object('type', 'attnum', 'value', 2)))),
'col1 IS NOT NULL'
'(col1) IS NOT NULL'
);
RETURN NEXT is(
msar.build_filter_expr(
Expand All @@ -3088,7 +3088,7 @@ BEGIN
'type', 'contains_case_insensitive', 'args', jsonb_build_array(
jsonb_build_object('type', 'attnum', 'value', 2),
jsonb_build_object('type', 'literal', 'value', 'ABc')))),
'col1 ILIKE ''%'' || ''ABc'' || ''%'''
'strpos(lower(col1), lower(''ABc''))::boolean'
);
RETURN NEXT is(
msar.build_filter_expr(
Expand All @@ -3097,61 +3097,91 @@ BEGIN
'type', 'starts_with_case_insensitive', 'args', jsonb_build_array(
jsonb_build_object('type', 'attnum', 'value', 2),
jsonb_build_object('type', 'literal', 'value', 'a''bc')))),
'col1 ILIKE ''a''''bc'' || ''%'''
'starts_with(lower(col1), lower(''a''''bc''))'
);
RETURN NEXT is(
-- composition for json_array_length_equals
msar.build_filter_expr(
rel_id,
jsonb_build_object(
'type', 'json_array_length_equals', 'args', jsonb_build_array(
jsonb_build_object('type', 'attnum', 'value', 2),
'type', 'equal', 'args', jsonb_build_array(
jsonb_build_object(
'type', 'json_array_length', 'args', jsonb_build_array(
jsonb_build_object('type', 'attnum', 'value', 2)
)
),
jsonb_build_object('type', 'literal', 'value', 500)))),
'jsonb_array_length(col1::jsonb) = ''500'''
'(jsonb_array_length((col1)::jsonb)) = (''500'')'
);
RETURN NEXT is(
-- composition for json_array_length_greater_than
msar.build_filter_expr(
rel_id,
jsonb_build_object(
'type', 'json_array_length_greater_than', 'args', jsonb_build_array(
jsonb_build_object('type', 'attnum', 'value', 2),
'type', 'greater', 'args', jsonb_build_array(
jsonb_build_object(
'type', 'json_array_length', 'args', jsonb_build_array(
jsonb_build_object('type', 'attnum', 'value', 2)
)
),
jsonb_build_object('type', 'literal', 'value', 500)))),
'jsonb_array_length(col1::jsonb) > ''500'''
'(jsonb_array_length((col1)::jsonb)) > (''500'')'
);
RETURN NEXT is(
msar.build_filter_expr(
-- composition for json_array_length_greater_or_equal
rel_id,
jsonb_build_object(
'type', 'json_array_length_greater_or_equal', 'args', jsonb_build_array(
jsonb_build_object('type', 'attnum', 'value', 2),
'type', 'greater_or_equal', 'args', jsonb_build_array(
jsonb_build_object(
'type', 'json_array_length', 'args', jsonb_build_array(
jsonb_build_object('type', 'attnum', 'value', 2)
)
),
jsonb_build_object('type', 'literal', 'value', 500)))),
'jsonb_array_length(col1::jsonb) >= ''500'''
'(jsonb_array_length((col1)::jsonb)) >= (''500'')'
);
RETURN NEXT is(
msar.build_filter_expr(
-- composition for json_array_length_less_than
rel_id,
jsonb_build_object(
'type', 'json_array_length_less_than', 'args', jsonb_build_array(
jsonb_build_object('type', 'attnum', 'value', 2),
'type', 'lesser', 'args', jsonb_build_array(
jsonb_build_object(
'type', 'json_array_length', 'args', jsonb_build_array(
jsonb_build_object('type', 'attnum', 'value', 2)
)
),
jsonb_build_object('type', 'literal', 'value', 500)))),
'jsonb_array_length(col1::jsonb) < ''500'''
'(jsonb_array_length((col1)::jsonb)) < (''500'')'
);
RETURN NEXT is(
msar.build_filter_expr(
-- composition for json_array_length_less_or_equal
rel_id,
jsonb_build_object(
'type', 'json_array_length_less_or_equal', 'args', jsonb_build_array(
jsonb_build_object('type', 'attnum', 'value', 2),
'type', 'lesser_or_equal', 'args', jsonb_build_array(
jsonb_build_object(
'type', 'json_array_length', 'args', jsonb_build_array(
jsonb_build_object('type', 'attnum', 'value', 2)
)
),
jsonb_build_object('type', 'literal', 'value', 500)))),
'jsonb_array_length(col1::jsonb) <= ''500'''
'(jsonb_array_length((col1)::jsonb)) <= (''500'')'
);
RETURN NEXT is(
msar.build_filter_expr(
-- composition for json_array_not_empty
rel_id,
jsonb_build_object(
'type', 'json_array_not_empty', 'args', jsonb_build_array(
jsonb_build_object('type', 'attnum', 'value', 2),
jsonb_build_object('type', 'literal', 'value', 500)))),
'jsonb_array_length(col1::jsonb) > 0'
'type', 'greater', 'args', jsonb_build_array(
jsonb_build_object(
'type', 'json_array_length', 'args', jsonb_build_array(
jsonb_build_object('type', 'attnum', 'value', 2)
)
),
jsonb_build_object('type', 'literal', 'value', 0)))),
'(jsonb_array_length((col1)::jsonb)) > (''0'')'
);
RETURN NEXT is(
msar.build_filter_expr(
Expand All @@ -3160,52 +3190,76 @@ BEGIN
'type', 'json_array_contains', 'args', jsonb_build_array(
jsonb_build_object('type', 'attnum', 'value', 2),
jsonb_build_object('type', 'literal', 'value', '"500"')))),
'col1 @> ''"500"'''
'(col1) @> (''"500"'')'
);
RETURN NEXT is(
msar.build_filter_expr(
-- composition for uri_scheme_equals
rel_id,
jsonb_build_object(
'type', 'uri_scheme_equals', 'args', jsonb_build_array(
jsonb_build_object('type', 'attnum', 'value', 2),
'type', 'equal', 'args', jsonb_build_array(
jsonb_build_object(
'type', 'uri_scheme', 'args', jsonb_build_array(
jsonb_build_object('type', 'attnum', 'value', 2)
)
),
jsonb_build_object('type', 'literal', 'value', 'https')))),
'mathesar_types.uri_scheme(col1) = ''https'''
'(mathesar_types.uri_scheme(col1)) = (''https'')'
);
RETURN NEXT is(
-- composition for uri_authority_contains
msar.build_filter_expr(
rel_id,
jsonb_build_object(
'type', 'uri_authority_contains', 'args', jsonb_build_array(
jsonb_build_object('type', 'attnum', 'value', 2),
'type', 'contains', 'args', jsonb_build_array(
jsonb_build_object(
'type', 'uri_authority', 'args', jsonb_build_array(
jsonb_build_object('type', 'attnum', 'value', 2)
)
),
jsonb_build_object('type', 'literal', 'value', 'google')))),
'mathesar_types.uri_authority(col1) LIKE ''%'' || ''google'' || ''%'''
'strpos((mathesar_types.uri_authority(col1)), (''google''))::boolean'
);
RETURN NEXT is(
-- composition for email_domain_equals
msar.build_filter_expr(
rel_id,
jsonb_build_object(
'type', 'email_domain_equals', 'args', jsonb_build_array(
jsonb_build_object('type', 'attnum', 'value', 2),
'type', 'equal', 'args', jsonb_build_array(
jsonb_build_object(
'type', 'email_domain', 'args', jsonb_build_array(
jsonb_build_object('type', 'attnum', 'value', 2)
)
),
jsonb_build_object('type', 'literal', 'value', 'gmail.com')))),
'mathesar_types.email_domain_name(col1) = ''gmail.com'''
'(mathesar_types.email_domain_name(col1)) = (''gmail.com'')'
);
RETURN NEXT is(
-- composition for email_domain_contains
msar.build_filter_expr(
rel_id,
jsonb_build_object(
'type', 'email_domain_contains', 'args', jsonb_build_array(
jsonb_build_object('type', 'attnum', 'value', 2),
'type', 'contains', 'args', jsonb_build_array(
jsonb_build_object(
'type', 'email_domain', 'args', jsonb_build_array(
jsonb_build_object('type', 'attnum', 'value', 2)
)
),
jsonb_build_object('type', 'literal', 'value', 'mail')))),
'mathesar_types.email_domain_name(col1) LIKE ''%'' || ''mail'' || ''%'''
'strpos((mathesar_types.email_domain_name(col1)), (''mail''))::boolean'
);
RETURN NEXT is(
msar.build_filter_expr(
rel_id,
jsonb_build_object(
'type', 'or', 'args', jsonb_build_array(
jsonb_build_object(
'type', 'email_domain_contains', 'args', jsonb_build_array(
jsonb_build_object('type', 'attnum', 'value', 2),
'type', 'contains', 'args', jsonb_build_array(
jsonb_build_object(
'type', 'email_domain', 'args', jsonb_build_array(
jsonb_build_object('type', 'attnum', 'value', 2)
)
),
jsonb_build_object('type', 'literal', 'value', 'mail'))
),
jsonb_build_object(
Expand All @@ -3216,7 +3270,7 @@ BEGIN
)
)
),
'(mathesar_types.email_domain_name(col1) LIKE ''%'' || ''mail'' || ''%'') OR (col2 = ''500'')'
'(strpos((mathesar_types.email_domain_name(col1)), (''mail''))::boolean) OR ((col2) = (''500''))'
);
RETURN NEXT is(
msar.build_filter_expr(
Expand Down Expand Up @@ -3245,7 +3299,7 @@ BEGIN
)
)
),
'((col2 = ''500'') AND (col3 < ''abcde'')) OR (id > ''20'')'
'(((col2) = (''500'')) AND ((col3) < (''abcde''))) OR ((id) > (''20''))'
);
END;
$$ LANGUAGE plpgsql;

0 comments on commit bb78a2e

Please sign in to comment.