feat: format Databricks with SQLQuack (#57)

* feat: format Databricks with SQLQuack

Databricks files are laid out by SQLQuack's Databricks profile, which
reads them with the Databricks scanner and parser. Of the 62 Databricks
corpus queries, 60 come out as SQLClean laid them out before. A ${x}
variable substitution, which the Databricks parser rejects, is left as
written with a warning, and get_json_object is upper-cased as every
other call is.

* test: expect format to lay out a Databricks file end to end

Author: kvlonge

Cargo.toml

@@ -31,7 +31,7 @@ similar = "2"
 sqlclean = { path = "crates/sqlclean" }
 # SQLQuack's formatter and lint engine, and the tokens and trees they read, which replace Polyglot.
 sqlquack-fingerprint = { git = "https://github.com/sqlstack-io/sqlquack.git", rev = "f53c6a076ad05c5c12047a2dcc9c58e3ddfbe293", features = ["bigquery", "databricks", "duckdb", "postgres", "snowflake", "sqlite", "sqlserver"] }
-sqlquack-format = { git = "https://github.com/sqlstack-io/sqlquack.git", rev = "f53c6a076ad05c5c12047a2dcc9c58e3ddfbe293", features = ["bigquery", "duckdb", "postgres", "snowflake", "sqlserver"] }
+sqlquack-format = { git = "https://github.com/sqlstack-io/sqlquack.git", rev = "f53c6a076ad05c5c12047a2dcc9c58e3ddfbe293", features = ["bigquery", "databricks", "duckdb", "postgres", "snowflake", "sqlserver"] }
 sqlquack-lint = { git = "https://github.com/sqlstack-io/sqlquack.git", rev = "f53c6a076ad05c5c12047a2dcc9c58e3ddfbe293", features = ["bigquery", "databricks", "duckdb", "postgres", "snowflake", "sqlite"] }
 sqlquack-parse = { git = "https://github.com/sqlstack-io/sqlquack.git", rev = "f53c6a076ad05c5c12047a2dcc9c58e3ddfbe293" }
 sqlquack-syntax = { git = "https://github.com/sqlstack-io/sqlquack.git", rev = "f53c6a076ad05c5c12047a2dcc9c58e3ddfbe293" }

crates/sqlclean-cli/skill/references/docs/configuration.md

@@ -125,7 +125,7 @@ error[SQLCLEAN999]: SQL could not be parsed as duckdb: Parse error at line 1, co
 
 SQLClean reads SQL with SQLQuack's parsers. `check` and `fix` read `bigquery`, `databricks`,
 `duckdb`, `generic`, `postgres`, `snowflake` and `sqlite` files, and `format` lays out `bigquery`,
-`duckdb`, `postgres`, `snowflake` and `tsql` files. A `databricks`, `generic` or `sqlite` file is
+`databricks`, `duckdb`, `postgres`, `snowflake` and `tsql` files. A `generic` or `sqlite` file is
 checked and fixed but left as written by `format`; a `tsql` file is formatted, by `fix` too, but its
 Rules are not checked yet. A file in any other dialect, such as `mysql`, is not checked, fixed or
 formatted: `check` reports it once as a warning (in JSON, under `unavailable`), `format` leaves it

crates/sqlclean/src/sql_lint/_helpers/formatter.rs

@@ -5,8 +5,8 @@ use sqlquack_syntax::Dialect as SqlQuackDialect;
 use crate::constants::INTERPOLATION_PLACEHOLDER_PREFIX;
 use crate::sql_lint::_helpers::cte_macros::{take_out_cte_macros, wrap_query_placeholders};
 use crate::sql_lint::constants::{
-    BIGQUERY_DIALECT, DUCKDB_DIALECT, LINT_API_VERSION, POSTGRES_DIALECTS, SNOWFLAKE_DIALECT,
-    TSQL_DIALECTS,
+    BIGQUERY_DIALECT, DATABRICKS_DIALECT, DUCKDB_DIALECT, LINT_API_VERSION, POSTGRES_DIALECTS,
+    SNOWFLAKE_DIALECT, TSQL_DIALECTS,
 };
 use crate::sql_lint::models::{FormatRequest, FormatResponse};
 
@@ -64,6 +64,7 @@ fn sqlquack_dialect(dialect: &str) -> Option<(FormatDialect, Option<SqlQuackDial
         SNOWFLAKE_DIALECT => Some((FormatDialect::Snowflake, Some(SqlQuackDialect::Snowflake))),
         DUCKDB_DIALECT => Some((FormatDialect::DuckDB, Some(SqlQuackDialect::DuckDB))),
         BIGQUERY_DIALECT => Some((FormatDialect::BigQuery, None)),
+        DATABRICKS_DIALECT => Some((FormatDialect::Databricks, None)),
         name if POSTGRES_DIALECTS.contains(&name) => {
             Some((FormatDialect::PostgreSQL, Some(SqlQuackDialect::PostgreSQL)))
         }

crates/sqlclean/src/sql_lint/constants.rs

@@ -5,6 +5,8 @@ pub(crate) const SNOWFLAKE_DIALECT: &str = "snowflake";
 pub(crate) const DUCKDB_DIALECT: &str = "duckdb";
 /// The dialect name SQLQuack lays out as BigQuery.
 pub(crate) const BIGQUERY_DIALECT: &str = "bigquery";
+/// The dialect name SQLQuack lays out as Databricks.
+pub(crate) const DATABRICKS_DIALECT: &str = "databricks";
 /// The dialect names SQLQuack lays out as PostgreSQL.
 pub(crate) const POSTGRES_DIALECTS: [&str; 2] = ["postgres", "postgresql"];
 /// The dialect names SQLQuack lays out as SQL Server.

crates/sqlclean/src/sql_lint/tests/fixtures/sqlquack_departures.json

@@ -593,5 +593,19 @@
   "formatted": null,
   "reason": "PostgreSQL's scanner reads `<<->>` as one operator, which the shared tokenizer splits",
   "receipt": null
+ },
+ {
+  "dialect": "databricks",
+  "sql": "SELECT ${x} FROM ${y} WHERE ${z} > 1",
+  "formatted": null,
+  "reason": "`${x}` is a variable substitution that Databricks replaces before parsing, so its grammar has no such token and the native parser rejects it; SQLClean formatted it",
+  "receipt": null
+ },
+ {
+  "dialect": "databricks",
+  "sql": "WITH t AS (SELECT '{\"x-y\": \"z\"}' AS c) SELECT get_json_object(c, '$.x-y') FROM t",
+  "formatted": "WITH t AS (\n  SELECT '{\"x-y\": \"z\"}' AS c\n)\n\nSELECT GET_JSON_OBJECT(c, '$.x-y')\nFROM t",
+  "reason": "SQLClean kept the call's authored case because its Polyglot oracle rewrites the JSON path `'$.x-y'` as `'$[\"x-y\"]'`, as SQLGlot does; SQLQuack upper-cases a called name in every dialect",
+  "receipt": null
  }
 ]

