Skip to content

Commit

Permalink
Fields of composite types are always nullable (#565)
Browse files Browse the repository at this point in the history
### What

We used to mark fields of composite type as not-nullable in the NDC
schema.

This is wrong. Nullability is a property of columns of tables, not of
fields of record types.

This is also demonstrated by the following transcript:
```
postgres=# create table a_table (nullable_text text, not_nullable_text text not null);
CREATE TABLE
postgres=# create table derived_table (a_table a_table, other text);
CREATE TABLE

postgres=# insert into derived_table values (('nullable', 'not nullable'), 'other text');
INSERT 0 1
postgres=# insert into derived_table values ((null, 'not nullable'), 'other text');
INSERT 0 1

postgres=# select (a_table).* from derived_table;
 nullable_text | not_nullable_text
---------------+-------------------
 nullable      | not nullable
               | not nullable
(2 rows)

postgres=# insert into a_table select (a_table).* from derived_table;
INSERT 0 2
postgres=# select * from a_table;
 nullable_text | not_nullable_text
---------------+-------------------
 nullable      | not nullable
               | not nullable
(2 rows)

-- We can easily construct a record with (not_nullable_text=null) when on **the composite type** a_table:
postgres=# insert into derived_table values (('nullable', null), 'other text');
INSERT 0 1
postgres=# select * from derived_table;
          a_table          |   other
---------------------------+------------
 (nullable,"not nullable") | other text
 (,"not nullable")         | other text
 (nullable,)               | other text
(3 rows)

postgres=# select (a_table).* from derived_table;
 nullable_text | not_nullable_text
---------------+-------------------
 nullable      | not nullable
               | not nullable
 nullable      |
(3 rows)

-- ... But we cannot insert this into **the table** a_table.
postgres=# insert into a_table select (a_table).* from derived_table;
ERROR:  null value in column "not_nullable_text" of relation "a_table" violates not-null constraint
DETAIL:  Failing row contains (nullable, null).
```

### How

We simply make the schema endpoint always return nullable fields of
composite types.

---------

Co-authored-by: Samir Talwar <[email protected]>
  • Loading branch information
plcplc and SamirTalwar authored Aug 12, 2024
1 parent 346d2db commit 1378805
Show file tree
Hide file tree
Showing 4 changed files with 226 additions and 99 deletions.
2 changes: 2 additions & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@

### Fixed

- Make fields of composite types nullable in the schema.
[#565](https://github.com/hasura/ndc-postgres/pull/565)
- Allow Native Operations that end with a semicolon when it's easy to remove them.
[#566](https://github.com/hasura/ndc-postgres/pull/566)
- Fix nested field relationships.
Expand Down
4 changes: 3 additions & 1 deletion crates/connectors/ndc-postgres/src/schema/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,9 @@ pub fn get_schema(
field_name.clone(),
models::ObjectField {
description: field_info.description.clone(),
r#type: type_to_type(&field_info.r#type),
r#type: models::Type::Nullable {
underlying_type: Box::new(type_to_type(&field_info.r#type)),
},
arguments: BTreeMap::new(),
},
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2965,14 +2965,20 @@ expression: result
"fields": {
"name": {
"type": {
"type": "named",
"name": "text"
"type": "nullable",
"underlying_type": {
"type": "named",
"name": "text"
}
}
},
"popularity": {
"type": {
"type": "named",
"name": "int8"
"type": "nullable",
"underlying_type": {
"type": "named",
"name": "int8"
}
}
}
}
Expand All @@ -2981,17 +2987,23 @@ expression: result
"fields": {
"members": {
"type": {
"type": "array",
"element_type": {
"type": "named",
"name": "chara"
"type": "nullable",
"underlying_type": {
"type": "array",
"element_type": {
"type": "named",
"name": "chara"
}
}
}
},
"name": {
"type": {
"type": "named",
"name": "text"
"type": "nullable",
"underlying_type": {
"type": "named",
"name": "text"
}
}
}
}
Expand All @@ -3000,17 +3012,23 @@ expression: result
"fields": {
"members": {
"type": {
"type": "array",
"element_type": {
"type": "named",
"name": "person_name"
"type": "nullable",
"underlying_type": {
"type": "array",
"element_type": {
"type": "named",
"name": "person_name"
}
}
}
},
"name": {
"type": {
"type": "named",
"name": "text"
"type": "nullable",
"underlying_type": {
"type": "named",
"name": "text"
}
}
}
}
Expand Down Expand Up @@ -3093,8 +3111,11 @@ expression: result
"fields": {
"only_occurring_here1": {
"type": {
"type": "named",
"name": "int8"
"type": "nullable",
"underlying_type": {
"type": "named",
"name": "int8"
}
}
}
}
Expand Down Expand Up @@ -3268,17 +3289,23 @@ expression: result
"fields": {
"committees": {
"type": {
"type": "array",
"element_type": {
"type": "named",
"name": "committee"
"type": "nullable",
"underlying_type": {
"type": "array",
"element_type": {
"type": "named",
"name": "committee"
}
}
}
},
"name": {
"type": {
"type": "named",
"name": "text"
"type": "nullable",
"underlying_type": {
"type": "named",
"name": "text"
}
}
}
}
Expand All @@ -3301,14 +3328,20 @@ expression: result
"fields": {
"address": {
"type": {
"type": "named",
"name": "person_address"
"type": "nullable",
"underlying_type": {
"type": "named",
"name": "person_address"
}
}
},
"name": {
"type": {
"type": "named",
"name": "person_name"
"type": "nullable",
"underlying_type": {
"type": "named",
"name": "person_name"
}
}
}
}
Expand All @@ -3319,15 +3352,21 @@ expression: result
"address_line_1": {
"description": "Address line No 1",
"type": {
"type": "named",
"name": "text"
"type": "nullable",
"underlying_type": {
"type": "named",
"name": "text"
}
}
},
"address_line_2": {
"description": "Address line No 2",
"type": {
"type": "named",
"name": "text"
"type": "nullable",
"underlying_type": {
"type": "named",
"name": "text"
}
}
}
}
Expand All @@ -3338,15 +3377,21 @@ expression: result
"first_name": {
"description": "The first name of a person",
"type": {
"type": "named",
"name": "text"
"type": "nullable",
"underlying_type": {
"type": "named",
"name": "text"
}
}
},
"last_name": {
"description": "The last name of a person",
"type": {
"type": "named",
"name": "text"
"type": "nullable",
"underlying_type": {
"type": "named",
"name": "text"
}
}
}
}
Expand Down
Loading

0 comments on commit 1378805

Please sign in to comment.