[SPARK-10741][SQL] Hive Query Having/OrderBy against Parquet table is not working

[cloud-fan]

https://issues.apache.org/jira/browse/SPARK-10741
I choose the second approach: do not change output exprIds when convert MetastoreRelation to LogicalRelation

[cloud-fan]

cc @yhuai

[yhuai]

I think we need to make it a case class because when a TreeNode makes copy, it copies parameters in its productIterator.

[marmbrus]

You can also manually implement product as is done here, but if possible I'd like to avoid that.

[yhuai]

How about we use the following?

private[sql] case class LogicalRelation(
    relation: BaseRelation,
    override val output: Seq[AttributeReference])
  extends LeafNode
  with MultiInstanceRelation {

  def this(relation: BaseRelation) = this(relation, relation.schema.toAttributes)

[yhuai]

Had a discussion with @marmbrus and @cloud-fan, we will keep it as a case class and change other places where we need to use unapply. We will also pass in a Seq[AttributeReference] and do the work of adding the correct exprIds inside LogicalRelation.

[cloud-fan]

cc @liancheng

[SparkQA]

Test build #42934 has finished for PR 8889 at commit 798bc73.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #42945 has finished for PR 8889 at commit 2e21513.

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

[SparkQA]

Test build #42962 has finished for PR 8889 at commit d317fb8.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[yhuai]

Let's add a check to make sure that attrs and expectedOutputAttributes have the same length. Otherwise, zip will silently drop elements.

[yhuai]

Ah, i see. You check the length at line 36.

[yhuai]

Then, let's add a comment to say where we check the length.

[SparkQA]

Test build #42975 has finished for PR 8889 at commit 56efdc2.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[yhuai]

Seems it failed some tests.

[SparkQA]

Test build #42986 has finished for PR 8889 at commit 30e930e.

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

[yhuai]

@liancheng can you also take a look?

[liancheng]

Nit: seems that we don't use i here.

[liancheng]

LGTM except for a few minor issues.

[SparkQA]

Test build #43036 has finished for PR 8889 at commit 8fb386a.

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

[yhuai]

test this please

[SparkQA]

Test build #43053 has finished for PR 8889 at commit 8fb386a.

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

[yhuai]

LGTM. Merging to master and branch 1.5.