Skip to content

Adding support for filtering on array column types #520

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 31 commits into from
May 31, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
a013f22
adding mock blog posts to seed data
mlabisi May 14, 2024
793254a
adding list type to filterable types
mlabisi May 14, 2024
4f08594
adding array filterops
mlabisi May 14, 2024
fb61169
using modified type for match
mlabisi May 15, 2024
163707e
adding array filter types to transpiler
mlabisi May 15, 2024
91b6b17
adding array filtering to builder
mlabisi May 15, 2024
c425f8f
fixing array filter input fields
mlabisi May 15, 2024
237a0a8
adding list filter types to schema
mlabisi May 15, 2024
809047a
correcting entity name for list filter
mlabisi May 15, 2024
ee149f2
fixup! adding mock blog posts to seed data
mlabisi May 15, 2024
9851b3c
simplifying array filter builders
mlabisi May 15, 2024
4cc6752
un-nesting list filter
mlabisi May 15, 2024
201154d
unwrapping non-nullable types
mlabisi May 15, 2024
a080c4f
enabling ordering by array columns
mlabisi May 15, 2024
0a45e97
disabling ordering by array columns
mlabisi May 15, 2024
83c5ea9
updating failing tests
mlabisi May 16, 2024
384e394
adding test cases for filtering by array columns
mlabisi May 16, 2024
a792bc1
removing archived status from seed data
mlabisi May 17, 2024
c2ee53d
removing extra comment from seed data
mlabisi May 17, 2024
dd9784a
removing unnecessary array handling from create_filters
mlabisi May 17, 2024
498d35c
removing unnecessary spacing change from create_filters
mlabisi May 17, 2024
26be47c
updating docs to include array column filters
mlabisi May 17, 2024
761f226
adding missing result to docs
mlabisi May 17, 2024
e658d83
bumping minor version
mlabisi May 19, 2024
3e3fb84
bumping patch version
mlabisi May 19, 2024
86b11fe
Merge remote-tracking branch 'origin/issue-460' into issue-460
mlabisi May 19, 2024
2e2a6be
removing bump on patch version
mlabisi May 26, 2024
c1a4ebe
removing unwrap
mlabisi May 26, 2024
02c273f
Merge branch 'refs/heads/master' into issue-460
mlabisi May 26, 2024
ca2890b
correcting ov test
mlabisi May 26, 2024
edcf8e0
correcting wording for cd docs
mlabisi May 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions dockerfiles/db/setup.sql
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ create table blog_post(
blog_id integer not null references blog(id) on delete cascade,
title varchar(255) not null,
body varchar(10000),
tags TEXT[],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This column was added to allow live validation beyond just the regression tests for filtering by array columns!

status blog_post_status not null,
created_at timestamp not null
);
Expand All @@ -79,5 +80,15 @@ values
((select id from account where email ilike 'a%'), 'A: Blog 3', 'a desc3', now()),
((select id from account where email ilike 'b%'), 'B: Blog 3', 'b desc1', now());

insert into blog_post (blog_id, title, body, tags, status, created_at)
values
((SELECT id FROM blog WHERE name = 'A: Blog 1'), 'Post 1 in A Blog 1', 'Content for post 1 in A Blog 1', '{"tech", "update"}', 'RELEASED', NOW()),
((SELECT id FROM blog WHERE name = 'A: Blog 1'), 'Post 2 in A Blog 1', 'Content for post 2 in A Blog 1', '{"announcement", "tech"}', 'PENDING', NOW()),
((SELECT id FROM blog WHERE name = 'A: Blog 2'), 'Post 1 in A Blog 2', 'Content for post 1 in A Blog 2', '{"personal"}', 'RELEASED', NOW()),
((SELECT id FROM blog WHERE name = 'A: Blog 2'), 'Post 2 in A Blog 2', 'Content for post 2 in A Blog 2', '{"update"}', 'RELEASED', NOW()),
((SELECT id FROM blog WHERE name = 'A: Blog 3'), 'Post 1 in A Blog 3', 'Content for post 1 in A Blog 3', '{"travel", "adventure"}', 'PENDING', NOW()),
((SELECT id FROM blog WHERE name = 'B: Blog 3'), 'Post 1 in B Blog 3', 'Content for post 1 in B Blog 3', '{"tech", "review"}', 'RELEASED', NOW()),
((SELECT id FROM blog WHERE name = 'B: Blog 3'), 'Post 2 in B Blog 3', 'Content for post 2 in B Blog 3', '{"coding", "tutorial"}', 'PENDING', NOW());


