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

SQL: Ability to generate exact distinct count queries. #3999

Merged
merged 1 commit into from Mar 3, 2017

Conversation

gianm
Copy link
Contributor

@gianm gianm commented Mar 3, 2017

Uses Calcite's AggregateExpandDistinctAggregatesRule to generate exact distinct count queries when useApproximateCountDistinct is "false". These query plans use nested groupBys.

@gianm gianm added this to the 0.10.1 milestone Mar 3, 2017
@gianm gianm requested review from fjy and jon-wei March 3, 2017 15:49
@gianm gianm closed this Mar 3, 2017
@gianm gianm reopened this Mar 3, 2017
@fjy
Copy link
Contributor

fjy commented Mar 3, 2017

👍

Copy link
Member

@nishantmonu51 nishantmonu51 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 looks good to me, very useful feature.

@@ -122,6 +122,14 @@ public int getMaxTopNLimit()
return 0;
}
};
private static final PlannerConfig PLANNER_CONFIG_NO_HLL = new PlannerConfig()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor nit: rename to PLANNER_CONFIG_NO_APPROXIMATION ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's also PLANNER_CONFIG_NO_TOPN to disable topn approximations, so I want to keep them different for that reason.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two kinds of approximations can be toggled separately

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok.

@gianm gianm removed the request for review from jon-wei March 3, 2017 18:04
@gianm
Copy link
Contributor Author

gianm commented Mar 3, 2017

I'm going to make one tweak to the wording, please don't merge just yet

@nishantmonu51 nishantmonu51 merged commit 4a56d7d into apache:master Mar 3, 2017
@gianm
Copy link
Contributor Author

gianm commented Mar 3, 2017

Haha, oops. I'll do another PR

@gianm gianm deleted the sql-exact-count-distinct branch March 3, 2017 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants