-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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 EXACT CARDINALITY commands #8984
Conversation
coordinator/statement_executor.go
Outdated
} | ||
|
||
return []*models.Row{&models.Row{ | ||
Columns: []string{"cardinality"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@benbjohnson @jsternberg question over what your thoughts are on this name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think cardinality
works for me. 👍
coordinator/statement_executor.go
Outdated
} | ||
|
||
return []*models.Row{&models.Row{ | ||
Columns: []string{"cardinality"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think cardinality
works for me. 👍
ce571d2
to
f1947fb
Compare
influxql/ast.go
Outdated
_, _ = buf.WriteString(" ON ") | ||
_, _ = buf.WriteString(QuoteIdent(s.Database)) | ||
} | ||
return buf.String() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to me like you should do this instead.
if s.Exact {
_, _ = buf.WriteString(" EXACT")
}
_, _ = buf.WriteString(" CARDINALITY")
if s.Database != "" {
// the stuff that's here
}
if !s.Exact {
return buf.String()
}
Then there's less code repetition.
influxql/ast.go
Outdated
_, _ = buf.WriteString("SHOW MEASUREMENT CARDINALITY") | ||
_, _ = buf.WriteString("SHOW MEASUREMENT") | ||
|
||
if !s.Exact { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same comment from above applies here.
49d204e
to
f7e0e41
Compare
@jsternberg changes made. Thanks. |
Required for all non-trivial PRs
This PR introduces five new commands:
Further, two versions of both the already-implemented
SHOW SERIES CARDINALITY
andSHOW MEASUREMENT CARDINALITY
commands have been converted to utilise the HLL++ sketches that are maintained in the index. These estimations are very accurate, and very fast.Specifically, the variants that currently support cardinality estimation are
All other
SHOW X CARDINALITY
commands will continue to use the exact-counting approaches of theSHOW X EXACT CARDINALITY
commands. They will be converted to estimations in future releases.