Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
support env argus in add jar, and set add jar ret to 0
  • Loading branch information
adrian-wang committed Apr 2, 2015
commit 95a40da98b570a5bb3beec2e5420d8a482762720
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import org.apache.hadoop.hive.common.{HiveInterruptCallback, HiveInterruptUtils,
import org.apache.hadoop.hive.conf.HiveConf
import org.apache.hadoop.hive.ql.Driver
import org.apache.hadoop.hive.ql.exec.Utilities
import org.apache.hadoop.hive.ql.processors.{SetProcessor, CommandProcessor, CommandProcessorFactory}
import org.apache.hadoop.hive.ql.processors.{AddResourceProcessor, SetProcessor, CommandProcessor, CommandProcessorFactory}
import org.apache.hadoop.hive.ql.session.SessionState
import org.apache.hadoop.hive.shims.ShimLoader
import org.apache.thrift.transport.TSocket
Expand Down Expand Up @@ -264,7 +264,8 @@ private[hive] class SparkSQLCLIDriver extends CliDriver with Logging {
val proc: CommandProcessor = HiveShim.getCommandProcessor(Array(tokens(0)), hconf)

if (proc != null) {
if (proc.isInstanceOf[Driver] || proc.isInstanceOf[SetProcessor]) {
if (proc.isInstanceOf[Driver] || proc.isInstanceOf[SetProcessor] ||
proc.isInstanceOf[AddResourceProcessor]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

just need "proc.isInstanceOf[AddResourceProcessor]", then "add file" will work.

Copy link
Contributor

Choose a reason for hiding this comment

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

agree

val driver = new SparkSQLDriver

driver.init()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,7 @@ class HiveCompatibilitySuite extends HiveQueryFileTest with BeforeAndAfter {
"load_file_with_space_in_the_name",
"loadpart1",
"louter_join_ppr",
"mapjoin_addjar",
"mapjoin_distinct",
"mapjoin_filter_on_outerjoin",
"mapjoin_mapjoin",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

package org.apache.spark.sql.hive.execution

import org.apache.hadoop.hive.ql.parse.VariableSubstitution

import org.apache.spark.sql.AnalysisException
import org.apache.spark.sql.catalyst.analysis.EliminateSubQueries
import org.apache.spark.sql.catalyst.util._
Expand Down Expand Up @@ -78,8 +80,10 @@ case class AddJar(path: String) extends RunnableCommand {
override def run(sqlContext: SQLContext): Seq[Row] = {
val hiveContext = sqlContext.asInstanceOf[HiveContext]
hiveContext.runSqlHive(s"ADD JAR $path")
hiveContext.sparkContext.addJar(path)
Seq.empty[Row]
val substituted =
new VariableSubstitution().substitute(sqlContext.asInstanceOf[HiveContext].hiveconf, path)
hiveContext.sparkContext.addJar(substituted)
Copy link
Contributor

Choose a reason for hiding this comment

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

As I remember, every SQL text is substitute in https://github.com/apache/spark/blob/master/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveContext.scala#L86
Can you confirm if we really need a variable substitution here?

Seq(Row(0))
Copy link
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
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
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).

}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ class TestHiveContext(sc: SparkContext) extends HiveContext(sc) {
// without restarting the JVM.
System.clearProperty("spark.hostPort")
CommandProcessorFactory.clean(hiveconf)
System.setProperty("hive.version", HiveShim.version)
Copy link
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.

System.setProperty(
"maven.local.repository",
System.getProperty("user.home") + File.separator + ".m2" + File.separator + "repository")

hiveconf.set("hive.plan.serialization.format", "javaXML")

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
0
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
0
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
0
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
2 val_2 2 blah
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
0