From 8fcd3d391a2867ff3927e389aaca8c0bc6b10c79 Mon Sep 17 00:00:00 2001 From: Xiaoyi Date: Sun, 25 Oct 2026 23:50:15 -0700 Subject: [PATCH 1/2] Derive: pass dialect method signatures through macro tokens Capture the Dialect method signatures while compiling sqlparser so expansion needs no source files or embedded build directory. Preserve invocation-site parameter names for expression overrides. Exercise all public entry points after removing the dependency sources. Direct macro callers now need sqlparser with derive-dialect or visitor. Fixes #2724 --- .github/workflows/rust.yml | 3 + derive/src/dialect.rs | 149 ++++++++++++++++--------------------- derive/src/lib.rs | 23 +++++- dev/test_derive_dialect.py | 89 ++++++++++++++++++++++ src/dialect/mod.rs | 5 ++ 5 files changed, 185 insertions(+), 84 deletions(-) create mode 100644 dev/test_derive_dialect.py diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index a07512da05..d71e83ab97 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -90,6 +90,9 @@ jobs: - uses: Swatinem/rust-cache@9d47c6ad4b02e050fd481d890b2ea34778fd09d6 # v2.7.8 - name: Test run: cargo test --locked --all-features + - name: Derive dialect without dependency sources + if: matrix.rust == 'stable' + run: python3 dev/test_derive_dialect.py coverage: runs-on: ubuntu-latest diff --git a/derive/src/dialect.rs b/derive/src/dialect.rs index 024e7ee437..ec6b732eb4 100644 --- a/derive/src/dialect.rs +++ b/derive/src/dialect.rs @@ -23,7 +23,7 @@ use std::collections::HashSet; use syn::{ braced, parse::{Parse, ParseStream}, - Error, Expr, File, FnArg, Ident, Item, LitBool, LitChar, Pat, ReceiverKind, ReturnType, + Error, Expr, FnArg, Ident, ItemTrait, LitBool, LitChar, Pat, ReceiverKind, ReturnType, Signature, Token, TraitItem, Type, }; @@ -110,22 +110,56 @@ impl Parse for DeriveDialectInput { } } -/// Entry point for the `derive_dialect!` macro -pub(crate) fn derive_dialect(input: DeriveDialectInput) -> proc_macro::TokenStream { +/// Pass the trait's signatures to each derived dialect without reading source files. +pub(crate) fn dialect_methods(item: ItemTrait) -> TokenStream { + let signatures = item.items.iter().filter_map(|item| match item { + TraitItem::Fn(method) => Some(&method.sig), + _ => None, + }); + quote! { + #item + + #[doc(hidden)] + #[macro_export] + macro_rules! __sqlparser_derive_dialect { + ($($args:tt)*) => { + $crate::dialect::__derive_dialect! { + trait Dialect { #(#signatures;)* } + $($args)* + } + }; + } + + #[doc(hidden)] + pub use __sqlparser_derive_dialect as __derive_dialect_with_methods; + } +} + +pub(crate) struct DeriveDialectWithMethods { + dialect: ItemTrait, + input: DeriveDialectInput, +} + +impl Parse for DeriveDialectWithMethods { + fn parse(input: ParseStream) -> syn::Result { + Ok(Self { + dialect: input.parse()?, + input: input.parse()?, + }) + } +} + +/// Entry point after the trait has supplied its method signatures. +pub(crate) fn derive_dialect(input: DeriveDialectWithMethods) -> proc_macro::TokenStream { match derive_dialect_inner(input) { Ok(tokens) => tokens.into(), Err(e) => e.to_compile_error().into(), } } -fn derive_dialect_inner(input: DeriveDialectInput) -> syn::Result { - let call_site = proc_macro2::Span::call_site(); - - let source = read_dialect_mod_file() - .map_err(|e| Error::new(call_site, format!("Failed to read dialect/mod.rs: {e}")))?; - let file: File = syn::parse_str::(&source) - .map_err(|e| Error::new(call_site, format!("Failed to parse source: {e}")))?; - let methods = extract_dialect_methods(&file)?; +fn derive_dialect_inner(input: DeriveDialectWithMethods) -> syn::Result { + let methods = extract_dialect_methods(&input.dialect, input.input.name.span()); + let input = input.input; // Validate overrides let method_names: HashSet<_> = methods.iter().map(|m| m.name.to_string()).collect(); @@ -254,86 +288,35 @@ fn extract_param_names(sig: &Signature) -> Vec<&Ident> { .collect() } -/// Read the `dialect/mod.rs` file that contains the Dialect trait. -/// -/// Searches for the file in the following order: -/// 1. `$CARGO_MANIFEST_DIR/src/dialect/mod.rs` - works when the macro is -/// invoked from within the `sqlparser` crate itself (e.g. in tests). -/// 2. `/../src/dialect/mod.rs` - works when -/// `sqlparser_derive` lives in a workspace alongside the main crate -/// (the standard `derive/` layout). -/// 3. Sibling directories of the compiled `sqlparser_derive` crate in the -/// Cargo registry - works when an external crate uses `derive_dialect!` -/// via a registry dependency. -fn read_dialect_mod_file() -> Result { - use std::path::{Path, PathBuf}; - - const DERIVE_CRATE_DIR: &str = env!("CARGO_MANIFEST_DIR"); - let derive_dir = Path::new(DERIVE_CRATE_DIR); - let mut candidates: Vec = Vec::new(); - - // The crate being compiled (eg: within sqlparser). - if let Ok(manifest_dir) = std::env::var("CARGO_MANIFEST_DIR") { - candidates.push(Path::new(&manifest_dir).join("src/dialect/mod.rs")); - } - // Workspace layout: the main crate is the parent of `derive/`. - candidates.push(derive_dir.join("../src/dialect/mod.rs")); - - // Cargo registry: look for sibling `sqlparser-*` directories (prefer newest). - if let Some(parent) = derive_dir.parent() { - if let Ok(entries) = std::fs::read_dir(parent) { - let mut siblings: Vec<_> = entries - .filter_map(|e| e.ok()) - .filter(|e| { - let name = e.file_name(); - let name = name.to_string_lossy(); - name.starts_with("sqlparser-") && !name.starts_with("sqlparser-derive") - }) - .collect(); - siblings.sort_by_key(|e| std::cmp::Reverse(e.file_name())); - candidates.extend( - siblings - .into_iter() - .map(|e| e.path().join("src/dialect/mod.rs")), - ); - } - } - for path in &candidates { - if let Ok(content) = std::fs::read_to_string(path) { - return Ok(content); - } - } - Err(format!( - "Could not find `sqlparser` dialect/mod.rs file. \ - Searched in $CARGO_MANIFEST_DIR/src/dialect/mod.rs and \ - the `sqlparser_derive` crate at {DERIVE_CRATE_DIR}" - )) -} - -/// Extract all methods from the `Dialect` trait (excluding `dialect` for TypeId) -fn extract_dialect_methods(file: &File) -> Result, Error> { - let dialect_trait = file - .items - .iter() - .find_map(|item| match item { - Item::Trait(t) if t.ident == "Dialect" => Some(t), - _ => None, - }) - .ok_or_else(|| Error::new(proc_macro2::Span::call_site(), "Dialect trait not found"))?; - +/// Extract all methods from the supplied trait (excluding `dialect` for TypeId). +fn extract_dialect_methods( + dialect_trait: &ItemTrait, + call_site: proc_macro2::Span, +) -> Vec { let mut methods: Vec<_> = dialect_trait .items .iter() .filter_map(|item| match item { - TraitItem::Fn(m) if m.sig.ident != "dialect" => Some(DialectMethod { - name: m.sig.ident.clone(), - signature: m.sig.clone(), - }), + TraitItem::Fn(m) if m.sig.ident != "dialect" => { + let mut signature = m.sig.clone(); + // Override expressions refer to parameters at the invocation site. + for arg in &mut signature.inputs { + if let FnArg::Typed(arg) = arg { + if let Pat::Ident(pat) = arg.pat.as_mut() { + pat.ident.set_span(call_site); + } + } + } + Some(DialectMethod { + name: m.sig.ident.clone(), + signature, + }) + } _ => None, }) .collect(); methods.sort_by_key(|m| m.name.to_string()); - Ok(methods) + methods } /// Check if a method signature is `fn name(&self) -> bool` diff --git a/derive/src/lib.rs b/derive/src/lib.rs index e3eaeea6d5..c233aa4fe2 100644 --- a/derive/src/lib.rs +++ b/derive/src/lib.rs @@ -56,8 +56,29 @@ pub fn derive_visit_immutable(input: proc_macro::TokenStream) -> proc_macro::Tok } /// Procedural macro for deriving new SQL dialects. +/// +/// Enable the `derive-dialect` feature on the `sqlparser` dependency. #[proc_macro] pub fn derive_dialect(input: proc_macro::TokenStream) -> proc_macro::TokenStream { - let input = parse_macro_input!(input as dialect::DeriveDialectInput); + let input = proc_macro2::TokenStream::from(input); + quote!(::sqlparser::dialect::__derive_dialect_with_methods!(#input);).into() +} + +/// Capture method signatures from the Dialect trait definition. +#[doc(hidden)] +#[proc_macro_attribute] +pub fn dialect_methods( + _args: proc_macro::TokenStream, + item: proc_macro::TokenStream, +) -> proc_macro::TokenStream { + let item = parse_macro_input!(item as syn::ItemTrait); + dialect::dialect_methods(item).into() +} + +/// Expand a dialect using the method signatures supplied by sqlparser. +#[doc(hidden)] +#[proc_macro] +pub fn __derive_dialect(input: proc_macro::TokenStream) -> proc_macro::TokenStream { + let input = parse_macro_input!(input as dialect::DeriveDialectWithMethods); dialect::derive_dialect(input) } diff --git a/dev/test_derive_dialect.py b/dev/test_derive_dialect.py new file mode 100644 index 0000000000..1af9ad414a --- /dev/null +++ b/dev/test_derive_dialect.py @@ -0,0 +1,89 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Compile a derived dialect after removing the dependency source trees.""" + +import json +import os +from pathlib import Path +import shutil +import subprocess +import tempfile + +ROOT = Path(__file__).resolve().parents[1] + +with tempfile.TemporaryDirectory(prefix="sqlparser-derive-") as scratch: + scratch = Path(scratch) + source = scratch / "source" + source.mkdir() + for directory in ("src", "derive", "sqlparser_bench", "tests", "examples"): + shutil.copytree(ROOT / directory, source / directory) + for filename in ("Cargo.toml", "Cargo.lock"): + shutil.copy2(ROOT / filename, source / filename) + target = scratch / "target" + result = subprocess.run( + [os.environ.get("CARGO", "cargo"), "build", "--locked", "-p", "sqlparser", + "--lib", "--no-default-features", "--features", "derive-dialect", + "--manifest-path", str(source / "Cargo.toml"), + "--target-dir", str(target), "--message-format=json"], + check=True, stdout=subprocess.PIPE, text=True, + ) + artifacts = {} + for line in result.stdout.splitlines(): + message = json.loads(line) + if message.get("reason") == "compiler-artifact": + name = message["target"]["name"] + if name in ("sqlparser", "sqlparser_derive"): + artifacts[name] = next( + path for path in message["filenames"] + if Path(path).suffix in (".rlib", ".so", ".dylib", ".dll") + ) + shutil.rmtree(source) + consumer = scratch / "consumer.rs" + consumer.write_text(r''' +use sqlparser::dialect::{Dialect, MySqlDialect}; + +sqlparser::derive_dialect!(RootDialect, MySqlDialect); +sqlparser::dialect::derive_dialect!(ModuleDialect, MySqlDialect, + overrides = { supports_string_literal_backslash_escape = false }); +sqlparser_derive::derive_dialect!(DirectDialect, MySqlDialect, + preserve_type_id = true, overrides = { + identifier_quote_style = None, + is_identifier_start = '_' == ch || ch.is_ascii_alphabetic(), + }); + +fn main() { + assert!(RootDialect::new().supports_string_literal_backslash_escape()); + assert!(!ModuleDialect::new().supports_string_literal_backslash_escape()); + let direct = DirectDialect::new(); + assert!(direct.supports_string_literal_backslash_escape()); + assert_eq!(direct.identifier_quote_style("x"), None); + assert!(direct.is_identifier_start('_')); + assert!(!direct.is_identifier_start('1')); + assert!((&direct as &dyn Dialect).is::()); + assert!(sqlparser::parser::Parser::parse_sql(&direct, "SELECT 1").is_ok()); +} +''') + binary = scratch / ("consumer.exe" if os.name == "nt" else "consumer") + command = [os.environ.get("RUSTC", "rustc"), "--edition=2021", str(consumer), + "-L", f"dependency={target / 'debug' / 'deps'}", "-o", str(binary)] + for name, path in artifacts.items(): + command.extend(["--extern", f"{name}={path}"]) + subprocess.run(command, check=True, cwd=scratch, + env={**os.environ, "CARGO_MANIFEST_DIR": str(source)}) + subprocess.run([str(binary)], check=True) + print("Derived dialects compile and run without dependency sources.") diff --git a/src/dialect/mod.rs b/src/dialect/mod.rs index db3f5c0661..75215cd7e2 100644 --- a/src/dialect/mod.rs +++ b/src/dialect/mod.rs @@ -151,6 +151,10 @@ pub use self::teradata::TeradataDialect; #[cfg(feature = "derive-dialect")] pub use sqlparser_derive::derive_dialect; +#[cfg(feature = "sqlparser_derive")] +#[doc(hidden)] +pub use sqlparser_derive::__derive_dialect; + use crate::ast::{ColumnOption, DataType, Expr, GranteesType, Ident, ObjectNamePart, Statement}; pub use crate::keywords; use crate::keywords::Keyword; @@ -231,6 +235,7 @@ pub enum BindPlaceholderStyle { /// ``` /// /// [module level documentation]: crate +#[cfg_attr(feature = "sqlparser_derive", sqlparser_derive::dialect_methods)] pub trait Dialect: Debug + Any { /// Determine the [`TypeId`] of this dialect. /// From 561a7204da89228f606bbe95065d2cb10a49d8d5 Mon Sep 17 00:00:00 2001 From: Xiaoyi Date: Sun, 25 Oct 2026 23:58:50 -0700 Subject: [PATCH 2/2] Tests: cover dialect macro paths in the existing Rust suite --- .github/workflows/rust.yml | 3 -- dev/test_derive_dialect.py | 89 ------------------------------- tests/sqlparser_derive_dialect.rs | 20 +++++++ 3 files changed, 20 insertions(+), 92 deletions(-) delete mode 100644 dev/test_derive_dialect.py diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index d71e83ab97..a07512da05 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -90,9 +90,6 @@ jobs: - uses: Swatinem/rust-cache@9d47c6ad4b02e050fd481d890b2ea34778fd09d6 # v2.7.8 - name: Test run: cargo test --locked --all-features - - name: Derive dialect without dependency sources - if: matrix.rust == 'stable' - run: python3 dev/test_derive_dialect.py coverage: runs-on: ubuntu-latest diff --git a/dev/test_derive_dialect.py b/dev/test_derive_dialect.py deleted file mode 100644 index 1af9ad414a..0000000000 --- a/dev/test_derive_dialect.py +++ /dev/null @@ -1,89 +0,0 @@ -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. - -"""Compile a derived dialect after removing the dependency source trees.""" - -import json -import os -from pathlib import Path -import shutil -import subprocess -import tempfile - -ROOT = Path(__file__).resolve().parents[1] - -with tempfile.TemporaryDirectory(prefix="sqlparser-derive-") as scratch: - scratch = Path(scratch) - source = scratch / "source" - source.mkdir() - for directory in ("src", "derive", "sqlparser_bench", "tests", "examples"): - shutil.copytree(ROOT / directory, source / directory) - for filename in ("Cargo.toml", "Cargo.lock"): - shutil.copy2(ROOT / filename, source / filename) - target = scratch / "target" - result = subprocess.run( - [os.environ.get("CARGO", "cargo"), "build", "--locked", "-p", "sqlparser", - "--lib", "--no-default-features", "--features", "derive-dialect", - "--manifest-path", str(source / "Cargo.toml"), - "--target-dir", str(target), "--message-format=json"], - check=True, stdout=subprocess.PIPE, text=True, - ) - artifacts = {} - for line in result.stdout.splitlines(): - message = json.loads(line) - if message.get("reason") == "compiler-artifact": - name = message["target"]["name"] - if name in ("sqlparser", "sqlparser_derive"): - artifacts[name] = next( - path for path in message["filenames"] - if Path(path).suffix in (".rlib", ".so", ".dylib", ".dll") - ) - shutil.rmtree(source) - consumer = scratch / "consumer.rs" - consumer.write_text(r''' -use sqlparser::dialect::{Dialect, MySqlDialect}; - -sqlparser::derive_dialect!(RootDialect, MySqlDialect); -sqlparser::dialect::derive_dialect!(ModuleDialect, MySqlDialect, - overrides = { supports_string_literal_backslash_escape = false }); -sqlparser_derive::derive_dialect!(DirectDialect, MySqlDialect, - preserve_type_id = true, overrides = { - identifier_quote_style = None, - is_identifier_start = '_' == ch || ch.is_ascii_alphabetic(), - }); - -fn main() { - assert!(RootDialect::new().supports_string_literal_backslash_escape()); - assert!(!ModuleDialect::new().supports_string_literal_backslash_escape()); - let direct = DirectDialect::new(); - assert!(direct.supports_string_literal_backslash_escape()); - assert_eq!(direct.identifier_quote_style("x"), None); - assert!(direct.is_identifier_start('_')); - assert!(!direct.is_identifier_start('1')); - assert!((&direct as &dyn Dialect).is::()); - assert!(sqlparser::parser::Parser::parse_sql(&direct, "SELECT 1").is_ok()); -} -''') - binary = scratch / ("consumer.exe" if os.name == "nt" else "consumer") - command = [os.environ.get("RUSTC", "rustc"), "--edition=2021", str(consumer), - "-L", f"dependency={target / 'debug' / 'deps'}", "-o", str(binary)] - for name, path in artifacts.items(): - command.extend(["--extern", f"{name}={path}"]) - subprocess.run(command, check=True, cwd=scratch, - env={**os.environ, "CARGO_MANIFEST_DIR": str(source)}) - subprocess.run([str(binary)], check=True) - print("Derived dialects compile and run without dependency sources.") diff --git a/tests/sqlparser_derive_dialect.rs b/tests/sqlparser_derive_dialect.rs index c8cff4e9f3..e5d2262279 100644 --- a/tests/sqlparser_derive_dialect.rs +++ b/tests/sqlparser_derive_dialect.rs @@ -220,3 +220,23 @@ fn test_literal_leading_bool_expression_override() { Some(sqlparser::dialect::BindPlaceholderStyle::DollarNumbered) ); } + +#[test] +fn test_direct_and_module_macro_paths() { + sqlparser_derive::derive_dialect!( + DirectDialect, + MySqlDialect, + overrides = { is_identifier_start = '_' == ch || ch.is_ascii_alphabetic() } + ); + sqlparser::dialect::derive_dialect!( + ModuleDialect, + MySqlDialect, + overrides = { supports_string_literal_backslash_escape = false } + ); + + let direct = DirectDialect::new(); + assert!(direct.is_identifier_start('_')); + assert!(!direct.is_identifier_start('1')); + assert!(direct.supports_string_literal_backslash_escape()); + assert!(!ModuleDialect::new().supports_string_literal_backslash_escape()); +}