Skip to content

Conversation

@marmbrus
Copy link
Contributor

@marmbrus marmbrus commented Mar 8, 2016

Follow-up to #11509, that simply refactors the interface that we use when resolving a pluggable DataSource.

  • Multiple functions share the same set of arguments so we make this a case class, called DataSource. Actual resolution is now done by calling a function on this class.
  • Instead of having multiple methods named apply (some of which do writing some of which do reading) we now explicitly have resolveRelation() and write(mode, df).
  • Get rid of Array[String] since this is an internal API and was forcing us to awkwardly call toArray in a bunch of places.

@marmbrus marmbrus changed the title [SPARK-13664][SQL] Cleanup Data Source resolution [SPARK-13738][SQL] Cleanup Data Source resolution Mar 8, 2016
@SparkQA
Copy link

SparkQA commented Mar 8, 2016

Test build #52624 has finished for PR 11572 at commit 36969f8.

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

@SparkQA
Copy link

SparkQA commented Mar 8, 2016

Test build #52625 has finished for PR 11572 at commit cf7c719.

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

@SparkQA
Copy link

SparkQA commented Mar 8, 2016

Test build #52637 has finished for PR 11572 at commit d83c2e2.

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

@SparkQA
Copy link

SparkQA commented Mar 8, 2016

Test build #52639 has finished for PR 11572 at commit dadd589.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class MetaStoreFileCatalog(

@SparkQA
Copy link

SparkQA commented Mar 8, 2016

Test build #52634 has finished for PR 11572 at commit 6f6f099.

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

* The main class responsible for representing a pluggable Data Source in Spark SQL. In addition to
* acting as the canonical set of parameters that can describe a Data Source, this class is used to
* resolve a description to a concrete implementation that can be used in a query plan
* (either batch or streaming) or to write out out data using an external library.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: write out ...

@HyukjinKwon
Copy link
Member

While taking a look, I also saw some nits in the previous PR, #11509.

At OrcRelation,

  1. import com.google.common.base.Objects is not being used.
  2. import org.apache.spark.sql.hive.HiveContext is not being used.
  3. the exception message, "call writeInternal" has double spaces.

@SparkQA
Copy link

SparkQA commented Mar 8, 2016

Test build #52674 has finished for PR 11572 at commit d7472ea.

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

@rxin
Copy link
Contributor

rxin commented Mar 8, 2016

LGTM - merging in master.

@asfgit asfgit closed this in 1e28840 Mar 8, 2016
roygao94 pushed a commit to roygao94/spark that referenced this pull request Mar 22, 2016
Follow-up to apache#11509, that simply refactors the interface that we use when resolving a pluggable `DataSource`.
 - Multiple functions share the same set of arguments so we make this a case class, called `DataSource`.  Actual resolution is now done by calling a function on this class.
 - Instead of having multiple methods named `apply` (some of which do writing some of which do reading) we now explicitly have `resolveRelation()` and `write(mode, df)`.
 - Get rid of `Array[String]` since this is an internal API and was forcing us to awkwardly call `toArray` in a bunch of places.

Author: Michael Armbrust <michael@databricks.com>

Closes apache#11572 from marmbrus/dataSourceResolution.
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.

5 participants