[SPARK-15116] In REPL we should create SparkSession first and get SparkContext from it

[cloud-fan]

What changes were proposed in this pull request?

see #12873 (comment). The problem is, if we create SparkContext first and then call SparkSession.builder.enableHiveSupport().getOrCreate(), we will reuse the existing SparkContext and the hive flag won't be set.

How was this patch tested?

verified it locally.

[cloud-fan]

cc @andrewor14 @rxin

[rxin]

Are these not called only once?

[SparkQA]

Test build #57732 has finished for PR 12890 at commit 49d653b.

• This patch fails build dependency tests.
• This patch merges cleanly.
• This patch adds no public classes.

[cloud-fan]

yea, but look at the logic of Builder.getOrCreate:

def getOrCreate(): SparkSession = synchronized {
      // Step 1. Create a SparkConf
      // Step 2. Get a SparkContext
      // Step 3. Get a SparkSession
      val sparkConf = new SparkConf()
      options.foreach { case (k, v) => sparkConf.set(k, v) }
      val sparkContext = SparkContext.getOrCreate(sparkConf)

      SQLContext.getOrCreate(sparkContext).sparkSession
    }

In REPL, we create the SparkContext first, then SparkSession, so the getOrCreate won't create a new SparkSession, and hive support is not enabled.

[cloud-fan]

retest this please

[rxin]

Sorry I still don't understand what's going on by looking at your comment. One thing is that we are going to remove withHiveSupport, so adding it back isn't a "fix" per se.

[rxin]

It seems like there is an underlying problem here. Can we fix that?

[SparkQA]

Test build #57733 has finished for PR 12890 at commit 49d653b.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #57742 has finished for PR 12890 at commit 2681e79.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[yhuai]

Is the problem that {{val sparkContext = SparkContext.getOrCreate(sparkConf)}} will give us a {{sparkContext}} that already created by the repl and its conf does not spark.sql.catalogImplementation to hive?

[yhuai]

I guess we want to use builder.config("spark.sql.catalogImplementation", "hive")?

[rxin]

that's what enableHiveSupport does?

[yhuai]

oh, right. We still have this method.

[andrewor14]

LGTM

[andrewor14]

By the way I think the comment #12873 (comment) captures the point but is a little misleading. The issue here is that if there's already a SparkContext then we just throw away all the conf set through the builder, including the spark.sql.catalogImplementation! Note that, however, we always create a new SparkSession even before this patch.

[rxin]

It seems like we might need a little bit more of design here to figure out what the right behavior should be. Let's talk more offline.

[andrewor14]

This patch itself has little to do with the design so let's just merge this first.
Merging into master 2.0.