(performance): fix double query execution when exceeding sizecap - #44
Open
McLavish wants to merge 2 commits into
Open
(performance): fix double query execution when exceeding sizecap#44McLavish wants to merge 2 commits into
McLavish wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a performance regression in the IPython jsoniq magic where exceeding conf.setResultSizeCap(...) could trigger an additional action (count()), causing the underlying query (including Spark jobs) to execute twice.
Changes:
- Remove the
response.count()call when results exceed the configured size cap. - Keep behavior limited to collecting/displaying only the first
ResultSizeCapitems.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
159
to
+160
| if len(capplusone) > rumble.getRumbleConf().getResultSizeCap(): | ||
| count = response.count() | ||
| print("The query output %s items, which is too many to display. Displaying the first %s items:" % (count, rumble.getRumbleConf().getResultSizeCap())) | ||
| print("The query output too many items to display. Displaying the first %s items:" % (rumble.getRumbleConf().getResultSizeCap())) |
Author
There was a problem hiding this comment.
Sorry @ghislainfourny I should disable automatic copilot reviews, they are really nitpicky and mostly pointless
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.
Expected behavior
When a query exceeds the configured
conf.setResultSizeCap(...), the output should be limited to the configured cap. Nothing else should happen underneath.Current behavior
When a query exceeds the configured
conf.setResultSizeCap(...), the query is re-executed through Spark, essentially causing it to execute twice.Case 1.1 - Correct behavior
Description:
JSONiq uses local JVM execution. No Spark job is executed, and no jobs are shown in Spark.
Case 1.2 - Unexpected behavior
Description:
JSONiq initially uses local JVM execution. Then, because the result exceeds
SizeCap(20), the extension triggers a second execution, this time on Spark.Case 2.1 - Correct behavior
Description:
JSONiq uses Spark to execute the query. The Spark job is executed exactly once.
Case 2.2 - Unexpected behavior
Description:
JSONiq uses Spark to execute the query. Then, because the result exceeds
SizeCap(20), the extension triggers a full second execution. As a result, the number of Spark jobs is doubled.