[GitHub] [spark] peter-toth commented on a change in pull request #26028: [SPARK-29359][SQL][TESTS] Better exception handling in (SQL|ThriftServer)QueryTestSuite

[GitBox <gi...@apache.org>]

peter-toth commented on a change in pull request #26028: [SPARK-29359][SQL][TESTS] Better exception handling in (SQL|ThriftServer)QueryTestSuite
URL: https://github.com/apache/spark/pull/26028#discussion_r331737306
 
 

 ##########
 File path: sql/core/src/test/resources/sql-tests/results/ansi/interval.sql.out
 ##########
 @@ -290,7 +290,7 @@ interval 4 weeks 2 days
 -- !query 22
 select 30 day day
 -- !query 22 schema
-struct<>
+
 
 Review comment:
   Thanks @dongjoon-hyun for the review, let me try to convince you that these changes make sense, but if you still disagree just let me know and I will drop them.
   
   The only cases where I replaced the expected schema from `struct<>` to nothing are those where some error occurs and an exception is thrown. In those cases there is no data returned, so there is no schema at all, not even an empty struct.
   ```
   -- !query 22
   select 30 day day
   -- !query 22 schema
   
   -- !query 22 output
   org.apache.spark.sql.catalyst.parser.ParseException
   ```
   IMHO `struct<>` makes sense where a statement was successful but data returned is empty and there are no columns in it. In those cases I left the expected output intact.
   ```
   -- !query 0
   CREATE OR REPLACE TEMPORARY VIEW view1 AS SELECT 2 AS i1
   -- !query 0 schema
   struct<>
   -- !query 0 output
   ```
   Empty expected schema is also useful to easily recognize a statement that ended up in an error (otherwise we probably need to check the output for containing `exception` which seems less elegant).
   I utilized empty expected schema to fix a sorting issue in `ThriftServerQueryTestSuite`: https://github.com/apache/spark/pull/26028/files#diff-b3ea3021602a88056e52bf83d8782de8R147, and there might be other cases in the future where this change could help.
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org