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

parser: add new agg function APPROX_COUNT_DISTINCT (#854) #906

Merged
merged 1 commit into from
Jun 18, 2020

Conversation

solotzg
Copy link
Contributor

@solotzg solotzg commented Jun 18, 2020

cherry-pick #854 to release-4.0

What problem does this PR solve?

What is changed and how it works?

Check List

Tests

  • Unit test

Signed-off-by: Tong Zhigao <[email protected]>
@codecov
Copy link

codecov bot commented Jun 18, 2020

Codecov Report

Merging #906 into release-4.0 will increase coverage by 0.04%.
The diff coverage is 84.09%.

@@               Coverage Diff               @@
##           release-4.0     #906      +/-   ##
===============================================
+ Coverage        78.25%   78.29%   +0.04%     
===============================================
  Files               40       40              
  Lines            14710    14788      +78     
===============================================
+ Hits             11511    11579      +68     
- Misses            2519     2524       +5     
- Partials           680      685       +5     

@@ -6384,6 +6387,10 @@ SumExpr:
$$ = &ast.AggregateFuncExpr{F: $1, Args: args}
}
}
| builtinApproxCountDistinct '(' ExpressionList ')'
{
$$ = &ast.AggregateFuncExpr{F: $1, Args: $3.([]ast.ExprNode), Distinct: false}
Copy link
Contributor

Choose a reason for hiding this comment

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

Distinct: false can be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe other logic use this field.

Copy link
Contributor

@XuHuaiyu XuHuaiyu left a comment

Choose a reason for hiding this comment

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

LGTM

@kennytm kennytm added status/LGT2 LGT2 and removed status/LGT1 LGT1 labels Jun 18, 2020
@kennytm kennytm merged commit 789c193 into pingcap:release-4.0 Jun 18, 2020
@solotzg solotzg deleted the release-4.0 branch June 18, 2020 11:32
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.

3 participants