Merge pull request #20 from hasura:nested_field_selector

Nested_field_selector

.github/workflows/build_and_test.yaml

@@ -0,0 +1,34 @@
+name: "Test Suite"
+on:
+  pull_request:
+
+jobs:
+  test:
+    name: cargo test
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions-rust-lang/setup-rust-toolchain@v1
+        with:
+          # this defaults to "-D warnings", making warnings fail the entire build.
+          # setting to empty strng to allow builds with warnings
+          # todo: consider removing this, and disallowing pushing with warnings?
+          rustflags: ""
+      - run: cargo test --all-features
+
+  # Check formatting with rustfmt
+  formatting:
+    name: cargo fmt
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      # Ensure rustfmt is installed and setup problem matcher
+      - uses: actions-rust-lang/setup-rust-toolchain@v1
+        with:
+          components: rustfmt
+          # this defaults to "-D warnings", making warnings fail the entire build.
+          # setting to empty strng to allow builds with warnings
+          # todo: consider removing this, and disallowing pushing with warnings?
+          rustflags: ""
+      - name: Rustfmt Check
+        uses: actions-rust-lang/rustfmt@v1

CHANGELOG.md

@@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [0.2.9]
+
+- Change namespaceing to use `.` separator instead of `_`. We assume table names are less likely to contain periods, so this reduces likelyhood of naming conflicts.This will change generated type names and will thus manifest as a breaking change for some users
+- Support `Nested` column types correctly, (previously these were treated as essentially Tuple columns)
+- Support subfield selection on complex column types.
+- Add support for relationships on column subfields, if the path to the subfield does not include lists
+- Fix datatype parser: add ability to parse SimpleAggregateFunction and AggregateFunction columns
+
 ## [0.2.8]
 
 - Make spans visible to cloud console users (tag spans with `internal.visibility = 'user'`)

Cargo.toml

@@ -6,5 +6,5 @@ members = [
 ]
 resolver = "2"
 
-package.version = "0.2.8"
+package.version = "0.2.9"
 package.edition = "2021"

crates/common/src/clickhouse_parser.rs

@@ -56,6 +56,8 @@ peg::parser! {
         / map()
         / tuple()
         / r#enum()
+        / aggregate_function()
+        / simple_aggregate_function()
         / nothing()
     rule nullable() -> DT = "Nullable(" t:data_type() ")" { DT::Nullable(Box::new(t)) }
     rule uint8() -> DT = "UInt8" { DT::UInt8 }
@@ -181,6 +183,16 @@ fn can_parse_clickhouse_data_type() {
                 (Some(Identifier::BacktickQuoted("u".to_string())), DT::UInt8),
             ]),
         ),
