Don't estimate OPTIONAL_JOIN size as 0 fixes #108 #110
Conversation
When the OPTIONAL_JOIN has a an empty side it would estimate it's result as empty. For estimates of size zero JOIN would then take the estimate as exact and stop early. The current fix is just incrementing the size estimate by one to make sure it's never 0. In the future we may want to get an exact estimate for 0. While we are here also remove redundant `getRootOperation()` calls in Join.cpp (`getResult()` already uses the `_rootOperation`) and add some useful `LOG(TRACE)`
niklas88
requested review from
Buchhold,
floriankramer and
joka921
and removed request for
floriankramer
There was a problem hiding this comment.
This looks good.
Just one thought: Do you always use std::endl which always performs a flush? This is good before lengthy computations but bad if there is a lot of output. I think this does not make a difference with our system, but I just wanted to make sure that this is done on purpose
src/engine/OptionalJoin.cpp
Outdated
| @@ -327,6 +327,8 @@ void OptionalJoin::computeSizeEstimateAndMultiplicities() { | |||
| } else { | |||
| _sizeEstimate = multResult * numDistinctResult; | |||
| } | |||
| // Don't estimate 0 since then upstream thinks we are sure it's zero | |||
There was a problem hiding this comment.
is "upstream" a fixed term in the QLever engine, or what is its meaning here?
There was a problem hiding this comment.
Not really, with it I mean in the parents of this node in the QueryExecutionTree. Particularly in Join when it checks for isKnownEmpty().
|
You are right about |
When the OPTIONAL_JOIN has a an empty side it would estimate it's result
as empty. For estimates of size zero JOIN would then take the estimate
as exact and stop early.
The current fix is just incrementing the size estimate by one to make
sure it's never 0. @floriankramer do you know in which cases we are sure
that the size will be 0? Also I'm really not happy with
getKnownEmpty()being just a check for
getSizeEstimate() == 0an estimate shouldprobably not be used as known even if it's exactly 0.
While we are here also remove redundant
getRootOperation()calls inJoin.cpp (
getResult()already uses the_rootOperation) and add someuseful
LOG(TRACE)