Repository navigation
Refinement keeps the view's written order_by - #3136
Merged
Merged
Conversation
An inline refinement that sets only a limit dropped the view's order_by, and a named-view refinement left a written ordering marked as a computed default, so a later inline refinement dropped it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
view + { limit: N }threw away the view'sorder_by, so the query silently returned a different top N, ordered by the default instead. We overlooked this in #2022: whentop: N bywas removed, the guard inrefineFrom()that keeps the view's ordering changed from "unless the refinement has its ownorder_by" to "unless it has its ownlimit". Wrong since 0.0.218.Refinement is computed two ways, chosen by how the right side is written, and this also fixes an ordering bug in the other one. Since the two are easy to confuse, here is how
x + yworks:x + { ... }): a query builder re-enters x's fields and runs y's statements as though they were written inside x. When both setlimit:ororder_by:, y wins.finalize()then computes a default ordering if none was written.x + w):refine()inrefine-utils.tsmerges two finished segments. When both setlimit:, or both have a writtenorder_by:, it is an error (WN-0006 Lenses #1444, Lenses), which keeps combining named views independent of their order. A named view must have fields, so a barelimit:ororder_by:can only be inline; in practice inline means "refine this view" and named means "combine pieces".A segment's
orderByis either written by the author or computed by the translator and markeddefaultOrderBy(#2022). Both paths must carry a written ordering forward and recompute a computed one. The named path copied a written ordering over a computed one but left the flag set, so a later inline refinement of the result threw it away.The named path also never recomputes a computed ordering for the merged fields. That is #3135; the test showing it is checked in skipped.