Add a comment about ColumnStatistics' unknown estimates semantics#254
Add a comment about ColumnStatistics' unknown estimates semantics#254martint merged 1 commit intotrinodb:masterfrom
Conversation
There was a problem hiding this comment.
SymbolStatsEstimate is not part of SPI, but rather part of engine, so connectors shouldn't assume any specific behavior.
The method that determines if SymbolStatsEstimate is unknown is io.prestosql.cost.SymbolStatsEstimate#isUnknown.
There was a problem hiding this comment.
I agree with your comment, but in this PR, I was trying to explain the engine's expectations from the connector statistics' estimation API, following prestodb/presto#12177 (comment).
Is it possible to change this comment to explain it better (e.g. what happens when some of the statistics are not available)?
There was a problem hiding this comment.
OTOH, we can add a link to the documentation (being added by #127) and explain it there. What do you think?
There was a problem hiding this comment.
It’s important for that kind of information to be in the API and accessible from the IDE. People don’t read manuals. Also, any API documentation that is not part of the code is even more likely to get stale as we continue to change the code.
Let’s keep it here but remove the reference to the non-spi class and phrase it in terms of how the engine may not be able to derive stats if any of the provided stats is unknown.
There was a problem hiding this comment.
Sounds good - pushed an updated commit 23cc24b523ba7d5b8f1284042fee980bc09fede7.
rzeyde-varada
force-pushed
the
add-estimate-comment
branch
from
e746c03 to
23cc24b
Compare
rzeyde-varada
force-pushed
the
add-estimate-comment
branch
from
23cc24b to
d8bd674
Compare
3 participants
Extracted from prestodb/presto#12177.