From 05a29212ffbf0704a620dfef1b850089d2efa1ab Mon Sep 17 00:00:00 2001
From: Nickolay Ponomarev <asqueella@gmail.com>
Date: Mon, 20 Apr 2020 04:07:21 +0300
Subject: [PATCH] Update comments (follow-up to PR #155)

https://github.com/andygrove/sqlparser-rs/pull/155
---
 src/ast/query.rs |  9 +++++----
 src/parser.rs    | 26 +++++++++++---------------
 2 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/src/ast/query.rs b/src/ast/query.rs
index a5eea141..3588257e 100644
--- a/src/ast/query.rs
+++ b/src/ast/query.rs
@@ -225,10 +225,11 @@ pub enum TableFactor {
         subquery: Box<Query>,
         alias: Option<TableAlias>,
     },
-    /// Represents a parenthesized join expression, such as
-    /// `(foo <JOIN> bar [ <JOIN> baz ... ])`.
-    /// The inner `TableWithJoins` can have no joins only if its
-    /// `relation` is itself a `TableFactor::NestedJoin`.
+    /// Represents a parenthesized table factor. The SQL spec only allows a
+    /// join expression (`(foo <JOIN> bar [ <JOIN> baz ... ])`) to be nested,
+    /// possibly several times, but the parser also accepts the non-standard
+    /// nesting of bare tables (`table_with_joins.joins.is_empty()`), so the
+    /// name `NestedJoin` is a bit of misnomer.
     NestedJoin(Box<TableWithJoins>),
 }
 
diff --git a/src/parser.rs b/src/parser.rs
index e988f9c0..cdaf8989 100644
--- a/src/parser.rs
+++ b/src/parser.rs
@@ -1771,7 +1771,6 @@ impl Parser {
             //                   ^ ^ ^ ^
             //                   | | | |
             //                   | | | |
-            //                   | | | |
             //                   | | | (4) belongs to a SetExpr::Query inside the subquery
             //                   | | (3) starts a derived table (subquery)
             //                   | (2) starts a nested join
@@ -1784,23 +1783,20 @@ impl Parser {
                 // case (3), and the next token would be `NATURAL`.
                 Ok(table_factor) => Ok(table_factor),
                 Err(_) => {
-                    // The '(' we've recently consumed does not start a derived
-                    // table. For valid input this can happen either when the
-                    // token following the paren can't start a query (e.g. `foo`
-                    // in `FROM (foo NATURAL JOIN bar)`, or when the '(' we've
-                    // consumed is followed by another '(' that starts a
-                    // derived table, like (3), or another nested join (2).
-                    //
-                    // Ignore the error and back up to where we were before.
-                    // Either we'll be able to parse a valid nested join, or
-                    // we won't, and we'll return that error instead.
-                    //
-                    // Even the SQL spec prohibits derived tables and bare
-                    // tables from appearing alone in parentheses, we allowed it
-                    // as some Db's allowed that (snowflake as example)
+                    // A parsing error from `parse_derived_table_factor` indicates that
+                    // the '(' we've recently consumed does not start a derived table
+                    // (cases 1, 2, or 4). Ignore the error and back up to where we
+                    // were before - right after the opening '('.
                     self.index = index;
+
+                    // Inside the parentheses we expect to find a table factor
+                    // followed by some joins or another level of nesting.
                     let table_and_joins = self.parse_table_and_joins()?;
                     self.expect_token(&Token::RParen)?;
+                    // The SQL spec prohibits derived and bare tables from appearing
+                    // alone in parentheses. We don't enforce this as some databases
+                    // (e.g. Snowflake) allow such syntax.
+
                     Ok(TableFactor::NestedJoin(Box::new(table_and_joins)))
                 }
             }