github-actions[bot] commented on code in PR #69448:
URL: https://github.com/apache/doris/pull/69448#discussion_r4357159601


##########
regression-test/suites/external_table_p0/tvf/test_s3_tvf.groovy:
##########
@@ -169,4 +169,46 @@ suite("test_s3_tvf", "p0,external") {
     } finally {
     }
 
+    // Exercise schema conversion with both readers, including failed casts, 
source NULLs and
+    // multiple batches. The BIGINT seed must be listed before the STRING file 
for TVF inference.
+    String castPrefix = 
"s3://${getS3BucketName()}/regression/parquet_invalid_string_cast/${UUID.randomUUID()}"
+    String castStorageProperties = """
+        "s3.endpoint" = "${s3_endpoint}",
+        "s3.region" = "${region}",
+        "s3.access_key" = "${ak}",
+        "s3.secret_key" = "${sk}"
+    """
+    sql """SELECT CAST(1 AS BIGINT) AS id, CAST('seed' AS STRING) AS name
+           INTO OUTFILE "${castPrefix}/00_seed/part_" FORMAT AS parquet
+           PROPERTIES (${castStorageProperties})"""
+    sql """SELECT CAST(CASE number % 4
+                          WHEN 0 THEN 'xx'
+                          WHEN 1 THEN '5'
+                          WHEN 2 THEN NULL
+                          ELSE '9' END AS STRING) AS id,
+                  CAST(number AS STRING) AS name
+           FROM numbers("number" = "4096")
+           INTO OUTFILE "${castPrefix}/01_strings/part_" FORMAT AS parquet
+           PROPERTIES (${castStorageProperties})"""
+
+    def previousCastSettings = sql "SELECT @@enable_file_scanner_v2, 
@@batch_size, @@enable_strict_cast"
+    try {
+        sql "SET batch_size=1024"
+        sql "SET enable_strict_cast=false"
+        // Materialize every value; COUNT(*) could use footer statistics and 
skip conversion.
+        // Compare the actual reader outputs rather than hardcoding expected 
rows in Groovy.
+        def castResults = [false, true].collect { scannerV2 ->
+            sql "SET enable_file_scanner_v2=${scannerV2}"

Review Comment:
   [P2] Exercise scanner V1 in the new differential regression
   
   `SET enable_file_scanner_v2=false` does not select V1 for this Parquet TVF 
at this head. `TVFScanNode` inherits `FileQueryScanNode.initSchemaParams()`, 
which sets `parquet_timestamp_semantics_version=1`; 
`FileScanLocalState::should_use_file_scanner_v2()` ORs that requirement with 
the session flag for non-load Parquet scans. Consequently both entries in 
`castResults` use V2, whose CAST path bypasses `PhysicalToLogicalConverter`, so 
this comparison cannot detect a regression in the conversion boundary fixed by 
this PR. Please add integration coverage that actually reaches the legacy 
reader/converter and verify scanner selection before treating these results as 
a V1/V2 comparison.



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