Remove isnot and notin operators and refactor

src/PostgREST/Parsers.hs

@@ -78,7 +78,7 @@ pFieldForest :: Parser [Tree SelectItem]
 pFieldForest = pFieldTree `sepBy1` lexeme (char ',')
 
 pFieldTree :: Parser (Tree SelectItem)
-pFieldTree =  try (Node <$> pRelationSelect <*> between (char '{') (char '}') pFieldForest)
+pFieldTree =  try (Node <$> pRelationSelect <*> between (char '{') (char '}') pFieldForest) -- TODO: "{}" deprecated
           <|> try (Node <$> pRelationSelect <*> between (char '(') (char ')') pFieldForest)
           <|> Node <$> pFieldSelect <*> pure []
 
@@ -130,14 +130,13 @@ pFieldSelect = lexeme $
     s <- pStar
     return ((s, Nothing), Nothing, Nothing, Nothing)
 
-pOpExpr :: Parser Text -> Parser [Text] -> Parser OpExpr
+pOpExpr :: Parser SingleVal -> Parser ListVal -> Parser OpExpr
 pOpExpr pSVal pLVal = try ( string "not" *> pDelimiter *> (OpExpr True <$> pOperation)) <|> OpExpr False <$> pOperation
   where
     pOperation :: Parser Operation
     pOperation =
           Op . toS <$> foldl1 (<|>) (try . ((<* pDelimiter) . string) . toS <$> M.keys ops) <*> pSVal
-      <|> In . toS <$> (string "in" <* pDelimiter) <*> pLVal
+      <|> In <$> (string "in" *> pDelimiter *> pLVal)
-      <|> In . toS <$> (string "notin" <* pDelimiter) <*> pLVal
       <|> pFts
       <?> "operator (eq, gt, ...)"
     pFts = do