comment on schema public is '@graphql({"inflect_names": true})';
25 changes: 22 additions & 3 deletions src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1085,7 +1085,7 @@ fn create_filters(
let kv_map = match validated {
gson::Value::Absent | gson::Value::Null => return Ok(filters),
gson::Value::Object(kv) => kv,
_ => return Err("Filter re-validation errror".to_string()),
_ => return Err("Filter re-validation error".to_string()),
};

for (k, op_to_v) in kv_map {
Expand Down Expand Up @@ -1137,7 +1137,7 @@ fn create_filters(
}
}
}
gson::Value::Array(values) if k == AND_FILTER_NAME || k == OR_FILTER_NAME => {
gson::Value::Array(values) => if k == AND_FILTER_NAME || k == OR_FILTER_NAME {
// If there are no inner filters we avoid creating an argumentless `and`/`or` expression
// which would have been anyways compiled away during transpilation
if !values.is_empty() {
Expand Down Expand Up @@ -1171,8 +1171,27 @@ fn create_filters(

filters.push(filter_builder);
}
} else {
if !values.is_empty() {
for value in values {
if let gson::Value::Object(filter_op_to_value_map) = value {
for (filter_op_str, filter_val) in filter_op_to_value_map {
let filter_op = FilterOp::from_str(filter_op_str)?;

// Skip absent
// Technically nulls should be treated as literals. It will always filter out all rows
// val <op> null is never true
if filter_val == &gson::Value::Absent {
continue;
}

filters.push(create_filter_builder_elem(filter_iv, filter_op, filter_val)?);
}
}
}
}
}
_ => return Err("Filter re-validation errror op_to_value map".to_string()),
_ => return Err("Filter re-validation error op_to_value map".to_string()),
}
}
Ok(filters)
Expand Down
145 changes: 132 additions & 13 deletions src/graphql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1116,6 +1116,7 @@ pub struct OrderByEntityType {
pub enum FilterableType {
Scalar(Scalar),
Enum(EnumType),
List(ListType),
}

#[derive(Clone, Debug, Eq, PartialEq, Hash)]
Expand All @@ -1129,6 +1130,10 @@ impl FilterTypeType {
match &self.entity {
FilterableType::Scalar(s) => s.name().expect("scalar name should exist"),
FilterableType::Enum(e) => e.name().expect("enum type name should exist"),
FilterableType::List(l) => match l.of_type().unwrap().name() {
None => panic!("inner list type name should exist"),
Some(name) => format!("{}List", name)
},
}
}
}
Expand Down Expand Up @@ -3328,6 +3333,9 @@ pub enum FilterOp {
ILike,
RegEx,
IRegEx,
Contains,
ContainedBy,
Overlap,
}