crates/sqlclean/src/sql_lint/tests/helpers.rs

@@ -145,22 +145,6 @@ fn only_keyword_case_differs(sql: &str, printed: &str, dialect: &str) -> Result<
     })
 }
 
-/// Check every corpus query of one dialect is left as written and reported unavailable; return how many.
-pub(crate) fn check_unavailable_corpus(corpus: &[Value], dialect: &str) -> Result<usize, String> {
-    let mut count: usize = 0;
-    for entry in corpus.iter().filter(|entry| entry["dialect"] == dialect) {
-        let sql = entry["sql"].as_str().unwrap_or_default();
-        let response = format_once(sql, dialect, Some(CORPUS_LINE_WIDTH))?;
-        assert_eq!(response["sql"], sql, "{dialect}: {sql}");
-        assert_eq!(response["formatted"], false, "{dialect}: {sql}");
-        assert_eq!(response["changed"], false, "{dialect}: {sql}");
-        assert!(response.get("reason").is_none(), "{dialect}: {sql}");
-        assert!(response["unavailable"].is_string(), "{dialect}: {sql}");
-        count += 1;
-    }
-    Ok(count)
-}
-
 /// Re-format a layout of an earlier formatter release, which must keep its fingerprint.
 fn relaid_previous_layout(previous: &str, dialect: &str) -> Result<String, String> {
     let relaid = format_once(previous, dialect, None)?;

crates/sqlclean/src/sql_lint/tests/test_format_corpus.rs

@@ -1,6 +1,6 @@
 use serde_json::Value;
 
-use crate::sql_lint::tests::helpers::{check_dialect_corpus, check_unavailable_corpus};
+use crate::sql_lint::tests::helpers::check_dialect_corpus;
 use crate::sql_lint::tests::test_types;
 
 const FORMAT_CORPUS: &str = include_str!("fixtures/format_corpus.json");
@@ -33,6 +33,12 @@ fn given_dialect_corpus_when_formatting_then_output_layout_and_refusals_are_stab
             expected_formatted: 124,
             expected_refused: 13,
         },
