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.

Sign up for GitHub

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

Jump to bottom

Add dialect param to use double precision for float64 in Postgres #11495

Merged
merged 5 commits into from
Jul 19, 2024
Merged

Add dialect param to use double precision for float64 in Postgres #11495

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
eb1c513
Add dialect param to use double precision for float64 in Postgres
Sevenannn Jul 16, 2024
d0d8b14
return ast data type instead of bool
Sevenannn Jul 16, 2024
3a41a9b
Merge branch 'main' into qianqian/float64-unparsing
Sevenannn Jul 18, 2024
7405dc8
Fix errors in merging
Sevenannn Jul 18, 2024
4d2b8e2
fix
Sevenannn Jul 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions datafusion/sql/src/unparser/dialect.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,12 @@ pub trait Dialect {
fn interval_style(&self) -> IntervalStyle {
IntervalStyle::PostgresVerbose
}

// Does the dialect use DOUBLE PRECISION to represent Float64 rather than DOUBLE?
// E.g. Postgres uses DOUBLE PRECISION instead of DOUBLE
fn use_double_precision_for_float64(&self) -> bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

Similarly to #11494 (comment) what do you think about making this simply return the AST type directly?

Suggested change
fn use_double_precision_for_float64(&self) -> bool {
fn use_double_precision_for_float64(&self) -> sqlparser::ast::DataType {

That is likely a more flexible API -- this formulation seems very narrow for postgres vs MySQL

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @alamb , thanks for the feedback! Directly return AST type seems clearer and more flexible to me. I update the changes in my newest commit, please take a look when you are available:

  • Return sqlparser::ast::DataType instead of bool
  • Change the method name from use_double_precision_for_float64 to float64_ast_dtype

false
}
}

/// `IntervalStyle` to use for unparsing
Expand Down Expand Up @@ -118,6 +124,7 @@ pub struct CustomDialect {
supports_nulls_first_in_sort: bool,
use_timestamp_for_date64: bool,
interval_style: IntervalStyle,
use_double_precision_for_float64: bool,
}

impl Default for CustomDialect {
Expand All @@ -127,6 +134,7 @@ impl Default for CustomDialect {
supports_nulls_first_in_sort: true,
use_timestamp_for_date64: false,
interval_style: IntervalStyle::SQLStandard,
use_double_precision_for_float64: false,
}
}
}
Expand Down Expand Up @@ -158,6 +166,10 @@ impl Dialect for CustomDialect {
fn interval_style(&self) -> IntervalStyle {
self.interval_style
}

fn use_double_precision_for_float64(&self) -> bool {
self.use_double_precision_for_float64
}
}

/// `CustomDialectBuilder` to build `CustomDialect` using builder pattern
Expand All @@ -179,6 +191,7 @@ pub struct CustomDialectBuilder {
supports_nulls_first_in_sort: bool,
use_timestamp_for_date64: bool,
interval_style: IntervalStyle,
use_double_precision_for_float64: bool,
}

impl Default for CustomDialectBuilder {
Expand All @@ -194,6 +207,7 @@ impl CustomDialectBuilder {
supports_nulls_first_in_sort: true,
use_timestamp_for_date64: false,
interval_style: IntervalStyle::PostgresVerbose,
use_double_precision_for_float64: false,
}
}

Expand All @@ -203,6 +217,7 @@ impl CustomDialectBuilder {
supports_nulls_first_in_sort: self.supports_nulls_first_in_sort,
use_timestamp_for_date64: self.use_timestamp_for_date64,
interval_style: self.interval_style,
use_double_precision_for_float64: self.use_double_precision_for_float64,
}
}

Expand Down Expand Up @@ -235,4 +250,12 @@ impl CustomDialectBuilder {
self.interval_style = interval_style;
self
}

pub fn with_use_double_precision_for_float64(
mut self,
use_double_precision_for_float64: bool,
) -> Self {
self.use_double_precision_for_float64 = use_double_precision_for_float64;
self
}
}
30 changes: 29 additions & 1 deletion datafusion/sql/src/unparser/expr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1237,7 +1237,11 @@ impl Unparser<'_> {
not_impl_err!("Unsupported DataType: conversion: {data_type:?}")
}
DataType::Float32 => Ok(ast::DataType::Float(None)),
DataType::Float64 => Ok(ast::DataType::Double),
DataType::Float64 => Ok(if self.dialect.use_double_precision_for_float64() {
ast::DataType::DoublePrecision
} else {
ast::DataType::Double
}),
DataType::Timestamp(_, tz) => {
let tz_info = match tz {
Some(_) => TimezoneInfo::WithTimeZone,
Expand Down Expand Up @@ -1819,6 +1823,30 @@ mod tests {
Ok(())
}

#[test]
fn custom_dialect_use_double_precision_for_float64() -> Result<()> {
for (use_double_precision_for_float64, identifier) in
[(false, "DOUBLE"), (true, "DOUBLE PRECISION")]
{
let dialect = CustomDialectBuilder::new()
.with_use_double_precision_for_float64(use_double_precision_for_float64)
.build();
let unparser = Unparser::new(&dialect);

let expr = Expr::Cast(Cast {
expr: Box::new(col("a")),
data_type: DataType::Float64,
});
let ast = unparser.expr_to_sql(&expr)?;

let actual = format!("{}", ast);

let expected = format!(r#"CAST(a AS {identifier})"#);
assert_eq!(actual, expected);
}
Ok(())
}

#[test]
fn customer_dialect_support_nulls_first_in_ort() -> Result<()> {
let tests: Vec<(Expr, &str, bool)> = vec![
Expand Down