Merge pull request #54 from hasura/djh/return-empty-array-when-no-res…

…ults

Return empty array when no rows results

crates/ndc-sqlserver/tests/query_tests.rs

@@ -36,14 +36,12 @@ mod predicates {
         insta::assert_json_snapshot!(result);
     }
 
-    /*
-    // this fails because empty responses don't return `{ rows: [] }` and instead return `{}`
     #[tokio::test]
     async fn select_where_album_id_less_than() {
         let result = run_query("select_where_album_id_less_than").await;
         insta::assert_json_snapshot!(result);
     }
-    */
+
     #[tokio::test]
     async fn select_where_album_id_less_than_or_equal_to() {
         let result = run_query("select_where_album_id_less_than_or_equal_to").await;
@@ -86,14 +84,12 @@ mod predicates {
         insta::assert_json_snapshot!(result);
     }
 
-    // need to run query for each set of variables
     #[tokio::test]
     async fn select_where_variable() {
         let result = run_query("select_where_variable").await;
         insta::assert_json_snapshot!(result);
     }
 
-    // need to run query for each set of variables
     #[tokio::test]
     async fn select_where_variable_int() {
         let result = run_query("select_where_variable_int").await;

crates/query-engine/src/phases/translation/sql/ast.rs

@@ -200,7 +200,7 @@ pub enum BinaryArrayOperator {
 
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum Function {
-    Coalesce,
+    IsNull,
     JsonAgg,
     Unknown(String),
 }

crates/query-engine/src/phases/translation/sql/convert.rs

@@ -363,8 +363,8 @@ impl BinaryArrayOperator {
 impl Function {
     pub fn to_sql(&self, sql: &mut SQL) {
         match self {
-            Function::Coalesce => sql.append_syntax("coalesce"),
-            Function::JsonAgg => sql.append_syntax("json_agg"),
+            Function::JsonAgg => sql.append_syntax("JSON_AGG"),
+            Function::IsNull => sql.append_syntax("ISNULL"),
             Function::Unknown(name) => sql.append_syntax(name),
         }
     }
@@ -386,7 +386,7 @@ impl CountType {
 impl Value {
     pub fn to_sql(&self, sql: &mut SQL) {
         match &self {
-            Value::EmptyJsonArray => sql.append_syntax("JSON_VALUE('[]','$')"),
+            Value::EmptyJsonArray => sql.append_syntax("'[]'"),
             Value::Int4(i) => sql.append_syntax(format!("{}", i).as_str()),
             Value::String(s) => sql.append_param(Param::String(s.clone())),
             Value::Variable(v) => sql.append_param(Param::Variable(v.clone())),

crates/query-engine/src/phases/translation/sql/helpers.rs

@@ -141,8 +141,8 @@ pub fn star_select(from: From) -> Select {
 
 /// given a set of rows and aggregate queries, combine them into
 /// one Select
-/// SELECT JSON_VALUE([aggregates].[json], "$.aggregates") as [aggregates],
-///  JSON_QUERY([rows].[json], "$.json") AS [rows]
+/// SELECT JSON_VALUE([aggregates].[aggregates_json], "$.aggregates_json") as [aggregates],
+///  JSON_QUERY(isnull([rows].[row_json],'[]'), "$.row_json") AS [rows]
 /// FROM (
 ///     SELECT *
 ///     FROM (
@@ -177,10 +177,16 @@ pub fn select_rowset(
         SelectSet::Rows(row_select) => {
             let rows_row = vec![(
                 make_column_alias("rows".to_string()),
-                Expression::ColumnName(ColumnName::AliasedColumn {
-                    name: row_column_alias.clone(),
-                    table: TableName::AliasedTable(row_table_alias.clone()),
-                }),
+                Expression::FunctionCall {
+                    function: Function::IsNull,
+                    args: vec![
+                        Expression::ColumnName(ColumnName::AliasedColumn {
+                            name: row_column_alias.clone(),
+                            table: TableName::AliasedTable(row_table_alias.clone()),
+                        }),
+                        Expression::Value(Value::EmptyJsonArray),
+                    ],
+                },
             )];
 
             let mut final_row_select = simple_select(rows_row);
@@ -227,10 +233,16 @@ pub fn select_rowset(
                 (
                     make_column_alias("rows".to_string()),
                     Expression::JsonQuery(
-                        Box::new(Expression::ColumnName(ColumnName::AliasedColumn {
-                            name: row_column_alias.clone(),
-                            table: TableName::AliasedTable(row_table_alias.clone()),
-                        })),
+                        Box::new(Expression::FunctionCall {
+                            function: Function::IsNull,
+                            args: vec![
+                                Expression::ColumnName(ColumnName::AliasedColumn {
+                                    name: row_column_alias.clone(),
+                                    table: TableName::AliasedTable(row_table_alias.clone()),
+                                }),
+                                Expression::Value(Value::EmptyJsonArray),
+                            ],
+                        }),
                         JsonPath {
                             elements: vec![row_column_alias.clone()],
                         },

crates/query-engine/tests/snapshots/tests__dup_array_relationship.snap

@@ -3,7 +3,7 @@ source: crates/query-engine/tests/tests.rs
 expression: result
 ---
 SELECT
-  [rows].[row_json] AS [rows]
+  ISNULL([rows].[row_json], '[]') AS [rows]
 FROM
   (
     SELECT
@@ -13,7 +13,7 @@ FROM
       [public].[Artist] AS [Artist]
       OUTER APPLY (
         SELECT
-          [rows].[row_json] AS [rows]
+          ISNULL([rows].[row_json], '[]') AS [rows]
         FROM
           (
             SELECT
@@ -32,7 +32,7 @@ FROM
       ) AS [albums]([json])
       OUTER APPLY (
         SELECT
-          [rows].[row_json] AS [rows]
+          ISNULL([rows].[row_json], '[]') AS [rows]
         FROM
           (
             SELECT

crates/query-engine/tests/snapshots/tests__it_aggregate_count_albums.snap

@@ -3,7 +3,7 @@ source: crates/query-engine/tests/tests.rs
 expression: result
 ---
 SELECT
-  JSON_QUERY([rows].[row_json], '$.row_json') AS [rows],
+  JSON_QUERY(ISNULL([rows].[row_json], '[]'), '$.row_json') AS [rows],
   JSON_QUERY(
     JSON_VALUE([aggregates].[agg_json], '$.agg_json'),
     '$'

crates/query-engine/tests/snapshots/tests__it_converts_select_with_limit.snap

@@ -3,7 +3,7 @@ source: crates/query-engine/tests/tests.rs
 expression: result
 ---
 SELECT
-  [rows].[row_json] AS [rows]
+  ISNULL([rows].[row_json], '[]') AS [rows]
 FROM
   (
     SELECT

crates/query-engine/tests/snapshots/tests__it_select_where_not_null.snap

@@ -3,7 +3,7 @@ source: crates/query-engine/tests/tests.rs
 expression: result
 ---
 SELECT
-  [rows].[row_json] AS [rows]
+  ISNULL([rows].[row_json], '[]') AS [rows]
 FROM
   (
     SELECT

crates/query-engine/tests/snapshots/tests__it_select_where_related_exists.snap

@@ -3,7 +3,7 @@ source: crates/query-engine/tests/tests.rs
 expression: result
 ---
 SELECT
-  [rows].[row_json] AS [rows]
+  ISNULL([rows].[row_json], '[]') AS [rows]
 FROM
   (
     SELECT
@@ -13,7 +13,7 @@ FROM
       [public].[Artist] AS [artist]
       OUTER APPLY (
         SELECT
-          [rows].[row_json] AS [rows]
+          ISNULL([rows].[row_json], '[]') AS [rows]
         FROM
           (
             SELECT

crates/query-engine/tests/snapshots/tests__it_select_where_string.snap

@@ -3,7 +3,7 @@ source: crates/query-engine/tests/tests.rs
 expression: result
 ---
 SELECT
-  [rows].[row_json] AS [rows]
+  ISNULL([rows].[row_json], '[]') AS [rows]
 FROM
   (
     SELECT

crates/query-engine/tests/snapshots/tests__it_select_where_unrelated_exists.snap

@@ -3,7 +3,7 @@ source: crates/query-engine/tests/tests.rs
 expression: result
 ---
 SELECT
-  [rows].[row_json] AS [rows]
+  ISNULL([rows].[row_json], '[]') AS [rows]
 FROM
   (
     SELECT

crates/query-engine/tests/snapshots/tests__it_simple_array_relationship.snap

@@ -3,7 +3,7 @@ source: crates/query-engine/tests/tests.rs
 expression: result
 ---
 SELECT
-  [rows].[row_json] AS [rows]
+  ISNULL([rows].[row_json], '[]') AS [rows]
 FROM
   (
     SELECT
@@ -12,7 +12,7 @@ FROM
       [public].[Artist] AS [Artist]
       OUTER APPLY (
         SELECT
-          [rows].[row_json] AS [rows]
+          ISNULL([rows].[row_json], '[]') AS [rows]
         FROM
           (
             SELECT

crates/query-engine/tests/snapshots/tests__it_simple_object_relationship.snap

@@ -3,7 +3,7 @@ source: crates/query-engine/tests/tests.rs
 expression: result
 ---
 SELECT
-  [rows].[row_json] AS [rows]
+  ISNULL([rows].[row_json], '[]') AS [rows]
 FROM
   (
     SELECT
@@ -12,7 +12,7 @@ FROM
       [public].[Album] AS [Album]
       OUTER APPLY (
         SELECT
-          [rows].[row_json] AS [rows]
+          ISNULL([rows].[row_json], '[]') AS [rows]
         FROM
           (
             SELECT

crates/query-engine/tests/snapshots/tests__nested_aggregates.snap

@@ -3,7 +3,7 @@ source: crates/query-engine/tests/tests.rs
 expression: result
 ---
 SELECT
-  [rows].[row_json] AS [rows]
+  ISNULL([rows].[row_json], '[]') AS [rows]
 FROM
   (
     SELECT
@@ -13,7 +13,7 @@ FROM
       [public].[Artist] AS [Artist]
       OUTER APPLY (
         SELECT
-          JSON_QUERY([rows].[row_json], '$.row_json') AS [rows],
+          JSON_QUERY(ISNULL([rows].[row_json], '[]'), '$.row_json') AS [rows],
           JSON_QUERY(
             JSON_VALUE([aggregates].[agg_json], '$.agg_json'),
             '$'

crates/query-engine/tests/snapshots/tests__nested_array_relationships.snap

@@ -3,7 +3,7 @@ source: crates/query-engine/tests/tests.rs
 expression: result
 ---
 SELECT
-  [rows].[row_json] AS [rows]
+  ISNULL([rows].[row_json], '[]') AS [rows]
 FROM
   (
     SELECT
@@ -12,7 +12,7 @@ FROM
       [public].[Artist] AS [Artist]
       OUTER APPLY (
         SELECT
-          [rows].[row_json] AS [rows]
+          ISNULL([rows].[row_json], '[]') AS [rows]
         FROM
           (
             SELECT
@@ -21,7 +21,7 @@ FROM
               [public].[Album] AS [Album]
               OUTER APPLY (
                 SELECT
-                  [rows].[row_json] AS [rows]
+                  ISNULL([rows].[row_json], '[]') AS [rows]
                 FROM
                   (
                     SELECT

crates/query-engine/tests/snapshots/tests__sorting_by_nested_relationship_column.snap

@@ -3,7 +3,7 @@ source: crates/query-engine/tests/tests.rs
 expression: result
 ---
 SELECT
-  [rows].[row_json] AS [rows]
+  ISNULL([rows].[row_json], '[]') AS [rows]
 FROM
   (
     SELECT

crates/query-engine/tests/snapshots/tests__sorting_by_relationship_column.snap

@@ -3,7 +3,7 @@ source: crates/query-engine/tests/tests.rs
 expression: result
 ---
 SELECT
-  [rows].[row_json] AS [rows]
+  ISNULL([rows].[row_json], '[]') AS [rows]
 FROM
   (
     SELECT

crates/query-engine/tests/snapshots/tests__sorting_by_relationship_count.snap

@@ -3,7 +3,7 @@ source: crates/query-engine/tests/tests.rs
 expression: result
 ---
 SELECT
-  [rows].[row_json] AS [rows]
+  ISNULL([rows].[row_json], '[]') AS [rows]
 FROM
   (
     SELECT

justfile

@@ -146,15 +146,14 @@ run-engine: start-dependencies
 
 # pasting multiline SQL into `sqlcmd` is a bad time, so here is a script to
 # smash a file in for rapid fire application development business value
-run-temp-sql: start-dependencies
+run-temp-sql:
+  docker compose up --wait sqlserver
   sqlcmd -S localhost,64003 -U SA -P "Password!" -d "Chinook" -i "./temp.sql"
 
 ## repl-sqlserver: start a sqlserver docker image and connect to it using sqlcmd
 repl-sqlserver:
   #!/usr/bin/env bash
-  docker compose up -d sqlserver
-  # need a proper health check here instead ... but yolo
-  sleep 2
+  docker compose up -wait sqlserver
   sqlcmd -S localhost,64003 -U SA -P "Password!" -d "Chinook"
 
 # run `clippy` linter

readme.md

@@ -21,7 +21,6 @@ Things we definitely don't have:
 - database introspection
 - explain queries
 - reinstate benchmarks
-- CI job for testing config server
 
 The best view of progress is probably `/crates/ndc-sqlserver/tests/`, and look
 at which tests are still commented out. If you'd to contribute, a very good
@@ -33,16 +32,16 @@ start would be to uncomment one and try to fix any query errors.
 
 1. Install [rustup](https://www.rust-lang.org/tools/install).
 2. Install additional tools:
-    - `cargo install cargo-watch cargo-insta`
-    - `rustup component add rust-analyzer`
-    - `rustup component add clippy`
-    - `rustup component add rustfmt`
+   - `cargo install cargo-watch cargo-insta`
+   - `rustup component add rust-analyzer`
+   - `rustup component add clippy`
+   - `rustup component add rustfmt`
 3. Install [just](https://github.com/casey/just)
 4. Install [Docker](https://www.docker.com/)
 5. Install protoc. Here are a few options:
-    - `brew install protobuf`
-    - `apt-get install protobuf-compiler`
-    - `dnf install protobuf-compiler`
+   - `brew install protobuf`
+   - `apt-get install protobuf-compiler`
+   - `dnf install protobuf-compiler`
 6. Clone [v3-engine](https://github.com/hasura/v3-engine) in a directory near this one:
    ```
    (cd .. && git clone [email protected]:hasura/v3-engine.git)
@@ -69,8 +68,8 @@ just run
    ```
    curl -H "Content-Type: application/json" \
      --data "@crates/ndc-sqlserver/tests/goldenfiles/select_where_variable.json" \
-	 http://localhost:8100/query \
-	 | jq
+    http://localhost:8100/query \
+    | jq
    ```
 
 Among the docker containers is a Jaeger instance for tracing/debugging, accessible at http://127.0.0.1:4002.
@@ -106,6 +105,7 @@ See [architecture.md](./architecture.md).
      }
    }
    ```
+
    (or `just test-integrated`)
 
 ## Write a database execution test