Skip to content

[SPARK-5794] [SQL] fix add jar#4586

Closed
adrian-wang wants to merge 7 commits into
apache:masterfrom
adrian-wang:addjar
Closed

[SPARK-5794] [SQL] fix add jar#4586
adrian-wang wants to merge 7 commits into
apache:masterfrom
adrian-wang:addjar

Conversation

@adrian-wang

Copy link
Copy Markdown
Contributor

No description provided.

@SparkQA

SparkQA commented Feb 13, 2015

Copy link
Copy Markdown

Test build #27427 has finished for PR 4586 at commit 275a8f7.

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

@SparkQA

SparkQA commented Feb 13, 2015

Copy link
Copy Markdown

Test build #27428 has finished for PR 4586 at commit 85db611.

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

@adrian-wang

Copy link
Copy Markdown
Contributor Author

This is because Jenkins do not have hcatalog.jar at /home/jenkins/.m2/repository/org/apache/hive/hcatalog/hive-hcatalog-core/0.13.1/hive-hcatalog-core-0.13.1.jar

I can remove the test from white list if the rest of this PR looks good.

@watermen

watermen commented Mar 2, 2015

Copy link
Copy Markdown
Contributor

I also found this issue, I fetched your branch and when I ran select * from src join t1 on src.key =t1.a; in mapjoin_addjar.q, it also threw ERROR CliDriver: java.lang.RuntimeException: java.lang.ClassNotFoundException: org.apache.hive.hcatalog.data.JsonSerDe like before.
I debuged the code and found it can't setup into if (proc.isInstanceOf[Driver] || proc.isInstanceOf[SetProcessor] in SparkSQLCLIDriver, so it can't run the run(sqlContext: SQLContext) in case class AddJar. Do I understand right?

@adrian-wang

Copy link
Copy Markdown
Contributor Author

@watermen , I re-compiled and it can step into that.

@watermen

watermen commented Mar 3, 2015

Copy link
Copy Markdown
Contributor

