From ac406c077c0d859d03e2f70878905281b50096d0 Mon Sep 17 00:00:00 2001
From: Jake Wheat <jakewheatmail@gmail.com>
Date: Sat, 14 Dec 2013 11:23:58 +0200
Subject: [PATCH] small refactoring in the parser

factor out the integer parser for interval literals to separate lexer
refactor the app parser to include windows here instead of later on
add notes on the fixity handling
formatting in the cabal file
---
 Language/SQL/SimpleSQL/Parser.lhs | 37 ++++++++++++++++++++++---------
 simple-sql-parser.cabal           | 10 ++++-----
 2 files changed, 31 insertions(+), 16 deletions(-)

diff --git a/Language/SQL/SimpleSQL/Parser.lhs b/Language/SQL/SimpleSQL/Parser.lhs
index 2f6086a..56ba67b 100644
--- a/Language/SQL/SimpleSQL/Parser.lhs
+++ b/Language/SQL/SimpleSQL/Parser.lhs
@@ -87,7 +87,7 @@ interval '5' month
 >     IntervalLit
 >     <$> stringLiteral
 >     <*> identifierString
->     <*> optionMaybe (try $ parens (read <$> many1 digit))
+>     <*> optionMaybe (try $ parens integerLiteral)
 
 > literal :: P ScalarExpr
 > literal = number <|> estring <|> interval
@@ -151,22 +151,26 @@ functioncall(args) over ([partition by ids] [order by orderitems])
 
 No support for explicit frames yet.
 
+The convention in this file is that the 'Suffix', erm, suffix on
+parser names means that they have been left factored.
+
 > windowSuffix :: ScalarExpr -> P ScalarExpr
-> windowSuffix e@(App f es) =
->     choice [try (keyword_ "over")
->             *> parens (WindowApp f es
->                        <$> option [] partitionBy
->                        <*> option [] orderBy)
->            ,return e]
+> windowSuffix (App f es) =
+>     try (keyword_ "over")
+>     *> parens (WindowApp f es
+>                <$> option [] partitionBy
+>                <*> option [] orderBy)
 >   where
 >     partitionBy = try (keyword_ "partition") >>
 >         keyword_ "by" >>
 >         commaSep1 scalarExpr'
+> windowSuffix _ = fail ""
 
-> windowSuffix e = return e
+TODO: review all the suffix functions, use the optionSuffix combinator
+to simplify the suffix parsers
 
 > app :: P ScalarExpr
-> app = aggOrApp >>= windowSuffix
+> app = aggOrApp >>= optionSuffix windowSuffix
 
 == case expression
 
@@ -411,9 +415,16 @@ split out the binary op parsing from this function to above
 >     keywords ks = unwords <$> mapM keyword ks
 
 TODO: create the fixity adjuster. This should take a list of operators
-with precendence and associativity and adjust a scalar expr tree to
+with precedence and associativity and adjust a scalar expr tree to
 match these. It shouldn't attempt to descend into scalar expressions
-inside nested query exprs in subqueries.
+inside nested query exprs in subqueries. This way we separate out
+parsing from handling the precedence and associativity. Is it a good
+idea to separate these? I'm not sure. I think it makes some error
+messages potentially a little less helpful without some extra work,
+but apart from that, I think it is a win in terms of code clarity. The
+errors which are harder to produce nicely I think are limited to
+chained binary operators with no parens which have no associativity
+which should be a parse error.
 
 > {-sqlFixities :: [HSE.Fixity]
 > sqlFixities = HSE.infixl_ 9 ["*", "/"]
@@ -698,6 +709,10 @@ make this choice.
 >         i <- int
 >         return (p ++ "e" ++ s ++ i)
 
+lexer for integer literals which appear in some places in sql
+
+> integerLiteral :: P Int
+> integerLiteral = read <$> many1 digit <* whiteSpace
 
 whitespace parser which skips comments also
 
diff --git a/simple-sql-parser.cabal b/simple-sql-parser.cabal
index 27f5db4..ff17b69 100644
--- a/simple-sql-parser.cabal
+++ b/simple-sql-parser.cabal
@@ -2,7 +2,7 @@ name:                simple-sql-parser
 version:             0.1.0.0
 synopsis:            A parser for SQL queries
 
-description:         A parser for SQL queries, using Parsec. Also includes pretty printer. Aims to support most of SQL2003 queries plus other dialects.
+description:         A parser for SQL queries, using Parsec. Also includes pretty printer. Aims to support most of SQL2003 queries plus other SQL dialects.
 
 homepage:            https://github.com/JakeWheat/simple_sql_parser
 license:             BSD3
@@ -24,12 +24,12 @@ library
   exposed-modules:     Language.SQL.SimpleSQL.Pretty,
                        Language.SQL.SimpleSQL.Parser,
                        Language.SQL.SimpleSQL.Syntax
-  -- other-modules:       
-  -- other-extensions:    
+  -- other-modules:
+  -- other-extensions:
   build-depends:       base >=4.6 && <4.7,
                        parsec >=3.1 && <3.2,
                        mtl >=2.1 && <2.2,
                        pretty >= 1.1 && < 1.2
-  -- hs-source-dirs:      
+  -- hs-source-dirs:
   default-language:    Haskell2010
-  ghc-options: -Wall
\ No newline at end of file
+  ghc-options:         -Wall
\ No newline at end of file