Repository navigation
test: move string expression coverage from Scala suites to Comet SQL tests #6626
Copy link
Copy link
Open
Labels
area:expressionsExpression evaluationExpression evaluationenhancementNew feature or requestNew feature or requesttestTesting relatedTesting related
Description
Part of #6615.
What / Why
This issue moves string expression coverage to Comet SQL tests under sql-tests/expressions/string/. The sources are CometStringExpressionSuite, the Spark 3.x CometStringDecodeSuite, and the string tests in CometExpressionSuite (substring, like, contains, char/varchar, read-side padding). Regex tests are in #6627. Static analysis on main at 3bc2faa found the following:
| Suite | Covered | Convert | Blocked | Split | Keep | Total |
|---|---|---|---|---|---|---|
CometStringExpressionSuite |
12 | 27 | 0 | 0 | 1 | 40 |
CometStringDecodeSuite |
0 | 1 | 0 | 0 | 0 | 1 |
CometExpressionSuite |
6 | 8 | 1 | 0 | 0 | 15 |
| Total | 18 | 36 | 1 | 0 | 1 | 56 |
Most of the work extends existing fixtures, which are thinner than the Scala tests. They have:
- no genuinely decomposed or combining characters (the Scala "combining" rows are themselves precomposed; see below);
- no binary
lpad/rpad; - no Parquet string-predicate pushdown;
- no
concat_wswith scalar subqueries or native array arguments; - no case-sensitivity check for
contains; - no escapes or Contains rewrite in
like.
Work
Line numbers refer to CometStringExpressionSuite, except L902, L914, L1269, L1306, L1321, L1349, L1573, L1952 and L2909, which are in CometExpressionSuite.
Existing fixtures to extend
string_lpad.sql/string_rpad.sql(L66, L70, L74)- Add decomposed
'é'and Telugu rows, also with a NULL length and with length 4. - Add literal non-positive lengths with a column string (
lpad(s, -6, 'x'),lpad(s, 0)) and the multi-character pad'zzz'with a column length.
- Add decomposed
split_rust.sql(L196, L207, L228)- Add limits 3 and 0, and the regex separators
'\\s+'and'\\d+'. The backslashes must be doubled; see test: move regex coverage to Comet SQL tests and fix fixtures whose patterns lose their backslashes #6627. - Add UTF-8 rows: CJK, emoji, a genuinely decomposed accent, RTL scripts, supplementary characters, mixed text with
'[a-z]+', and a zero-width space.
- Add limits 3 and 0, and the regex separators
chr.sql(L334)- Add negative and large column values (61231, -61231, -1700, 512) and the literals 555, -555 and
cast(0.5 as int).
- Add negative and large column values (61231, -61231, -1700, 512) and the literals 555, -555 and
init_cap_enabled.sql(L354): add'james ähtäri'.string_trim.sql(L378)- Add
trim('SL', s),ltrim('SL', s)andrtrim('SL', s). - Add
btrim(s),btrim('SL', s)(where the column is the trim set) andbtrim(s, 'SL'). - Add
upper(trim(s))and its variants.
- Add
string_instr.sql,string_replace.sql,string_translate_enabled.sql(L409)- Add
instr(s, '手机')over'苹果手机', the two-argumentreplace(s, 'SQL'), and a nativetranslate(s, 'SL苹', '123').
- Add
length.sql(L423): add anX'00FF'row to the binary table.substring.sql- Covers nine
CometStringExpressionSuitetests (L577-L759) andCometExpressionSuiteL902 and L914. - Rows:
'a','ab','x','hello','world!', mixed-width'aé苹😀','苹果手机','😀🎉🔥', and real decomposed, precomposed and Telugu strings. - Add the missing start/length combinations, the
-2147483647cases, the literal(-300, 3),substring(s, -2, -2),substring(s, 0, 0), the two-argumentsubstring(s, 0), and four multi-byte literals.
- Covers nine
like.sql- L1321: a
'%..%'pattern (the Contains rewrite) and use in a WHERE clause over mixed-case data. - L1349:
LIKE '%\_%'andLIKE '%$_%' ESCAPE '$'. Spark's unescaping keeps\_and\%as written.
- L1321: a
contains.sql(L1573): rows that differ only in case, a needle at the start of the string, and use in a WHERE clause.misc/codegen_dispatch_disabled_fallback.sql(L1952)- Add
initcapwithexpect_fallback(spark.comet.exec.scalaUDF.codegen.enabled=false). - The opt-in half is already in
init_cap_enabled.sql.
- Add
decode.sql, plus a newdecode_disabled.sql(MaxSparkVersion: 3.5), for theCometStringDecodeSuitetest- Set
spark.comet.expression.StringDecode.enabled=false. - Assert
expect_fallback(Expression support is disabled. Set spark.comet.expression.StringDecode.enabled=true to enable it.).
- Set
New fixtures
binary_pad.sql(L95, L99): binarylpad/rpadwith column and literal str/len/pad combinations. These go through the dispatcher asStaticInvoke.like_parquet_pushdown.sql(L456, L496)- Use a ConfigMatrix over
spark.sql.parquet.filterPushdown.stringPredicate(andparquet.enable.dictionary), plusparquet.block.size=512. - Write one-file tables with
range(0, n, 1, 1)and add LIKE / NOT LIKE filters.
- Use a ConfigMatrix over
concat_ws_scalar_subquery.sql(L782): dispatcher off, a 32-row single-file table, and six scalar-subquery queries.concat_ws_native.sql(L799)- Turn the dispatcher off and set
StringSplit.allowIncompatible=true, so a plainqueryproves native execution. - Add
concat_ws('', arr, s, array('x', NULL, 'y')).
- Turn the dispatcher off and set
char_varchar.sql- L1269:
char(7),char(8)andvarchar(6)columns undersubstring. - L2909: a
CHAR(2)column holding'é','é',''and'ab'.
- L1269:
collation_native_opt_in.sql(L43,MinSparkVersion: 4.0)- Dispatcher off, with
Concat.allowIncompatible=trueandReverse.allowIncompatible=true. expect_native(concat)/expect_native(reverse)overCAST('abc' AS STRING COLLATE UTF8_LCASE). Because CometCast folds a literal cast, this does produce the collatedLiteralthe Scala test builds.
- Dispatcher off, with
Blocked
CometExpressionSuiteL1306 (char/varchar over-length values). It needs a table over files that already contain over-length values. A fixture cannot point a table at a location, and the write-side length check rejects such values. Keep it in Scala unless the framework gainsLOCATIONsupport.
Stays in Scala
- L444 reads
test-data/dictionary-string-binary.parquet, an Arrow dictionary-typed file that SQL cannot write.
Notes on the Scala tests
- "split string with UTF-8 characters". Its "combining" row is byte-identical to the precomposed row: both use U+00E9 and U+00EF.
- "substring - unicode multi-byte characters".
"café" // combining accentis precomposed. Neither test covers combining characters, so the fixtures add real ones. - "Upper and Lower" and "trim". They set
Upper.allowIncompatible/Lower.allowIncompatible, which do nothing: the native path is gated onspark.comet.caseConversion.enabled. Both tests therefore run the dispatcher. - "lpad/rpad with NULL length" says the generator never emits NULL integers. It does now.
- All-literal pad combinations. Spark folds them in the Scala suite. They really run in Comet once in a fixture.
concat_ws.sql:44-45usesspark_answer_onlyon all-literal arguments, and its comment says the plan dispatches. Pin it withexpect_dispatch(concat_ws).
Done when
- Every test in the table below is accounted for. Either a fixture covers it (the same inputs, configs and assertion strength, or stronger), the fixture that already covers it is cited, or it stays in Scala for the reason given above.
- Each converted Scala test is deleted in the PR that adds its coverage. A suite left empty is deleted and removed from the suite lists in
.github/workflows/pr_build_linux.ymlandpr_build_macos.yml. - The new and changed fixtures pass on every Spark profile. Apply
run-all-spark-profiles, because pull requests run only the default profile.
Every Scala test in scope, with its verdict and target fixture
| Suite | Line | Test | Verdict | Target |
|---|---|---|---|---|
CometExpressionSuite |
902 | string type and substring | convert | string/substring.sql |
CometExpressionSuite |
914 | substring with start < 1 | convert | string/substring.sql |
CometExpressionSuite |
925 | substring with dictionary | covered | string/substring.sql |
CometExpressionSuite |
1269 | charvarchar | convert | string/char_varchar.sql (new) |
CometExpressionSuite |
1306 | char varchar over length values | blocked: other | string/char_varchar.sql (new) |
CometExpressionSuite |
1321 | like (LikeSimplification enabled) | convert | string/like.sql |
CometExpressionSuite |
1349 | like with custom escape | convert | string/like.sql |
CometExpressionSuite |
1573 | contains | convert | string/contains.sql |
CometExpressionSuite |
1613 | contains with both columns | covered | string/contains.sql |
CometExpressionSuite |
1621 | startswith | covered | string/starts_with.sql |
CometExpressionSuite |
1637 | endswith | covered | string/ends_with.sql |
CometExpressionSuite |
1952 | enable incompat expression using dynamic config | convert | misc/codegen_dispatch_disabled_fallback.sql |
CometExpressionSuite |
2049 | unhex | covered | string/unhex.sql |
CometExpressionSuite |
2909 | readSidePadding | convert | string/char_varchar.sql (new) |
CometExpressionSuite |
3752 | test length function | covered | string/length.sql |
CometStringDecodeSuite |
32 | StringDecode honors spark.comet.expression.StringDecode.enabled | convert | string/decode.sql; string/decode_disabled.sql (new) |
CometStringExpressionSuite |
43 | collated strings preserve native opt-in routing | convert | string/collation_native_opt_in.sql (new) |
CometStringExpressionSuite |
66 | lpad string | convert | string/string_lpad.sql |
CometStringExpressionSuite |
70 | rpad string | convert | string/string_rpad.sql |
CometStringExpressionSuite |
74 | lpad/rpad with NULL length | convert | string/string_lpad.sql; string/string_rpad.sql |
CometStringExpressionSuite |
95 | lpad binary | convert | string/binary_pad.sql (new) |
CometStringExpressionSuite |
99 | rpad binary | convert | string/binary_pad.sql (new) |
CometStringExpressionSuite |
186 | split string basic | covered | string/split_rust.sql |
CometStringExpressionSuite |
196 | split string with limit | convert | string/split_rust.sql |
CometStringExpressionSuite |
207 | split string with regex patterns | convert | string/split_rust.sql |
CometStringExpressionSuite |
220 | split string edge cases | covered | string/split_rust.sql |
CometStringExpressionSuite |
228 | split string with UTF-8 characters | convert | string/split_rust.sql |
CometStringExpressionSuite |
286 | Various String scalar functions | covered | string/ascii.sql; string/bit_length.sql; string/octet_length.sql |
CometStringExpressionSuite |
298 | Upper and Lower | covered | string/upper.sql; string/lower.sql |
CometStringExpressionSuite |
313 | Chr | covered | string/chr.sql |
CometStringExpressionSuite |
323 | Chr with null character | covered | string/chr.sql |
CometStringExpressionSuite |
334 | Chr with negative and large value | convert | string/chr.sql |
CometStringExpressionSuite |
354 | InitCap compatible cases | convert | string/init_cap_enabled.sql |
CometStringExpressionSuite |
368 | InitCap incompatible cases | covered | string/init_cap.sql |
CometStringExpressionSuite |
378 | trim | convert | string/string_trim.sql |
CometStringExpressionSuite |
400 | string repeat | covered | string/string_repeat.sql |
CometStringExpressionSuite |
409 | length, reverse, instr, replace, translate | convert | string/string_instr.sql; string/string_replace.sql; string/string_translate_enabled.sql |
CometStringExpressionSuite |
423 | length on binary input runs natively | convert | string/length.sql |
CometStringExpressionSuite |
444 | length on dictionary-typed string and binary columns from an Arrow-written file | keep | |
CometStringExpressionSuite |
456 | string predicate filter | convert | string/like_parquet_pushdown.sql (new) |
CometStringExpressionSuite |
496 | filter pushdown - StringPredicate | convert | string/like_parquet_pushdown.sql (new) |
CometStringExpressionSuite |
537 | string_space | covered | string/string_space.sql |
CometStringExpressionSuite |
543 | string_space with dictionary | covered | string/string_space.sql |
CometStringExpressionSuite |
553 | substring | covered | string/substring.sql |
CometStringExpressionSuite |
577 | substring - negative start boundary cases | convert | string/substring.sql |
CometStringExpressionSuite |
598 | substring - negative start with zero and negative length | convert | string/substring.sql |
CometStringExpressionSuite |
614 | substring - single character and empty strings | convert | string/substring.sql |
CometStringExpressionSuite |
627 | substring - unicode multi-byte characters | convert | string/substring.sql |
CometStringExpressionSuite |
656 | substring - decomposed and combining unicode characters | convert | string/substring.sql |
CometStringExpressionSuite |
686 | substring - large start and length values | convert | string/substring.sql |
CometStringExpressionSuite |
698 | substring - dictionary encoded strings | convert | string/substring.sql |
CometStringExpressionSuite |
731 | substring - scalar inputs | convert | string/substring.sql |
CometStringExpressionSuite |
759 | substring - scalar inputs with multi-byte | convert | string/substring.sql |
CometStringExpressionSuite |
782 | concat_ws with scalar subqueries over a multi-row batch | convert | string/concat_ws_scalar_subquery.sql (new) |
CometStringExpressionSuite |
799 | concat_ws with array arguments | convert | string/concat_ws_native.sql (new); string/concat_ws.sql |
CometStringExpressionSuite |
840 | levenshtein routes collated strings through the codegen dispatcher (issue #5591) | covered | string/levenshtein_collation.sql; string/routing_string_collation_enabled.sql |
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
area:expressionsExpression evaluationExpression evaluationenhancementNew feature or requestNew feature or requesttestTesting relatedTesting related
Activity