Skip to content

Commit

Permalink
Use lists to encode sets in Thrift
Browse files Browse the repository at this point in the history
Summary:
The set type in Hack is very limited and cannot have e.g. predicates as elements. Since the thrift set type translates to the Hack set type, that means that we cannot use sets in thrift to encode sets in Angle.

This is really sad because it means that we cannot use sets in other language APIs, like Haskell :-(

Reviewed By: donsbot

Differential Revision: D66158787

fbshipit-source-id: 05beed53d0e5de40c185848b6eae16c62024354c
  • Loading branch information
Josef Svenningsson authored and facebook-github-bot committed Nov 21, 2024
1 parent 61f6255 commit 7a1b27e
Show file tree
Hide file tree
Showing 6 changed files with 15 additions and 18 deletions.
3 changes: 2 additions & 1 deletion glean/db/Glean/Query/JSON.hs
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,8 @@ thriftType NatTy{} = 6
thriftType StringTy{} = 8
thriftType (ArrayTy ByteTy{}) = 8
thriftType ArrayTy{} = 9
thriftType SetTy{} = 10
thriftType (SetTy ByteTy{}) = 8
thriftType SetTy{} = 9
thriftType RecordTy{} = 12
thriftType SumTy{} = 12
thriftType (NamedTy (ExpandedType _ ty)) = thriftType ty
Expand Down
3 changes: 1 addition & 2 deletions glean/schema/gen/Glean/Schema/Gen/Haskell.hs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,6 @@ header here deps = Text.unlines $
, "import qualified Data.ByteString"
, "import qualified Data.Default"
, "import qualified Data.Text"
, "import qualified Data.Set"
, ""
-- we should use qualified imports as far as possible to avoid
-- clashing with Thrift-generated code
Expand Down Expand Up @@ -246,7 +245,7 @@ haskellTy_ withId genSub here t = case t of
SumTy{} -> shareTypeDef genSub here t
SetTy tInner -> do
inner <- haskellTy_ PredName genSub here tInner
return $ "Data.Set.Set " <> inner
return $ "[" <> inner <> "]"
MaybeTy ty -> do
inner <- haskellTy_ PredName genSub here ty
return (optionalize inner)
Expand Down
2 changes: 1 addition & 1 deletion glean/schema/gen/Glean/Schema/Gen/Thrift.hs
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ thriftTy here t = case t of
SumTy{} -> shareTypeDef here t
SetTy ty -> do
inner <- thriftTy here ty
return $ "set<" <> inner <> ">"
return $ "list<" <> inner <> ">"
MaybeTy tInner -> do
inner <- thriftTy here tInner
return (optionalize inner)
Expand Down
8 changes: 3 additions & 5 deletions glean/test/lib/TestData.hs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ module TestData
) where

import Data.Default
import qualified Data.Set as Set

import Glean.Typed
import Glean.Types
Expand Down Expand Up @@ -43,9 +42,8 @@ kitchenSink1 = def
, Glean.Test.kitchenSink_maybe_ = Just def
, Glean.Test.kitchenSink_bool_ = True
, Glean.Test.kitchenSink_string_ = "Hello\0world!\0"
, Glean.Test.kitchenSink_set_of_nat =
Set.fromList [toNat 1, toNat 2, toNat 65535]
, Glean.Test.kitchenSink_set_of_string = Set.fromList ["apa", "bepa"]
, Glean.Test.kitchenSink_set_of_nat = [toNat 65535, toNat 1, toNat 2 ]
, Glean.Test.kitchenSink_set_of_string = ["apa", "bepa"]
}

mkTestFacts :: NewFact m => (m () -> m ()) -> (m () -> m ()) -> m ()
Expand Down Expand Up @@ -108,7 +106,7 @@ mkTestFacts first second = do
, Glean.Test.kitchenSink_named_sum_ = Glean.Test.Sum_wed True
, Glean.Test.kitchenSink_named_record_ = rec
, Glean.Test.kitchenSink_maybe_ = Nothing
, Glean.Test.kitchenSink_set_of_pred = Set.fromList [kitchenSink2Fact0]
, Glean.Test.kitchenSink_set_of_pred = [kitchenSink2Fact0]
}

kitchenSink2Term1b = kitchenSink2Term1
Expand Down
10 changes: 5 additions & 5 deletions glean/test/tests/Angle/AngleTest.hs
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,11 @@ angleTest modify = dbTestCase $ \env repo -> do
glean.test.Predicate { named_sum_ = { tue = 37 } }
|]
print results
assertBool "angle - glean.test.Predicate 1" $
case results of
[Glean.Test.Predicate{Glean.Test.predicate_key = Just k}] ->
ignorePredK k == ignorePredK kitchenSink1
_ -> False
case results of
[Glean.Test.Predicate{Glean.Test.predicate_key = Just k}] ->
assertEqual "angle - glean.test.Predicate 1"
(ignorePredK kitchenSink1) (ignorePredK k)
_ -> assertBool "angle - glean.test.Predicate 1" False

-- match all results (two)
results <- runQuery_ env repo $ modify $ angle @Glean.Test.Predicate
Expand Down
7 changes: 3 additions & 4 deletions glean/test/tests/EncodingTest.hs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import qualified Data.ByteString.Base16 as Hex
import qualified Data.ByteString.Char8 as ByteString
import Data.Default
import Data.IntMap.Strict (IntMap)
import qualified Data.Set as Set
import Foreign.C.String
import Foreign.C.Types
import Foreign.Ptr
Expand Down Expand Up @@ -90,11 +89,11 @@ kitchenSink = KitchenSink
, kitchenSink_array2_of_string =
[ [ "a", "bc", "def" ] ]
, kitchenSink_set_of_nat =
Set.fromList [ Nat 1, Nat 3 ]
[ Nat 1, Nat 3 ]
, kitchenSink_set_of_string =
Set.fromList [ "foo", "bar" ]
[ "foo", "bar" ]
, kitchenSink_set_of_pred =
Set.fromList [ Glean.Test.Predicate 5432 Nothing ]
[ Glean.Test.Predicate 5432 Nothing ]
}

data E = E
Expand Down

0 comments on commit 7a1b27e

Please sign in to comment.