Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow a prefix for ?? placeholders #1018

Merged
merged 6 commits into from
Jan 28, 2020
Merged

Conversation

parsonsmatt
Copy link
Collaborator

@parsonsmatt parsonsmatt commented Jan 17, 2020

Before submitting your PR, check that you've:

After submitting your PR:

  • Update the Changelog.md file with a link to your PR
  • Check that CI passes (or if it fails, for reasons unrelated to your change, like CI timeouts)

OK, so this is going to be trickier than it should be. The code that actually does the name prefixing is here:

instance
    (PersistEntity record, PersistEntityBackend record ~ backend, IsPersistBackend backend) =>
    RawSql (Entity record) where
    rawSqlCols escape _ent = (length sqlFields, [intercalate ", " sqlFields])
        where
          sqlFields = map (((name <> ".") <>) . escape)
              $ map fieldDB
              -- Hacky for a composite key because
              -- it selects the same field multiple times
              $ entityKeyFields entDef ++ entityFields entDef
          name = escape (entityDB entDef)
          entDef = entityDef (Nothing :: Maybe record)

So we arrive at name from the entityDef, and then we immediately collapse that information into a concatenated string. We don't have a good hook to intercept this.

A possible solution is a newtype:

newtype EntityWithPrefix (sym :: Symbol) (record :: Type) = EntityWithPrefix (Entity record)

instance
    (KnownSymbol prefix, PersistEntity record, PersistEntityBackend record ~ backend, IsPersistBackend backend) =>
    RawSql (EntityWithPrefix prefix record) where
    rawSqlCols escape _ent = (length sqlFields, [intercalate ", " sqlFields])
        where
          sqlFields = map (((name <> ".") <>) . escape)
              $ map fieldDB
              -- Hacky for a composite key because
              -- it selects the same field multiple times
              $ entityKeyFields entDef ++ entityFields entDef
          name = symbolVal (Proxy :: Proxy prefix)
          entDef = entityDef (Nothing :: Maybe record)

This would require the user to write:

result :: [(EntityWithPrefix "parent" Relationship, EntityWithPrefix "child" Relationship
  <- rawSql "SELECT ??, ?? FROM relationship AS parent LEFT JOIN relationship AS child ON ..." []

I'm not super pleased with this, but it's a drop-in solution that doesn't require modifying any existing code.

Another option is to parse the column substitutions returned from this and re-splice the desired prefix. Kind of gross, but it should work seamlessly - replace (escape entityName) (escape desiredPrefix) columnSubstitutions. How do we get the desired name, though?

Currently, the implementation splits on ??, performs substitution, and then rejoins:

-- module Database.Persist.Sql.Raw
      withStmt' colSubsts params sink = do
            srcRes <- rawQueryRes sql params
            liftIO $ with srcRes (\src -> runConduit $ src .| sink)
          where
            sql = T.concat $ makeSubsts colSubsts $ T.splitOn placeholder stmt
            placeholder = "??"
            makeSubsts (s:ss) (t:ts) = t : s : makeSubsts ss ts
            makeSubsts []     []     = []
            makeSubsts []     ts     = [T.intercalate placeholder ts]
            makeSubsts ss     []     = error (concat err)
                where
                  err = [ "rawsql: there are still ", show (length ss)
                        , "'??' placeholder substitutions to be made "
                        , "but all '??' placeholders have already been "
                        , "consumed.  Please read 'rawSql's documentation "
                        , "on how '??' placeholders work."
                        ]

If we take T.uncons on the text, and that is a ., then we should be able to dig out the previous few characters from it and pick that as the desired name. This will need to respect escaping etc so it's not going to be trivial to write the code to capture it. But a good first pass can merely require that it's an "easy" identifier.

This is a bit nasty, but it's not a breaking change and doesn't require any newtype wrangling.

Fixes #1017

@parsonsmatt parsonsmatt changed the title [WIP] Allow a prefix for ?? placeholders Allow a prefix for ?? placeholders Jan 27, 2020

it "commit/rollback" $ do
caseCommitRollback runDb
runDb cleanDB
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was a mix of 2 space and 4 space indent :| Fixed it here, but it gives a noisy diff.

@parsonsmatt parsonsmatt added this to the 2.10.5 milestone Jan 28, 2020
@parsonsmatt parsonsmatt merged commit 6665283 into master Jan 28, 2020
@parsonsmatt parsonsmatt deleted the matt/prefix-entity-placeholder branch March 31, 2020 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow prefixing of ??
1 participant