[
https://issues.apache.org/jira/browse/CALCITE-3947?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17093818#comment-17093818
]
Julian Hyde edited comment on CALCITE-3947 at 4/27/20, 6:30 PM:
----------------------------------------------------------------
Thanks [~hyuan].
Sorry I didn't get further than "maybe". But when I'm triaging 500 emails every
morning I don't have much time. But our Apache process means that we need to
move at the speed of the slowest reviewer, not at the speed of the developer
for whom this is the only task that needs to be accomplished today. (Of course
the reviewer needs to give reasonable latency.)
The point I wanted to make was this. {{LinkedHashSet}} is a more costly data
structure, in terms of time and space, than {{HashSet}}. It needs to be
justified.
The only code that needs a predictable iteration order on the set is the code
that creates operands. That code would be better if it sorted the classes by
name, rather than by insertion order. That is, in some sense, more
deterministic, and also more useful.
was (Author: julianhyde):
Thanks [~hyuan].
Sorry I didn't get further than "maybe". But when I'm triaging 500 emails every
morning I don't have much time.
The point I wanted to make was this. {{LinkedHashSet}} is a more costly data
structure, in terms of time and space, than {{HashSet}}. It needs to be
justified.
The only code that needs a predictable iteration order on the set is the code
that creates operands. That code would be better if it sorted the classes by
name, rather than by insertion order. That is, in some sense, more
deterministic, and also more useful.
> AbstractRelOptPlanner.classes should be LinkedHashSet so that rule match
> order is deterministic across runs
> -----------------------------------------------------------------------------------------------------------
>
> Key: CALCITE-3947
> URL: https://issues.apache.org/jira/browse/CALCITE-3947
> Project: Calcite
> Issue Type: Improvement
> Reporter: Botong Huang
> Priority: Minor
> Time Spent: 10m
> Remaining Estimate: 0h
>
> AbstractRelOptPlanner.classes is used by subClasses() to determine things to
> put into VolcanoPlanner.classOperands, which is then used in
> VolcanoPlanner.fireRules(). Since AbstractRelOptPlanner.classes is now a
> HashSet, its iteration order is not deterministic across runs, making
> debugging hard. It should be LinkedHashSet just like many other fields in the
> planner.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
