diff --git a/src/PostgREST/Parsers.hs b/src/PostgREST/Parsers.hs index ede99707b..e7612e412 100644 --- a/src/PostgREST/Parsers.hs +++ b/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 . toS <$> (string "notin" <* pDelimiter) <*> pLVal + <|> In <$> (string "in" *> 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) diff --git a/src/PostgREST/QueryBuilder.hs b/src/PostgREST/QueryBuilder.hs index 054628665..b678c1188 100644 --- a/src/PostgREST/QueryBuilder.hs +++ b/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) <> ")" - where notOp = if hasNot_ then "NOT" else "" +pgFmtLogicTree qi (Expr hasNot op forest) = notOp <> " (" <> intercalate (" " <> show op <> " ") (pgFmtLogicTree qi <$> forest) <> ")" + where notOp = if hasNot then "NOT" else "" pgFmtLogicTree qi (Stmnt flt) = pgFmtFilter qi flt pgFmtJsonPath :: Maybe JsonPath -> SqlFragment diff --git a/src/PostgREST/Types.hs b/src/PostgREST/Types.hs index 20f5f9d32..f7e93714d 100644 --- a/src/PostgREST/Types.hs +++ b/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 | - In Operator [Text] | - Fts FtsMode (Maybe Language) Text | +data Operation = Op Operator SingleVal | + In ListVal | + 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 diff --git a/test/Feature/QuerySpec.hs b/test/Feature/QuerySpec.hs index 5510fa833..6a08a52de 100644 --- a/test/Feature/QuerySpec.hs +++ b/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 - get "/items_with_different_col_types?int_data=notin.()&select=int_data" `shouldRespondWith` - [json| [{int_data: 1}] |] { matchHeaders = [matchContentTypeJson] } + it "returns all results when the not.in value is empty between parentheses" $ get "/items_with_different_col_types?int_data=not.in.()&select=int_data" `shouldRespondWith` [json| [{int_data: 1}] |] { matchHeaders = [matchContentTypeJson] }