From e748d3de30178c71bd7b5ac26dbfd6b81ce2d0bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Og=C3=B3rek?= Date: Fri, 20 Oct 2023 13:56:36 +0200 Subject: [PATCH 1/2] ref: Make jsonb type work with any valid json and add more tests to cover it --- wrappers/dockerfiles/airtable/server.py | 12 ++- wrappers/src/fdw/airtable_fdw/result.rs | 9 +- wrappers/src/fdw/airtable_fdw/tests.rs | 136 ++++++++++++++++++------ 3 files changed, 115 insertions(+), 42 deletions(-) diff --git a/wrappers/dockerfiles/airtable/server.py b/wrappers/dockerfiles/airtable/server.py index 9b2c700e..c85bc6b8 100644 --- a/wrappers/dockerfiles/airtable/server.py +++ b/wrappers/dockerfiles/airtable/server.py @@ -41,21 +41,29 @@ def do_GET(self): client.create( test_table, { + "bool_field": True, "numeric_field": 1, "string_field": "two", "timestamp_field": "2023-07-19T06:39:15.000Z", - "strings_array_field": ["foo", "bar"], "object_field": {"foo": "bar"}, + "strings_array_field": ["foo", "bar"], + "numerics_array_field": [1, 2], + "bools_array_field": [False], + "objects_array_field": [{"foo": "bar"}, {"foo": "baz"}] }, ) client.create( test_table, { + "bool_field": False, "numeric_field": 2, "string_field": "three", "timestamp_field": "2023-07-20T06:39:15.000Z", - "strings_array_field": ["baz", "qux"], "object_field": {"foo": "baz"}, + "strings_array_field": ["baz", "qux"], + "numerics_array_field": [3, 4], + "bools_array_field": [True, False, True], + "objects_array_field": [{"foo": "qux"}] }, ) diff --git a/wrappers/src/fdw/airtable_fdw/result.rs b/wrappers/src/fdw/airtable_fdw/result.rs index 4d76f38b..15364aa2 100644 --- a/wrappers/src/fdw/airtable_fdw/result.rs +++ b/wrappers/src/fdw/airtable_fdw/result.rs @@ -213,15 +213,10 @@ impl AirtableRecord { } }, ), + // TODO: Think about adding support for BOOLARRAYOID, NUMERICARRAYOID, TEXTARRAYOID and rest of array types. pg_sys::JSONBOID => self.fields.0.get(&col.name).map_or_else( || Ok(None), - |val| { - if val.is_array() || val.is_object() { - Ok(Some(Cell::Json(pgrx::JsonB(val.clone())))) - } else { - Err(()) - } - }, + |val| Ok(Some(Cell::Json(pgrx::JsonB(val.clone())))), ), _ => { return Err(AirtableFdwError::UnsupportedColumnType(col.name.clone())); diff --git a/wrappers/src/fdw/airtable_fdw/tests.rs b/wrappers/src/fdw/airtable_fdw/tests.rs index 7095d7d5..800bf9ac 100644 --- a/wrappers/src/fdw/airtable_fdw/tests.rs +++ b/wrappers/src/fdw/airtable_fdw/tests.rs @@ -1,7 +1,7 @@ #[cfg(any(test, feature = "pg_test"))] #[pgrx::pg_schema] mod tests { - use pgrx::{prelude::*, JsonB}; + use pgrx::prelude::*; #[pg_test] fn airtable_smoketest() { @@ -27,11 +27,15 @@ mod tests { c.update( r#" CREATE FOREIGN TABLE airtable_table ( + bool_field bool, numeric_field numeric, string_field text, timestamp_field timestamp, + object_field jsonb, strings_array_field jsonb, - object_field jsonb + numerics_array_field jsonb, + bools_array_field jsonb, + objects_array_field jsonb ) SERVER airtable_server OPTIONS ( @@ -46,11 +50,7 @@ mod tests { c.update( r#" CREATE FOREIGN TABLE airtable_view ( - numeric_field numeric, - string_field text, - timestamp_field timestamp, - strings_array_field jsonb, - object_field jsonb + string_field text ) SERVER airtable_server OPTIONS ( @@ -64,60 +64,130 @@ mod tests { ) .unwrap(); + #[derive(serde::Deserialize)] + struct Foo { + foo: String, + } + /* The table data below comes from the code in wrappers/dockerfiles/airtable/server.py */ let results = c .select( - "SELECT string_field FROM airtable_table WHERE numeric_field = 1", + "SELECT bool_field FROM airtable_table WHERE bool_field = False", None, None, ) - .unwrap() - .filter_map(|r| r.get_by_name::<&str, _>("string_field").unwrap()) + .expect("No results for a given query") + .filter_map(|r| { + r.get_by_name::("bool_field") + .expect("bool_field is missing") + }) .collect::>(); - assert_eq!(results, vec!["two"]); + assert_eq!(results, vec![false]); let results = c - .select( - "SELECT strings_array_field FROM airtable_table WHERE numeric_field = 1", - None, - None, - ) - .unwrap() + .select("SELECT string_field FROM airtable_table", None, None) + .expect("No results for a given query") .filter_map(|r| { - r.get_by_name::("strings_array_field") - .expect("strings_array_field is missing") - .and_then(|v| serde_json::from_value::>(v.0.to_owned()).ok()) + r.get_by_name::<&str, _>("string_field") + .expect("string_field is missing") }) .collect::>(); + assert_eq!(results, vec!["two", "three"]); - assert_eq!(results, vec![vec!["foo", "bar"]]); + let results = c + .select("SELECT numeric_field FROM airtable_table", None, None) + .expect("No results for a given query") + .filter_map(|r| { + r.get_by_name::("numeric_field") + .expect("numeric_field is missing") + }) + .collect::>(); + assert_eq!( + results, + vec![pgrx::AnyNumeric::from(1), pgrx::AnyNumeric::from(2)] + ); - #[derive(serde::Deserialize)] - struct Foo { - foo: String, - } + let results = c + .select("SELECT timestamp_field FROM airtable_table", None, None) + .expect("No results for a given query") + .filter_map(|r| { + r.get_by_name::("timestamp_field") + .expect("timestamp_field is missing") + .map(|v| v.to_iso_string()) + }) + .collect::>(); + assert_eq!(results, vec!["2023-07-19T06:39:15", "2023-07-20T06:39:15"]); + + let results = c + .select("SELECT object_field FROM airtable_table", None, None) + .expect("No results for a given query") + .filter_map(|r| { + r.get_by_name::("object_field") + .expect("object_field is missing") + .and_then(|v| serde_json::from_value::(v.0.to_owned()).ok()) + .map(|v| v.foo) + }) + .collect::>(); + assert_eq!(results, vec!["bar", "baz"]); + + let results = c + .select("SELECT strings_array_field FROM airtable_table", None, None) + .expect("No results for a given query") + .filter_map(|r| { + r.get_by_name::("strings_array_field") + .expect("strings_array_field is missing") + .and_then(|v| serde_json::from_value::>(v.0.to_owned()).ok()) + }) + .collect::>(); + assert_eq!(results, vec![vec!["foo", "bar"], vec!["baz", "qux"]]); let results = c .select( - "SELECT object_field FROM airtable_table WHERE numeric_field = 1", + "SELECT numerics_array_field FROM airtable_table", None, None, ) - .unwrap() + .expect("No results for a given query") .filter_map(|r| { - r.get_by_name::("object_field") - .expect("object_field is missing") - .and_then(|v| serde_json::from_value::(v.0.to_owned()).ok()) + r.get_by_name::("numerics_array_field") + .expect("numerics_array_field is missing") + .and_then(|v| serde_json::from_value::>(v.0.to_owned()).ok()) + }) + .collect::>(); + assert_eq!(results, vec![vec![1, 2], vec![3, 4]]); + + let results = c + .select("SELECT bools_array_field FROM airtable_table", None, None) + .expect("No results for a given query") + .filter_map(|r| { + r.get_by_name::("bools_array_field") + .expect("bools_array_field is missing") + .and_then(|v| serde_json::from_value::>(v.0.to_owned()).ok()) }) .collect::>(); - assert_eq!(results[0].foo, "bar"); + assert_eq!(results, vec![vec![false], vec![true, false, true]]); + + let results = c + .select("SELECT objects_array_field FROM airtable_table", None, None) + .expect("No results for a given query") + .filter_map(|r| { + r.get_by_name::("objects_array_field") + .expect("objects_array_field is missing") + .and_then(|v| serde_json::from_value::>(v.0.to_owned()).ok()) + .map(|v| v.into_iter().map(|f| f.foo).collect::>()) + }) + .collect::>(); + assert_eq!(results, vec![vec!["bar", "baz"], vec!["qux"]]); let results = c .select("SELECT string_field FROM airtable_view", None, None) - .unwrap() - .filter_map(|r| r.get_by_name::<&str, _>("string_field").unwrap()) + .expect("No results for a given query") + .filter_map(|r| { + r.get_by_name::<&str, _>("string_field") + .expect("string_field is missing") + }) .collect::>(); assert_eq!(results, vec!["three"]); }); From 8390488e90edefdca91d859b0a1996b02692cddd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Og=C3=B3rek?= Date: Fri, 20 Oct 2023 14:02:45 +0200 Subject: [PATCH 2/2] chore: bump version to 0.1.18 --- .github/workflows/release.yml | 1 + wrappers/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 95de3fc8..7c08bccf 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -101,6 +101,7 @@ jobs: cp ${extension_dir}/${{ matrix.extension_name }}--${deb_version}.sql ${extension_dir}/${{ matrix.extension_name }}--0.1.14--${deb_version}.sql cp ${extension_dir}/${{ matrix.extension_name }}--${deb_version}.sql ${extension_dir}/${{ matrix.extension_name }}--0.1.15--${deb_version}.sql cp ${extension_dir}/${{ matrix.extension_name }}--${deb_version}.sql ${extension_dir}/${{ matrix.extension_name }}--0.1.16--${deb_version}.sql + cp ${extension_dir}/${{ matrix.extension_name }}--${deb_version}.sql ${extension_dir}/${{ matrix.extension_name }}--0.1.17--${deb_version}.sql # Create installable package mkdir archive diff --git a/wrappers/Cargo.toml b/wrappers/Cargo.toml index de77af0b..db65f10c 100644 --- a/wrappers/Cargo.toml +++ b/wrappers/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "wrappers" -version = "0.1.17" +version = "0.1.18" edition = "2021" [lib]