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

Allow testing records with sibling whitespace in SLT tests and add more string tests #13197

Merged
merged 4 commits into from
Nov 1, 2024
Merged

Allow testing records with sibling whitespace in SLT tests and add more string tests #13197

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
02cf32e
empty
findepi Oct 31, 2024
0d5abd6
Allow testing values with trailing whitespace in SLT tests
findepi Oct 31, 2024
1f98c54
Update SLT tests for "Allow testing values with trailing whitespace ..."
findepi Oct 31, 2024
fa96eb3
Add empty string to string test data
findepi Oct 31, 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: 21 additions & 2 deletions datafusion/sqllogictest/bin/sqllogictests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use std::path::{Path, PathBuf};
use clap::Parser;
use datafusion_sqllogictest::{DataFusion, TestContext};
use futures::stream::StreamExt;
use itertools::Itertools;
use log::info;
use sqllogictest::strict_column_validator;

Expand All @@ -39,6 +40,23 @@ pub fn main() -> Result<()> {
.block_on(run_tests())
}

fn value_validator(actual: &[Vec<String>], expected: &[String]) -> bool {
let expected = expected
.iter()
// Trailing whitespace from lines in SLT will typically be removed, but do not fail if it is not
// If particular test wants to cover trailing whitespace on a value,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is probably a good thing to add in the user documentation (aka https://github.com/apache/datafusion/blob/main/datafusion/sqllogictest/README.md)

I think it would be hard to find here for someone writing sqllogictests

Copy link
Member Author

Choose a reason for hiding this comment

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

it feels so edge-case'y. but yes, we can add

Copy link
Contributor

Choose a reason for hiding this comment

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

I will make a follow on PR

Copy link
Contributor

@alamb alamb Nov 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

// it should project additional non-whitespace column on the right.
.map(|s| s.trim_end().to_owned())
.collect::<Vec<_>>();
let actual = actual
.iter()
.map(|strs| strs.iter().join(" "))
Copy link
Contributor

Choose a reason for hiding this comment

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

we prob can do this in a sinlge map?

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not quite sure what you are suggesting here

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm suggesting to have a single collection iteration instead of two

.map(|strs| strs.iter().join(" ").trim_end().to_owned())

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, we can do this in single map.
i placed this into two map() steps so that the common part of actual and expected processing looks the same and so that i can write a nice comment

// Editors do not preserve trailing whitespace, so expected may or may not lack it included
.map(|s| s.trim_end().to_owned())
.collect::<Vec<_>>();
actual == expected
}

