Test OfflineClusterIntegrationTest.testDistinctCountHll in both engines#11427
Open
gortiz wants to merge 1 commit intoapache:masterfrom
Open
Test OfflineClusterIntegrationTest.testDistinctCountHll in both engines#11427gortiz wants to merge 1 commit intoapache:masterfrom
gortiz wants to merge 1 commit intoapache:masterfrom
Conversation
gortiz
force-pushed
the
multiStageHllBug
branch
from
ab3d546 to
74394f7
Compare
Codecov Report
@@ Coverage Diff @@
## master #11427 +/- ##
=============================================
- Coverage 62.93% 14.43% -48.50%
+ Complexity 1111 201 -910
=============================================
Files 2318 2318
Lines 124328 124321 -7
Branches 18980 18980
=============================================
- Hits 78241 17944 -60297
- Misses 40533 104862 +64329
+ Partials 5554 1515 -4039
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1496 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
gortiz
force-pushed
the
multiStageHllBug
branch
from
74394f7 to
bca1f51
Compare
gortiz
force-pushed
the
multiStageHllBug
branch
from
bca1f51 to
372c3ba
Compare
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
2 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.
This PR executes OfflineClusterIntegrationTest.testDistinctCountHll in both single and multi stage query engine.
While doing that, I found that V1 and V2 returned different values when the default
log2mwas used. The reason was that while V1 picked the default value fromdefault.hyperloglog.log2mconfig, V2 was using the default value without reading that config. And the reason for that is that the implementation in V1 does a rewrite in the broker. While V2 doesn't have anything like that.Ideally we should change the implementation of all different HLL operations in order to correctly use the value specified in the configuration. But I assume V1 doesn't do that for some reason (probably because operation implementation is static and cannot read the configuration). Therefore I decided to follow the same approach in V2 and do a rewrite.
In order to do so, I created a calcite RelRule.