From 77c889f356b08cd7b336f790b8556405454d05eb Mon Sep 17 00:00:00 2001
From: Mees Delzenne <DelSkayn@users.noreply.github.com>
Date: Mon, 21 Aug 2023 17:29:50 +0200
Subject: [PATCH] Minor parser fixes (#2479)

---
 lib/src/sql/comment.rs    | 21 ++++++++++------
 lib/src/sql/permission.rs | 11 ++++++++-
 lib/src/sql/view.rs       | 52 ++++++++++++++++++++++++++++++---------
 3 files changed, 64 insertions(+), 20 deletions(-)

diff --git a/lib/src/sql/comment.rs b/lib/src/sql/comment.rs
index 4f1032d6..d95603b7 100644
--- a/lib/src/sql/comment.rs
+++ b/lib/src/sql/comment.rs
@@ -5,15 +5,16 @@ use nom::character::complete::char;
 use nom::character::complete::multispace0;
 use nom::character::complete::multispace1;
 use nom::character::complete::not_line_ending;
+use nom::multi::many0;
 use nom::multi::many1;
 
 pub fn mightbespace(i: &str) -> IResult<&str, ()> {
-	let (i, _) = alt((comment, blank))(i)?;
+	let (i, _) = many0(alt((comment, space)))(i)?;
 	Ok((i, ()))
 }
 
 pub fn shouldbespace(i: &str) -> IResult<&str, ()> {
-	let (i, _) = alt((comment, space))(i)?;
+	let (i, _) = many1(alt((comment, space)))(i)?;
 	Ok((i, ()))
 }
 
@@ -58,12 +59,18 @@ pub fn hash(i: &str) -> IResult<&str, ()> {
 	Ok((i, ()))
 }
 
-fn blank(i: &str) -> IResult<&str, ()> {
-	let (i, _) = multispace0(i)?;
-	Ok((i, ()))
-}
-
 fn space(i: &str) -> IResult<&str, ()> {
 	let (i, _) = multispace1(i)?;
 	Ok((i, ()))
 }
+
+#[cfg(test)]
+mod test {
+	use crate::sql::parse;
+
+	#[test]
+	fn any_whitespace() {
+		let sql = "USE /* white space and comment between */ NS test;";
+		assert!(parse(sql).is_ok());
+	}
+}
diff --git a/lib/src/sql/permission.rs b/lib/src/sql/permission.rs
index 0a126aaa..3b5966f8 100644
--- a/lib/src/sql/permission.rs
+++ b/lib/src/sql/permission.rs
@@ -9,6 +9,7 @@ use crate::sql::value::{value, Value};
 use nom::branch::alt;
 use nom::bytes::complete::tag_no_case;
 use nom::combinator::map;
+use nom::multi::separated_list1;
 use nom::{multi::separated_list0, sequence::tuple};
 use revision::revisioned;
 use serde::{Deserialize, Serialize};
@@ -138,7 +139,7 @@ fn full(i: &str) -> IResult<&str, Permissions> {
 }
 
 fn specific(i: &str) -> IResult<&str, Permissions> {
-	let (i, perms) = separated_list0(commasorspace, permission)(i)?;
+	let (i, perms) = separated_list1(commasorspace, permission)(i)?;
 	Ok((
 		i,
 		Permissions {
@@ -277,4 +278,12 @@ mod tests {
 			}
 		);
 	}
+
+	#[test]
+	fn no_empty_permissions() {
+		// This was previouslly allowed,
+		let sql = "PERMISSION ";
+		let res = dbg!(permission(sql));
+		assert!(dbg!(res.is_err()));
+	}
 }
diff --git a/lib/src/sql/view.rs b/lib/src/sql/view.rs
index 4a911114..e6a7e9f6 100644
--- a/lib/src/sql/view.rs
+++ b/lib/src/sql/view.rs
@@ -4,7 +4,8 @@ use crate::sql::error::IResult;
 use crate::sql::field::{fields, Fields};
 use crate::sql::group::{group, Groups};
 use crate::sql::table::{tables, Tables};
-use nom::bytes::complete::tag_no_case;
+use nom::branch::alt;
+use nom::bytes::complete::{tag, tag_no_case};
 use nom::combinator::opt;
 use nom::sequence::preceded;
 use revision::revisioned;
@@ -34,19 +35,29 @@ impl fmt::Display for View {
 }
 
 pub fn view(i: &str) -> IResult<&str, View> {
+	let select_view = |i| {
+		let (i, _) = tag_no_case("SELECT")(i)?;
+		let (i, _) = shouldbespace(i)?;
+		let (i, expr) = fields(i)?;
+		let (i, _) = shouldbespace(i)?;
+		let (i, _) = tag_no_case("FROM")(i)?;
+		let (i, _) = shouldbespace(i)?;
+		let (i, what) = tables(i)?;
+		let (i, cond) = opt(preceded(shouldbespace, cond))(i)?;
+		let (i, group) = opt(preceded(shouldbespace, group))(i)?;
+		Ok((i, (expr, what, cond, group)))
+	};
+
+	let select_view_delimited = |i| {
+		let (i, _) = tag("(")(i)?;
+		let (i, res) = select_view(i)?;
+		let (i, _) = tag(")")(i)?;
+		Ok((i, res))
+	};
+
 	let (i, _) = tag_no_case("AS")(i)?;
 	let (i, _) = shouldbespace(i)?;
-	let (i, _) = opt(tag_no_case("("))(i)?;
-	let (i, _) = tag_no_case("SELECT")(i)?;
-	let (i, _) = shouldbespace(i)?;
-	let (i, expr) = fields(i)?;
-	let (i, _) = shouldbespace(i)?;
-	let (i, _) = tag_no_case("FROM")(i)?;
-	let (i, _) = shouldbespace(i)?;
-	let (i, what) = tables(i)?;
-	let (i, cond) = opt(preceded(shouldbespace, cond))(i)?;
-	let (i, group) = opt(preceded(shouldbespace, group))(i)?;
-	let (i, _) = opt(tag_no_case(")"))(i)?;
+	let (i, (expr, what, cond, group)) = alt((select_view, select_view_delimited))(i)?;
 	Ok((
 		i,
 		View {
@@ -98,4 +109,21 @@ mod tests {
 		let out = res.unwrap().1;
 		assert_eq!("AS SELECT temp FROM test WHERE temp != NONE GROUP BY temp", format!("{}", out))
 	}
+
+	#[test]
+	fn view_disallow_unbalanced_brackets() {
+		let sql = "AS (SELECT temp FROM test WHERE temp IS NOT NONE GROUP BY temp";
+		let res = view(sql);
+		assert!(res.is_err());
+		let sql = "AS SELECT temp FROM test WHERE temp IS NOT NONE GROUP BY temp)";
+		let res = view(sql);
+		// The above test won't return an error since the trailing ) might be part of a another
+		// pair.
+		if let Ok((i, _)) = res {
+			// but it should not be parsed.
+			assert_eq!(i, ")")
+		} else {
+			panic!()
+		}
+	}
 }