[SPARK-8077][SQL] Optimization for TreeNodes with large numbers of children

[MickDavies]

For example large IN clauses

Large IN clauses are parsed very slowly. For example SQL below (10K items in IN) takes 45-50s.

s"""SELECT * FROM Person WHERE ForeName IN ('${(1 to 10000).map("n" + _).mkString("','")}')"""

This is principally due to TreeNode which repeatedly call contains on children, where children in this case is a List that is 10K long. In effect parsing for large IN clauses is O(N squared).
A lazily initialised Set based on children for contains reduces parse time to around 2.5s

[MickDavies]

I'm not sure this works as children is a def, and therefore no guarantees that set of children is immutable. I need to think about it a bit more.

[MickDavies]

Regarding generating lazy val childrenSet from children, given lack of guarantee that children produces an unchanging sequence? It looks like the intention is that children will not change?

[marmbrus]

ok to test

[marmbrus]

Space after :.

[cloud-fan]

how about naming it containsChild so that we can use it like containsChild(arg)?

[MickDavies]

Good idea.

Thanks

Mick

On 12 Jun 2015, at 05:34, Wenchen Fan [email protected] wrote:

In sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/trees/TreeNode.scala #6673 (comment):

@@ -62,6 +62,8 @@ abstract class TreeNode[BaseType <: TreeNode[BaseType]] {
/** Returns a Seq of the children of this node */
def children: Seq[BaseType]

• lazy val childrenSet:Set[TreeNode[_]] = children.toSet
  how about naming it containsChild so that we can use it like containsChild(arg)?

—
Reply to this email directly or view it on GitHub https://github.com/apache/spark/pull/6673/files#r32289725.

[marmbrus]

I think its reasonable to assume children will not change as TreeNodes are generally expected to be immutable. I'd add this requirement to the method's scala doc though.

[SparkQA]

Test build #34737 has finished for PR 6673 at commit e6be8be.

• This patch fails Scala style tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #34762 has finished for PR 6673 at commit 38cd425.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[marmbrus]

Thanks! Merging to master.