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.

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

Add sample and check to FunnelBaseAggregationFunction #15251

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

vrajat
Copy link
Collaborator

@vrajat vrajat commented Mar 12, 2025

Fix for #15249

The PR adds a sample call to FunnelBaseAggregationFunction.java

A test is also added. PerQueryCPUMemResourceUsageAccountant clears state at the end of a query.
It cannot be used in tests to check if resources are being accounted. This class is a simple extension of PerQueryCPUMemResourceUsageAccountant that aggregates memory usage by query id.

Note that this is useful only in simple scenarios when one query is running.
This class has to be defined in the accounting package so that base & server starter classes can find it. It is not meant to be used outside of tests.

@codecov-commenter
Copy link

codecov-commenter commented Mar 12, 2025

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 63.63%. Comparing base (59551e4) to head (b8cfcd8).
Report is 1835 commits behind head on master.

Files with missing lines Patch % Lines
...n/funnel/window/FunnelBaseAggregationFunction.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #15251      +/-   ##
============================================
+ Coverage     61.75%   63.63%   +1.88%     
- Complexity      207     1461    +1254     
============================================
  Files          2436     2772     +336     
  Lines        133233   156248   +23015     
  Branches      20636    23981    +3345     
============================================
+ Hits          82274    99436   +17162     
- Misses        44911    49335    +4424     
- Partials       6048     7477    +1429     
Flag Coverage Δ
custom-integration1 100.00% <ø> (+99.99%) ⬆️
integration 100.00% <ø> (+99.99%) ⬆️
integration1 100.00% <ø> (+99.99%) ⬆️
integration2 0.00% <ø> (ø)
java-11 63.61% <0.00%> (+1.90%) ⬆️
java-21 63.51% <0.00%> (+1.89%) ⬆️
skip-bytebuffers-false 63.63% <0.00%> (+1.88%) ⬆️
skip-bytebuffers-true 63.50% <0.00%> (+35.77%) ⬆️
temurin 63.63% <0.00%> (+1.88%) ⬆️
unittests 63.63% <0.00%> (+1.88%) ⬆️
unittests1 56.19% <0.00%> (+9.30%) ⬆️
unittests2 34.17% <0.00%> (+6.44%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@vrajat vrajat requested review from xiangfu0 and yashmayya March 13, 2025 05:41
@vrajat vrajat added the bugfix label Mar 13, 2025
@vrajat vrajat marked this pull request as ready for review March 13, 2025 05:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants