Thank you for raising this, Julian. How much performance improvement can we get?
Best, Chunwei On Thu, Nov 5, 2020 at 5:32 AM Julian Hyde <[email protected]> wrote: > Currently class SqlNodeList [1] implements Iterable<SqlNode> but it > does not implement List<SqlNode>. How do people feel about doing that? > (Or Collection<SqlNode>?) > > The main potential benefit is performance. Consider this code: > > SqlNodeList nodeList; > ImmutableList immutableList1 = ImmutableList.copyOf(nodeList); > > List<SqlNode> list = nodeList.toList(); > ImmutableList immutableList2 = ImmutableList.copyOf(list); > > Today, the second form is faster, because ImmutableList.copyOf can > call size() and preallocate a list of the right size. > > The second benefit is that we can remove calls to '.toList()'. > > The downside is that a few locations are overloaded. For example, > class Span has overloaded methods > > static Span of(SqlNode node); > static Span of(Collection<? extends SqlNode> nodes) > > If SqlNodeList implements List<? extends SqlNode>, then the code > > SqlNodeList nodeList; > Span s = Span.of(nodeList); > > becomes ambiguous. We can disambiguate by adding Span.of(SqlNodeList). > But there may be other locations in client code that cannot be > disambiguated. > > Julian > > [1] > https://github.com/apache/calcite/blob/155276591288615c4d02d55fb7d77eceb2e24b2d/core/src/main/java/org/apache/calcite/sql/SqlNodeList.java#L38 >
