From e74cb3c27b521aee8edcec211e0038349b548422 Mon Sep 17 00:00:00 2001 From: Kousuke Saruta Date: Sat, 2 Sep 2023 00:49:10 +0900 Subject: [PATCH 1/4] Revert "Make sqllogictest distinguish NaN from -NaN (#7403)" This reverts commit fd7e612d5f2827cef8aa3d6470d8e2e197fed23f. --- .../sqllogictest/src/engines/conversion.rs | 18 ++-------- datafusion/sqllogictest/test_files/math.slt | 6 ++-- .../sqllogictest/test_files/predicates.slt | 2 +- datafusion/sqllogictest/test_files/scalar.slt | 34 +++++++++---------- datafusion/sqllogictest/test_files/select.slt | 6 ---- 5 files changed, 24 insertions(+), 42 deletions(-) diff --git a/datafusion/sqllogictest/src/engines/conversion.rs b/datafusion/sqllogictest/src/engines/conversion.rs index 2f6fa6263ad4..757b9deb8bd7 100644 --- a/datafusion/sqllogictest/src/engines/conversion.rs +++ b/datafusion/sqllogictest/src/engines/conversion.rs @@ -41,11 +41,7 @@ pub(crate) fn varchar_to_str(value: &str) -> String { pub(crate) fn f16_to_str(value: f16) -> String { if value.is_nan() { - if value.is_sign_positive() { - "NaN".to_string() - } else { - "-NaN".to_string() - } + "NaN".to_string() } else if value == f16::INFINITY { "Infinity".to_string() } else if value == f16::NEG_INFINITY { @@ -57,11 +53,7 @@ pub(crate) fn f16_to_str(value: f16) -> String { pub(crate) fn f32_to_str(value: f32) -> String { if value.is_nan() { - if value.is_sign_positive() { - "NaN".to_string() - } else { - "-NaN".to_string() - } + "NaN".to_string() } else if value == f32::INFINITY { "Infinity".to_string() } else if value == f32::NEG_INFINITY { @@ -73,11 +65,7 @@ pub(crate) fn f32_to_str(value: f32) -> String { pub(crate) fn f64_to_str(value: f64) -> String { if value.is_nan() { - if value.is_sign_positive() { - "NaN".to_string() - } else { - "-NaN".to_string() - } + "NaN".to_string() } else if value == f64::INFINITY { "Infinity".to_string() } else if value == f64::NEG_INFINITY { diff --git a/datafusion/sqllogictest/test_files/math.slt b/datafusion/sqllogictest/test_files/math.slt index 5ad9e9475603..cd55e018e99c 100644 --- a/datafusion/sqllogictest/test_files/math.slt +++ b/datafusion/sqllogictest/test_files/math.slt @@ -95,10 +95,10 @@ SELECT atan2(2.0, 1.0), atan2(-2.0, 1.0), atan2(2.0, -1.0), atan2(-2.0, -1.0), a 1.107148717794 -1.107148717794 2.034443935796 -2.034443935796 NULL NULL NULL # nanvl -query RRB -SELECT nanvl(asin(10), 1.0), nanvl(1.0, 2.0), isnan(nanvl(asin(10), asin(10))) +query RRR +SELECT nanvl(asin(10), 1.0), nanvl(1.0, 2.0), nanvl(asin(10), asin(10)) ---- -1 1 true +1 1 NaN # isnan query BBBB diff --git a/datafusion/sqllogictest/test_files/predicates.slt b/datafusion/sqllogictest/test_files/predicates.slt index 0a16d86d3fdc..a168dfe361b9 100644 --- a/datafusion/sqllogictest/test_files/predicates.slt +++ b/datafusion/sqllogictest/test_files/predicates.slt @@ -286,7 +286,7 @@ SELECT * FROM test_float WHERE column1 IN (0.0, 1.2, 'NaN'::double, '-NaN'::doub ---- 1.2 NaN --NaN +NaN ### # Test logical plan simplifies large OR chains diff --git a/datafusion/sqllogictest/test_files/scalar.slt b/datafusion/sqllogictest/test_files/scalar.slt index ca52163758b9..49782164c026 100644 --- a/datafusion/sqllogictest/test_files/scalar.slt +++ b/datafusion/sqllogictest/test_files/scalar.slt @@ -543,10 +543,10 @@ select ln(0); query RRR rowsort select round(ln(a), 5), round(ln(b), 5), round(ln(c), 5) from signed_integers; ---- --NaN 4.60517 -NaN --NaN 9.21034 -NaN -0.69315 -NaN 4.81218 +0.69315 NaN 4.81218 1.38629 NULL NULL +NaN 4.60517 NaN +NaN 9.21034 NaN ## log @@ -595,10 +595,10 @@ Infinity 2 2 query RRR rowsort select log(a, 64) a, log(b), log(10, b) from signed_integers; ---- --NaN 2 2 --NaN 4 4 3 NULL NULL -6 -NaN -NaN +6 NaN NaN +NaN 2 2 +NaN 4 4 ## log10 @@ -625,10 +625,10 @@ select log10(0); query RRR rowsort select round(log(a), 5), round(log(b), 5), round(log(c), 5) from signed_integers; ---- --NaN 2 -NaN --NaN 4 -NaN -0.30103 -NaN 2.08991 +0.30103 NaN 2.08991 0.60206 NULL NULL +NaN 2 NaN +NaN 4 NaN ## log2 @@ -655,10 +655,10 @@ select log2(0); query RRR rowsort select round(log2(a), 5), round(log2(b), 5), round(log2(c), 5) from signed_integers; ---- --NaN 13.28771 -NaN --NaN 6.64386 -NaN -1 -NaN 6.94251 +1 NaN 6.94251 2 NULL NULL +NaN 13.28771 NaN +NaN 6.64386 NaN ## nanvl @@ -779,10 +779,10 @@ NULL query RRR rowsort select round(power(a, b), 5), round(power(c, d), 5), round(power(e, f), 5) from small_floats; ---- --NaN -NaN 2.32282 -1.1487 0 -NaN +1.1487 0 NaN 1.17462 1 0.31623 NULL NULL NULL +NaN NaN 2.32282 ## radians @@ -918,10 +918,10 @@ NULL query RRR rowsort select round(sqrt(a), 5), round(sqrt(b), 5), round(sqrt(c), 5) from signed_integers; ---- --NaN 10 -NaN --NaN 100 -NaN -1.41421 -NaN 11.09054 +1.41421 NaN 11.09054 2 NULL NULL +NaN 10 NaN +NaN 100 NaN ## tan diff --git a/datafusion/sqllogictest/test_files/select.slt b/datafusion/sqllogictest/test_files/select.slt index 96c2370c58ca..7b39bccf3f26 100644 --- a/datafusion/sqllogictest/test_files/select.slt +++ b/datafusion/sqllogictest/test_files/select.slt @@ -224,12 +224,6 @@ select ---- false true false true true false false true false true true false true true false false true -# select NaNs -query RRRR -select 'NaN'::double a, '-NaN'::double b, 'NaN'::float c, '-NaN'::float d ----- -NaN -NaN NaN -NaN - # select limit clause query I select * from (select 1 a union all select 2) b order by a limit 1; From aa6bcf94f58f9054fe08f2f5d15a733dd67b1a77 Mon Sep 17 00:00:00 2001 From: Kousuke Saruta Date: Sat, 2 Sep 2023 01:12:05 +0900 Subject: [PATCH 2/4] Make sqllogictests platform-independent for the sign of NaNs --- .../sqllogictest/test_files/predicates.slt | 46 +++++++++++-------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/datafusion/sqllogictest/test_files/predicates.slt b/datafusion/sqllogictest/test_files/predicates.slt index a168dfe361b9..0c0dd9a5cbd7 100644 --- a/datafusion/sqllogictest/test_files/predicates.slt +++ b/datafusion/sqllogictest/test_files/predicates.slt @@ -254,39 +254,45 @@ foo fazzz statement ok -CREATE TABLE IF NOT EXISTS test_float AS VALUES(1.2),(2.1),(NULL),('NaN'::double),('-NaN'::double); +CREATE TABLE IF NOT EXISTS test_float AS VALUES + ('a', 1.2), + ('b', 2.1), + ('c', NULL), + ('d', 'NaN'::double), + ('e', '-NaN'::double) + ; # IN expr for float -query R -SELECT * FROM test_float WHERE column1 IN (0.0, -1.2) +query T +SELECT column1 FROM test_float WHERE column2 IN (0.0, -1.2) ---- -query R -SELECT * FROM test_float WHERE column1 IN (0.0, 1.2) +query T +SELECT column1 FROM test_float WHERE column2 IN (0.0, 1.2) ---- -1.2 +a -query R -SELECT * FROM test_float WHERE column1 IN (2.1, 1.2) +query T +SELECT column1 FROM test_float WHERE column2 IN (2.1, 1.2) ---- -1.2 -2.1 +a +b -query R -SELECT * FROM test_float WHERE column1 IN (0.0, 1.2, NULL) +query T +SELECT column1 FROM test_float WHERE column2 IN (0.0, 1.2, NULL) ---- -1.2 +a -query R -SELECT * FROM test_float WHERE column1 IN (0.0, -1.2, NULL) +query T +SELECT column1 FROM test_float WHERE column2 IN (0.0, -1.2, NULL) ---- -query R -SELECT * FROM test_float WHERE column1 IN (0.0, 1.2, 'NaN'::double, '-NaN'::double) +query T +SELECT column1 FROM test_float WHERE column2 IN (0.0, 1.2, 'NaN'::double, '-NaN'::double) ---- -1.2 -NaN -NaN +a +d +e ### # Test logical plan simplifies large OR chains From 350d22058645267d10de87cda635a9ef9f5ce73e Mon Sep 17 00:00:00 2001 From: Kousuke Saruta Date: Sat, 2 Sep 2023 02:00:14 +0900 Subject: [PATCH 3/4] Add comment --- datafusion/sqllogictest/src/engines/conversion.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/datafusion/sqllogictest/src/engines/conversion.rs b/datafusion/sqllogictest/src/engines/conversion.rs index 757b9deb8bd7..909539b3131b 100644 --- a/datafusion/sqllogictest/src/engines/conversion.rs +++ b/datafusion/sqllogictest/src/engines/conversion.rs @@ -41,6 +41,8 @@ pub(crate) fn varchar_to_str(value: &str) -> String { pub(crate) fn f16_to_str(value: f16) -> String { if value.is_nan() { + // The sign of NaN can be different depending on platform. + // So the string representation of NaN ignores the sign. "NaN".to_string() } else if value == f16::INFINITY { "Infinity".to_string() @@ -53,6 +55,8 @@ pub(crate) fn f16_to_str(value: f16) -> String { pub(crate) fn f32_to_str(value: f32) -> String { if value.is_nan() { + // The sign of NaN can be different depending on platform. + // So the string representation of NaN ignores the sign. "NaN".to_string() } else if value == f32::INFINITY { "Infinity".to_string() @@ -65,6 +69,8 @@ pub(crate) fn f32_to_str(value: f32) -> String { pub(crate) fn f64_to_str(value: f64) -> String { if value.is_nan() { + // The sign of NaN can be different depending on platform. + // So the string representation of NaN ignores the sign. "NaN".to_string() } else if value == f64::INFINITY { "Infinity".to_string() From 1a5b5a87f0f1baa345e33fc611976cea3977f9ba Mon Sep 17 00:00:00 2001 From: Kousuke Saruta Date: Thu, 7 Sep 2023 19:39:16 +0900 Subject: [PATCH 4/4] Ensure -NaN is parsed as -NaN --- datafusion/sqllogictest/test_files/select.slt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/datafusion/sqllogictest/test_files/select.slt b/datafusion/sqllogictest/test_files/select.slt index 7b39bccf3f26..a7ed2bf5c743 100644 --- a/datafusion/sqllogictest/test_files/select.slt +++ b/datafusion/sqllogictest/test_files/select.slt @@ -224,6 +224,12 @@ select ---- false true false true true false false true false true true false true true false false true +# select NaNs +query BBBB +select (isnan('NaN'::double) AND 'NaN'::double > 0) a, (isnan('-NaN'::double) AND '-NaN'::double < 0) b, (isnan('NaN'::float) AND 'NaN'::float > 0) c, (isnan('-NaN'::float) AND '-NaN'::float < 0) d +---- +true true true true + # select limit clause query I select * from (select 1 a union all select 2) b order by a limit 1;