[SPARK-1852] prevents queries with sorts submitting jobs prematurely

[liancheng]

JIRA issue: SPARK-1852

This issue is related to SPARK-1021, but this PR doesn't try to solve that one. Worked around by only forcing query planning when running DDL and other native commands.

[AmplabJenkins]

Merged build triggered.

[AmplabJenkins]

Merged build started.

[AmplabJenkins]

Merged build finished.

[AmplabJenkins]

Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/15376/

[AmplabJenkins]

Merged build triggered.

[AmplabJenkins]

Merged build started.

[AmplabJenkins]

Merged build finished. All automated tests passed.

[AmplabJenkins]

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/15380/

[marmbrus]

Thanks for looking into this! Just a thought: what do you think about instead putting a method called process() or something into QueryExecution. That way we wouldn't have to duplicate the logic in two places, and subclasses of QueryExecution could also override it if there were special cases?

[marmbrus]

/cc @concretevitamin who is looking at similar parts of the code at the moment.

[concretevitamin]

Removing duplication is definitely a good thing. The other benefit is that it seems more natural to me to push such DDL/command processing logic into QueryExecution, instead of putting them into two thin entry methods (sql and hql).

I am super new to Spark SQL so bear with me if this is silly -- what is the reason we don't do this as well for InsertIntoHiveTable and InsertIntoParquetTable?

[liancheng]

@marmbrus Agree. I'll try to remove the lazy qualifier of SchemaRDD.queryExecution since the field object itself is rather cheap (all of its fields are lazy), and then call the process() method in QueryExecution constructor to evaluate native commands eagerly.

@concretevitamin InsertIntoHiveTable and InsertIntoParquetTable are physical operations, which are parts of physical plans. However, we only need to take care of logical plan here. InsertIntoTable and InsertIntoCreatedTable are translated into those physical operators after logical plan optimization.

[AmplabJenkins]

Merged build triggered.

[AmplabJenkins]

Merged build triggered.

[liancheng]

@marmbrus @concretevitamin While working on the cache table SQL command and reviewing PR #956, I think this PR may make both changes cleaner, so I tried to finish this one first. For example, we won't need processCmd and eagerlyProcess anymore since all commands and insertions are eagerly executed in a unified way.

Instead of QueryExecution, I factored the duplicated code into SchemaRDDLike to make the change cleaner and more straightforward (after some experiments, it turned out that moving code to QueryExecution complicates things a lot).

[concretevitamin]

Hey @liancheng - I think this refactoring will solve this particular ticket (i.e. queries w/ sorts will not be eagerly executed anymore). However, I don't see why we don't need processCmd and eagerlyProcess anymore; for instance, for set commands this PR's changes will eagerly call toRdd, but we still need some logic in the two toRdd to handle the set. Are you proposing to just take that piece of logic and put them into SchemaRDDLike? That could work. And what about processCmd then?

[AmplabJenkins]

Merged build started.

[AmplabJenkins]

Merged build finished.

[AmplabJenkins]

Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/15596/

[AmplabJenkins]

Merged build started.

[AmplabJenkins]

Merged build finished.

[AmplabJenkins]

Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/15598/

[liancheng]

Just a memo: please refer to this comment and SPARK-2094 for more details about this PR and PR #956.

[liancheng]

Closing this PR because corresponding change is merged into another up coming PR that aims to solve SPARK-2094.