bombsimon commented on code in PR #1884:
URL: 
https://github.com/apache/datafusion-sqlparser-rs/pull/1884#discussion_r2150371186


##########
src/ast/mod.rs:
##########
@@ -3670,17 +3727,24 @@ pub enum Statement {
         /// END;
         /// ```
         statements: Vec<Statement>,
-        /// Statements of an exception clause.
+        /// Exception handling with exception clauses and raises.
         /// Example:
         /// ```sql
         /// BEGIN
         ///     SELECT 1;
-        /// EXCEPTION WHEN ERROR THEN
-        ///     SELECT 2;
-        ///     SELECT 3;
+        /// EXCEPTION
+        ///     WHEN EXCEPTION_1 THEN
+        ///         SELECT 2;
+        ///     WHEN EXCEPTION_2 OR EXCEPTION_3 THEN
+        ///         SELECT 3;
+        ///     WHEN OTHER THEN
+        ///         SELECT 4;
+        /// RAISE;

Review Comment:
   An exception is represented by an `ExceptionClause` which consists of a 
`Vec` of `ExceptionWhen` (each `WHEN` arm and its statements) and a potential 
`RAISE` statement.
   
   For BigQuery, [the 
docs](https://cloud.google.com/bigquery/docs/reference/standard-sql/procedural-language#raise)
 says this which I interpreted as it can't be used without `EXCEPTION`:
   
   > **When USING MESSAGE isn't supplied**
   > The RAISE statement must only be used within an EXCEPTION clause. The 
RAISE statement will re-raise the exception that was caught, and preserve the 
original stack trace.
   
   For Snowflake we could likely do this, but there are other concerns. I know 
the parser isn't requiring _valid_ syntax, but since this specific `RAISE` is 
within error handling where you can omit the exception and re-raise what's 
being handled, I think it makes more sense to put it here. I would thing that 
if the `RAISE` was on the `StartTransaction` we would always have an exception 
as described in the [Snowflake 
docs](https://docs.snowflake.com/en/sql-reference/snowflake-scripting/raise). 
It's mentioned 
[here](https://docs.snowflake.com/en/developer-guide/snowflake-scripting/exceptions#raising-the-same-exception-again-in-an-exception-handler).
   
   If I move the `RAISE` to the `StartTransaction`, should I allow it both with 
and without re-raising and should I move the `Vec<ExceptionWhen>` directly 
under `StartTransaction` as well?



-- 
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: github-unsubscr...@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to