You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by "ngsg (via GitHub)" <gi...@apache.org> on 2025/03/04 12:17:15 UTC
[PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
ngsg opened a new pull request, #5674:
URL: https://github.com/apache/hive/pull/5674
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://cwiki.apache.org/confluence/display/Hive/HowToContribute
2. Ensure that you have created an issue on the Hive project JIRA: https://issues.apache.org/jira/projects/HIVE/summary
3. Ensure you have added or run the appropriate tests for your PR:
4. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]HIVE-XXXXX: Your PR title ...'.
5. Be sure to keep the PR description updated to reflect all changes.
6. Please write your PR title to summarize what this PR proposes.
7. If possible, provide a concise example to reproduce the issue for a faster review.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
This patch implements a sanity-check in `LongColumnStatsAggregator`. The added check ensures that the estimated NDV is always less than or equal to the maximum number of integers within the estimated range.
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
If the estimated range of a long type column is `[A, B]`, then the maximum number of distinct values in the column should be `B - A + 1` (assuming no nulls in the column). Therefore, any integer that is larger than `B - A + 1` cannot be the estimated value of NDV.
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description, screenshot and/or a reproducable example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Hive versions or within the unreleased branches such as master.
If no, write 'No'.
-->
No
### Is the change a dependency upgrade?
<!--
If yes, please attach a file with output from mvn dependency:tree to validate a complete upgrade of dependency.
If no, write 'No'.
-->
No
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
The patch is tested manually by printing some debug messages in bitvector-enabled HMS. I'll add a unit test for it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5674:
URL: https://github.com/apache/hive/pull/5674#discussion_r2023059194
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java:
##########
@@ -242,6 +242,24 @@ public ColumnStatisticsObj aggregate(List<ColStatsObjWithSourceInfo> colStatsWit
}
statsObj.setStatsData(columnStatisticsData);
+
+ // NDV cannot exceed the number of integers within the given range.
+ long numIntInRange = columnStatisticsData.getLongStats().getHighValue()
Review Comment:
if `high`=`low`, `densityAvgSum` would be `0` and when evaluating `estimation`, we'll be dividing by `0`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5674:
URL: https://github.com/apache/hive/pull/5674#discussion_r2025030382
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java:
##########
@@ -343,4 +356,23 @@ public void extrapolate(ColumnStatisticsData extrapolateData, int numParts,
extrapolateLongData.setNumNulls(numNulls);
extrapolateLongData.setNumDVs(ndv);
}
+
+ private void adjustNDV(ColumnStatisticsData columnStatisticsData) {
+ // NDV cannot exceed the number of integers within the given range.
+ long numIntInRange = columnStatisticsData.getLongStats().getHighValue()
Review Comment:
@ngsg , could we please rename var to `estimation`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5674:
URL: https://github.com/apache/hive/pull/5674#issuecomment-2713643483
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5674) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5674&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5674&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5674&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5674&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5674&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5674)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5674:
URL: https://github.com/apache/hive/pull/5674#discussion_r2009155605
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java:
##########
@@ -242,6 +242,24 @@ public ColumnStatisticsObj aggregate(List<ColStatsObjWithSourceInfo> colStatsWit
}
statsObj.setStatsData(columnStatisticsData);
+
+ // NDV cannot exceed the number of integers within the given range.
Review Comment:
should we move the NDV correction logic inside of `extrapolate`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "ngsg (via GitHub)" <gi...@apache.org>.
ngsg commented on PR #5674:
URL: https://github.com/apache/hive/pull/5674#issuecomment-2766514905
@deniskuzZ , sorry for the delay. I missed your comments but have checked them now. I'll address your review and update the PR soon. Thanks for the reminder.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5674:
URL: https://github.com/apache/hive/pull/5674#discussion_r2023130779
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java:
##########
@@ -192,7 +207,13 @@ public ColumnStatisticsObj aggregate(List<ColStatsObjWithSourceInfo> colStatsWit
csd.setLongStats(aggregateData);
adjustedStatsMap.put(pseudoPartName.toString(), csd);
if (useDensityFunctionForNDVEstimation) {
- densityAvgSum += ((double) (aggregateData.getHighValue() - aggregateData.getLowValue())) / aggregateData.getNumDVs();
+ if (aggregateData.getNumDVs() == 0) {
Review Comment:
Could it be `0` after `ndvEstimator.estimateNumDistinctValues()`? Do we have a test validating this?
if yes, isn't it the same here?
https://github.com/apache/hive/pull/5674/files#diff-004a5c809d016b31028b52e5a5e591d30b446d974b0d22e386c35acfee845d59R248
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5674:
URL: https://github.com/apache/hive/pull/5674#discussion_r2009150534
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java:
##########
@@ -242,6 +242,24 @@ public ColumnStatisticsObj aggregate(List<ColStatsObjWithSourceInfo> colStatsWit
}
statsObj.setStatsData(columnStatisticsData);
+
+ // NDV cannot exceed the number of integers within the given range.
+ long numIntInRange = columnStatisticsData.getLongStats().getHighValue()
Review Comment:
should we fix this as well since we could try to divide by 0
````
densityAvgSum += ((double) (newData.getHighValue() - newData.getLowValue())) / newData.getNumDVs();
double densityAvg = densityAvgSum / partNames.size();
estimation = (long) ((aggregateData.getHighValue() - aggregateData.getLowValue()) / densityAvg);
````
`extrapolate` method has `densityAvg != 0.0` check
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "ngsg (via GitHub)" <gi...@apache.org>.
ngsg commented on code in PR #5674:
URL: https://github.com/apache/hive/pull/5674#discussion_r2024629146
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java:
##########
@@ -192,7 +207,13 @@ public ColumnStatisticsObj aggregate(List<ColStatsObjWithSourceInfo> colStatsWit
csd.setLongStats(aggregateData);
adjustedStatsMap.put(pseudoPartName.toString(), csd);
if (useDensityFunctionForNDVEstimation) {
- densityAvgSum += ((double) (aggregateData.getHighValue() - aggregateData.getLowValue())) / aggregateData.getNumDVs();
+ if (aggregateData.getNumDVs() == 0) {
Review Comment:
You're right; according to my previous comment, this should be also nonzero. Thank you for pointing out my mistake. I'll remove is-zero test here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5674:
URL: https://github.com/apache/hive/pull/5674#issuecomment-2774391276
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5674) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5674&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5674&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5674&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5674&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5674&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5674)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "ngsg (via GitHub)" <gi...@apache.org>.
ngsg commented on code in PR #5674:
URL: https://github.com/apache/hive/pull/5674#discussion_r2025889555
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java:
##########
@@ -343,4 +356,23 @@ public void extrapolate(ColumnStatisticsData extrapolateData, int numParts,
extrapolateLongData.setNumNulls(numNulls);
extrapolateLongData.setNumDVs(ndv);
}
+
+ private void adjustNDV(ColumnStatisticsData columnStatisticsData) {
+ // NDV cannot exceed the number of integers within the given range.
+ long numIntInRange = columnStatisticsData.getLongStats().getHighValue()
Review Comment:
Sure, I renamed it to `estimation`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5674:
URL: https://github.com/apache/hive/pull/5674#discussion_r2023138045
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java:
##########
@@ -163,7 +172,13 @@ public ColumnStatisticsObj aggregate(List<ColStatsObjWithSourceInfo> colStatsWit
String partName = csp.getPartName();
LongColumnStatsData newData = cso.getStatsData().getLongStats();
if (useDensityFunctionForNDVEstimation) {
- densityAvgSum += ((double) (newData.getHighValue() - newData.getLowValue())) / newData.getNumDVs();
+ if (newData.getNumDVs() == 0) {
Review Comment:
could it be `0`? if yes, isn't this the same here?
https://github.com/apache/hive/pull/5674/files#diff-004a5c809d016b31028b52e5a5e591d30b446d974b0d22e386c35acfee845d59R105
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on PR #5674:
URL: https://github.com/apache/hive/pull/5674#issuecomment-2769887634
@ngsg, added a few comments, please. take a look. I think `0` check is only needed at line #132
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "ngsg (via GitHub)" <gi...@apache.org>.
ngsg commented on code in PR #5674:
URL: https://github.com/apache/hive/pull/5674#discussion_r2025889153
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java:
##########
@@ -343,4 +356,23 @@ public void extrapolate(ColumnStatisticsData extrapolateData, int numParts,
extrapolateLongData.setNumNulls(numNulls);
extrapolateLongData.setNumDVs(ndv);
}
+
+ private void adjustNDV(ColumnStatisticsData columnStatisticsData) {
Review Comment:
I changed the parameter to `LongColumnStatsData columnStats`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5674:
URL: https://github.com/apache/hive/pull/5674#discussion_r2023085228
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java:
##########
@@ -192,7 +207,13 @@ public ColumnStatisticsObj aggregate(List<ColStatsObjWithSourceInfo> colStatsWit
csd.setLongStats(aggregateData);
adjustedStatsMap.put(pseudoPartName.toString(), csd);
if (useDensityFunctionForNDVEstimation) {
- densityAvgSum += ((double) (aggregateData.getHighValue() - aggregateData.getLowValue())) / aggregateData.getNumDVs();
+ if (aggregateData.getNumDVs() == 0) {
+ // If NDV is not present, assume that the density is 0.5.
+ densityAvgSum += 0.5;
+ } else {
+ double range = aggregateData.getHighValue() - aggregateData.getLowValue();
Review Comment:
no need for `range` var
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5674:
URL: https://github.com/apache/hive/pull/5674#discussion_r2023138045
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java:
##########
@@ -163,7 +172,13 @@ public ColumnStatisticsObj aggregate(List<ColStatsObjWithSourceInfo> colStatsWit
String partName = csp.getPartName();
LongColumnStatsData newData = cso.getStatsData().getLongStats();
if (useDensityFunctionForNDVEstimation) {
- densityAvgSum += ((double) (newData.getHighValue() - newData.getLowValue())) / newData.getNumDVs();
+ if (newData.getNumDVs() == 0) {
Review Comment:
could it be `0` here as well?
https://github.com/apache/hive/pull/5674/files#diff-004a5c809d016b31028b52e5a5e591d30b446d974b0d22e386c35acfee845d59R105
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5674:
URL: https://github.com/apache/hive/pull/5674#discussion_r2009155605
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java:
##########
@@ -242,6 +242,24 @@ public ColumnStatisticsObj aggregate(List<ColStatsObjWithSourceInfo> colStatsWit
}
statsObj.setStatsData(columnStatisticsData);
+
+ // NDV cannot exceed the number of integers within the given range.
Review Comment:
should we move the NDV correction logic inside of `extrapolate` or extract to a separate method like `adjustNDV()`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5674:
URL: https://github.com/apache/hive/pull/5674#issuecomment-2776042282
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5674) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5674&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5674&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5674&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5674&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5674&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5674)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5674:
URL: https://github.com/apache/hive/pull/5674#discussion_r2023064382
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java:
##########
@@ -242,6 +242,24 @@ public ColumnStatisticsObj aggregate(List<ColStatsObjWithSourceInfo> colStatsWit
}
statsObj.setStatsData(columnStatisticsData);
+
+ // NDV cannot exceed the number of integers within the given range.
Review Comment:
ok, but isn't correction applied at the very end after `extrapolate ` now?
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java:
##########
@@ -242,6 +242,24 @@ public ColumnStatisticsObj aggregate(List<ColStatsObjWithSourceInfo> colStatsWit
}
statsObj.setStatsData(columnStatisticsData);
+
+ // NDV cannot exceed the number of integers within the given range.
Review Comment:
ok, but isn't correction applied at the end after `extrapolate ` now?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5674:
URL: https://github.com/apache/hive/pull/5674#discussion_r2023091932
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java:
##########
@@ -192,7 +207,13 @@ public ColumnStatisticsObj aggregate(List<ColStatsObjWithSourceInfo> colStatsWit
csd.setLongStats(aggregateData);
adjustedStatsMap.put(pseudoPartName.toString(), csd);
if (useDensityFunctionForNDVEstimation) {
- densityAvgSum += ((double) (aggregateData.getHighValue() - aggregateData.getLowValue())) / aggregateData.getNumDVs();
+ if (aggregateData.getNumDVs() == 0) {
Review Comment:
could it be `0` here as well?
https://github.com/apache/hive/pull/5674/files#diff-004a5c809d016b31028b52e5a5e591d30b446d974b0d22e386c35acfee845d59L227
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5674:
URL: https://github.com/apache/hive/pull/5674#issuecomment-2768594175
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5674) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5674&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5674&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5674&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5674&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5674&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5674)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5674:
URL: https://github.com/apache/hive/pull/5674#discussion_r2023064382
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java:
##########
@@ -242,6 +242,24 @@ public ColumnStatisticsObj aggregate(List<ColStatsObjWithSourceInfo> colStatsWit
}
statsObj.setStatsData(columnStatisticsData);
+
+ // NDV cannot exceed the number of integers within the given range.
Review Comment:
ok, but isn't correction applied at the end, after `extrapolate ` now?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5674:
URL: https://github.com/apache/hive/pull/5674#issuecomment-2774958920
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5674) **Quality Gate passed**
Issues
 [12 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5674&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5674&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5674&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5674&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5674&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5674)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "ngsg (via GitHub)" <gi...@apache.org>.
ngsg commented on PR #5674:
URL: https://github.com/apache/hive/pull/5674#issuecomment-2772588355
@deniskuzZ , I have checked your comments and agree that we only need to guard division-by-zero issue at `#132`, as you mentioned. I have added a comment for each case and updated the patch. Could you please confirm them? Thank you.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5674:
URL: https://github.com/apache/hive/pull/5674#issuecomment-2700597211
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5674) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5674&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5674&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5674&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5674&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5674&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5674)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5674:
URL: https://github.com/apache/hive/pull/5674#discussion_r2009155605
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java:
##########
@@ -242,6 +242,24 @@ public ColumnStatisticsObj aggregate(List<ColStatsObjWithSourceInfo> colStatsWit
}
statsObj.setStatsData(columnStatisticsData);
+
+ // NDV cannot exceed the number of integers within the given range.
Review Comment:
should we move the NDV correction logic inside of `extrapolate` and/or extract to a separate method like `adjustNDV()`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "ngsg (via GitHub)" <gi...@apache.org>.
ngsg commented on code in PR #5674:
URL: https://github.com/apache/hive/pull/5674#discussion_r2021220624
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java:
##########
@@ -242,6 +242,24 @@ public ColumnStatisticsObj aggregate(List<ColStatsObjWithSourceInfo> colStatsWit
}
statsObj.setStatsData(columnStatisticsData);
+
+ // NDV cannot exceed the number of integers within the given range.
+ long numIntInRange = columnStatisticsData.getLongStats().getHighValue()
Review Comment:
```
densityAvgSum += ((double) (newData.getHighValue() - newData.getLowValue())) / newData.getNumDVs();
```
There might be an invariant such as columnStats being present implies `NDV != 0`, but it's safer to explicitly guard against division by zero. I'll update the code accordingly.
---
```
double densityAvg = densityAvgSum / partNames.size();
```
If `partNames.size()` is 0, we should return an empty ColumnStats or null, as there's nothing to aggregate. I'll check the convention and adjust the code to handle this case properly.
---
```
estimation = (long) ((aggregateData.getHighValue() - aggregateData.getLowValue()) / densityAvg);
```
It seems that `densityAvg` is 0 in some cases. I'll fix it as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "ngsg (via GitHub)" <gi...@apache.org>.
ngsg commented on PR #5674:
URL: https://github.com/apache/hive/pull/5674#issuecomment-2768472166
@deniskuzZ I've addressed your review and updated the patch accordingly. For the division by zero concerns, I didn't modify the division by `partNames.size()` and `ndvEstimator.estimateNumDistinctValues()` because, if I understand them correctly, they are always positive. Could you please review the changes? Thank you.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5674:
URL: https://github.com/apache/hive/pull/5674#discussion_r2023091932
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java:
##########
@@ -192,7 +207,13 @@ public ColumnStatisticsObj aggregate(List<ColStatsObjWithSourceInfo> colStatsWit
csd.setLongStats(aggregateData);
adjustedStatsMap.put(pseudoPartName.toString(), csd);
if (useDensityFunctionForNDVEstimation) {
- densityAvgSum += ((double) (aggregateData.getHighValue() - aggregateData.getLowValue())) / aggregateData.getNumDVs();
+ if (aggregateData.getNumDVs() == 0) {
Review Comment:
could it be `0` here as well?
https://github.com/apache/hive/pull/5674/files#diff-004a5c809d016b31028b52e5a5e591d30b446d974b0d22e386c35acfee845d59L227
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5674:
URL: https://github.com/apache/hive/pull/5674#discussion_r2023130779
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java:
##########
@@ -192,7 +207,13 @@ public ColumnStatisticsObj aggregate(List<ColStatsObjWithSourceInfo> colStatsWit
csd.setLongStats(aggregateData);
adjustedStatsMap.put(pseudoPartName.toString(), csd);
if (useDensityFunctionForNDVEstimation) {
- densityAvgSum += ((double) (aggregateData.getHighValue() - aggregateData.getLowValue())) / aggregateData.getNumDVs();
+ if (aggregateData.getNumDVs() == 0) {
Review Comment:
Could it be `0` after `ndvEstimator.estimateNumDistinctValues()`?
if yes, isn't it the same here?
https://github.com/apache/hive/pull/5674/files#diff-004a5c809d016b31028b52e5a5e591d30b446d974b0d22e386c35acfee845d59R248
Do we have a test validating this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "ngsg (via GitHub)" <gi...@apache.org>.
ngsg commented on code in PR #5674:
URL: https://github.com/apache/hive/pull/5674#discussion_r2024816977
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java:
##########
@@ -163,7 +172,13 @@ public ColumnStatisticsObj aggregate(List<ColStatsObjWithSourceInfo> colStatsWit
String partName = csp.getPartName();
LongColumnStatsData newData = cso.getStatsData().getLongStats();
if (useDensityFunctionForNDVEstimation) {
- densityAvgSum += ((double) (newData.getHighValue() - newData.getLowValue())) / newData.getNumDVs();
+ if (newData.getNumDVs() == 0) {
Review Comment:
I write a test to validate that `newData.getNumDVs() == 0`. I check the test using LlapLocalDriver with an additional modification to `data/conf/llap/hivemetastore-site.xml`, specifically setting `metastore.stats.ndv.densityfunction` to `true`.
However, I think is-zero test is unnecessary at this point, as JVM does not throw an error even when the divisor is 0 (cf. p672 of the [Java Language Specification](https://docs.oracle.com/javase/specs/jls/se24/jls24.pdf)).
If NDV is 0, this likely indicates that there are no rows in the given partition. In such cases, if high and low values are present(which is a weird situation), the density should be set to infinity. I don’t think we need to address this situation explicitly, as it should not occur under normal circumstances.
For documentation purposes, I attach the test I wrote below:
```
create table x_inventory (inv_quantity_on_hand int) partitioned by (inv_date_sk bigint);
insert into table x_inventory (inv_quantity_on_hand, inv_date_sk) values (0, 2000), (1, 2001), (2, 2002);
set hive.stats.ndv.algo=fm;
analyze table x_inventory compute statistics for columns;
alter table x_inventory partition (inv_date_sk = 2002) update statistics for column inv_quantity_on_hand set('numDVs'='2','numNulls'='0');
alter table x_inventory partition (inv_date_sk = 2001) update statistics for column inv_quantity_on_hand set('numDVs'='1','numNulls'='0');
alter table x_inventory partition (inv_date_sk = 2000) update statistics for column inv_quantity_on_hand set('numDVs'='0','numNulls'='0');
-- at this point, stats uses FMSketch.
set hive.stats.ndv.algo=hll;
insert into table x_inventory (inv_quantity_on_hand, inv_date_sk) values (3, 2003);
-- so there are FMSketch and HyperLogLog.
set hive.stats.column.autogather=false;
insert into table x_inventory (inv_quantity_on_hand, inv_date_sk) values (4, 2004);
-- Finally, this ensures that the colstats is incomplete and not mergable.
desc formatted x_inventory partition (inv_date_sk=2002) inv_quantity_on_hand;
desc formatted x_inventory partition (inv_date_sk=2003) inv_quantity_on_hand;
desc formatted x_inventory partition (inv_date_sk=2004) inv_quantity_on_hand;
-- This requests aggregated column stats of inv_quantity_on_hand.
explain select count(distinct inv_quantity_on_hand) from x_inventory;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5674:
URL: https://github.com/apache/hive/pull/5674#issuecomment-2778136269
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5674) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5674&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5674&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5674&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5674&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5674&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5674)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5674:
URL: https://github.com/apache/hive/pull/5674#issuecomment-2781004274
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5674) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5674&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5674&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5674&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5674&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5674&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5674)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5674:
URL: https://github.com/apache/hive/pull/5674#issuecomment-2779476296
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5674) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5674&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5674&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5674&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5674&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5674&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5674)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ merged PR #5674:
URL: https://github.com/apache/hive/pull/5674
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5674:
URL: https://github.com/apache/hive/pull/5674#discussion_r2023059194
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java:
##########
@@ -242,6 +242,24 @@ public ColumnStatisticsObj aggregate(List<ColStatsObjWithSourceInfo> colStatsWit
}
statsObj.setStatsData(columnStatisticsData);
+
+ // NDV cannot exceed the number of integers within the given range.
+ long numIntInRange = columnStatisticsData.getLongStats().getHighValue()
Review Comment:
if `high`=`low`, densityAvgSum would be 0 and when evaluating `estimation` we'll be dividing by `0`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5674:
URL: https://github.com/apache/hive/pull/5674#discussion_r2023130779
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java:
##########
@@ -192,7 +207,13 @@ public ColumnStatisticsObj aggregate(List<ColStatsObjWithSourceInfo> colStatsWit
csd.setLongStats(aggregateData);
adjustedStatsMap.put(pseudoPartName.toString(), csd);
if (useDensityFunctionForNDVEstimation) {
- densityAvgSum += ((double) (aggregateData.getHighValue() - aggregateData.getLowValue())) / aggregateData.getNumDVs();
+ if (aggregateData.getNumDVs() == 0) {
Review Comment:
Could it be `0` after `ndvEstimator.estimateNumDistinctValues()`? Do we have a test validating this? According to https://github.com/apache/hive/pull/5674#issuecomment-2768472166 that is not expected/possible.
if yes, isn't it the same here?
https://github.com/apache/hive/pull/5674/files#diff-004a5c809d016b31028b52e5a5e591d30b446d974b0d22e386c35acfee845d59R248
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5674:
URL: https://github.com/apache/hive/pull/5674#discussion_r2023071554
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java:
##########
@@ -129,7 +129,16 @@ public ColumnStatisticsObj aggregate(List<ColStatsObjWithSourceInfo> colStatsWit
// We have estimation, lowerbound and higherbound. We use estimation
// if it is between lowerbound and higherbound.
double densityAvg = densityAvgSum / partNames.size();
- estimation = (long) ((aggregateData.getHighValue() - aggregateData.getLowValue()) / densityAvg);
+ if (densityAvg == 0) {
+ // This means each partition has exactly one value. (highValue == lowValue and NDV = 1).
Review Comment:
👍
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5674:
URL: https://github.com/apache/hive/pull/5674#discussion_r2023138045
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java:
##########
@@ -163,7 +172,13 @@ public ColumnStatisticsObj aggregate(List<ColStatsObjWithSourceInfo> colStatsWit
String partName = csp.getPartName();
LongColumnStatsData newData = cso.getStatsData().getLongStats();
if (useDensityFunctionForNDVEstimation) {
- densityAvgSum += ((double) (newData.getHighValue() - newData.getLowValue())) / newData.getNumDVs();
+ if (newData.getNumDVs() == 0) {
Review Comment:
Could it be `0`? Do we have a test validating this?
if yes, isn't this the same here?
https://github.com/apache/hive/pull/5674/files#diff-004a5c809d016b31028b52e5a5e591d30b446d974b0d22e386c35acfee845d59R105
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5674:
URL: https://github.com/apache/hive/pull/5674#discussion_r2023130779
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java:
##########
@@ -192,7 +207,13 @@ public ColumnStatisticsObj aggregate(List<ColStatsObjWithSourceInfo> colStatsWit
csd.setLongStats(aggregateData);
adjustedStatsMap.put(pseudoPartName.toString(), csd);
if (useDensityFunctionForNDVEstimation) {
- densityAvgSum += ((double) (aggregateData.getHighValue() - aggregateData.getLowValue())) / aggregateData.getNumDVs();
+ if (aggregateData.getNumDVs() == 0) {
Review Comment:
could it be `0` after `ndvEstimator.estimateNumDistinctValues()`? if yes, isn't it the same here?
https://github.com/apache/hive/pull/5674/files#diff-004a5c809d016b31028b52e5a5e591d30b446d974b0d22e386c35acfee845d59R248
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5674:
URL: https://github.com/apache/hive/pull/5674#discussion_r2023138045
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java:
##########
@@ -163,7 +172,13 @@ public ColumnStatisticsObj aggregate(List<ColStatsObjWithSourceInfo> colStatsWit
String partName = csp.getPartName();
LongColumnStatsData newData = cso.getStatsData().getLongStats();
if (useDensityFunctionForNDVEstimation) {
- densityAvgSum += ((double) (newData.getHighValue() - newData.getLowValue())) / newData.getNumDVs();
+ if (newData.getNumDVs() == 0) {
Review Comment:
could it be `0`? if yes, isn't this the same usage?
https://github.com/apache/hive/pull/5674/files#diff-004a5c809d016b31028b52e5a5e591d30b446d974b0d22e386c35acfee845d59R105
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on PR #5674:
URL: https://github.com/apache/hive/pull/5674#issuecomment-2766243139
hi @ngsg, not sure if you saw the comments
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5674:
URL: https://github.com/apache/hive/pull/5674#issuecomment-2772715201
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5674) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5674&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5674&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5674&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5674&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5674&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5674)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5674:
URL: https://github.com/apache/hive/pull/5674#discussion_r2009150534
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java:
##########
@@ -242,6 +242,24 @@ public ColumnStatisticsObj aggregate(List<ColStatsObjWithSourceInfo> colStatsWit
}
statsObj.setStatsData(columnStatisticsData);
+
+ // NDV cannot exceed the number of integers within the given range.
+ long numIntInRange = columnStatisticsData.getLongStats().getHighValue()
Review Comment:
should we fix this as well since we could try to divide by 0
````
double densityAvg = densityAvgSum / partNames.size();
estimation = (long) ((aggregateData.getHighValue() - aggregateData.getLowValue()) / densityAvg);
````
`extrapolate` method has `densityAvg != 0.0` check
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5674:
URL: https://github.com/apache/hive/pull/5674#discussion_r2009150534
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java:
##########
@@ -242,6 +242,24 @@ public ColumnStatisticsObj aggregate(List<ColStatsObjWithSourceInfo> colStatsWit
}
statsObj.setStatsData(columnStatisticsData);
+
+ // NDV cannot exceed the number of integers within the given range.
+ long numIntInRange = columnStatisticsData.getLongStats().getHighValue()
Review Comment:
should we fix this as well
````
densityAvgSum += ((double) (newData.getHighValue() - newData.getLowValue())) / newData.getNumDVs();
````
since we could try to divide by 0
````
double densityAvg = densityAvgSum / partNames.size();
estimation = (long) ((aggregateData.getHighValue() - aggregateData.getLowValue()) / densityAvg);
````
````
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "ngsg (via GitHub)" <gi...@apache.org>.
ngsg commented on code in PR #5674:
URL: https://github.com/apache/hive/pull/5674#discussion_r2021219099
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java:
##########
@@ -242,6 +242,24 @@ public ColumnStatisticsObj aggregate(List<ColStatsObjWithSourceInfo> colStatsWit
}
statsObj.setStatsData(columnStatisticsData);
+
+ // NDV cannot exceed the number of integers within the given range.
Review Comment:
We may use a probabilistic data structure instead of an extrapolation method, so we could not embed the NDV correction logic inside `extrapolate`. I'll extract the correction logic into a separate method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "ngsg (via GitHub)" <gi...@apache.org>.
ngsg commented on code in PR #5674:
URL: https://github.com/apache/hive/pull/5674#discussion_r2024835915
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java:
##########
@@ -192,7 +207,13 @@ public ColumnStatisticsObj aggregate(List<ColStatsObjWithSourceInfo> colStatsWit
csd.setLongStats(aggregateData);
adjustedStatsMap.put(pseudoPartName.toString(), csd);
if (useDensityFunctionForNDVEstimation) {
- densityAvgSum += ((double) (aggregateData.getHighValue() - aggregateData.getLowValue())) / aggregateData.getNumDVs();
+ if (aggregateData.getNumDVs() == 0) {
+ // If NDV is not present, assume that the density is 0.5.
+ densityAvgSum += 0.5;
+ } else {
+ double range = aggregateData.getHighValue() - aggregateData.getLowValue();
Review Comment:
I removed `range` variable and reverted the code to original version.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5674:
URL: https://github.com/apache/hive/pull/5674#discussion_r2025039136
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java:
##########
@@ -343,4 +356,23 @@ public void extrapolate(ColumnStatisticsData extrapolateData, int numParts,
extrapolateLongData.setNumNulls(numNulls);
extrapolateLongData.setNumDVs(ndv);
}
+
+ private void adjustNDV(ColumnStatisticsData columnStatisticsData) {
Review Comment:
please change param to `LongColumnStatsData columnStats` <-- `columnStatisticsData.getLongStats()`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28800: Improve LongColumnStatsAggregator by preventing the overestimation of NDV [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5674:
URL: https://github.com/apache/hive/pull/5674#discussion_r2023130779
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java:
##########
@@ -192,7 +207,13 @@ public ColumnStatisticsObj aggregate(List<ColStatsObjWithSourceInfo> colStatsWit
csd.setLongStats(aggregateData);
adjustedStatsMap.put(pseudoPartName.toString(), csd);
if (useDensityFunctionForNDVEstimation) {
- densityAvgSum += ((double) (aggregateData.getHighValue() - aggregateData.getLowValue())) / aggregateData.getNumDVs();
+ if (aggregateData.getNumDVs() == 0) {
Review Comment:
Could it be `0` after `ndvEstimator.estimateNumDistinctValues()`? Do we have a test validating this?
if yes, isn't it the same here?
https://github.com/apache/hive/pull/5674/files#diff-004a5c809d016b31028b52e5a5e591d30b446d974b0d22e386c35acfee845d59R248
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org