Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom scalar to sql overrides support for DuckDB Unparser dialect(#68) #13915

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sgrebnov
Copy link
Member

@sgrebnov sgrebnov commented Dec 27, 2024

Which issue does this PR close?

Unparser does not currently support extending dialect with custom handlers for internal UDFs or new engine extensions. Possible workarounds here are creating a custom dialect wrapper around original dialect with overrided scalar_function_to_sql_overrides or using CustomDialect. This is not very convenient and robust as requires tracking changes to the main dialect and reflecting the same improvements in the custom dialect.

Rationale for this change

Propose introducing a pattern and approach for extending scalar_function_to_sql_overrides for dialects with additional handlers, example usage.

/// Create a new instance of the `DuckDB` dialect with support of custom internal UDFs
let dialect = DuckDBDialect::new().with_custom_scalar_overrides(vec![(
    "cosine_distance",
    Box::new(duckdb::cosine_distance_to_sql) as ScalarFnToSqlHandler,
)]);

What changes are included in this PR?

PR introduces new pattern for extending dialects with custom scalar_function_to_sql_overrides handlers starting from DuckDB that could be extended to other dialects next

pub type ScalarFnToSqlHandler = Box<dyn Fn(&Unparser, &[Expr]) -> Result<Option<ast::Expr>> + Send + Sync>;

pub fn with_custom_scalar_overrides(
        mut self,
        handlers: Vec<(&str, ScalarFnToSqlHandler)>,
    ) -> Self {
        ...
        self
    }

Are these changes tested?

Added unit test test_custom_scalar_overrides_duckdb

Are there any user-facing changes?

Breaking change: as we are adding additional field to DuckDB dialect, existing approach to construct DuckDBDialect {} can't be used and needs to be updated to DuckDBDialect::new()

@github-actions github-actions bot added the sql SQL Planner label Dec 27, 2024
@sgrebnov sgrebnov marked this pull request as ready for review December 27, 2024 05:44
@sgrebnov sgrebnov force-pushed the sgrebnov/custom-scalar-fn-unparser branch from ac20a64 to 13254d2 Compare December 27, 2024 05:46
@xudong963 xudong963 self-requested a review December 27, 2024 06:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sql SQL Planner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant