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.

Sign up for GitHub

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

Jump to bottom

Avoid rebinding PrimExprs that were created in the Aggregator itself #586

Merged
merged 3 commits into from
Feb 6, 2024
Merged

Avoid rebinding PrimExprs that were created in the Aggregator itself #586

merged 3 commits into from
Feb 6, 2024

Commits on Jan 21, 2024

  1. Fix typo

    @tomjaguarpaw
    tomjaguarpaw committed Jan 21, 2024
    Configuration menu
    Copy the full SHA
    b735e9a View commit details
    Browse the repository at this point in the history

  2. Add another, more subtle, failing test

    @tomjaguarpaw
    tomjaguarpaw committed Jan 21, 2024
    Configuration menu
    Copy the full SHA
    4dcf913 View commit details
    Browse the repository at this point in the history

  3. Avoid rebinding PrimExprs that were created in the Aggregator itself 

    It's redundant because we only need to rebind PrimExprs that came from
a lateral subquery.  As explained at [1], rather than carefully
analysing which PrimExprs came from a lateral subquery we can just
rebind everything.  A previous commit [2] changed things so that
everything mentioned in aggregator order expressions is rebound.
Instead we probably should have done what this commit does, that is,
added an Unpackspec constraint to aggregate.

Fixes #587

This is a simpler approach to resolving the issues discussed at

* #585
* #578

This still suffers from the problem described at:

#578 (comment)

i.e. if we find a way of duplicating field names, such as

    O.asc (\x -> snd x O..++ snd x)

then we can still create crashing queries.  The benefit of this
comment is that there is a way of generating non-crashing queries!

[1] https://github.com/tomjaguarpaw/haskell-opaleye/blob/52a4063188dd617ff91050dc0f2e27fc0570633c/src/Opaleye/Internal/Aggregate.hs#L111-L114

[2] d848317, part of
    #576
    @tomjaguarpaw
    tomjaguarpaw committed Jan 21, 2024
    Configuration menu
    Copy the full SHA
    0a2ef1b View commit details
    Browse the repository at this point in the history