hasura · BenoitRanque · Jun 20, 2024 · Mar 14, 2024 · Mar 18, 2024
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "clickhouse_gdc"
-version = "2.37.1"
+version = "2.37.2"
 edition = "2021"
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

diff --git a/changelog.md b/changelog.md
@@ -1,5 +1,14 @@
 # ChangeLog
 
+## 2.37.2
+
+- fix breaking change: don't use schema qualification for the `default` schema.
+  > Supporting non-default schema meant qualifying tables, which also meant introducing the schema name in the table name array.
+  > This was a breaking change for two reasons: older metadata needed to be updated to qualify tables with the `default` schema,
+  > and it also changed the generated hasura types. This fix removes the `default` schema from the table name array, but keeps any other schemas.
+
+## 2.37.1
+
 - fix introspection 4: when introspecting for specific tables, allow qualified table names. Note we only filter on table names with no regards for schemas, this may need to be fixed later
 
 ## 2.36.0

diff --git a/src/server.rs b/src/server.rs
@@ -9,7 +9,7 @@ mod config;
 mod error;
 mod routes;
 use self::{error::ServerError, routes::*};
-pub use config::Config;
+pub use config::{Config, TableConfig};
 
 pub fn router() -> Router {
     Router::new()

diff --git a/src/server/config.rs b/src/server/config.rs
@@ -26,6 +26,8 @@ pub struct Config {
 pub struct TableConfig {
     /// The table name
     pub name: String,
+    /// The table schema (optional, defaults to "default")
+    pub schema: Option<String>,
     /// Optional alias for this table. Required if the table name is not a valid graphql name
     pub alias: Option<String>,
     /// Optional configuration for table columns
@@ -116,6 +118,12 @@ pub fn get_openapi_config_schema_response() -> ConfigSchemaResponse {
                             "nullable": false,
                             "type": "string"
                         },
+                        "schema": {
+                            "title": "Schema",
+                            "description": "Optional schema qualifying this table. Defaults to 'default' if omitted",
+                            "nullable": true,
+                            "type": "string"
+                        },
                         "alias": {
                             "title": "Alias",
                             "description": "Optional alias for this table. Required if the table name is not a valid graphql name",

diff --git a/src/server/routes/post_explain.rs b/src/server/routes/post_explain.rs
@@ -10,7 +10,7 @@ use crate::{
         config::{SourceConfig, SourceName},
         error::ServerError,
     },
-    sql::{apply_aliases_to_query_request, QueryBuilder},
+    sql::QueryBuilder,
 };
 
 #[axum_macros::debug_handler]
@@ -19,8 +19,7 @@ pub async fn post_explain(
     SourceConfig(config): SourceConfig,
     WithRejection(Json(request), _): WithRejection<Json<QueryRequest>, ServerError>,
 ) -> Result<Json<ExplainResponse>, ServerError> {
-    let request = apply_aliases_to_query_request(request, &config)?;
-    let statement = QueryBuilder::build_sql_statement(&request, false)?;
+    let statement = QueryBuilder::build_sql_statement(&request, false, &config)?;
     let statement_string = statement.to_string();
     let explain_statement = format!("EXPLAIN {}", statement_string);
 

diff --git a/src/server/routes/post_query.rs b/src/server/routes/post_query.rs
@@ -9,7 +9,7 @@ use crate::{
         config::{SourceConfig, SourceName},
         error::ServerError,
     },
-    sql::{apply_aliases_to_query_request, QueryBuilder},
+    sql::QueryBuilder,
 };
 
 #[axum_macros::debug_handler]
@@ -18,8 +18,7 @@ pub async fn post_query(
     SourceConfig(config): SourceConfig,
     WithRejection(Json(request), _): WithRejection<Json<QueryRequest>, ServerError>,
 ) -> Result<Json<QueryResponse>, ServerError> {
-    let request = apply_aliases_to_query_request(request, &config)?;
-    let statement = QueryBuilder::build_sql_statement(&request, false)?;
+    let statement = QueryBuilder::build_sql_statement(&request, false, &config)?;
 
     let statement_string = statement.to_string();
 

diff --git a/src/server/routes/post_schema.rs b/src/server/routes/post_schema.rs
@@ -4,7 +4,8 @@ use std::str::FromStr;
 use axum::Json;
 use axum_extra::extract::WithRejection;
 use gdc_rust_types::{
-    ColumnInfo, ColumnType, ErrorResponseType, SchemaRequest, SchemaResponse, TableInfo, TableType,
+    ColumnInfo, ColumnType, ErrorResponseType, SchemaRequest, SchemaResponse, TableInfo, TableName,
+    TableType,
 };
 use serde::{Deserialize, Serialize};
 
@@ -16,7 +17,7 @@ use crate::server::{
         clickhouse_data_type::{ClickhouseDataType, Identifier},
         ScalarType,
     },
-    Config,
+    Config, TableConfig,
 };
 
 #[axum_macros::debug_handler]
@@ -85,6 +86,8 @@ fn get_schema_response(
                     columns,
                 } = table;
 
+                let table_config = get_table_config(config, &table_schema, &table_name);
+
                 let columns = if let Some(columns) = columns {
                     Some(
                         columns
@@ -101,7 +104,7 @@ fn get_schema_response(
                                     .unwrap_or(ScalarType::Unknown);
 
                                 Ok(ColumnInfo {
-                                    name: aliased_column_name(&table_name, &column_name, config),
+                                    name: get_column_alias(&table_config, &column_name),
                                     description: None,
                                     nullable,
                                     insertable: None,
@@ -116,8 +119,15 @@ fn get_schema_response(
                     None
                 };
 
+                let primary_key = primary_key.map(|primary_key| {
+                    primary_key
+                        .into_iter()
+                        .map(|column| get_column_alias(&table_config, &column))
+                        .collect()
+                });
+
                 Ok(TableInfo {
-                    name: vec![table_schema, aliased_table_name(&table_name, config)],
+                    name: get_table_alias(&table_config, &table_schema, &table_name),
                     description: None,
                     r#type: table_type.map(|table_type| match table_type {
                         IntrospectionTableType::BaseTable => TableType::Table,
@@ -137,41 +147,48 @@ fn get_schema_response(
     Ok(response)
 }
 
-fn aliased_table_name(table_name: &str, config: &Config) -> String {
-    if let Some(tables) = &config.tables {
-        if let Some(table_config) = tables
-            .iter()
-            .find(|table_config| table_config.name == table_name)
-        {
-            if let Some(alias) = &table_config.alias {
-                return alias.to_owned();
-            }
-        }
-    }
+fn get_table_config<'a>(
+    config: &'a Config,
+    table_schema: &str,
+    table_name: &str,
+) -> Option<&'a TableConfig> {
+    let matching_schema_name = |table_config: &TableConfig| match &table_config.schema {
+        Some(schema) => schema == table_schema,
+        None => table_schema == "default",
+    };
 
-    table_name.to_owned()
+    config.tables.as_ref().and_then(|tables| {
+        tables.iter().find(|table_config| {
+            table_config.name == table_name && matching_schema_name(table_config)
+        })
+    })
 }
 
-fn aliased_column_name(table_name: &str, column_name: &str, config: &Config) -> String {
-    if let Some(tables) = &config.tables {
-        if let Some(table_config) = tables
-            .iter()
-            .find(|table_config| table_config.name == table_name)
-        {
-            if let Some(columns) = &table_config.columns {
-                if let Some(column_config) = columns
-                    .iter()
-                    .find(|column_config| column_config.name == column_name)
-                {
-                    if let Some(alias) = &column_config.alias {
-                        return alias.to_owned();
-                    }
-                }
-            }
-        }
+fn get_table_alias(
+    table_config: &Option<&TableConfig>,
+    table_schema: &str,
+    table_name: &str,
+) -> TableName {
+    let aliased_table_name = table_config
+        .and_then(|table_config| table_config.alias.to_owned())
+        .unwrap_or_else(|| table_name.to_owned());
+
+    if table_schema == "default" {
+        vec![aliased_table_name]
+    } else {
+        vec![table_schema.to_owned(), aliased_table_name]
     }
-
-    column_name.to_owned()
+}
+fn get_column_alias(table_config: &Option<&TableConfig>, column_name: &str) -> String {
+    table_config
+        .and_then(|table_config| table_config.columns.as_ref())
+        .and_then(|columns| {
+            columns
+                .iter()
+                .find(|column| column.name == column_name)
+                .and_then(|column_config| column_config.alias.to_owned())
+        })
+        .unwrap_or_else(|| column_name.to_owned())
 }
 
 fn get_scalar_type(data_type: &ClickhouseDataType) -> ScalarType {

diff --git a/src/sql.rs b/src/sql.rs
@@ -1,5 +1,3 @@
 mod ast;
 mod query_builder;
-pub use query_builder::{
-    aliasing::apply_aliases_to_query_request, QueryBuilder, QueryBuilderError,
-};
+pub use query_builder::{QueryBuilder, QueryBuilderError};