-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
feat: estimate query cost in Postgres #12130
feat: estimate query cost in Postgres #12130
Conversation
@@ -132,7 +132,8 @@ class PrestoEngineSpec(BaseEngineSpec): # pylint: disable=too-many-public-metho | |||
} | |||
|
|||
@classmethod | |||
def get_allow_cost_estimate(cls, version: Optional[str] = None) -> bool: | |||
def get_allow_cost_estimate(cls, extra: Dict[str, Any]) -> bool: | |||
version = extra.get("version") | |||
return version is not None and StrictVersion(version) >= StrictVersion("0.319") |
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.
one liner:
return extra.get("version") and StrictVersion(version) >= StrictVersion("0.319")
5fe9d66
to
f37bbd6
Compare
Codecov Report
@@ Coverage Diff @@
## master #12130 +/- ##
==========================================
- Coverage 66.22% 63.62% -2.60%
==========================================
Files 972 481 -491
Lines 48098 29708 -18390
Branches 4752 0 -4752
==========================================
- Hits 31852 18903 -12949
+ Misses 16108 10805 -5303
+ Partials 138 0 -138
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
f37bbd6
to
9ceefc9
Compare
What's the unit of this cost? Seconds? |
It's a relative value, so not super useful. But it's possible to collect stats about queries, and define a custom We had this running at Lyft for Presto, and we would show wall time, dollar cost and carbon footprint this way. See #8470. |
Thanks for the explanation! In that case, do you think it's worth adding the link to Postgres doc somewhere in the modal. Or at least a one-liner to explain how is this calculated? E.g.
|
@ktmud unfortunately the modal is not DB-specific, so we can't add that message to it. I also don't expect people to use it without an admin setting up a custom formatter that translates the number into a meaningful value. I added an example to Here's a screenshot with that config in place: |
@betodealmeida Thanks for the explanation. This makes sense. |
SUMMARY
Currently the estimate query cost feature is only enabled for Presto, making it hard to test the functionality and iterate on SQL Lab without breaking it.
This PR adds a simple query cost estimator to Postgres.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TEST PLAN
See screenshot.
ADDITIONAL INFORMATION