@@ -149,14 +148,14 @@ pOpExpr pSVal pLVal = try ( string "not" *> pDelimiter *> (OpExpr True <$> pOper
           <|> try (string "fts" *> pDelimiter) *> pure Nothing
           <|> try (string "@@" *> pDelimiter)  *> pure Nothing -- TODO: '@@' deprecated
       Fts mode (toS <$> lang) <$> pSVal
-    ops = M.filterWithKey (const . flip notElem ["in", "notin", "fts", "@@"]) operators -- TODO: '@@' deprecated
+    ops = M.filterWithKey (const . flip notElem ["in", "fts", "@@"]) operators -- TODO: '@@' deprecated
 
-pSingleVal :: Parser Text
+pSingleVal :: Parser SingleVal
 pSingleVal = toS <$> many anyChar
 
-pListVal :: Parser [Text]
+pListVal :: Parser ListVal
 pListVal =    try (lexeme (char '(') *> pListElement `sepBy1` char ',' <* lexeme (char ')'))
-          <|> lexeme pListElement `sepBy1` char ','
+          <|> lexeme pListElement `sepBy1` char ',' -- TODO: "in.3,4,5" deprecated, parens e.g. "in.(3,4,5)" should be used
 
 pListElement :: Parser Text
 pListElement = try pQuotedValue <|> (toS <$> many (noneOf ",)"))
@@ -201,9 +200,10 @@ pLogicTree = Stmnt <$> try pLogicFilter
                <|> string "or" *> pure Or
                <?> "logic operator (and, or)"
 
-pLogicSingleVal :: Parser Text
+pLogicSingleVal :: Parser SingleVal
 pLogicSingleVal = try pQuotedValue <|> try pPgArray <|> (toS <$> many (noneOf ",)"))
   where
+    -- TODO: "{}" deprecated, after removal pPgArray can be removed
     pPgArray :: Parser Text
     pPgArray =  do
       a <- string "{"
@@ -211,7 +211,7 @@ pLogicSingleVal = try pQuotedValue <|> try pPgArray <|> (toS <$> many (noneOf ",
       c <- string "}"
       toS <$> pure (a ++ b ++ c)
 
-pLogicListVal :: Parser [Text]
+pLogicListVal :: Parser ListVal
 pLogicListVal = lexeme (char '(') *> pListElement `sepBy1` char ',' <* lexeme (char ')')
 
 pLogicPath :: Parser (EmbedPath, Text)

src/PostgREST/QueryBuilder.hs

@@ -413,15 +413,19 @@ pgFmtSelectItem table (f@(_, jp), Nothing, alias, _) = pgFmtField table f <> pgF
 pgFmtSelectItem table (f@(_, jp), Just cast, alias, _) = "CAST (" <> pgFmtField table f <> " AS " <> cast <> " )" <> pgFmtAs jp alias
 
 pgFmtFilter :: QualifiedIdentifier -> Filter -> SqlFragment
-pgFmtFilter table (Filter fld (OpExpr hasNot_ oper)) = notOp <> " " <> case oper of
+pgFmtFilter table (Filter fld (OpExpr hasNot oper)) = notOp <> " " <> case oper of
    Op op val  -> pgFmtFieldOp op <> " " <> case op of
      "like"   -> unknownLiteral (T.map star val)
      "ilike"  -> unknownLiteral (T.map star val)
      "is"     -> whiteList val
-     "isnot"  -> whiteList val
      _        -> unknownLiteral val
 
-   In op vals -> pgFmtIn op vals -- in and notin
+   In vals -> pgFmtField table fld <> " " <>
+    let emptyValForIn = "= any('{}') " in -- Workaround because for postgresql "col IN ()" is invalid syntax, we instead do "col = any('{}')"
+    case ((&&) (length vals == 1) . T.null) <$> headMay vals of
+      Just False -> sqlOperator "in" <> "(" <> intercalate ", " (map unknownLiteral vals) <> ") "
+      Just True  -> emptyValForIn
+      Nothing    -> emptyValForIn
 
    Fts mode lang val ->
      pgFmtFieldOp "fts" <> " " <> case mode of
@@ -435,27 +439,17 @@ pgFmtFilter table (Filter fld (OpExpr hasNot_ oper)) = notOp <> " " <> case oper
  where
    pgFmtFieldOp op = pgFmtField table fld <> " " <> sqlOperator op
    sqlOperator o = HM.lookupDefault "=" o operators
-   notOp = if hasNot_ then "NOT" else ""
+   notOp = if hasNot then "NOT" else ""
    star c = if c == '*' then '%' else c
    unknownLiteral = (<> "::unknown ") . pgFmtLit
    whiteList :: Text -> SqlFragment
    whiteList v = fromMaybe
      (toS (pgFmtLit v) <> "::unknown ")
      (find ((==) . toLower $ v) ["null","true","false"])
-   pgFmtIn :: Operator -> [Text] -> SqlFragment
-   pgFmtIn op vals =
-    -- Workaround because for postgresql "col IN ()" is invalid syntax, we instead do "col = any('{}')"
-    let emptyValForIn o = (if "not" `isInfixOf` o then "NOT " else "") -- handle case of "notin" operator
-                          <> pgFmtField table fld <> " = any('{}') " in
-    case T.null <$> headMay vals of
-      Just isNull -> if isNull && length vals == 1
-                        then emptyValForIn op
-                        else pgFmtFieldOp op <> "(" <> intercalate ", " (map unknownLiteral vals) <> ") "
-      Nothing -> emptyValForIn op
 
 pgFmtLogicTree :: QualifiedIdentifier -> LogicTree -> SqlFragment
-pgFmtLogicTree qi (Expr hasNot_ op forest) = notOp <> " (" <> intercalate (" " <> show op <> " ") (pgFmtLogicTree qi <$> forest) <> ")"
+pgFmtLogicTree qi (Expr hasNot op forest) = notOp <> " (" <> intercalate (" " <> show op <> " ") (pgFmtLogicTree qi <$> forest) <> ")"
-  where notOp =  if hasNot_ then "NOT" else ""
+  where notOp =  if hasNot then "NOT" else ""
 pgFmtLogicTree qi (Stmnt flt) = pgFmtFilter qi flt
 
 pgFmtJsonPath :: Maybe JsonPath -> SqlFragment

src/PostgREST/Types.hs

@@ -158,8 +158,6 @@ operators = M.fromList [
   ("like", "LIKE"),
   ("ilike", "ILIKE"),
   ("in", "IN"),
-  ("notin", "NOT IN"),
-  ("isnot", "IS NOT"),
   ("is", "IS"),
   ("fts", "@@"),
   ("cs", "@>"),
@@ -176,13 +174,17 @@ operators = M.fromList [
   ("<@", "<@")]
 
 data OpExpr = OpExpr Bool Operation deriving (Eq, Show)
-data Operation = Op Operator Text |
+data Operation = Op Operator SingleVal |
-                 In Operator [Text] |
+                 In ListVal |
-                 Fts FtsMode (Maybe Language) Text |
+                 Fts FtsMode (Maybe Language) SingleVal |
                  Join QualifiedIdentifier ForeignKey deriving (Eq, Show)
 
 data FtsMode = Normal | Plain | Phrase deriving (Eq, Show)
 type Language = Text
+-- | Represents a single value in a filter, e.g. id=eq.singleval
+type SingleVal = Text
+-- | Represents a list value in a filter, e.g. id=in.(val1,val2,val3)
+type ListVal = [Text]
 
 data LogicOperator = And | Or deriving Eq
 instance Show LogicOperator where

test/Feature/QuerySpec.hs

@@ -55,11 +55,6 @@ spec = do
         `shouldRespondWith` [json| [{"id":1},{"id":3},{"id":5}] |]
         { matchHeaders = ["Content-Range" <:> "0-2/*"] }
 
-    it "matches items NOT IN" $
-      get "/items?id=notin.2,4,6,7,8,9,10,11,12,13,14,15"
-        `shouldRespondWith` [json| [{"id":1},{"id":3},{"id":5}] |]
-        { matchHeaders = ["Content-Range" <:> "0-2/*"] }
-
     it "matches items NOT IN using not operator" $
       get "/items?id=not.in.2,4,6,7,8,9,10,11,12,13,14,15"
         `shouldRespondWith` [json| [{"id":1},{"id":3},{"id":5}] |]
@@ -892,7 +887,7 @@ spec = do
           , matchHeaders = []
           }
 
-  describe "values with quotes in IN and NOTIN operators" $ do
+  describe "values with quotes in IN and NOT IN" $ do
     it "succeeds when only quoted values are present" $ do
       get "/w_or_wo_comma_names?name=in.\"Hebdon, John\"" `shouldRespondWith`
         [json| [{"name":"Hebdon, John"}] |]
@@ -900,9 +895,6 @@ spec = do
       get "/w_or_wo_comma_names?name=in.\"Hebdon, John\",\"Williams, Mary\",\"Smith, Joseph\"" `shouldRespondWith`
         [json| [{"name":"Hebdon, John"},{"name":"Williams, Mary"},{"name":"Smith, Joseph"}] |]
         { matchHeaders = [matchContentTypeJson] }
-      get "/w_or_wo_comma_names?name=notin.\"Hebdon, John\",\"Williams, Mary\",\"Smith, Joseph\"" `shouldRespondWith`
-        [json| [{"name":"David White"},{"name":"Larry Thompson"}] |]
-        { matchHeaders = [matchContentTypeJson] }
       get "/w_or_wo_comma_names?name=not.in.\"Hebdon, John\",\"Williams, Mary\",\"Smith, Joseph\"" `shouldRespondWith`
         [json| [{"name":"David White"},{"name":"Larry Thompson"}] |]
         { matchHeaders = [matchContentTypeJson] }
@@ -914,9 +906,6 @@ spec = do
       get "/w_or_wo_comma_names?name=not.in.\"Hebdon, John\",Larry Thompson,\"Smith, Joseph\"" `shouldRespondWith`
         [json| [{"name":"Williams, Mary"},{"name":"David White"}] |]
         { matchHeaders = [matchContentTypeJson] }
-      get "/w_or_wo_comma_names?name=notin.\"Hebdon, John\",David White,\"Williams, Mary\",Larry Thompson" `shouldRespondWith`
-        [json| [{"name":"Smith, Joseph"}] |]
-        { matchHeaders = [matchContentTypeJson] }
 
     it "checks well formed quoted values" $ do
       get "/w_or_wo_comma_names?name=in.\"\"Hebdon, John\"" `shouldRespondWith`
@@ -953,10 +942,6 @@ spec = do
         get "/items_with_different_col_types?time_data=in." `shouldRespondWith`
           [json| [] |] { matchHeaders = [matchContentTypeJson] }
 
-    it "returns all results for notin when no value is present" $
-      get "/items_with_different_col_types?int_data=notin.&select=int_data" `shouldRespondWith`
-        [json| [{int_data: 1}] |] { matchHeaders = [matchContentTypeJson] }
-
     it "returns all results for not.in when no value is present" $
       get "/items_with_different_col_types?int_data=not.in.&select=int_data" `shouldRespondWith`
         [json| [{int_data: 1}] |] { matchHeaders = [matchContentTypeJson] }
@@ -972,9 +957,7 @@ spec = do
       get "/items_with_different_col_types?int_data=in.()" `shouldRespondWith`
         [json| [] |] { matchHeaders = [matchContentTypeJson] }
 
-    it "returns all results when the notin value is empty between parentheses" $ do
+    it "returns all results when the not.in value is empty between parentheses" $
-      get "/items_with_different_col_types?int_data=notin.()&select=int_data" `shouldRespondWith`
-        [json| [{int_data: 1}] |] { matchHeaders = [matchContentTypeJson] }
       get "/items_with_different_col_types?int_data=not.in.()&select=int_data" `shouldRespondWith`
         [json| [{int_data: 1}] |] { matchHeaders = [matchContentTypeJson] }