fix: respect SORT direction when sorting GROUP BY keys - #2666
Open
hgabreu wants to merge 1 commit into
Open
Conversation
When a GROUP BY is followed by a SORT ... DESC, the group keys were always sorted ascending because the internal sort in executeCore() was hardcoded to ascending order. Fix: look ahead in the ops array from the GROUP BY step for an immediately following SORT operation and use its first field's direction to set the sort factor (-1 for descending, +1 for ascending/default). Fixes behaviour reported in issues blacksmithgu#966 and blacksmithgu#1671.
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
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.
Problem
When a
GROUP BYis followed bySORT ... DESC, the group keys are always sorted ascending. This is because the internal sort insideexecuteCore()was hardcoded to ascending order, regardless of the subsequentSORToperation.Queries like:
…produce groups in ascending order instead of descending.
Related issues: #966, #1671
Fix
In
executeCore(), changed thefor...ofloop to an indexed loop so the current position in the ops array is known. When processing agroupop, look ahead for an immediately followingsortop and read its first field's direction. Use that to set a sort factor (-1for descending,+1for ascending/default) applied to the group key comparator.Test
Verified locally by deploying the built plugin to an Obsidian vault and confirming that
GROUP BY ... SORT ... DESCnow produces groups in descending order.🤖 Generated with Claude Code