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

sub_select is not properly parenthesized under some conditions #79

Open
tysonzero opened this issue Mar 18, 2018 · 9 comments
Open

sub_select is not properly parenthesized under some conditions #79

tysonzero opened this issue Mar 18, 2018 · 9 comments
Labels

Comments

@tysonzero
Copy link

tysonzero commented Mar 18, 2018

Specifically the following fails with syntax error at or near "SELECT", because the inner sub_select is not properly enclosed in parenthesis.

aggrPrevDate :: MonadIO m => ReaderT SqlBackend m ()
aggrPrevDate = insertSelect . from $ \date -> do
    groupBy $ date ^. DateDate
    orderBy [asc $ date ^. DateDate]
    pure $ PrevDate <# (date ^. DateDate) <&> sub_select (getPrevDate date)
  where
    getPrevDate date = from $ \d -> do
        where_ $ d ^. DateDate <. date ^. DateDate
        pure . max_ $ d ^. DateDate

This is with:

esqueleto 2.5.3
persistent 2.7.1
persistent-postgresql 2.6.2.1
@tysonzero
Copy link
Author

Any update on this / any workarounds?

@bitemyapp
Copy link
Owner

Patches warmly welcomed!

@tysonzero
Copy link
Author

I can try and look into it whenever I next have some free time. It's just going to take me a long time to understand the internals of esqueleto, so I was hoping it would be a quick fix for someone who already understood the library.

@parsonsmatt
Copy link
Collaborator

Opened #126 with a failing test case.

@parsonsmatt
Copy link
Collaborator

In the interim, we're solving this in our work codebase with:

-- | The extra parens are needed when the argument is a `SELECT` statement.
unsafeSqlFunctionWithExtraParens
  :: UnsafeSqlFunctionArgument a
  => TLB.Builder
  -> a
  -> SqlExpr (Value b)
unsafeSqlFunctionWithExtraParens name' arg =
  ERaw Never $ \info ->
    let (argsTLB, argsVals) =
          uncommas' $ map (\(ERaw _ f) -> f info) $ toArgList arg
    in (name' <> parens (parens argsTLB), argsVals)

-- | If the argument is a 'sub_select' then you have to use this, otherwise it
-- goes boom.
sumSelect :: (Num a, Num b) => SqlExpr (Value a) -> SqlExpr (Value (Maybe b))
sumSelect = unsafeSqlFunctionWithExtraParens "SUM"

@hdgarrood
Copy link
Contributor

I think this might be due to the parens field of ERaw being ignored in makeWhere:

makeWhere :: IdentInfo -> WhereClause -> (TLB.Builder, [PersistValue])
makeWhere _    NoWhere                       = mempty
makeWhere info (Where v) = first ("\nWHERE " <>) $ x info
  where
    x =
        case v of
            ERaw _ f             -> f
            EAliasedValue i _    -> aliasedValueIdentToRawSql i
            EValueReference i i' -> valueReferenceToRawSql i i'
            ECompositeKey _      -> throw (CompositeKeyError -> EsqueletoError
CompositeKeyErr MakeWhereError)

@belevy
Copy link
Collaborator

belevy commented Jan 12, 2021

@hdgarrood that sounds plausible

@hdgarrood
Copy link
Contributor

Having looked a little bit more closely I think the issue I'm running into might be different from this one.

@belevy
Copy link
Collaborator

belevy commented Jan 12, 2021

It should be possible to avoid sub_select through the use of lateral joins (or just regular subquery joins if you don't need to correlate the subquery)

EDIT: oh didn't notice that this issue is in the insertSelect. Don't think .Experimental works with that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants