Skip to content

[SPARK-8970][SQL] remove unnecessary abstraction for ExtractValue #7339

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

[SPARK-8970][SQL] remove unnecessary abstraction for ExtractValue #7339

wants to merge 2 commits into from

Conversation

@cloud-fan
Copy link
Contributor

@cloud-fan cloud-fan commented Jul 10, 2015

No description provided.

@SparkQA
Copy link

SparkQA commented Jul 10, 2015

Test build #37010 has finished for PR 7339 at commit 394d3ac.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class GetArrayItem(child: Expression, ordinal: Expression) extends BinaryExpression
    • case class GetMapValue(child: Expression, key: Expression) extends BinaryExpression

@cloud-fan
Copy link
Contributor Author

cloud-fan commented Jul 10, 2015

cc @rxin

rxin
rxin reviewed Jul 10, 2015
View reviewed changes
...atalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeExtractors.scala Outdated
Copy link
Contributor

@rxin rxin Jul 10, 2015

Choose a reason for hiding this comment

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

I find it odd to return null in the 2nd field. Why is the semantics of that?

Copy link
Contributor Author

@cloud-fan cloud-fan Jul 10, 2015

Choose a reason for hiding this comment

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

This unapply is only used in optimizer to save code. I have removed it.

cloud-fan
cloud-fan reviewed Jul 10, 2015
View reviewed changes
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala Outdated
Copy link
Contributor Author

@cloud-fan cloud-fan Jul 10, 2015

Choose a reason for hiding this comment

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

Maybe we can simplify null propagation for binary and unary expression. Will do that in another PR.

@SparkQA
Copy link

SparkQA commented Jul 10, 2015

Test build #37044 has finished for PR 7339 at commit 3bc01aa.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class GetArrayItem(child: Expression, ordinal: Expression) extends BinaryExpression
    • case class GetMapValue(child: Expression, key: Expression) extends BinaryExpression

@SparkQA
Copy link

SparkQA commented Jul 10, 2015

Test build #37042 has finished for PR 7339 at commit 6a37c12.

  • This patch fails PySpark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class KafkaRDD(RDD):
    • class KafkaDStream(DStream):
    • class KafkaTransformedDStream(TransformedDStream):
    • case class GetArrayItem(child: Expression, ordinal: Expression) extends BinaryExpression
    • case class GetMapValue(child: Expression, key: Expression) extends BinaryExpression
    • class GenericInternalRowWithSchema(values: Array[Any], override val schema: StructType)
    • implicit class DebugQuery(query: DataFrame) extends Logging
    • class HiveContext(sc: SparkContext) extends SQLContext(sc) with Logging
    • case class StreamInputInfo(

@rxin
Copy link
Contributor

rxin commented Jul 10, 2015

Jenkins, retest this please.

@SparkQA
Copy link

SparkQA commented Jul 10, 2015

Test build #37048 has finished for PR 7339 at commit 3bc01aa.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class GetArrayItem(child: Expression, ordinal: Expression) extends BinaryExpression
    • case class GetMapValue(child: Expression, key: Expression) extends BinaryExpression

@rxin
Copy link
Contributor

rxin commented Jul 10, 2015

Jenkins, retest this please.

@SparkQA
Copy link

SparkQA commented Jul 10, 2015

Test build #37061 has finished for PR 7339 at commit 3bc01aa.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class GetArrayItem(child: Expression, ordinal: Expression) extends BinaryExpression
    • case class GetMapValue(child: Expression, key: Expression) extends BinaryExpression

@SparkQA
Copy link

SparkQA commented Jul 11, 2015

Test build #37075 has finished for PR 7339 at commit 84a2128.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class GetArrayItem(child: Expression, ordinal: Expression) extends BinaryExpression
    • case class GetMapValue(child: Expression, key: Expression) extends BinaryExpression

@cloud-fan
Copy link
Contributor Author

cloud-fan commented Jul 11, 2015

I saw the JIRA is marked as resolved, any problems here?

cloud-fan added a commit that referenced this pull request Jul 11, 2015
@cloud-fan @rxin
[SPARK-8970][SQL] remove unnecessary abstraction for ExtractValue
c472eb1 
Author: Wenchen Fan <[email protected]>

Closes #7339 from cloud-fan/minor and squashes the following commits:

84a2128 [Wenchen Fan] remove unapply
6a37c12 [Wenchen Fan] remove unnecessary abstraction for ExtractValue
@rxin
Copy link
Contributor

rxin commented Jul 12, 2015

It's been merged -- but github didn't close this. Can you close the pr?

@cloud-fan cloud-fan closed this Jul 13, 2015
@cloud-fan cloud-fan deleted the minor branch July 13, 2015 02:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@cloud-fan @SparkQA @rxin