MOD-12984 Persist query warnings across cursor reads #7865
Merged
MOD-12984 Persist query warnings across cursor reads #7865
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
meiravgri
force-pushed
the
meiravg_persist_warnings_in_areq
branch
from
c7684c8 to
7a9740e
Compare
meiravgri
force-pushed
the
meiravg_persist_warnings_in_areq
branch
from
0c37fab to
63ec332
Compare
![jit-ci[bot]](https://avatars.githubusercontent.com/in/142441?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Jit has detected 1 important finding in this PR that you should review.
Jit encountered an internal error and cannot comment on each finding.
You can ask a Jit admin to comment #jit_ignore_all on this PR to ignore the findings.
Here are the findings in this PR:
- Security Control: Static Code Analysis Semgrep Pro
- Type: Yaml.Github-Actions.Security.Run-Shell-Injection.Run-Shell-Injection
- Description: Using variable interpolation
${{...}}withgithubcontext data in arun:step could allow an attacker to inject their own code into the runner. This would allow them to steal secrets and code.githubcontext data can have arbitrary user input and should be treated as untrusted. Instead, use an intermediate environment variable withenv:to store the data and use the environment variable in therun:script. Be sure to use double-quotes the environment variable, like this: "$ENVVAR". - Severity: HIGH
- Learn More: Link
- Filename: .github/workflows/flow-build-artifacts.yml
- Lines: 69-90
There was a problem hiding this comment.
❌ Jit has detected 2 important findings in this PR that you should review.
Jit encountered an internal error and cannot comment on each finding.
You can ask a Jit admin to comment #jit_ignore_all on this PR to ignore the findings.
Here are the findings in this PR:
- Security Control: Static Code Analysis Semgrep Pro
- Type: Yaml.Github-Actions.Security.Run-Shell-Injection.Run-Shell-Injection
- Description: Using variable interpolation
${{...}}withgithubcontext data in arun:step could allow an attacker to inject their own code into the runner. This would allow them to steal secrets and code.githubcontext data can have arbitrary user input and should be treated as untrusted. Instead, use an intermediate environment variable withenv:to store the data and use the environment variable in therun:script. Be sure to use double-quotes the environment variable, like this: "$ENVVAR". - Severity: HIGH
- Learn More: Link
- Filename: .github/workflows/flow-build-artifacts.yml
- Lines: 70-92
- Security Control: Static Code Analysis Semgrep Pro
- Type: Yaml.Github-Actions.Security.Run-Shell-Injection.Run-Shell-Injection
- Description: Using variable interpolation
${{...}}withgithubcontext data in arun:step could allow an attacker to inject their own code into the runner. This would allow them to steal secrets and code.githubcontext data can have arbitrary user input and should be treated as untrusted. Instead, use an intermediate environment variable withenv:to store the data and use the environment variable in therun:script. Be sure to use double-quotes the environment variable, like this: "$ENVVAR". - Severity: HIGH
- Learn More: Link
- Filename: .github/workflows/task-build-artifacts.yml
- Lines: 260-276
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7865 +/- ##
==========================================
- Coverage 84.23% 84.04% -0.19%
==========================================
Files 361 361
Lines 56301 55098 -1203
Branches 15720 14508 -1212
==========================================
- Hits 47425 46308 -1117
+ Misses 8660 8599 -61
+ Partials 216 191 -25
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
introduce ProfileWarnings
return all warnings in sendchunk resp3 return all warnings as an array in profil;e
add testProfileMultipleWarnings and testPersistProfileWarning_MaxPrefixExpansions and test_multiple_warnings
meiravgri
changed the title
meiravgri
changed the title
lerman25
reviewed
Dec 31, 2025
JoanFM
reviewed
Dec 31, 2025
Contributor
|
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin 8.2
git worktree add -d .worktree/backport-7865-to-8.2 origin/8.2
cd .worktree/backport-7865-to-8.2
git switch --create backport-7865-to-8.2
git cherry-pick -x 2f474323ba2cec615af4a250b30556016927c0b2 |
Contributor
|
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin 8.4
git worktree add -d .worktree/backport-7865-to-8.4 origin/8.4
cd .worktree/backport-7865-to-8.4
git switch --create backport-7865-to-8.4
git cherry-pick -x 2f474323ba2cec615af4a250b30556016927c0b2 |
meiravgri
added a commit
that referenced
this pull request
Dec 31, 2025
* bit map place holder * save profilectx in AREQ introduce ProfileWarnings * return all warnings in coord reply return all warnings in sendchunk resp3 return all warnings as an array in profil;e * align tests to the new format add testProfileMultipleWarnings and testPersistProfileWarning_MaxPrefixExpansions and test_multiple_warnings * more fixes * test fixes * revert multiple warnings code relted changes * fixes * remove tests relarted to #7892 * imp testPersistProfileWarning_MaxPrefixExpansions * remove temporary test * fix mising BG indexing OOM * fix test * move to header * add oom test * asserts * detect timeout when there are multiple warnings in netCursorCallback * fix assert * enhance test_multiple_warnings to test we have exactly 1 cursor read * fix assert * test with oom (remove in the next commit) * fix test - only test with timeout * comment on ProfileWarningType * fix test * remove unused (cherry picked from commit 2f47432)
meiravgri
added a commit
that referenced
this pull request
Jan 1, 2026
* bit map place holder * save profilectx in AREQ introduce ProfileWarnings * return all warnings in coord reply return all warnings in sendchunk resp3 return all warnings as an array in profil;e * align tests to the new format add testProfileMultipleWarnings and testPersistProfileWarning_MaxPrefixExpansions and test_multiple_warnings * more fixes * test fixes * revert multiple warnings code relted changes * fixes * remove tests relarted to #7892 * imp testPersistProfileWarning_MaxPrefixExpansions * remove temporary test * fix mising BG indexing OOM * fix test * move to header * add oom test * asserts * detect timeout when there are multiple warnings in netCursorCallback * fix assert * enhance test_multiple_warnings to test we have exactly 1 cursor read * fix assert * test with oom (remove in the next commit) * fix test - only test with timeout * comment on ProfileWarningType * fix test * remove unused (cherry picked from commit 2f47432)
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jan 1, 2026
* MOD-12984 Persist query warnings across cursor reads (#7865) * bit map place holder * save profilectx in AREQ introduce ProfileWarnings * return all warnings in coord reply return all warnings in sendchunk resp3 return all warnings as an array in profil;e * align tests to the new format add testProfileMultipleWarnings and testPersistProfileWarning_MaxPrefixExpansions and test_multiple_warnings * more fixes * test fixes * revert multiple warnings code relted changes * fixes * remove tests relarted to #7892 * imp testPersistProfileWarning_MaxPrefixExpansions * remove temporary test * fix mising BG indexing OOM * fix test * move to header * add oom test * asserts * detect timeout when there are multiple warnings in netCursorCallback * fix assert * enhance test_multiple_warnings to test we have exactly 1 cursor read * fix assert * test with oom (remove in the next commit) * fix test - only test with timeout * comment on ProfileWarningType * fix test * remove unused (cherry picked from commit 2f47432) * fix includes and
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jan 1, 2026
* MOD-12984 Persist query warnings across cursor reads (#7865) * bit map place holder * save profilectx in AREQ introduce ProfileWarnings * return all warnings in coord reply return all warnings in sendchunk resp3 return all warnings as an array in profil;e * align tests to the new format add testProfileMultipleWarnings and testPersistProfileWarning_MaxPrefixExpansions and test_multiple_warnings * more fixes * test fixes * revert multiple warnings code relted changes * fixes * remove tests relarted to #7892 * imp testPersistProfileWarning_MaxPrefixExpansions * remove temporary test * fix mising BG indexing OOM * fix test * move to header * add oom test * asserts * detect timeout when there are multiple warnings in netCursorCallback * fix assert * enhance test_multiple_warnings to test we have exactly 1 cursor read * fix assert * test with oom (remove in the next commit) * fix test - only test with timeout * comment on ProfileWarningType * fix test * remove unused (cherry picked from commit 2f47432) * push last
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
3 participants
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
Profile warnings (timeout, max prefix expansions, OOM, etc.) triggered during early cursor reads were lost by the time the final profile was printed. This happened because warning state was computed fresh at print time from
QueryErrorflags, which is allocated on the stack each read.Solution
Introduce a
ProfileWarningsbitset that accumulates warnings throughout query execution, ensuring all warnings triggered during any cursor read are preserved and reported in the final profile output.Changes
New
ProfileWarningsbitset (profile.h)ProfileWarningstype (uint8_t bitset) with warning type enum flagsProfileWarnings_Add()andProfileWarnings_Has()functionsRefactored
ProfilePrinterCtx(profile.h){warnings, cursor_reads}cursor_readsfromAREQtoProfilePrinterCtxAREQ.has_timedout(now tracked in bitset)Updated warning tracking (
aggregate_exec.c)ProfileWarnings_Add()when each warning condition is detectedUpdated
Profile_Print(profile.c)AREQ*directly and reads warnings viaProfileWarnings_Has()AREQ*Note
Introduces persistent profiling warnings and refactors profile context to ensure warnings raised on any cursor read are preserved and printed.
ProfileWarningsbitset and helpers; refactorProfilePrinterCtxto{warnings, cursor_reads}and removeAREQ.has_timedout/movecursor_readsinto profile contextaggregate_exec.c) to accumulate warnings (timeout, query OOM, BG scan OOM, max prefix expansions) including empty-reply flows; invokeprofile(reply, req)withAREQ*AREQ*through profile printers (dist_aggregate.c) and iterate RESP3 warnings indist_utils.cfor timeout detectionProfile_Printto read warnings from bitset and report internal cursor reads fromprofileCtxWritten by Cursor Bugbot for commit 72b9b98. This will update automatically on new commits. Configure here.