andygrove commented on code in PR #5032:
URL: https://github.com/apache/datafusion-comet/pull/5032#discussion_r3692980440
##########
spark/src/test/resources/sql-tests/expressions/string/string_translate.sql:
##########
@@ -15,29 +15,29 @@
-- specific language governing permissions and limitations
-- under the License.
--- translate is gated as Incompatible by default. DataFusion's translate
iterates over Unicode
--- graphemes (Spark uses code points) and substitutes U+0000 instead of
treating it as a deletion
--- sentinel, so the native path silently diverges from Spark for
combining-mark inputs and for
--- to=NUL. These default-config tests assert that the expression falls back
cleanly to Spark.
--- See string_translate_enabled.sql for the opt-in native path.
+-- translate runs through the codegen dispatcher by default so results match
Spark exactly. The
+-- native path diverges from Spark (DataFusion iterates over Unicode graphemes
where Spark uses code
+-- points, and substitutes U+0000 instead of treating it as a deletion
sentinel), so it is opt-in
+-- via spark.comet.expression.StringTranslate.allowIncompatible. See
string_translate_enabled.sql
+-- for the opt-in native path.
statement
CREATE TABLE test_translate(s string, from_str string, to_str string) USING
parquet
statement
INSERT INTO test_translate VALUES ('hello', 'el', 'ip'), ('hello', 'aeiou',
'12345'), ('', 'a', 'b'), (NULL, 'a', 'b'), ('hello', '', ''), ('abc', 'abc',
'x')
Review Comment:
Added in 26994c919, built with `decode()` as you suggested so the fixture
stays ASCII:
```sql
(concat('caf', decode(X'65CC81', 'UTF-8')), 'e', 'E'), -- "e" + U+0301:
one grapheme, two code points
('hello', 'l', decode(X'00', 'UTF-8')) -- U+0000 as the `to`
argument
```
I also checked that they actually discriminate rather than passing
vacuously, since that is the point of them. Adding the same two rows to
`string_translate_enabled.sql`, which opts into the native path, fails:
```
- sql-file: expressions/string/string_translate_enabled.sql *** FAILED ***
Error executing SQL 'SELECT translate(s, from_str, to_str) FROM
test_translate_enabled'
Results do not match for query
```
So the rows pass under the dispatcher and fail under the native path, which
makes them a real regression test for the routing. I left
`string_translate_enabled.sql` as it was — its header explicitly says it covers
inputs where the two implementations agree, so the divergent rows belong in the
default file only.
##########
spark/src/test/resources/sql-tests/expressions/csv/to_csv.sql:
##########
@@ -0,0 +1,41 @@
+-- Licensed to the Apache Software Foundation (ASF) under one
+-- or more contributor license agreements. See the NOTICE file
+-- distributed with this work for additional information
+-- regarding copyright ownership. The ASF licenses this file
+-- to you under the Apache License, Version 2.0 (the
+-- "License"); you may not use this file except in compliance
+-- with the License. You may obtain a copy of the License at
+--
+-- http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing,
+-- software distributed under the License is distributed on an
+-- "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+-- KIND, either express or implied. See the License for the
+-- specific language governing permissions and limitations
+-- under the License.
+
+-- to_csv runs through the codegen dispatcher by default so results match
Spark exactly, including
+-- quoting and escaping. The native path is opt-in via
+-- spark.comet.expression.StructsToCsv.allowIncompatible.
+
+statement
+CREATE TABLE test_to_csv(a int, b string, c double) USING parquet
+
+statement
+INSERT INTO test_to_csv VALUES
+ (1, 'x', 2.5),
+ (-3, 'hello,world', 0.0),
+ (0, 'has "quote"', -1.5),
+ (NULL, NULL, NULL),
+ (7, '', 3.0)
+
+-- column struct: values with delimiters and quotes exercise Spark's CSV
quoting rules
+query
+SELECT to_csv(named_struct('a', a, 'b', b, 'c', c)) FROM test_to_csv
Review Comment:
Added in 26994c919. `to_csv.sql` now covers `map('sep', ';')` over the
existing rows (which also changes which values need quoting), plus a
`timestampFormat`/`dateFormat` query over a new date+timestamp table — so the
same query exercises both the options axis and the #3232 types that previously
fell back. The dispatcher path had no options coverage anywhere before this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
