alamb commented on code in PR #21577:
URL: https://github.com/apache/datafusion/pull/21577#discussion_r3072519435


##########
datafusion/sqllogictest/test_files/set_variable.slt:
##########
@@ -404,6 +404,11 @@ SHOW datafusion.format.date_format
 ----
 datafusion.format.date_format %d-%m-%Y
 
+query D
+SELECT DATE '2026-04-07'
+----
+07-04-2026

Review Comment:
   I think these tests are now testing the wiring into the sqllogictest 
harness. I was thinking we could avoid any changes to sqllogictest by 
converting the values to strings first, which would then test that this 
formatting logic was wired into the functions correctly
   
   So somethinglike
   ```sql
   select arrow_cast(DATE '2026-04-07', 'Utf8');
   ```
   
   Which would convert the date into a string (which should use the formatting 
I think). Or maybe the format strings are used for parsing (I am not sure)



##########
datafusion/sqllogictest/src/engines/datafusion_engine/runner.rs:
##########
@@ -212,7 +212,12 @@ async fn run_query(
     let stream = execute_stream(plan, task_ctx)?;
     let types = normalize::convert_schema_to_types(stream.schema().fields());
     let results: Vec<RecordBatch> = collect(stream).await?;
-    let rows = normalize::convert_batches(&schema, results, is_spark_path)?;
+
+    let df_format = get_format_options(ctx)?;
+    let arrow_format: arrow::util::display::FormatOptions<'_> =
+        (&df_format).try_into()?;
+    let rows =
+        normalize::convert_batches(&schema, results, is_spark_path, 
&arrow_format)?;

Review Comment:
   This code is kind of hard to read as it 
   1. uses `rrow::util::display::FormatOptions<'_> ` which I find hard to read
   2. Requires some work to get the FormatOptions
   
   If we are going to mess with the runner, would it be possible to encapsulate 
the work more into `normalize` -- so something like
   
   ```rust
   // Pass in `ctx` to use its output format options
   let rows =
           normalize::convert_batches(&ctx, &schema, results, is_spark_path)?;
   ```



-- 
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]

Reply via email to