Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-7943] [SPARK-8105] [SPARK-8435] [SPARK-8714] [SPARK-8561] Fixes multi-database support #7623

Closed
wants to merge 4 commits into from

Conversation

liancheng
Copy link
Contributor

This PR fixes a set of issues related to multi-database. A new data structure TableIdentifier is introduced to identify a table among multiple databases. We should stop using a single String (table name without database name), or Seq[String] (optional database name plus table name) to identify tables internally.

@SparkQA
Copy link

SparkQA commented Jul 23, 2015

Test build #38244 has finished for PR 7623 at commit 611b61d.

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


case _ =>
val cmd =
CreateTableUsingAsSelect(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should refactor CreateTableUsingAsSelect to use TableIdentifier.

@SparkQA
Copy link

SparkQA commented Jul 23, 2015

Test build #38246 has finished for PR 7623 at commit 5a94169.

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

table(new SqlParser().parseTableIdentifier(tableName))
}

private[sql] def table(tableIdent: TableIdentifier): DataFrame = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this called frequently enough to justify a function? It would be great to remove this if possible, since this API is also accessible in Java, and Java users won't see this as a private API.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's called right above... I can make it private instead of private[sql] though.

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 the call above. Why having this if it is only used once?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To provide better support for multiple databases, internally we should always use TableIdentifier instead of String or Seq[String]. That's why I made this private[sql].

@SparkQA
Copy link

SparkQA commented Jul 26, 2015

Test build #38460 has finished for PR 7623 at commit ec5f903.

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

@SparkQA
Copy link

SparkQA commented Jul 26, 2015

Test build #38459 has finished for PR 7623 at commit c49cb5c.

  • This patch passes all tests.
  • This patch does not merge cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jul 27, 2015

Test build #38527 has finished for PR 7623 at commit f3bcd4b.

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

@liancheng
Copy link
Contributor Author

Merging to master.

@asfgit asfgit closed this in 72981bc Jul 27, 2015
@liancheng liancheng deleted the spark-8131-multi-db branch July 27, 2015 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants