Introduce `sqlAll`, `sqlAny` and related state types #112

zlondrej · 2024-08-23T19:03:03Z

This allows writing certain helper functions which weren't possible before.

Specific use case is for filters:

data EqFilter a
  = Eq a
  | In [a]

newtype SomeUserInfo = SomeUserInfo { someUserInfo :: Text }

...hs
-- This should convert the `EqFilter` into `hpqtypes-extras` expressions
applyEqFilter :: forall v n . (MonadState v n, SqlWhere v) => EqFilter SomeUserInfo -> n ()
applyEqFilter = \case
  Eq value -> sqlWhereSomeInfo sqlWhereEq value
  In value -> sqlWhereSomeInfo sqlWhereIn value
  where
    -- Without the type that's used for `sqlWhereAny`, I couldn't figure out
    -- how to write the type signature so it would actually type check.
    sqlWhereSomeInfo :: (SQL -> a -> State SqlWhereAny ()) -> a -> n ()
    sqlWhereSomeInfo sqlCondition value =
      sqlWhereAny
        [ sqlCondition "first_name" value
        , sqlCondition "last_name" value
        , sqlCondition "email" value
        ]

Without the type available, one can only use constraints, but then there's an issue of ambiguous type variables:

    • Could not deduce (SqlWhere v2)
        arising from a use of ‘sqlWhereSomeInfo’
      from the context: (MonadState v n, SqlWhere v)
        bound by the type signature for:
                   applyEqFilter :: forall v (n :: * -> *).
                                          (MonadState v n, SqlWhere v) =>
                                          EqFilter SomeUserInfo -> n ()
        at ...
      The type variable ‘v2’ is ambiguous
      Potentially matching instances:
        instance SqlWhere SqlDelete
          -- Defined in ‘Database.PostgreSQL.PQTypes.SQL.Builder’
        instance SqlWhere SqlInsertSelect
          -- Defined in ‘Database.PostgreSQL.PQTypes.SQL.Builder’
        instance SqlWhere SqlSelect
          -- Defined in ‘Database.PostgreSQL.PQTypes.SQL.Builder’
        instance SqlWhere SqlUpdate
          -- Defined in ‘Database.PostgreSQL.PQTypes.SQL.Builder’
        ...plus one instance involving out-of-scope types
          Potentially matching instance:
            instance SqlWhere
                       Database.PostgreSQL.PQTypes.SQL.Builder.SqlAll
    • In the expression: sqlWhereSomeInfo sqlWhereEq value
      In a \case alternative:
          Eq value -> sqlWhereSomeInfo sqlWhereEq value
    |
    |           Eq value -> sqlWhereSomeInfo sqlWhereEq value
    |                       ^^^^^^^^^^^^^^^^

Note: I realized later that there's is a way to write the helper, which is by omitting the type signature entirely, but I don't see any reason why this type has to be hidden.

src/Database/PostgreSQL/PQTypes/SQL/Builder.hs

arybczak

There are a few hlint warnings btw.

arybczak · 2024-09-09T08:43:26Z

src/Database/PostgreSQL/PQTypes/SQL/Builder.hs

+-- | Type representing a set of conditions that are joined by 'AND'.
+--
+-- This is usually not needed as the default behavior is to join conditions
+-- by 'AND', but it is used to implement `sqlWhereAny`.


Is it? Is says the same thing about SqlWhereAny, it's a bit confusing.

I guess I'll remove the implementation details from the comments.

arybczak · 2024-09-09T08:45:29Z

src/Database/PostgreSQL/PQTypes/SQL/Builder.hs

+-- | Run monad that joins all conditions with 'AND' operator.
+--
+-- When no conditions are given, the result is 'TRUE'.
+sqlAll :: State SqlWhereAll () -> SQL


What is it for? Apart from being used in sqlWhereAny. Tests?

Because sqlWhere . sqlAll is redundant.

Well for nesting ANDs in OR's and parity with sqlAny. How else are you going to write (a OR (b AND c))?

I mean, you could use sqlWhereAny, but if you want to go with sqlWhere . sqlAny $ ..., then at one point you have to use sqlWhere . sqlAll $ ... inside.

Ah, I see. The comment you added to the code makes it clear, thanks.

arybczak · 2024-09-09T08:46:44Z

src/Database/PostgreSQL/PQTypes/SQL/Builder.hs

+-- | Run monad that joins all conditions with 'OR' operator.
+--
+-- When no conditions are given, the result is 'FALSE'.
+sqlAny :: State SqlWhereAny () -> SQL


Perhaps we should deprecate sqlWhereAny and start using sqlWhere . sqlAny $ do ... instead?

