mirrors/datafusion-sqlparse
1
0
Fork 0
mirror of https://github.com/apache/datafusion-sqlparser-rs.git synced 2025-07-14 12:05:00 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Don't lose precision when parsing decimal fractions

Browse source 
The SQL standard requires that numeric literals with a decimal point,
like 1.23, are represented exactly, up to some precision. That means
that parsing these literals into f64s is invalid, as it is impossible
to represent many decimal numbers exactly in binary floating point (for
example, 0.3).

This commit parses all numeric literals into a new `Value` variant
`Number(String)`, removing the old `Long(u64)` and `Double(f64)`
variants. This is slightly less convenient for downstream consumers, but
far more flexible, as numbers that do not fit into a u64 and f64 are now
representable.
This commit is contained in:
Nikhil Benesch 2019-07-08 14:15:16 -04:00
parent 2bef9ec30a
commit b5621c0fe8
No known key found for this signature in database
GPG key ID: FCF98542083C5A69
5 changed files with 82 additions and 77 deletions
Download patch file Download diff file Expand all files Collapse all files

1
Cargo.toml
View file

@ -20,7 +20,6 @@ path = "src/lib.rs"
[dependencies]
log = "0.4.5"
ordered-float = "1.0.2"
[dev-dependencies]
simple_logger = "1.0.1"

10
src/ast/value.rs
View file