impl ToString for FilterOp {
Expand All @@ -3346,6 +3354,9 @@ impl ToString for FilterOp {
Self::ILike => "ilike",
Self::RegEx => "regex",
Self::IRegEx => "iregex",
Self::Contains => "cs",
Self::ContainedBy => "cd",
Self::Overlap => "ov",
}
.to_string()
}
Expand All @@ -3369,6 +3380,9 @@ impl FromStr for FilterOp {
"ilike" => Ok(Self::ILike),
"regex" => Ok(Self::RegEx),
"iregex" => Ok(Self::IRegEx),
"cs" => Ok(Self::Contains),
"cd" => Ok(Self::ContainedBy),
"ov" => Ok(Self::Overlap),
_ => Err("Invalid filter operation".to_string()),
}
}
Expand Down Expand Up @@ -3541,6 +3555,8 @@ impl ___Type for FilterTypeType {
default_value: None,
sql_type: None,
},
// shouldn't happen since we've covered all cases in supported_ops
_ => panic!("encountered unknown FilterOp")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Scalar's supported_ops won't contain any of the array-only filter operations, so we're safe to ignore them here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should restructure the FilterOp and related types so that we can avoid this catchall panic. As this PR is already long enough, I'm ok with doing that as a separate PR. Just open a ticket to handle this later.

})
.collect()
}
Expand Down Expand Up @@ -3583,6 +3599,36 @@ impl ___Type for FilterTypeType {
},
]
}
FilterableType::List(list_type) => {
let supported_ops = vec![
FilterOp::Contains,
FilterOp::ContainedBy,
FilterOp::Equal,
FilterOp::GreaterThan,
FilterOp::GreaterThanEqualTo,
FilterOp::LessThan,
FilterOp::LessThanEqualTo,
FilterOp::NotEqual,
FilterOp::Overlap,
];

supported_ops
.iter()
.map(|op| match op {
_ => __InputValue {
name_: op.to_string(),
type_: __Type::List(ListType {
type_: Box::new(__Type::NonNull(NonNullType {
type_: Box::new(*list_type.type_.clone()),
})),
}),
description: None,
default_value: None,
sql_type: None,
},
})
.collect()
}
};