+        (
+            "SimpleAggregateFunction(sum, UInt64)",
+            DT::SimpleAggregateFunction {
+                function: AggregateFunctionDefinition {
+                    name: Identifier::Unquoted("sum".to_string()),
+                    parameters: None,
+                },
+                arguments: vec![DT::UInt64],
+            },
+        ),
     ];
 
     for (s, t) in data_types {

crates/common/src/clickhouse_parser/datatype.rs

@@ -20,6 +20,16 @@ pub enum Identifier {
     Unquoted(String),
 }
 
+impl Identifier {
+    pub fn value(&self) -> &str {
+        match self {
+            Identifier::DoubleQuoted(s)
+            | Identifier::BacktickQuoted(s)
+            | Identifier::Unquoted(s) => s,
+        }
+    }
+}
+
 impl Display for Identifier {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {

crates/common/src/config.rs

@@ -10,6 +10,7 @@ use crate::{
 pub struct ServerConfig {
     /// the connection part of the config is not part of the config file
     pub connection: ConnectionConfig,
+    pub namespace_separator: String,
     pub table_types: BTreeMap<ReturnTypeRef, TableType>,
     pub tables: BTreeMap<String, TableConfig>,
     pub queries: BTreeMap<String, ParameterizedQueryConfig>,

crates/ndc-clickhouse/Cargo.toml

@@ -14,6 +14,7 @@ reqwest = { version = "0.12.3", features = [
   "json",
   "rustls-tls",
 ], default-features = false }
+schemars = "0.8.16"
 serde = { version = "1.0.197", features = ["derive"] }
 serde_json = "1.0.114"
 sqlformat = "0.2.3"

crates/ndc-clickhouse/src/connector.rs

@@ -287,6 +287,9 @@ pub async fn read_server_config(
 
     let config = ServerConfig {
         connection,
+        // hardcoding separator for now, to avoid prematurely exposing configuration options we may not want to keep
+        // if we make this configurable, we must default to this separator when the option is not provided
+        namespace_separator: ".".to_string(),
         table_types,
         tables,
         queries,

crates/ndc-clickhouse/src/connector/handler/query.rs

@@ -1,9 +1,5 @@
 use common::{client::execute_query, config::ServerConfig};
-use ndc_sdk::{
-    connector::QueryError,
-    json_response::JsonResponse,
-    models::{self, QueryResponse},
-};
+use ndc_sdk::{connector::QueryError, json_response::JsonResponse, models};
 use tracing::{Instrument, Level};
 
 use crate::{connector::state::ServerState, sql::QueryBuilder};

crates/ndc-clickhouse/src/connector/handler/schema.rs

@@ -1,7 +1,7 @@
 use crate::schema::ClickHouseTypeDefinition;
 use common::{
     clickhouse_parser::{
-        datatype::{ClickHouseDataType, Identifier},
+        datatype::ClickHouseDataType,
         parameterized_query::{Parameter, ParameterType, ParameterizedQueryElement},
     },
     config::ServerConfig,
@@ -23,6 +23,7 @@ pub async fn schema(
                 &column_type,
                 &column_alias,
                 &type_name,
+                &configuration.namespace_separator,
             );
 
             let (scalars, objects) = type_definition.type_definitions();
@@ -61,6 +62,7 @@ pub async fn schema(
                 &argument_type,
                 &argument_name,
                 table_alias,
+                &configuration.namespace_separator,
             );
             let (scalars, objects) = type_definition.type_definitions();
 
@@ -79,19 +81,15 @@ pub async fn schema(
     for (query_alias, query_config) in &configuration.queries {
         for element in &query_config.query.elements {
             if let ParameterizedQueryElement::Parameter(Parameter { name, r#type }) = element {
-                let argument_alias = match name {
-                    Identifier::DoubleQuoted(n)
-                    | Identifier::BacktickQuoted(n)
-                    | Identifier::Unquoted(n) => n,
-                };
                 let data_type = match r#type {
                     ParameterType::Identifier => &ClickHouseDataType::String,
                     ParameterType::DataType(t) => t,
                 };
                 let type_definition = ClickHouseTypeDefinition::from_query_argument(
                     data_type,
-                    &argument_alias,
+                    name.value(),
                     query_alias,
+                    &configuration.namespace_separator,
                 );
 
                 let (scalars, objects) = type_definition.type_definitions();
@@ -123,6 +121,7 @@ pub async fn schema(
                         &argument_type,
                         &argument_name,
                         table_alias,
+                        &configuration.namespace_separator,
                     );
                     (
                         argument_name.to_owned(),
@@ -164,23 +163,19 @@ pub async fn schema(
                 .filter_map(|element| match element {
                     ParameterizedQueryElement::String(_) => None,
                     ParameterizedQueryElement::Parameter(Parameter { name, r#type }) => {
-                        let argument_alias = match name {
-                            Identifier::DoubleQuoted(n)
-                            | Identifier::BacktickQuoted(n)
-                            | Identifier::Unquoted(n) => n,
-                        };
                         let data_type = match r#type {
                             ParameterType::Identifier => &ClickHouseDataType::String,
                             ParameterType::DataType(t) => &t,
                         };
                         let type_definition = ClickHouseTypeDefinition::from_query_argument(
                             data_type,
-                            &argument_alias,
+                            name.value(),
                             query_alias,
+                            &configuration.namespace_separator,
                         );
 
                         Some((
-                            argument_alias.to_owned(),
+                            name.value().to_owned(),
                             models::ArgumentInfo {
                                 description: None,
                                 argument_type: type_definition.type_identifier(),