/// Sets up an empty directory at test_files/scratch/<name>
/// creating it if needed and clearing any file contents if it exists
/// This allows tests for inserting to external tables or copy to
Expand Down Expand Up @@ -140,6 +158,7 @@ async fn run_test_file(test_file: TestFile) -> Result<()> {
))
});
runner.with_column_validator(strict_column_validator);
runner.with_validator(value_validator);
runner
.run_file_async(path)
.await
Expand All @@ -158,6 +177,7 @@ async fn run_test_file_with_postgres(test_file: TestFile) -> Result<()> {
let mut runner =
sqllogictest::Runner::new(|| Postgres::connect(relative_path.clone()));
runner.with_column_validator(strict_column_validator);
runner.with_validator(value_validator);
runner
.run_file_async(path)
.await
Expand All @@ -176,7 +196,6 @@ async fn run_complete_file(test_file: TestFile) -> Result<()> {
path,
relative_path,
} = test_file;
use sqllogictest::default_validator;

info!("Using complete mode to complete: {}", path.display());

Expand All @@ -196,7 +215,7 @@ async fn run_complete_file(test_file: TestFile) -> Result<()> {
.update_test_file(
path,
col_separator,
default_validator,
value_validator,
strict_column_validator,
)
.await
Expand Down
32 changes: 16 additions & 16 deletions datafusion/sqllogictest/test_files/avro.slt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,22 +198,22 @@ NULL
query IT
SELECT id, CAST(string_col AS varchar) FROM alltypes_plain_multi_files
----
4 0
5 1
6 0
7 1
2 0
3 1
0 0
1 1
4 0
5 1
6 0
7 1
2 0
3 1
0 0
1 1
4 0
5 1
6 0
7 1
2 0
3 1
0 0
1 1
4 0
5 1
6 0
7 1
2 0
3 1
0 0
1 1

# test avro nested records
query ????
Expand Down
4 changes: 2 additions & 2 deletions datafusion/sqllogictest/test_files/map.slt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ SELECT MAP {'a': 1, null: 2}
query ?
SELECT MAP {[1,2,3]:1, [2,4]:2};
----
{[1, 2, 3]: 1, [2, 4]: 2}
{[1, 2, 3]: 1, [2, 4]: 2}

# array with different type as key
# expect to fail due to type coercion error
Expand Down Expand Up @@ -483,7 +483,7 @@ SELECT MAP { MAP {1:'a', 2:'b'}:1, MAP {1:'c', 2:'d'}:2 };
query ?
SELECT MAP { MAP {1:'a', 2:'b', 3:'c'}:1, MAP {2:'c', 4:'d'}:2 };
----
{{1: a, 2: b, 3: c}: 1, {2: c, 4: d}: 2}
{{1: a, 2: b, 3: c}: 1, {2: c, 4: d}: 2}

# map as value
query ?
Expand Down
6 changes: 3 additions & 3 deletions datafusion/sqllogictest/test_files/pg_compat/pg_compat_types.slt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,18 @@ NULL
query TT
select 'a'::VARCHAR, ''::VARCHAR
----
a (empty)
a (empty)

skipif postgres
query TT
select 'a'::CHAR, ''::CHAR
----
a (empty)
a (empty)

query TT
select 'a'::TEXT, ''::TEXT
----
a (empty)
a (empty)

skipif postgres
query I
Expand Down
6 changes: 3 additions & 3 deletions datafusion/sqllogictest/test_files/select.slt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ VALUES (-1)
query IIB
VALUES (2+1,2-1,2>1)
----
3 1 true
3 1 true

# multiple rows values
query I rowsort
Expand All @@ -472,8 +472,8 @@ VALUES (1),(2)
query IT rowsort
VALUES (1,'a'),(2,'b')
----
1 a
2 b
1 a
2 b

# table foo for distinct order by
statement ok
Expand Down
2 changes: 2 additions & 0 deletions datafusion/sqllogictest/test_files/string/dictionary_utf8.slt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ Xiangpeng datafusion数据融合 false true false true
Raphael datafusionДатаФусион false false false false
under_score un iść core false false false false
percent pan Tadeusz ma iść w kąt false false false false
(empty) (empty) false false false false
NULL NULL NULL NULL NULL NULL
NULL NULL NULL NULL NULL NULL

#
Expand Down
2 changes: 2 additions & 0 deletions datafusion/sqllogictest/test_files/string/init_data.slt.part
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ create table test_source as values
('Raphael', 'R', 'datafusionДатаФусион', 'аФус'),
('under_score', 'un_____core', 'un iść core', 'chrząszcz na łące w 東京都'),
('percent', 'p%t', 'pan Tadeusz ma iść w kąt', 'Pan Tadeusz ma frunąć stąd w kąt'),
('', '%', '', ''),
(NULL, '%', NULL, NULL),
(NULL, 'R', NULL, '🔥');

# --------------------------------------
Expand Down
4 changes: 4 additions & 0 deletions datafusion/sqllogictest/test_files/string/large_string.slt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ Xiangpeng Xiangpeng datafusion数据融合 datafusion数据融合
Raphael R datafusionДатаФусион аФус
under_score un_____core un iść core chrząszcz na łące w 東京都
percent p%t pan Tadeusz ma iść w kąt Pan Tadeusz ma frunąć stąd w kąt
(empty) % (empty) (empty)
NULL % NULL NULL
NULL R NULL 🔥

# TODO: move it back to `string_query.slt.part` after fixing the issue
Expand All @@ -61,6 +63,8 @@ Xiangpeng datafusion数据融合 false true false true
Raphael datafusionДатаФусион false false false false
under_score un iść core false false false false
percent pan Tadeusz ma iść w kąt false false false false
(empty) (empty) false false false false
NULL NULL NULL NULL NULL NULL
NULL NULL NULL NULL NULL NULL

#
Expand Down
12 changes: 12 additions & 0 deletions datafusion/sqllogictest/test_files/string/string.slt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ Xiangpeng datafusion数据融合 false true false true
Raphael datafusionДатаФусион false false false false
under_score un iść core false false false false
percent pan Tadeusz ma iść w kąt false false false false
(empty) (empty) false false false false
NULL NULL NULL NULL NULL NULL
NULL NULL NULL NULL NULL NULL

#
Expand All @@ -72,6 +74,9 @@ SELECT unicode_2, 'is LIKE', ascii_2 FROM test_basic_operator WHERE unicode_2 LI
UNION ALL
SELECT unicode_2, 'is NOT LIKE', ascii_2 FROM test_basic_operator WHERE unicode_2 NOT LIKE ascii_2
----
(empty) is LIKE %
(empty) is LIKE %
(empty) is LIKE %
Andrew is NOT LIKE X
Pan Tadeusz ma frunąć stąd w kąt is NOT LIKE p%t
Raphael is NOT LIKE R
Expand Down Expand Up @@ -100,7 +105,9 @@ SELECT
(unicode_2 LIKE ascii_2) AS unicode_2_like_ascii_2
FROM test_basic_operator
----
(empty) % (empty) (empty) true false true true
Andrew X datafusion📊🔥 🔥 false false false false
NULL % NULL NULL NULL NULL NULL NULL
NULL R NULL 🔥 NULL NULL NULL false
Raphael R datafusionДатаФусион аФус false false false false
Xiangpeng Xiangpeng datafusion数据融合 datafusion数据融合 true true false false
Expand All @@ -122,6 +129,9 @@ SELECT unicode_2, 'is ILIKE', ascii_2 FROM test_basic_operator WHERE unicode_2 I
UNION ALL
SELECT unicode_2, 'is NOT ILIKE', ascii_2 FROM test_basic_operator WHERE unicode_2 NOT ILIKE ascii_2
----
(empty) is ILIKE %
(empty) is ILIKE %
(empty) is ILIKE %
Andrew is NOT ILIKE X
Pan Tadeusz ma frunąć stąd w kąt is ILIKE p%t
Raphael is NOT ILIKE R
Expand Down Expand Up @@ -150,7 +160,9 @@ SELECT
(unicode_2 ILIKE ascii_2) AS unicode_2_ilike_ascii_2
FROM test_basic_operator
----
(empty) % (empty) (empty) true false true true
Andrew X datafusion📊🔥 🔥 false false false false
NULL % NULL NULL NULL NULL NULL NULL
NULL R NULL 🔥 NULL NULL NULL false
Raphael R datafusionДатаФусион аФус false false false false
Xiangpeng Xiangpeng datafusion数据融合 datafusion数据融合 true true false false
Expand Down
5 changes: 5 additions & 0 deletions datafusion/sqllogictest/test_files/string/string_literal.slt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -816,3 +816,8 @@ query B
SELECT starts_with('foobar', 'bar')
----
false

query TT
select ' ', '|'
----
|
Loading