@ -10,16 +10,13 @@
// See the License for the specific language governing permissions and
// limitations under the License.
use ordered_float::OrderedFloat;
use std::fmt;
/// Primitive SQL values such as number and string
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub enum Value {
/// Unsigned integer value
Long(u64),
/// Unsigned floating point value
Double(OrderedFloat<f64>),
/// Numeric literal
Number(String),
/// 'string value'
SingleQuotedString(String),
/// N'string value'
@ -60,8 +57,7 @@ pub enum Value {
impl fmt::Display for Value {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
Value::Long(v) => write!(f, "{}", v),
Value::Double(v) => write!(f, "{}", v),
Value::Number(v) => write!(f, "{}", v),
Value::SingleQuotedString(v) => write!(f, "'{}'", escape_single_quote_string(v)),
Value::NationalStringLiteral(v) => write!(f, "N'{}'", v),
Value::HexStringLiteral(v) => write!(f, "X'{}'", v),

13
src/parser.rs
View file

@ -1183,14 +1183,7 @@ impl Parser {
return parser_err!(format!("No value parser for keyword {}", k.keyword));
}
},
Token::Number(ref n) if n.contains('.') => match n.parse::<f64>() {
Ok(n) => Ok(Value::Double(n.into())),
Err(e) => parser_err!(format!("Could not parse '{}' as f64: {}", n, e)),
},
Token::Number(ref n) => match n.parse::<u64>() {
Ok(n) => Ok(Value::Long(n)),
Err(e) => parser_err!(format!("Could not parse '{}' as u64: {}", n, e)),
},
Token::Number(ref n) => Ok(Value::Number(n.to_string())),
Token::SingleQuotedString(ref s) => Ok(Value::SingleQuotedString(s.to_string())),
Token::NationalStringLiteral(ref s) => {
Ok(Value::NationalStringLiteral(s.to_string()))
@ -1864,7 +1857,7 @@ impl Parser {
Ok(None)
} else {
self.parse_literal_uint()
.map(|n| Some(Expr::Value(Value::Long(n))))
.map(|n| Some(Expr::Value(Value::Number(n.to_string()))))
}
}
@ -1872,7 +1865,7 @@ impl Parser {
pub fn parse_offset(&mut self) -> Result<Expr, ParserError> {
let value = self
.parse_literal_uint()
.map(|n| Expr::Value(Value::Long(n)))?;
.map(|n| Expr::Value(Value::Number(n.to_string())))?;
self.expect_one_of_keywords(&["ROW", "ROWS"])?;
Ok(value)
}

131
tests/sqlparser_common.rs
View file

@ -27,9 +27,9 @@ use sqlparser::test_utils::{all_dialects, expr_from_projection, only};
#[test]
fn parse_insert_values() {
let row = vec![
Expr::Value(Value::Long(1)),
Expr::Value(Value::Long(2)),
Expr::Value(Value::Long(3)),
Expr::Value(Value::Number("1".into())),
Expr::Value(Value::Number("2".into())),
Expr::Value(Value::Number("3".into())),
];
let rows1 = vec![row.clone()];
let rows2 = vec![row.clone(), row];
@ -107,15 +107,15 @@ fn parse_update() {
vec![
Assignment {
id: "a".into(),
value: Expr::Value(Value::Long(1)),
value: Expr::Value(Value::Number("1".into())),
},
Assignment {
id: "b".into(),
value: Expr::Value(Value::Long(2)),
value: Expr::Value(Value::Number("2".into())),
},
Assignment {
id: "c".into(),
value: Expr::Value(Value::Long(3)),
value: Expr::Value(Value::Number("3".into())),
},
]
);
@ -181,7 +181,7 @@ fn parse_where_delete_statement() {
Expr::BinaryOp {
left: Box::new(Expr::Identifier("name".to_string())),
op: Eq,
right: Box::new(Expr::Value(Value::Long(5))),
right: Box::new(Expr::Value(Value::Number("5".into()))),
},
selection.unwrap(),
);
@ -205,17 +205,17 @@ fn parse_simple_select() {
assert_eq!(false, select.distinct);
assert_eq!(3, select.projection.len());
let select = verified_query(sql);
assert_eq!(Some(Expr::Value(Value::Long(5))), select.limit);
assert_eq!(Some(Expr::Value(Value::Number("5".into()))), select.limit);
}
#[test]
fn parse_limit_is_not_an_alias() {
// In dialects supporting LIMIT it shouldn't be parsed as a table alias
let ast = verified_query("SELECT id FROM customer LIMIT 1");
assert_eq!(Some(Expr::Value(Value::Long(1))), ast.limit);
assert_eq!(Some(Expr::Value(Value::Number("1".into()))), ast.limit);
let ast = verified_query("SELECT 1 LIMIT 5");
assert_eq!(Some(Expr::Value(Value::Long(5))), ast.limit);
assert_eq!(Some(Expr::Value(Value::Number("5".into()))), ast.limit);
}
#[test]
@ -286,7 +286,7 @@ fn parse_column_aliases() {
} = only(&select.projection)
{
assert_eq!(&BinaryOperator::Plus, op);
assert_eq!(&Expr::Value(Value::Long(1)), right.as_ref());
assert_eq!(&Expr::Value(Value::Number("1".into())), right.as_ref());
assert_eq!("newname", alias);
} else {
panic!("Expected ExprWithAlias")
@ -527,9 +527,9 @@ fn parse_not_precedence() {
Expr::UnaryOp {
op: UnaryOperator::Not,
expr: Box::new(Expr::Between {
expr: Box::new(Expr::Value(Value::Long(1))),
low: Box::new(Expr::Value(Value::Long(1))),
high: Box::new(Expr::Value(Value::Long(2))),
expr: Box::new(Expr::Value(Value::Number("1".into()))),
low: Box::new(Expr::Value(Value::Number("1".into()))),
high: Box::new(Expr::Value(Value::Number("2".into()))),
negated: true,
}),
},
@ -658,8 +658,8 @@ fn parse_between() {
assert_eq!(
Expr::Between {
expr: Box::new(Expr::Identifier("age".to_string())),
low: Box::new(Expr::Value(Value::Long(25))),
high: Box::new(Expr::Value(Value::Long(32))),
low: Box::new(Expr::Value(Value::Number("25".into()))),
high: Box::new(Expr::Value(Value::Number("32".into()))),
negated,
},
select.selection.unwrap()
@ -676,16 +676,16 @@ fn parse_between_with_expr() {
let select = verified_only_select(sql);
assert_eq!(
Expr::IsNull(Box::new(Expr::Between {
expr: Box::new(Expr::Value(Value::Long(1))),
expr: Box::new(Expr::Value(Value::Number("1".into()))),
low: Box::new(Expr::BinaryOp {
left: Box::new(Expr::Value(Value::Long(1))),
left: Box::new(Expr::Value(Value::Number("1".into()))),
op: Plus,
right: Box::new(Expr::Value(Value::Long(2))),
right: Box::new(Expr::Value(Value::Number("2".into()))),
}),
high: Box::new(Expr::BinaryOp {
left: Box::new(Expr::Value(Value::Long(3))),
left: Box::new(Expr::Value(Value::Number("3".into()))),
op: Plus,
right: Box::new(Expr::Value(Value::Long(4))),
right: Box::new(Expr::Value(Value::Number("4".into()))),
}),
negated: false,
})),
@ -697,19 +697,19 @@ fn parse_between_with_expr() {
assert_eq!(
Expr::BinaryOp {
left: Box::new(Expr::BinaryOp {
left: Box::new(Expr::Value(Value::Long(1))),
left: Box::new(Expr::Value(Value::Number("1".into()))),
op: BinaryOperator::Eq,
right: Box::new(Expr::Value(Value::Long(1))),
right: Box::new(Expr::Value(Value::Number("1".into()))),
}),
op: BinaryOperator::And,
right: Box::new(Expr::Between {
expr: Box::new(Expr::BinaryOp {
left: Box::new(Expr::Value(Value::Long(1))),
left: Box::new(Expr::Value(Value::Number("1".into()))),
op: BinaryOperator::Plus,
right: Box::new(Expr::Identifier("x".to_string())),
}),
low: Box::new(Expr::Value(Value::Long(1))),
high: Box::new(Expr::Value(Value::Long(2))),
low: Box::new(Expr::Value(Value::Number("1".into()))),
high: Box::new(Expr::Value(Value::Number("2".into()))),
negated: false,
}),
},
@ -763,7 +763,7 @@ fn parse_select_order_by_limit() {
],
select.order_by
);
assert_eq!(Some(Expr::Value(Value::Long(2))), select.limit);
assert_eq!(Some(Expr::Value(Value::Number("2".into()))), select.limit);
}
#[test]
@ -792,7 +792,7 @@ fn parse_select_having() {
distinct: false
})),
op: BinaryOperator::Gt,
right: Box::new(Expr::Value(Value::Long(1)))
right: Box::new(Expr::Value(Value::Number("1".into())))
}),
select.having
);
@ -988,7 +988,7 @@ fn parse_create_table_with_options() {
},
SqlOption {
name: "a".into(),
value: Value::Long(123)
value: Value::Number("123".into())
},
],
with_options
@ -1178,6 +1178,23 @@ fn parse_aggregate_with_group_by() {
//TODO: assertions
}
#[test]
fn parse_literal_decimal() {
// These numbers were explicitly chosen to not roundtrip if represented as
// f64s (i.e., as 64-bit binary floating point numbers).
let sql = "SELECT 0.300000000000000004, 9007199254740993.0";
let select = verified_only_select(sql);
assert_eq!(2, select.projection.len());
assert_eq!(
&Expr::Value(Value::Number("0.300000000000000004".into())),
expr_from_projection(&select.projection[0]),
);
assert_eq!(
&Expr::Value(Value::Number("9007199254740993.0".into())),
expr_from_projection(&select.projection[1]),
)
}
#[test]
fn parse_literal_string() {
let sql = "SELECT 'one', N'national string', X'deadBEEF'";
@ -1421,12 +1438,12 @@ fn parse_searched_case_expr() {
BinaryOp {
left: Box::new(Identifier("bar".to_string())),
op: Eq,
right: Box::new(Expr::Value(Value::Long(0)))
right: Box::new(Expr::Value(Value::Number("0".into())))
},
BinaryOp {
left: Box::new(Identifier("bar".to_string())),
op: GtEq,
right: Box::new(Expr::Value(Value::Long(0)))
right: Box::new(Expr::Value(Value::Number("0".into())))
}
],
results: vec![
@ -1451,7 +1468,7 @@ fn parse_simple_case_expr() {
assert_eq!(
&Case {
operand: Some(Box::new(Identifier("foo".to_string()))),
conditions: vec![Expr::Value(Value::Long(1))],
conditions: vec![Expr::Value(Value::Number("1".into()))],
results: vec![Expr::Value(Value::SingleQuotedString("Y".to_string())),],
else_result: Some(Box::new(Expr::Value(Value::SingleQuotedString(
"N".to_string()
@ -2063,7 +2080,7 @@ fn parse_create_view_with_options() {
},
SqlOption {
name: "a".into(),
value: Value::Long(123)
value: Value::Number("123".into())
},
],
with_options
@ -2197,26 +2214,26 @@ fn parse_invalid_subquery_without_parens() {
#[test]
fn parse_offset() {
let ast = verified_query("SELECT foo FROM bar OFFSET 2 ROWS");
assert_eq!(ast.offset, Some(Expr::Value(Value::Long(2))));
assert_eq!(ast.offset, Some(Expr::Value(Value::Number("2".into()))));
let ast = verified_query("SELECT foo FROM bar WHERE foo = 4 OFFSET 2 ROWS");
assert_eq!(ast.offset, Some(Expr::Value(Value::Long(2))));
assert_eq!(ast.offset, Some(Expr::Value(Value::Number("2".into()))));
let ast = verified_query("SELECT foo FROM bar ORDER BY baz OFFSET 2 ROWS");
assert_eq!(ast.offset, Some(Expr::Value(Value::Long(2))));
assert_eq!(ast.offset, Some(Expr::Value(Value::Number("2".into()))));
let ast = verified_query("SELECT foo FROM bar WHERE foo = 4 ORDER BY baz OFFSET 2 ROWS");
assert_eq!(ast.offset, Some(Expr::Value(Value::Long(2))));
assert_eq!(ast.offset, Some(Expr::Value(Value::Number("2".into()))));
let ast = verified_query("SELECT foo FROM (SELECT * FROM bar OFFSET 2 ROWS) OFFSET 2 ROWS");
assert_eq!(ast.offset, Some(Expr::Value(Value::Long(2))));
assert_eq!(ast.offset, Some(Expr::Value(Value::Number("2".into()))));
match ast.body {
SetExpr::Select(s) => match only(s.from).relation {
TableFactor::Derived { subquery, .. } => {
assert_eq!(subquery.offset, Some(Expr::Value(Value::Long(2))));
assert_eq!(subquery.offset, Some(Expr::Value(Value::Number("2".into()))));
}
_ => panic!("Test broke"),
},
_ => panic!("Test broke"),
}
let ast = verified_query("SELECT 'foo' OFFSET 0 ROWS");
assert_eq!(ast.offset, Some(Expr::Value(Value::Long(0))));
assert_eq!(ast.offset, Some(Expr::Value(Value::Number("0".into()))));
}
#[test]
@ -2229,15 +2246,15 @@ fn parse_singular_row_offset() {
#[test]
fn parse_fetch() {
const FETCH_FIRST_TWO_ROWS_ONLY: Fetch = Fetch {
let fetch_first_two_rows_only = Some(Fetch {
with_ties: false,
percent: false,
quantity: Some(Expr::Value(Value::Long(2))),
};
quantity: Some(Expr::Value(Value::Number("2".into()))),
});
let ast = verified_query("SELECT foo FROM bar FETCH FIRST 2 ROWS ONLY");
assert_eq!(ast.fetch, Some(FETCH_FIRST_TWO_ROWS_ONLY));
assert_eq!(ast.fetch, fetch_first_two_rows_only);
let ast = verified_query("SELECT 'foo' FETCH FIRST 2 ROWS ONLY");
assert_eq!(ast.fetch, Some(FETCH_FIRST_TWO_ROWS_ONLY));
assert_eq!(ast.fetch, fetch_first_two_rows_only);
let ast = verified_query("SELECT foo FROM bar FETCH FIRST ROWS ONLY");
assert_eq!(
ast.fetch,
@ -2248,9 +2265,9 @@ fn parse_fetch() {
})
);
let ast = verified_query("SELECT foo FROM bar WHERE foo = 4 FETCH FIRST 2 ROWS ONLY");
assert_eq!(ast.fetch, Some(FETCH_FIRST_TWO_ROWS_ONLY));
assert_eq!(ast.fetch, fetch_first_two_rows_only);
let ast = verified_query("SELECT foo FROM bar ORDER BY baz FETCH FIRST 2 ROWS ONLY");
assert_eq!(ast.fetch, Some(FETCH_FIRST_TWO_ROWS_ONLY));
assert_eq!(ast.fetch, fetch_first_two_rows_only);
let ast = verified_query(
"SELECT foo FROM bar WHERE foo = 4 ORDER BY baz FETCH FIRST 2 ROWS WITH TIES",
);
@ -2259,7 +2276,7 @@ fn parse_fetch() {
Some(Fetch {
with_ties: true,
percent: false,
quantity: Some(Expr::Value(Value::Long(2))),
quantity: Some(Expr::Value(Value::Number("2".into()))),
})
);
let ast = verified_query("SELECT foo FROM bar FETCH FIRST 50 PERCENT ROWS ONLY");
@ -2268,35 +2285,35 @@ fn parse_fetch() {
Some(Fetch {
with_ties: false,
percent: true,
quantity: Some(Expr::Value(Value::Long(50))),
quantity: Some(Expr::Value(Value::Number("50".into()))),
})
);
let ast = verified_query(
"SELECT foo FROM bar WHERE foo = 4 ORDER BY baz OFFSET 2 ROWS FETCH FIRST 2 ROWS ONLY",
);
assert_eq!(ast.offset, Some(Expr::Value(Value::Long(2))));
assert_eq!(ast.fetch, Some(FETCH_FIRST_TWO_ROWS_ONLY));
assert_eq!(ast.offset, Some(Expr::Value(Value::Number("2".into()))));
assert_eq!(ast.fetch, fetch_first_two_rows_only);
let ast = verified_query(
"SELECT foo FROM (SELECT * FROM bar FETCH FIRST 2 ROWS ONLY) FETCH FIRST 2 ROWS ONLY",
);
assert_eq!(ast.fetch, Some(FETCH_FIRST_TWO_ROWS_ONLY));
assert_eq!(ast.fetch, fetch_first_two_rows_only);
match ast.body {
SetExpr::Select(s) => match only(s.from).relation {
TableFactor::Derived { subquery, .. } => {
assert_eq!(subquery.fetch, Some(FETCH_FIRST_TWO_ROWS_ONLY));
assert_eq!(subquery.fetch, fetch_first_two_rows_only);
}
_ => panic!("Test broke"),
},
_ => panic!("Test broke"),
}
let ast = verified_query("SELECT foo FROM (SELECT * FROM bar OFFSET 2 ROWS FETCH FIRST 2 ROWS ONLY) OFFSET 2 ROWS FETCH FIRST 2 ROWS ONLY");
assert_eq!(ast.offset, Some(Expr::Value(Value::Long(2))));
assert_eq!(ast.fetch, Some(FETCH_FIRST_TWO_ROWS_ONLY));
assert_eq!(ast.offset, Some(Expr::Value(Value::Number("2".into()))));
assert_eq!(ast.fetch, fetch_first_two_rows_only);
match ast.body {
SetExpr::Select(s) => match only(s.from).relation {
TableFactor::Derived { subquery, .. } => {
assert_eq!(subquery.offset, Some(Expr::Value(Value::Long(2))));
assert_eq!(subquery.fetch, Some(FETCH_FIRST_TWO_ROWS_ONLY));
assert_eq!(subquery.offset, Some(Expr::Value(Value::Number("2".into()))));
assert_eq!(subquery.fetch, fetch_first_two_rows_only);
}
_ => panic!("Test broke"),
},

4
tests/sqlparser_postgres.rs
View file

@ -163,7 +163,7 @@ fn parse_create_table_with_defaults() {
vec![
SqlOption {
name: "fillfactor".into(),
value: Value::Long(20)
value: Value::Number("20".into())
},
SqlOption {
name: "user_catalog_table".into(),
@ -171,7 +171,7 @@ fn parse_create_table_with_defaults() {
},
SqlOption {
name: "autovacuum_vacuum_threshold".into(),
value: Value::Long(100)
value: Value::Number("100".into())
},
]
);