Skip to content

[SPARK-16406][SQL] Improve performance of LogicalPlan.resolve - #231

Merged
csd-jenkins merged 2 commits into
alteryx:csd-2.2from
markhamstra:csd-2.2
May 15, 2018
Merged

csd-jenkins merged 2 commits into
alteryx:csd-2.2from
markhamstra:csd-2.2

Conversation

@markhamstra

Copy link
Copy Markdown

LogicalPlan.resolve(...) uses linear searches to find an attribute matching a name. This is fine in normal cases, but gets problematic when you try to resolve a large number of columns on a plan with a large number of attributes.

This PR adds an indexing structure to resolve(...) in order to find potential matches quicker. This PR improves the reference resolution time for the following code by 4x (11.8s -> 2.4s):

val n = 4000
val values = (1 to n).map(_.toString).mkString(", ")
val columns = (1 to n).map("column" + _).mkString(", ")
val query =
  s"""
     |SELECT $columns
     |FROM VALUES ($values) T($columns)
     |WHERE 1=2 AND 1 IN ($columns)
     |GROUP BY $columns
     |ORDER BY $columns
     |""".stripMargin

spark.time(sql(query))

Existing tests.

Author: Herman van Hovell [email protected]

Closes apache#14083 from hvanhovell/SPARK-16406.

What changes were proposed in this pull request?

(Please fill in changes proposed in this fix)

How was this patch tested?

(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)

Please review http://spark.apache.org/contributing.html before opening a pull request.

`LogicalPlan.resolve(...)` uses linear searches to find an attribute matching a name. This is fine in normal cases, but gets problematic when you try to resolve a large number of columns on a plan with a large number of attributes.

This PR adds an indexing structure to `resolve(...)` in order to find potential matches quicker. This PR improves the reference resolution time for the following code by 4x (11.8s -> 2.4s):

``` scala
val n = 4000
val values = (1 to n).map(_.toString).mkString(", ")
val columns = (1 to n).map("column" + _).mkString(", ")
val query =
  s"""
     |SELECT $columns
     |FROM VALUES ($values) T($columns)
     |WHERE 1=2 AND 1 IN ($columns)
     |GROUP BY $columns
     |ORDER BY $columns
     |""".stripMargin

spark.time(sql(query))
```

Existing tests.

Author: Herman van Hovell <[email protected]>

Closes apache#14083 from hvanhovell/SPARK-16406.
@markhamstra
markhamstra requested a review from ianlcsd May 8, 2018 21:11
@markhamstra

Copy link
Copy Markdown
Author

JMWG

@ianlcsd ianlcsd left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@markhamstra

Copy link
Copy Markdown
Author

jttp

@markhamstra

Copy link
Copy Markdown
Author

retest this please

@markhamstra

Copy link
Copy Markdown
Author

JMWG

@csd-jenkins

Copy link
Copy Markdown

Saw merge directive 'JMWG'. CSD Jenkins auto merging

@csd-jenkins
csd-jenkins merged commit cdcacda into alteryx:csd-2.2 May 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants