Skip to content

use dialect in SqlExec::sql#353

Closed
kczimm wants to merge 1 commit into
datafusion-contrib:spiceaifrom
kczimm:kczimm/use-dialect-in-sql-exec
Closed

use dialect in SqlExec::sql#353
kczimm wants to merge 1 commit into
datafusion-contrib:spiceaifrom
kczimm:kczimm/use-dialect-in-sql-exec

Conversation

@kczimm
Copy link
Copy Markdown

@kczimm kczimm commented May 20, 2025

Use the dialect given to SqlTable in SqlExec::sql.

} else {
let dialect = self.engine.map_or(
Arc::new(DefaultDialect {}) as Arc<dyn Dialect + Send + Sync>,
self.dialect.clone().unwrap_or(Arc::new(DefaultDialect {})),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for learning purpose: when would the .sql() method be used when it uses the dialect that's other than the engine.dialect()

Comment thread src/duckdb/sql_table.rs
filters,
limit,
Some(Engine::DuckDB),
None,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread src/mysql/sql_table.rs
filters,
limit,
Some(Engine::MySQL),
None,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines +281 to +288
let quote = |name| {
let quote = self
.dialect
.as_ref()
.and_then(|d| d.identifier_quote_style(name))
.unwrap_or('"');
format!("{quote}{name}{quote}")
};
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this inline closure be moved to its own function?

Comment thread src/sqlite/sql_table.rs
filters,
limit,
Some(Engine::SQLite),
None,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The base table on the SQLiteTable has a dialect that could be passed down.

@kczimm
Copy link
Copy Markdown
Author

kczimm commented May 21, 2025

Closing this in favor of a different solution

@kczimm kczimm closed this May 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants