Skip to content

Conversation

nongli
Copy link
Contributor

@nongli nongli commented Oct 28, 2015

  1. Supporting expanding structs in Projections. i.e.
    "SELECT s.*" where s is a struct type.
    This is fixed by allowing the expand function to handle structs in addition to tables.
  2. Supporting expanding * inside aggregate functions of structs.
    "SELECT max(struct(col1, structCol.*))"
    This requires recursively expanding the expressions. In this case, it it the aggregate
    expression "max(...)" and we need to recursively expand its children inputs.

@yhuai
Copy link
Contributor

yhuai commented Oct 28, 2015

ok to test

@SparkQA
Copy link

SparkQA commented Oct 28, 2015

Test build #44549 has finished for PR 9343 at commit 54e155e.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):\n * case class UnresolvedStar(target: Option[String]) extends Star with Unevaluable\n

Copy link
Member

Choose a reason for hiding this comment

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

'nameas' -> 'name as'?

@SparkQA
Copy link

SparkQA commented Oct 29, 2015

Test build #44555 has finished for PR 9343 at commit df65944.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):\n * class HasSolver(Params):\n * case class UnresolvedStar(target: Option[String]) extends Star with Unevaluable\n

Copy link
Contributor

Choose a reason for hiding this comment

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

you can make it more dataframe style like testData2.select(struct("a", "b").as("record"))

@SparkQA
Copy link

SparkQA commented Oct 30, 2015

Test build #44704 has finished for PR 9343 at commit be20b33.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):\n * case class UnresolvedStar(target: Option[String]) extends Star with Unevaluable\n

Copy link
Contributor

Choose a reason for hiding this comment

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

newChildren => newArgs?

@SparkQA
Copy link

SparkQA commented Nov 3, 2015

Test build #44840 has finished for PR 9343 at commit c55a7d2.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):\n * case class UnresolvedStar(target: Option[Seq[String]]) extends Star with Unevaluable\n

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems you want the following?

if (!expandedAttributes.isEmpty) {
  expandedAttributes.zip(input.output).map {
    case (n: NamedExpression, _) => n
    case (e, originalAttribute) =>
      Alias(e, originalAttribute.name)(qualifiers = originalAttribute.qualifiers)
  }
}

1. Supporting expanding structs in Projections. i.e.
  "SELECT s.*" where s is a struct type.
  This is fixed by allowing the expand function to handle structs in addition to tables.

2. Supporting expanding * inside aggregate functions of structs.
   "SELECT max(struct(col1, structCol.*))"
   This requires recursively expanding the expressions. In this case, it it the aggregate
   expression "max(...)" and we need to recursively expand its children inputs.
@SparkQA
Copy link

SparkQA commented Nov 3, 2015

Test build #44862 has finished for PR 9343 at commit 886acc7.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):\n * case class UnresolvedStar(target: Option[Seq[String]]) extends Star with Unevaluable\n

Copy link
Contributor

Choose a reason for hiding this comment

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

nonEmpty?

@yhuai
Copy link
Contributor

yhuai commented Nov 3, 2015

Thanks @nongli ! Overall LGTM. I am going to merge it to master. Can you create a follow-up PR to address my comments? Thanks!

@asfgit asfgit closed this in 9cb5c73 Nov 3, 2015
Copy link
Contributor

Choose a reason for hiding this comment

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

please fix the style here. e.g. ).+ instead of ). +, remove the extra { ... }, follow the original indent

Copy link
Contributor

Choose a reason for hiding this comment

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

We can use exists instead of filter(...).nonEmpty

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.

5 participants