[SPARK-6145][SQL] fix ORDER BY on nested fields

[marmbrus]

This PR is based on work by @cloud-fan in #4904, but with two differences:

• We isolate the logic for Sort's special handling into ResolveSortReferences
• We avoid creating UnresolvedGetField expressions during resolution. Instead we either resolve GetField or we return None. This avoids us going down the wrong path early on.

[SparkQA]

Test build #29187 has finished for PR 5189 at commit e35b27c.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • class RegexTokenizer extends UnaryTransformer[String, Seq[String], RegexTokenizer]

[cloud-fan]

Since we decide to resolve Sort in ResolveSortReferences, then we need to stop resolve Sort in ResolveReferences. For example,

test("quick") {
    jsonRDD(sparkContext.makeRDD(
      """{"a": {"b": 1}, "b": 2}""" ::
      """{"a": {"b": 2}, "b": 1}""" :: Nil)).registerTempTable("t")
    checkAnswer(sql("SELECT a.b FROM t ORDER BY b"), Seq(Row(2), Row(1)))
  }

This test will fail, because we have mistakenly resolved b as a.b in ResolveReferences(we try to resolve b on top of Projection("a.b") and success). A quick fix is just make sure we don't resolve Sort in ResolveReferences like case u @ UnresolvedAttribute(name) if !q.isInstanceOf[Sort]. Do you have better idea @marmbrus ?

[cloud-fan]

Even there is no missing in project, we still need to build new Sort with resolved SortOrder.

[cloud-fan]

Another problem, for something like SELECT DISTINCT a FROM t ORDER BY a, we can't resolve this Sort because its child is not Project or Aggregate, thus can't handled by ResolveSortReferences.

[marmbrus]

@cloud-fan, thanks for the further comments. Regarding not resolving sort in in ResolveReferences, I agree that is a little confusing and I think you are correct for standard SQL. However, this is not how hive does resolution and I don't think that we can differ from their implementation. Consider the following:

  test("hive orderby queries") {
    val df = jsonRDD(sparkContext.makeRDD(
      """{"a": {"b": 1}, "b": 2}""" ::
      """{"a": {"b": 2}, "b": 1}""" :: Nil))registerTempTable("t")
    sql("CREATE TABLE t1 AS SELECT * FROM t")
    runSqlHive("SELECT a.b FROM t1 ORDER BY b").foreach(println)
    sql("SELECT a.b FROM t1 ORDER BY b").collect().foreach(println)
  }

Which outputs:

1
2
[1]
[2]

So given this, I think the current way we resolve things (i.e. first resolve based on what comes from the select clause and then fall back on its child only when this fails) is non-standard but what we must do to avoid breaking existing HiveQL queries.

[yhuai]

Update comment? Seems we will not add UnresolvedGetField any more.

[SparkQA]

Test build #29255 has finished for PR 5189 at commit ca02d85.

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

[cloud-fan]

Ah, so SELECT a.b is equal to SELECT a.b AS b, and the b in ORDER BY b should reference to a.b, that's how hive do resolution and makes sense. I have sent you a PR to remove the duplicated resolveGetField and others LGTM.

[SparkQA]

Test build #29367 has finished for PR 5189 at commit b8cae45.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • class Analyzer(
  • trait CheckAnalysis
• This patch does not change any dependencies.

[yhuai]

LGTM