Depends if you'd like to remove it eventually. But if you want to deprecate sqlWhereAny, then I'd like to deprecate sqlOR, sqlConcatAND and sqlConcatOR as well.

If it comes to kontrakcja, sqlOR is used 1 time, sqlConcatAND 5 times and sqlConcatOR 4 times, so it seems feasible. Let's do it a separate PR though.

This allows writing cartain helper functions which weren't possible before.

Previous commit introduced some inconsistencies in terms of documentation vs implementation for `SqlWhereAny`. For that reason, I dediced to expand on the scope a little bit and introduce `WHERE` wrappers for `AND` and `OR` conditions.

arybczak

Looks good, thanks. Feel free to merge after addressing the comment about empty line in tests.

arybczak · 2024-09-20T12:41:52Z

src/Database/PostgreSQL/PQTypes/SQL/Builder.hs

+-- | Run monad that joins all conditions with 'AND' operator.
+--
+-- When no conditions are given, the result is 'TRUE'.
+sqlAll :: State SqlWhereAll () -> SQL


Ah, I see. The comment you added to the code makes it clear, thanks.

arybczak · 2024-09-20T12:46:41Z

test/Main.hs

+          , sqlWhere "cond4"
+          ]
+  ]
+


Why the empty line? :)

arybczak · 2024-09-20T12:48:10Z

src/Database/PostgreSQL/PQTypes/SQL/Builder.hs

+-- | Run monad that joins all conditions with 'OR' operator.
+--
+-- When no conditions are given, the result is 'FALSE'.
+sqlAny :: State SqlWhereAny () -> SQL


If it comes to kontrakcja, sqlOR is used 1 time, sqlConcatAND 5 times and sqlConcatOR 4 times, so it seems feasible. Let's do it a separate PR though.

arybczak · 2024-09-20T12:51:48Z

I forgot, please update the PR title (IIUC it's outdated) and add changelog entry.

zlondrej requested a review from kubek2k August 23, 2024 19:03

arybczak reviewed Sep 3, 2024

View reviewed changes

src/Database/PostgreSQL/PQTypes/SQL/Builder.hs Show resolved Hide resolved

zlondrej requested a review from arybczak September 4, 2024 10:09

arybczak reviewed Sep 9, 2024

View reviewed changes

zlondrej added 5 commits September 9, 2024 15:36

Export SqlWhereAny type so it can be used in type signatures

dfc7204

This allows writing cartain helper functions which weren't possible before.

Reintroduce SqlWhereAll back and add tests

37f6fd9

Previous commit introduced some inconsistencies in terms of documentation vs implementation for `SqlWhereAny`. For that reason, I dediced to expand on the scope a little bit and introduce `WHERE` wrappers for `AND` and `OR` conditions.

Remove redundant import

2e27eb2

Fix hlint reports

85c3c26

Update comments

79de921

zlondrej force-pushed the dev-janosik-export-sql-all branch from 083fba7 to 79de921 Compare September 9, 2024 13:38

zlondrej requested a review from arybczak September 9, 2024 13:38

arybczak approved these changes Sep 20, 2024

View reviewed changes

Format & update changelog

a5ad0ad

zlondrej changed the title ~~Export SqlWhereAny type so it can be used in type signatures~~ Introduce sqlAll, sqlAny and related state types Sep 30, 2024

zlondrej merged commit 573a865 into master Sep 30, 2024
7 checks passed

zlondrej deleted the dev-janosik-export-sql-all branch September 30, 2024 14:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce `sqlAll`, `sqlAny` and related state types #112

Introduce `sqlAll`, `sqlAny` and related state types #112

zlondrej commented Aug 23, 2024

arybczak left a comment

arybczak Sep 9, 2024 •

edited

Loading

zlondrej Sep 9, 2024

arybczak Sep 9, 2024

zlondrej Sep 9, 2024

arybczak Sep 20, 2024

arybczak Sep 9, 2024

zlondrej Sep 9, 2024

arybczak Sep 20, 2024

arybczak left a comment

arybczak Sep 20, 2024

arybczak Sep 20, 2024

arybczak Sep 20, 2024

arybczak commented Sep 20, 2024

Introduce sqlAll, sqlAny and related state types #112

Introduce sqlAll, sqlAny and related state types #112

Conversation

zlondrej commented Aug 23, 2024

arybczak left a comment

Choose a reason for hiding this comment

arybczak Sep 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arybczak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arybczak commented Sep 20, 2024

Introduce `sqlAll`, `sqlAny` and related state types #112

Introduce `sqlAll`, `sqlAny` and related state types #112

arybczak Sep 9, 2024 •

edited

Loading