Skip to content
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

fix a bug in which a UDF call returned null #441

Merged
merged 4 commits into from
Oct 24, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
49 changes: 48 additions & 1 deletion docs/functions.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ Functions marked `immutable` or `stable` are available on the query type. Functi
## Supported Return Types


Built-in GraphQL scalar types `Int`, `Float`, `String`, `Boolean` and [custom scalar types](api.md#custom-scalars) are supported as function arguments and return types. Function types returning a table or view are supported as well:
Built-in GraphQL scalar types `Int`, `Float`, `String`, `Boolean` and [custom scalar types](api.md#custom-scalars) are supported as function arguments and return types. Function types returning a table or view are supported as well. Such functions implement the [Node interface](api.md#node):

=== "Function"

Expand Down Expand Up @@ -125,6 +125,7 @@ Built-in GraphQL scalar types `Int`, `Float`, `String`, `Boolean` and [custom sc
accountById(accountId: 1) {
id
email
nodeId
}
}
```
Expand All @@ -137,11 +138,57 @@ Built-in GraphQL scalar types `Int`, `Float`, `String`, `Boolean` and [custom sc
"accountById": {
"id": 1,
"email": "[email protected]"
"nodeId": "WyJwdWJsaWMiLCAiYWNjb3VudCIsIDFd"
}
}
}
```

If all the columns of a row are null, the result can be a little surprising. Instead of an object with all columns null, the top-level field is null:
imor marked this conversation as resolved.
Show resolved Hide resolved

=== "Function"

```sql
create table account(
id int,
email varchar(255),
name text null
);

insert into account(id, email, name)
values
(1, '[email protected]', 'aardvark'),
(2, '[email protected]', null),
(null, null, null);

create function returns_account_with_all_null_columns()
returns account language sql stable
as $$ select id, email, name from account where id is null; $$;
```

=== "Query"

```graphql
query {
returnsAccountWithAllNullColumns {
id
email
name
__typename
}
}
```

=== "Response"

```json
{
"data": {
"returnsAccountWithAllNullColumns": null
}
}
```

Functions returning multiple rows of a table or view are exposed as [collections](api.md#collections).

=== "Function"
Expand Down
2 changes: 1 addition & 1 deletion src/sql_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,7 @@ impl Table {
pub fn primary_key_columns(&self) -> Vec<&Arc<Column>> {
self.primary_key()
.map(|x| x.column_names)
.unwrap_or(vec![])
.unwrap_or_default()
.iter()
.map(|col_name| {
self.columns
Expand Down
2 changes: 1 addition & 1 deletion src/transpile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@ impl FunctionCallBuilder {
} else {
select_clause
};
format!("select coalesce((select {select_clause} from {func_schema}.{func_name}{args_clause} {block_name} where {block_name} is not null), null::jsonb);")
format!("select coalesce((select {select_clause} from {func_schema}.{func_name}{args_clause} {block_name} where not ({block_name} is null)), null::jsonb);")
}
FuncCallReturnTypeBuilder::Connection(connection_builder) => {
let from_clause = format!("{func_schema}.{func_name}{args_clause}");
Expand Down
114 changes: 114 additions & 0 deletions test/expected/function_calls.out
Original file line number Diff line number Diff line change
Expand Up @@ -2195,4 +2195,118 @@ begin;
}
(1 row)

rollback to savepoint a;
create table account(
id int,
email varchar(255),
name text null
);
create function returns_all_columns_non_null_account()
returns account language sql stable
as $$ select id, email, name from account where id = 1; $$;
create function returns_one_column_null_account()
returns account language sql stable
as $$ select id, email, name from account where id = 2; $$;
create function returns_all_columns_null_account()
returns account language sql stable
as $$ select id, email, name from account where id is null; $$;
create function returns_null_account()
returns account language sql stable
as $$ select id, email, name from account where id = 9; $$;
insert into account(id, email, name)
values
(1, '[email protected]', 'aardvark'),--all columns non-null
(2, '[email protected]', null),--mixed: some null, some non-null
(null, null, null);--all columns null
-- comment on table account is e'@graphql({"totalCount": {"enabled": true}})';
select jsonb_pretty(graphql.resolve($$
query {
returnsAllColumnsNonNullAccount {
id
email
name
__typename
}
}
$$));
jsonb_pretty
----------------------------------------------
{ +
"data": { +
"returnsAllColumnsNonNullAccount": {+
"id": 1, +
"name": "aardvark", +
"email": "[email protected]", +
"__typename": "Account" +
} +
} +
}
(1 row)

select jsonb_pretty(graphql.resolve($$
query {
returnsOneColumnNullAccount {
id
email
name
__typename
}
}
$$));
jsonb_pretty
------------------------------------------
{ +
"data": { +
"returnsOneColumnNullAccount": {+
"id": 2, +
"name": null, +
"email": "[email protected]", +
"__typename": "Account" +
} +
} +
}
(1 row)

-- With current implementation we can't distinguish between
-- when all columns of a composite type are null and when
-- the composite type itself is null. In both these cases
-- the result will be null for the top-level field.
select jsonb_pretty(graphql.resolve($$
query {
returnsAllColumnsNullAccount {
id
email
name
__typename
}
}
$$));
jsonb_pretty
----------------------------------------------
{ +
"data": { +
"returnsAllColumnsNullAccount": null+
} +
}
(1 row)

select jsonb_pretty(graphql.resolve($$
query {
returnsNullAccount {
id
email
name
__typename
}
}
$$));
jsonb_pretty
------------------------------------
{ +
"data": { +
"returnsNullAccount": null+
} +
}
(1 row)

rollback;
81 changes: 81 additions & 0 deletions test/sql/function_calls.sql
Original file line number Diff line number Diff line change
Expand Up @@ -782,4 +782,85 @@ begin;
returnsEventTrigger
}
$$));

rollback to savepoint a;

create table account(
id int,
email varchar(255),
name text null
);

create function returns_all_columns_non_null_account()
returns account language sql stable
as $$ select id, email, name from account where id = 1; $$;

create function returns_one_column_null_account()
returns account language sql stable
as $$ select id, email, name from account where id = 2; $$;

create function returns_all_columns_null_account()
returns account language sql stable
as $$ select id, email, name from account where id is null; $$;

create function returns_null_account()
returns account language sql stable
as $$ select id, email, name from account where id = 9; $$;

insert into account(id, email, name)
values
(1, '[email protected]', 'aardvark'),--all columns non-null
(2, '[email protected]', null),--mixed: some null, some non-null
(null, null, null);--all columns null

-- comment on table account is e'@graphql({"totalCount": {"enabled": true}})';

select jsonb_pretty(graphql.resolve($$
query {
returnsAllColumnsNonNullAccount {
id
email
name
__typename
}
}
$$));

select jsonb_pretty(graphql.resolve($$
query {
returnsOneColumnNullAccount {
id
email
name
__typename
}
}
$$));

-- With current implementation we can't distinguish between
-- when all columns of a composite type are null and when
-- the composite type itself is null. In both these cases
-- the result will be null for the top-level field.
select jsonb_pretty(graphql.resolve($$
query {
returnsAllColumnsNullAccount {
id
email
name
__typename
}
}
$$));

select jsonb_pretty(graphql.resolve($$
query {
returnsNullAccount {
id
email
name
__typename
}
}
$$));

rollback;
Loading