From 742382fcc05739a8bc96445cd44c2b137c2ac200 Mon Sep 17 00:00:00 2001 From: Jake Wheat Date: Thu, 8 Feb 2024 10:43:11 +0000 Subject: [PATCH] restrict parsing of * and X.* as term in expressions --- Language/SQL/SimpleSQL/Parse.hs | 33 ++- Language/SQL/SimpleSQL/Pretty.hs | 1 + Language/SQL/SimpleSQL/Syntax.hs | 4 +- expected-parse-errors/golden | 195 +++++++----------- .../SQL/SimpleSQL/QueryExprComponents.hs | 15 ++ .../Language/SQL/SimpleSQL/SQL2011Queries.hs | 5 +- tests/Language/SQL/SimpleSQL/ScalarExprs.hs | 9 +- 7 files changed, 127 insertions(+), 135 deletions(-) diff --git a/Language/SQL/SimpleSQL/Parse.hs b/Language/SQL/SimpleSQL/Parse.hs index d9f1ee5..b898265 100644 --- a/Language/SQL/SimpleSQL/Parse.hs +++ b/Language/SQL/SimpleSQL/Parse.hs @@ -604,12 +604,19 @@ simpleLiteral = numberLit <|> stringLit === star used in select *, select x.*, and agg(*) variations, and some other -places as well. The parser doesn't attempt to check that the star is -in a valid context, it parses it OK in any scalar expression context. +places as well. The parser makes an attempt to not parse star in +most contexts, to provide better experience when the user makes a mistake +in an expression containing * meaning multiple. It will parse a * +at the top level of a select item, or in arg in a app argument list. -} star :: Parser ScalarExpr -star = Star <$ symbol "*" +star = + hidden $ choice + [Star <$ symbol "*" + -- much easier to use try here than to left factor where + -- this is allowed and not allowed + ,try (QStar <$> (names <* symbol "." <* symbol "*"))] {- == parameter @@ -957,12 +964,12 @@ app :: Parser ([Name] -> ScalarExpr) app = hidden openParen *> choice [hidden duplicates - <**> (commaSep1 scalarExpr + <**> (commaSep1 scalarExprOrStar <**> ((hoption [] orderBy <* closeParen) <**> (hoptional afilter <$$$$$> AggregateApp))) -- separate cases with no all or distinct which must have at -- least one scalar expr - ,commaSep1 scalarExpr + ,commaSep1 scalarExprOrStar <**> choice [closeParen *> hidden (choice [window @@ -1310,13 +1317,19 @@ documenting/fixing. scalarExpr :: Parser ScalarExpr scalarExpr = label "expression" $ E.makeExprParser term (opTable False) +-- used when parsing contexts where a * or x.* is allowed +-- currently at the top level of a select item or top level of +-- argument passed to an app-like. This list may need to be extended. + +scalarExprOrStar :: Parser ScalarExpr +scalarExprOrStar = label "expression" (star <|> scalarExpr) + term :: Parser ScalarExpr term = label "expression" $ choice [simpleLiteral ,parameter ,positionalArg - ,star ,parensExpr ,caseExpr ,cast @@ -1383,8 +1396,12 @@ duplicates = -} selectItem :: Parser (ScalarExpr,Maybe Name) -selectItem = label "select item" ((,) <$> scalarExpr <*> optional als) - where als = label "alias" $ optional (keyword_ "as") *> name +selectItem = + label "select item" $ choice + [(,Nothing) <$> star + ,(,) <$> scalarExpr <*> optional als] + where + als = label "alias" $ optional (keyword_ "as") *> name selectList :: Parser [(ScalarExpr,Maybe Name)] selectList = commaSep1 selectItem diff --git a/Language/SQL/SimpleSQL/Pretty.hs b/Language/SQL/SimpleSQL/Pretty.hs index 7b0d1eb..7ff71a2 100644 --- a/Language/SQL/SimpleSQL/Pretty.hs +++ b/Language/SQL/SimpleSQL/Pretty.hs @@ -87,6 +87,7 @@ scalarExpr _ (IntervalLit s v f t) = <+> me (\x -> pretty "to" <+> intervalTypeField x) t scalarExpr _ (Iden i) = names i scalarExpr _ Star = pretty "*" +scalarExpr _ (QStar nms) = names nms <> pretty ".*" scalarExpr _ Parameter = pretty "?" scalarExpr _ (PositionalArg n) = pretty $ T.cons '$' $ showText n scalarExpr _ (HostParameter p i) = diff --git a/Language/SQL/SimpleSQL/Syntax.hs b/Language/SQL/SimpleSQL/Syntax.hs index e8f8cec..9aa99d7 100644 --- a/Language/SQL/SimpleSQL/Syntax.hs +++ b/Language/SQL/SimpleSQL/Syntax.hs @@ -105,8 +105,10 @@ data ScalarExpr -- | identifier with parts separated by dots | Iden [Name] - -- | star, as in select *, t.*, count(*) + -- | star, as in select *, count(*) | Star + -- | qualified star, as in a.*, b.c.* + | QStar [Name] | Parameter -- ^ Represents a ? in a parameterized query | PositionalArg Int -- ^ Represents an e.g. $1 in a parameterized query diff --git a/expected-parse-errors/golden b/expected-parse-errors/golden index 806c8b7..cadeb51 100644 --- a/expected-parse-errors/golden +++ b/expected-parse-errors/golden @@ -3878,105 +3878,97 @@ expecting expression or query expr scalarExpr ansi2011 a >* -BinOp (Iden [ Name Nothing "a" ]) [ Name Nothing ">" ] Star + +1:4: + | +1 | a >* + | ^ +unexpected * +expecting expression + queryExpr ansi2011 select a >* -Select - { qeSetQuantifier = SQDefault - , qeSelectList = - [ ( BinOp (Iden [ Name Nothing "a" ]) [ Name Nothing ">" ] Star - , Nothing - ) - ] - , qeFrom = [] - , qeWhere = Nothing - , qeGroupBy = [] - , qeHaving = Nothing - , qeOrderBy = [] - , qeOffset = Nothing - , qeFetchFirst = Nothing - } +1:11: + | +1 | select a >* + | ^ +unexpected * +expecting expression + queryExpr ansi2011 select a >*, -1:13: +1:11: | 1 | select a >*, - | ^ -unexpected end of input -expecting select item + | ^ +unexpected * +expecting expression queryExpr ansi2011 select a >* from -1:17: +1:11: | 1 | select a >* from - | ^ -unexpected end of input -expecting table ref + | ^ +unexpected * +expecting expression scalarExpr ansi2011 a >* b -1:6: +1:4: | 1 | a >* b - | ^ -unexpected b + | ^ +unexpected * +expecting expression queryExpr ansi2011 select a >* b -Select - { qeSetQuantifier = SQDefault - , qeSelectList = - [ ( BinOp (Iden [ Name Nothing "a" ]) [ Name Nothing ">" ] Star - , Just (Name Nothing "b") - ) - ] - , qeFrom = [] - , qeWhere = Nothing - , qeGroupBy = [] - , qeHaving = Nothing - , qeOrderBy = [] - , qeOffset = Nothing - , qeFetchFirst = Nothing - } +1:11: + | +1 | select a >* b + | ^ +unexpected * +expecting expression + queryExpr ansi2011 select a >* b, -1:15: +1:11: | 1 | select a >* b, - | ^ -unexpected end of input -expecting select item + | ^ +unexpected * +expecting expression queryExpr ansi2011 select a >* b from -1:19: +1:11: | 1 | select a >* b from - | ^ -unexpected end of input -expecting table ref + | ^ +unexpected * +expecting expression scalarExpr @@ -5147,94 +5139,61 @@ queryExpr ansi2011 select * as a -Select - { qeSetQuantifier = SQDefault - , qeSelectList = [ ( Star , Just (Name Nothing "a") ) ] - , qeFrom = [] - , qeWhere = Nothing - , qeGroupBy = [] - , qeHaving = Nothing - , qeOrderBy = [] - , qeOffset = Nothing - , qeFetchFirst = Nothing - } +1:10: + | +1 | select * as a + | ^^ +unexpected as +expecting from + queryExpr ansi2011 select t.* as a -Select - { qeSetQuantifier = SQDefault - , qeSelectList = - [ ( BinOp (Iden [ Name Nothing "t" ]) [ Name Nothing "." ] Star - , Just (Name Nothing "a") - ) - ] - , qeFrom = [] - , qeWhere = Nothing - , qeGroupBy = [] - , qeHaving = Nothing - , qeOrderBy = [] - , qeOffset = Nothing - , qeFetchFirst = Nothing - } +1:12: + | +1 | select t.* as a + | ^^ +unexpected as +expecting from + queryExpr ansi2011 select 3 + * -Select - { qeSetQuantifier = SQDefault - , qeSelectList = - [ ( BinOp (NumLit "3") [ Name Nothing "+" ] Star , Nothing ) ] - , qeFrom = [] - , qeWhere = Nothing - , qeGroupBy = [] - , qeHaving = Nothing - , qeOrderBy = [] - , qeOffset = Nothing - , qeFetchFirst = Nothing - } +1:12: + | +1 | select 3 + * + | ^ +unexpected * +expecting expression + queryExpr ansi2011 select case when * then 1 end -Select - { qeSetQuantifier = SQDefault - , qeSelectList = - [ ( Case - { caseTest = Nothing - , caseWhens = [ ( [ Star ] , NumLit "1" ) ] - , caseElse = Nothing - } - , Nothing - ) - ] - , qeFrom = [] - , qeWhere = Nothing - , qeGroupBy = [] - , qeHaving = Nothing - , qeOrderBy = [] - , qeOffset = Nothing - , qeFetchFirst = Nothing - } +1:18: + | +1 | select case when * then 1 end + | ^ +unexpected * +expecting expression + queryExpr ansi2011 select (*) -Select - { qeSetQuantifier = SQDefault - , qeSelectList = [ ( Parens Star , Nothing ) ] - , qeFrom = [] - , qeWhere = Nothing - , qeGroupBy = [] - , qeHaving = Nothing - , qeOrderBy = [] - , qeOffset = Nothing - , qeFetchFirst = Nothing - } +1:9: + | +1 | select (*) + | ^ +unexpected * +expecting expression or query expr + queryExpr ansi2011 diff --git a/tests/Language/SQL/SimpleSQL/QueryExprComponents.hs b/tests/Language/SQL/SimpleSQL/QueryExprComponents.hs index 77a922b..49e3ff5 100644 --- a/tests/Language/SQL/SimpleSQL/QueryExprComponents.hs +++ b/tests/Language/SQL/SimpleSQL/QueryExprComponents.hs @@ -73,7 +73,22 @@ selectLists = Group "selectLists" [(BinOp (Iden [Name Nothing "a"]) [Name Nothing "+"] (BinOp (Iden [Name Nothing "b"]) [Name Nothing "*"] (Iden [Name Nothing "c"])) ,Nothing)]} + ,q "select * from t" + $ toQueryExpr $ makeSelect {msSelectList = [(Star,Nothing)] + ,msFrom = [TRSimple [Name Nothing "t"]]} + ,q "select t.* from t" + $ toQueryExpr $ makeSelect {msSelectList = [(QStar [Name Nothing "t"],Nothing)] + ,msFrom = [TRSimple [Name Nothing "t"]]} + + ,q "select t.*, a as b, u.* from t" + $ toQueryExpr $ makeSelect + {msSelectList = + [(QStar [Name Nothing "t"],Nothing) + ,(Iden [Name Nothing "a"], Just $ Name Nothing "b") + ,(QStar [Name Nothing "u"],Nothing)] + ,msFrom = [TRSimple [Name Nothing "t"]]} + ] whereClause :: TestItem diff --git a/tests/Language/SQL/SimpleSQL/SQL2011Queries.hs b/tests/Language/SQL/SimpleSQL/SQL2011Queries.hs index 047bd2f..916db65 100644 --- a/tests/Language/SQL/SimpleSQL/SQL2011Queries.hs +++ b/tests/Language/SQL/SimpleSQL/SQL2011Queries.hs @@ -3283,8 +3283,9 @@ querySpecification = Group "query specification" ,("select distinct a from t",toQueryExpr $ ms {msSetQuantifier = Distinct}) ,("select * from t", toQueryExpr $ ms {msSelectList = [(Star,Nothing)]}) ,("select a.* from t" - ,toQueryExpr $ ms {msSelectList = [(BinOp (Iden [Name Nothing "a"]) [Name Nothing "."] Star - ,Nothing)]}) + ,toQueryExpr $ ms {msSelectList = + [(QStar [Name Nothing "a"] + ,Nothing)]}) ,("select a b from t" ,toQueryExpr $ ms {msSelectList = [(Iden [Name Nothing "a"], Just $ Name Nothing "b")]}) ,("select a as b from t" diff --git a/tests/Language/SQL/SimpleSQL/ScalarExprs.hs b/tests/Language/SQL/SimpleSQL/ScalarExprs.hs index 6aa075a..7088be2 100644 --- a/tests/Language/SQL/SimpleSQL/ScalarExprs.hs +++ b/tests/Language/SQL/SimpleSQL/ScalarExprs.hs @@ -68,9 +68,8 @@ identifiers = Group "identifiers" star :: TestItem star = Group "star" - [t "*" Star - --,("t.*", Star2 "t") - --,("ROW(t.*,42)", App "ROW" [Star2 "t", NumLit "42"]) + [t "count(*)" $ App [Name Nothing "count"] [Star] + ,t "ROW(t.*,42)" $ App [Name Nothing "ROW"] [QStar [Name Nothing "t"], NumLit "42"] ] parameter :: TestItem @@ -81,10 +80,8 @@ parameter = Group "parameter" dots :: TestItem dots = Group "dot" [t "t.a" $ Iden [Name Nothing "t",Name Nothing "a"] - ,t "t.*" $ BinOp (Iden [Name Nothing "t"]) [Name Nothing "."] Star ,t "a.b.c" $ Iden [Name Nothing "a",Name Nothing "b",Name Nothing "c"] - ,t "ROW(t.*,42)" - $ App [Name Nothing "ROW"] [BinOp (Iden [Name Nothing "t"]) [Name Nothing "."] Star, NumLit "42"] + ,t "ROW(t.*,42)" $ App [Name Nothing "ROW"] [QStar [Name Nothing "t"], NumLit "42"] ] app :: TestItem