@adrian-wang, you mean it can step into if (proc.isInstanceOf[Driver] || proc.isInstanceOf[SetProcessor] or run(sqlContext: SQLContext)? Because add jar is instance of AddResourceProcessor.

@adrian-wang

Copy link
Copy Markdown
Contributor Author

If you are using HiveContext, it will step into the else branch, and trigger by
ret = proc.run(cmd_1).getResponseCode
so that will be executed by hive. if you are using SqlContext, then the parser will call the add jar case class.

@watermen

watermen commented Mar 3, 2015

Copy link
Copy Markdown
Contributor

@adrian-wang yeah, I'm using HiveContext, and threw ERROR CliDriver: java.lang.RuntimeException: java.lang.ClassNotFoundException: org.apache.hive.hcatalog.data.JsonSerDerun when running the test case in mapjoin_addjar.q. I know it will be executed by hive, so it's strange.

@adrian-wang

Copy link
Copy Markdown
Contributor Author

@waterman do you have that jar in your maven local repository then?

@adrian-wang

Copy link
Copy Markdown
Contributor Author

just rebased twice...

@watermen

watermen commented Mar 3, 2015

Copy link
Copy Markdown
Contributor

@adrian-wang I'm running the sql in spark-sql shell, and use add jar /home/qiyadong/GitHub/temp/hive-hcatalog-core-0.13.1.jar; to add jar. You mean I should check the jar in maven local repository when I run maven test?

@adrian-wang

Copy link
Copy Markdown
Contributor Author

@watermen there must be something wrong. I just did a clean pick and verified that this works well. You may need to clean and rebuild your assembly jar.

@adrian-wang

Copy link
Copy Markdown
Contributor Author

@watermen Ah, I think I know what's wrong now. I'll fix that, thanks for your reminding!

@watermen

watermen commented Mar 3, 2015

Copy link
Copy Markdown
Contributor

@adrian-wang Look forward to your commit.

@SparkQA

SparkQA commented Mar 3, 2015

Copy link
Copy Markdown

Test build #28221 has finished for PR 4586 at commit 97cab24.

  • This patch fails Spark unit tests.
  • This patch does not merge cleanly.
  • This patch adds no public classes.

@SparkQA

SparkQA commented Mar 3, 2015

Copy link
Copy Markdown

Test build #28223 has finished for PR 4586 at commit 1ce40c1.

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

@SparkQA

SparkQA commented Mar 3, 2015

Copy link
Copy Markdown

Test build #28222 has finished for PR 4586 at commit b42b15c.

  • This patch fails Spark unit tests.
  • This patch does not merge cleanly.
  • This patch adds no public classes.

@SparkQA

SparkQA commented Mar 3, 2015

Copy link
Copy Markdown

Test build #28225 has finished for PR 4586 at commit 2b46223.

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

@watermen

watermen commented Mar 3, 2015

Copy link
Copy Markdown
Contributor

@adrian-wang I try if (proc.isInstanceOf[Driver] || proc.isInstanceOf[SetProcessor] || proc.isInstanceOf[AddResourceProcessor]) like you before, and the exception also threw. But bin/spark-sql --jars xxx.jar or set xxx.jar in bin/beeline can be run successfully.

@watermen

watermen commented Mar 3, 2015

Copy link
Copy Markdown
Contributor

@adrian-wang I think sparkContext.addJar as same as bin/spark-sql --jars xxx.jar, but the former is fail and the latter is success.

@adrian-wang

Copy link
Copy Markdown
Contributor Author

@watermen I have test my code this time and it should work. I'm trying to figure out where the problem lies, can you try this code first?

@SparkQA

SparkQA commented Mar 5, 2015

Copy link
Copy Markdown

Test build #28275 has finished for PR 4586 at commit 48f93c2.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • logInfo("Added " + substituted + " to class path")

@watermen

watermen commented Mar 5, 2015

Copy link
Copy Markdown
Contributor

@adrian-wang I test it and it works.

@adrian-wang adrian-wang changed the title [SPARK-5794] [SQL] support env argus in add jar, and set add jar ret to 0 [SPARK-5794] [SQL] fix add jar Mar 5, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There is a SerDe jar at sql/hive/src/test/resources/data/files/TestSerDe.jar , Instead of whitelisting the mapjoin_addjar in HiveCompatibilitySuite.scala, we can create a new unit test in HiveQuerySuite for simplifying the unit testing. As setting the local maven repo like this seems a little bit hacky to me, it probably doesn't work if people modify the default location or the jar hive-hcatalog-core-*.jar doesn't exist in some cases.

@pzzs

pzzs commented Mar 12, 2015

Copy link
Copy Markdown
Contributor

LGTM

@the-sea

the-sea commented Mar 13, 2015

Copy link
Copy Markdown

greate feature!

@SparkQA

SparkQA commented Apr 2, 2015

Copy link
Copy Markdown

Test build #29600 has finished for PR 4586 at commit 0810e71.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • logInfo("Added " + path + " to class path")
  • This patch does not change any dependencies.

@adrian-wang adrian-wang changed the title [SPARK-5794] [SQL] [WIP] fix add jar [SPARK-5794] [SQL] fix add jar Apr 8, 2015
@SparkQA

SparkQA commented Apr 8, 2015

Copy link
Copy Markdown

Test build #29840 has finished for PR 4586 at commit 9957d87.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
  • This patch does not change any dependencies.

@SparkQA

SparkQA commented Apr 8, 2015

Copy link
Copy Markdown

Test build #29847 has finished for PR 4586 at commit 6c707e8.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
  • This patch does not change any dependencies.

@SparkQA

SparkQA commented Apr 8, 2015

Copy link
Copy Markdown

Test build #29845 has finished for PR 4586 at commit 32c4fb8.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
  • This patch does not change any dependencies.

@adrian-wang

Copy link
Copy Markdown
Contributor Author

retest this please.

@SparkQA

SparkQA commented Apr 8, 2015

Copy link
Copy Markdown

Test build #29851 has finished for PR 4586 at commit 6c707e8.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
  • This patch does not change any dependencies.

@adrian-wang

Copy link
Copy Markdown
Contributor Author

ping.
/cc @liancheng

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you put this jar somewhere else? The stuff inside of resources/data are often replaced completely with files from hive. Perhaps just in the top level resources dir.

@SparkQA

SparkQA commented Apr 9, 2015

Copy link
Copy Markdown

Test build #29916 has finished for PR 4586 at commit efdd602.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
  • This patch does not change any dependencies.

@adrian-wang

Copy link
Copy Markdown
Contributor Author

ping @marmbrus

1 similar comment
@adrian-wang

Copy link
Copy Markdown
Contributor Author

ping @marmbrus

@marmbrus

Copy link
Copy Markdown
Contributor

Thanks, merged to master.

@asfgit asfgit closed this in b45059d Apr 14, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@adrian-wang Why we need a Row with a value of 0 at here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hive would return a 0 for add jar command.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(I thought it may be better to comment at the original pr).

OK, I see. In future, let's make sure we also update the output if the result of a command is not an empty Seq (#5350 will change the schema for AddJar).

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.

9 participants