+        test_types::FormatCorpusTestCase {
+            description: "Databricks identity corpus",
+            dialect: "databricks",
+            expected_formatted: 61,
+            expected_refused: 1,
+        },
         test_types::FormatCorpusTestCase {
             description: "T-SQL identity corpus",
             dialect: "tsql",
@@ -53,25 +59,3 @@ fn given_dialect_corpus_when_formatting_then_output_layout_and_refusals_are_stab
     }
     Ok(())
 }
-
-/// Dialects SQLQuack does not format yet are reported unavailable, with every query left as written.
-#[test]
-fn given_dialects_without_a_native_formatter_when_formatting_then_each_query_is_unavailable()
--> Result<(), String> {
-    let test_cases = [test_types::UnavailableCorpusTestCase {
-        description: "Databricks corpus",
-        dialect: "databricks",
-        expected_unavailable: 62,
-    }];
-    let corpus: Vec<Value> =
-        serde_json::from_str(FORMAT_CORPUS).map_err(|error| error.to_string())?;
-    for test_case in &test_cases {
-        assert_eq!(
-            check_unavailable_corpus(&corpus, test_case.dialect)?,
-            test_case.expected_unavailable,
-            "{}",
-            test_case.description
-        );
-    }
-    Ok(())
-}

crates/sqlclean/src/sql_lint/tests/test_sql_lint.rs

@@ -1894,10 +1894,10 @@ fn given_each_documented_dialect_when_reading_sql_then_sqlquack_was_built_for_it
             expected_formatted: true,
         },
         test_types::DialectReadersTestCase {
-            description: "databricks is checked, and reported unavailable to be formatted",
+            description: "databricks is checked and formatted",
             dialect: "databricks",
             expected_linted: true,
-            expected_formatted: false,
+            expected_formatted: true,
         },
         test_types::DialectReadersTestCase {
             description: "duckdb is checked and formatted",

crates/sqlclean/src/sql_lint/tests/test_types.rs

@@ -159,12 +159,6 @@ pub(crate) struct CrossJoinSubjectTestCase {
     pub expected_subject: Option<(&'static str, Option<&'static str>)>,
 }
 
-pub(crate) struct UnavailableCorpusTestCase {
-    pub description: &'static str,
-    pub dialect: &'static str,
-    pub expected_unavailable: usize,
-}
-
 pub(crate) struct DialectReadersTestCase {
     pub description: &'static str,
     pub dialect: &'static str,

tests/e2e/src/sqlclean/hosting/test_sqlclean_command.py

@@ -318,12 +318,12 @@ def test_given_project_when_checking_then_reports_findings_and_exit_status(
             },
         ),
         RewriteTestCase(
-            description="format leaves a Databricks file, which SQLQuack parses but does not format, unchanged",
+            description="format lays out a Databricks file",
             config='dialect = "databricks"\n',
             files={"models/q.sql": "select a from t\n"},
-            arguments=("format", "--check"),
+            arguments=("format",),
             expected_exit=0,
-            expected_files={"models/q.sql": "select a from t\n"},
+            expected_files={"models/q.sql": "SELECT a\nFROM t\n"},
         ),
         RewriteTestCase(
             description="format lays out BigQuery and PostgreSQL files",