Hi Julian, Vladimir
Actually, I found TpchTest#testQuery07 can directly reproduce the
situation. Put this in VolcanoRuleCall#match, delete the previous code,
and run the test, we can see the prints are full of the console(about 8k+).
I will take a look into this, to see if we can remove the outdated rel
in mapRel2Subset.
if (subset != rel && !subset.getRelList().contains(rel)) {
System.out.println("Rule [{}] not fired because operand #{}
({}) belongs to obsolete set");;
}
Actually you can run TpchTest#testQuery03, the prints only occur 3 times,
more frindly for debug.
Regards!
Aron Tao
Julian Hyde <[email protected]> 于2020年12月4日周五 上午11:49写道：
I think Vladimir is very likely correct that the performance
regression was due to algorithmic changes, that getRelList should not
be used, and that improving its performance is not solving the real
problem. So, I will not merge a fix for getRelList performance until
after the algorithmic problem has been resolved.
On Thu, Dec 3, 2020 at 6:54 PM JiaTao Tao <[email protected]> wrote:
>
> Hi Julian
>
> Thanks, Julian, your advice is really helpful! Do you mind I pull a
request
> to merge your code so that we can improve the performance very quickly?
> About " stored the list of RelNodes ", do you concern about the duplicate
> "relnode" store in "relsubset"? IMO, what we store is a reference, seems
OK.
>
> Regards!
>
> Aron Tao
>
>
> Vladimir Sitnikov <[email protected]> 于2020年12月4日周五 上午5:13写道：
>
> The improvement to RelTraitSet.satisfies would be nice, however, I still
> believe getRelList must not be used in 2222.
>
> It looks like the only possibility of the case when rel points to a relset
> which does not include the rel is the case when the rel
> has been replaced with an equivalent.
>
> In other words, the planner finds that the rel is equivalent to another
> rel, and it removes the new one from its set.
> Should we remove the entry from mapRel2Subset as well?
> Then RuleCall could treat "missing entry in mapRel2Subset" as a sign of
the
> stale rel.
>
> Vladimir