infields.sort_by_key(|a| a.name());
Expand Down Expand Up @@ -3620,14 +3666,11 @@ impl ___Type for FilterEntityType {
.columns
.iter()
.filter(|x| x.permissions.is_selectable)
// No filtering on arrays
.filter(|x| !x.type_name.ends_with("[]"))
// No filtering on composites
.filter(|x| !self.schema.context.is_composite(x.type_oid))
// No filtering on json/b. they do not support = or <>
.filter(|x| !["json", "jsonb"].contains(&x.type_name.as_ref()))
.filter_map(|col| {
// Should be a scalar
if let Some(utype) = sql_column_to_graphql_type(col, &self.schema) {
let column_graphql_name = self.schema.graphql_column_field_name(col);

Expand All @@ -3641,22 +3684,33 @@ impl ___Type for FilterEntityType {
not_column_exists = true;
}

match utype.unmodified_type() {
__Type::Scalar(s) => Some(__InputValue {
match utype.nullable_type() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that we support filtering by array columns, we want to respect the fact that the utype is an array, so we'll only unwrap Non-Nullables.

__Type::Scalar(s) => {
Some(__InputValue {
name_: column_graphql_name,
type_: __Type::FilterType(FilterTypeType {
entity: FilterableType::Scalar(s),
schema: Arc::clone(&self.schema),
}),
description: None,
default_value: None,
sql_type: Some(NodeSQLType::Column(Arc::clone(col))),
})
},
__Type::Enum(e) => Some(__InputValue {
name_: column_graphql_name,
type_: __Type::FilterType(FilterTypeType {
entity: FilterableType::Scalar(s),
entity: FilterableType::Enum(e),
schema: Arc::clone(&self.schema),
}),
description: None,
default_value: None,
sql_type: Some(NodeSQLType::Column(Arc::clone(col))),
}),
// ERROR HERE
__Type::Enum(s) => Some(__InputValue {
__Type::List(l) => Some(__InputValue {
name_: column_graphql_name,
type_: __Type::FilterType(FilterTypeType {
entity: FilterableType::Enum(s),
entity: FilterableType::List(l),
schema: Arc::clone(&self.schema),
}),
description: None,
Expand Down Expand Up @@ -3829,13 +3883,12 @@ impl ___Type for OrderByEntityType {
.columns
.iter()
.filter(|x| x.permissions.is_selectable)
// No filtering on arrays
// No ordering by arrays
.filter(|x| !x.type_name.ends_with("[]"))
// No filtering on composites
// No ordering by composites
.filter(|x| !self.schema.context.is_composite(x.type_oid))
// No filtering on json/b. they do not support = or <>
// No ordering by json/b. they do not support = or <>
.filter(|x| !["json", "jsonb"].contains(&x.type_name.as_ref()))
// TODO filter out arrays, json and composites
.map(|col| __InputValue {
name_: self.schema.graphql_column_field_name(col),
type_: __Type::OrderBy(OrderByType {}),
Expand Down Expand Up @@ -3968,6 +4021,72 @@ impl __Schema {
entity: FilterableType::Scalar(Scalar::Opaque),
schema: Arc::clone(&schema_rc),
}),
__Type::FilterType(FilterTypeType {
entity: FilterableType::List(ListType {
type_: Box::new(__Type::Scalar(Scalar::ID))
}),
schema: Arc::clone(&schema_rc),
}),
__Type::FilterType(FilterTypeType {
entity: FilterableType::List(ListType {
type_: Box::new(__Type::Scalar(Scalar::Int))
}),
schema: Arc::clone(&schema_rc),
}),
__Type::FilterType(FilterTypeType {
entity: FilterableType::List(ListType {
type_: Box::new(__Type::Scalar(Scalar::Float))
}),
schema: Arc::clone(&schema_rc),
}),
__Type::FilterType(FilterTypeType {
entity: FilterableType::List(ListType {
type_: Box::new(__Type::Scalar(Scalar::String(None)))
}),
schema: Arc::clone(&schema_rc),
}),
__Type::FilterType(FilterTypeType {
entity: FilterableType::List(ListType {
type_: Box::new(__Type::Scalar(Scalar::Boolean))
}),
schema: Arc::clone(&schema_rc),
}),
__Type::FilterType(FilterTypeType {
entity: FilterableType::List(ListType {
type_: Box::new(__Type::Scalar(Scalar::Date))
}),
schema: Arc::clone(&schema_rc),
}),
__Type::FilterType(FilterTypeType {
entity: FilterableType::List(ListType {
type_: Box::new(__Type::Scalar(Scalar::Time))
}),
schema: Arc::clone(&schema_rc),
}),
__Type::FilterType(FilterTypeType {
entity: FilterableType::List(ListType {
type_: Box::new(__Type::Scalar(Scalar::Datetime))
}),
schema: Arc::clone(&schema_rc),
}),
__Type::FilterType(FilterTypeType {
entity: FilterableType::List(ListType {
type_: Box::new(__Type::Scalar(Scalar::BigInt))
}),
schema: Arc::clone(&schema_rc),
}),
__Type::FilterType(FilterTypeType {
entity: FilterableType::List(ListType {
type_: Box::new(__Type::Scalar(Scalar::UUID))
}),
schema: Arc::clone(&schema_rc),
}),
__Type::FilterType(FilterTypeType {
entity: FilterableType::List(ListType {
type_: Box::new(__Type::Scalar(Scalar::BigFloat))
}),
schema: Arc::clone(&schema_rc),
}),
__Type::Query(QueryType {
schema: Arc::clone(&schema_rc),
}),
Expand Down
6 changes: 6 additions & 0 deletions src/transpile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -714,6 +714,9 @@ impl FilterBuilderElem {
_ => {
let cast_type_name = match op {
FilterOp::In => format!("{}[]", column.type_name),
FilterOp::Contains => format!("{}[]", column.type_name),
FilterOp::ContainedBy => format!("{}[]", column.type_name),
FilterOp::Overlap => format!("{}[]", column.type_name),
_ => column.type_name.clone(),
};

Expand All @@ -735,6 +738,9 @@ impl FilterBuilderElem {
FilterOp::ILike => "ilike",
FilterOp::RegEx => "~",
FilterOp::IRegEx => "~*",
FilterOp::Contains => "@>",
FilterOp::ContainedBy => "<@",
FilterOp::Overlap => "&&",
FilterOp::Is => {
return Err("Error transpiling Is filter".to_string());
}
Expand Down
6 changes: 4 additions & 2 deletions test/expected/omit_exotic_types.out
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
begin;
/*
Composite and array types are not currently supported as inputs
Composite types are not currently supported as inputs
- confirm composites are not allowed anywhere
- confirm arrays are not allowed as input
*/
create type complex as (r int, i int);
create table something(
Expand Down Expand Up @@ -107,6 +106,9 @@ begin;
{ +
"name": "name" +
}, +
{ +
"name": "tags" +
}, +
{ +
"name": "nodeId" +
}, +
Expand Down
Loading