Skip to content

Conversation

@liancheng
Copy link
Contributor

Adds logical and physical command classes for the "add jar" command.

Note that this PR conflicts with and should be merged after #2215.

Copy link
Contributor

Choose a reason for hiding this comment

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

I saw SetCommand takes the context as the parameter, as well as the other Commands in the file commands.scala, like:

case class SetCommand(
    key: Option[String], value: Option[String], output: Seq[Attribute])(
    @transient context: SQLContext)

Should we follow the same pattern? The default sqlContext will take value from a ThreadLocal variable, which probably a little different.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The ThreadLocal version was added in #993, and I see this as the more recommended manner to pass SQLContext/HiveContext since it saves boilerplate code, @marmbrus is it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, you should use the provide sqlContext. This will always be provided by the planner.

@chenghao-intel
Copy link
Contributor

I saw people complaints the add jar doesn't work in the maillist, that's great to have this feature, otherwise we may not able to add the UDF jars in runtime. BTW, ./sql/hive/src/test/resources/data/files/TestSerDe.jar can be used for the add jar unit test, just in case we need one.

@liancheng
Copy link
Contributor Author

Cool, thanks for the suggestion for testing, will add a test case for this.

@liancheng
Copy link
Contributor Author

ok to test

@SparkQA
Copy link

SparkQA commented Sep 3, 2014

QA tests have started for PR 2242 at commit 522856e.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Sep 3, 2014

QA tests have finished for PR 2242 at commit 522856e.

  • This patch passes unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class AddJar(path: String) extends LeafNode with Command

@SparkQA
Copy link

SparkQA commented Sep 3, 2014

QA tests have started for PR 2242 at commit ec7366d.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Sep 3, 2014

QA tests have finished for PR 2242 at commit ec7366d.

  • This patch fails unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class AddJar(path: String) extends LeafNode with Command

@liancheng
Copy link
Contributor Author

Build failure caused by unrelated GraphX suite, maybe a GraphX regression, saw this multiple times this morning @rxin @ankurdave

@liancheng
Copy link
Contributor Author

ok to test

@SparkQA
Copy link

SparkQA commented Sep 3, 2014

QA tests have started for PR 2242 at commit ec7366d.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Sep 4, 2014

QA tests have finished for PR 2242 at commit ec7366d.

  • This patch fails unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class AddJar(path: String) extends LeafNode with Command

@liancheng
Copy link
Contributor Author

Build failure caused by streaming suites.

@liancheng
Copy link
Contributor Author

retest this please

@marmbrus
Copy link
Contributor

marmbrus commented Sep 4, 2014

Rebase please :)

@SparkQA
Copy link

SparkQA commented Sep 4, 2014

QA tests have started for PR 2242 at commit e43a2f1.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Sep 4, 2014

QA tests have finished for PR 2242 at commit e43a2f1.

  • This patch passes unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class BlockManagerMaster(
    • class AttributeMap[A](baseMap: Map[ExprId, (Attribute, A)])
    • case class AddJar(path: String) extends LeafNode with Command

@liancheng
Copy link
Contributor Author

@marmbrus Rebased, this is also ready to go!

@marmbrus
Copy link
Contributor

marmbrus commented Sep 5, 2014

Thanks! I've merged this to master.

@asfgit asfgit closed this in ee575f1 Sep 5, 2014
@liancheng liancheng deleted the add-jar branch September 24, 2014 00:05
tbfenet added a commit to tbfenet/spark that referenced this pull request Dec 10, 2014
Adds logical and physical command classes for the "add jar" command.

Note that this PR conflicts with and should be merged after apache#2215.

Author: Cheng Lian <lian.cs.zju@gmail.com>

Closes apache#2242 from liancheng/add-jar and squashes the following commits:

e43a2f1 [Cheng Lian] Updates AddJar according to conventions introduced in apache#2215
b99107f [Cheng Lian] Added test case for ADD JAR command
095b2c7 [Cheng Lian] Also forward ADD JAR command to Hive
9be031b [Cheng Lian] Trims Jar path string
8195056 [Cheng Lian] Added support for the "add jar" command

Conflicts:
	sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveStrategies.scala
	sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/commands.scala
	sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveQuerySuite.scala
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.

4 participants