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-30603][SQL] Move RESERVED_PROPERTIES from SupportsNamespaces and TableCatalog to CatalogV2Util #27318

Closed
wants to merge 9 commits into from

Conversation

yaooqinn
Copy link
Member

@yaooqinn yaooqinn commented Jan 22, 2020

What changes were proposed in this pull request?

In this PR, I propose to move the RESERVED_PROPERTIES s from SupportsNamespaces and TableCatalog to CatalogV2Util, which can keep RESERVED_PROPERTIES safe for interval usages only.

Why are the changes needed?

the RESERVED_PROPERTIES should not be changed by subclasses

Does this PR introduce any user-facing change?

no

How was this patch tested?

existing uts

@SparkQA
Copy link

SparkQA commented Jan 22, 2020

Test build #117228 has finished for PR 27318 at commit e193bc3.

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

@SparkQA
Copy link

SparkQA commented Jan 22, 2020

Test build #117233 has finished for PR 27318 at commit 938aeea.

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

* A property to specify the location of the namespace. If the namespace
* needs to store files, it should be under this location.
*/
String PROP_LOCATION = "location";
Copy link
Contributor

Choose a reason for hiding this comment

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

These property names should remain in the public class, as we need the implementations to read and know them.

The only thing interval is which properties are reserved. We should move the RESERVED_PROPERTIES to CatalogV2Utils

Copy link
Member Author

Choose a reason for hiding this comment

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

ah I see

@yaooqinn yaooqinn changed the title [SPARK-30603][SQL] Keep the reserved properties of namespaces and tables private [SPARK-30603][SQL] move RESERVED_PROPERTIES from SupportsNamespaces and TableCatalog to CatalogV2Util Jan 23, 2020
@cloud-fan
Copy link
Contributor

sorry it conflicts, can you resolve it frist?

@SparkQA
Copy link

SparkQA commented Jan 23, 2020

Test build #117282 has finished for PR 27318 at commit 93d316a.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@yaooqinn
Copy link
Member Author

thanks for your reminding @cloud-fan, I've cleaned the conflicts.

@SparkQA
Copy link

SparkQA commented Jan 23, 2020

Test build #117287 has finished for PR 27318 at commit 3d54845.

  • This patch fails SparkR unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
  • case class UnresolvedV2Relation(
  • case class ResolvedView(identifier: Identifier) extends LeafNode
  • case class Like(left: Expression, right: Expression, escapeChar: Char)
  • case class AlterTableAddColumnsStatement(
  • case class AlterTableAlterColumnStatement(
  • case class AlterTableRenameColumnStatement(
  • case class AlterTableDropColumnsStatement(
  • case class AlterTableSetPropertiesStatement(
  • case class AlterTableUnsetPropertiesStatement(
  • case class AlterTableSetLocationStatement(
  • case class AlterTable(
  • case class SparkListenerSQLAdaptiveSQLMetricUpdates(

@cloud-fan
Copy link
Contributor

retest this please

@cloud-fan
Copy link
Contributor

sorry it conflicts again...

@yaooqinn
Copy link
Member Author

updated

@SparkQA
Copy link

SparkQA commented Jan 23, 2020

Test build #117308 has finished for PR 27318 at commit 3d54845.

  • This patch fails SparkR unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
  • case class UnresolvedV2Relation(
  • case class ResolvedView(identifier: Identifier) extends LeafNode
  • case class Like(left: Expression, right: Expression, escapeChar: Char)
  • case class AlterTableAddColumnsStatement(
  • case class AlterTableAlterColumnStatement(
  • case class AlterTableRenameColumnStatement(
  • case class AlterTableDropColumnsStatement(
  • case class AlterTableSetPropertiesStatement(
  • case class AlterTableUnsetPropertiesStatement(
  • case class AlterTableSetLocationStatement(
  • case class AlterTable(
  • case class SparkListenerSQLAdaptiveSQLMetricUpdates(

@SparkQA
Copy link

SparkQA commented Jan 23, 2020

Test build #117314 has finished for PR 27318 at commit c9943ca.

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

@SparkQA
Copy link

SparkQA commented Jan 23, 2020

Test build #117315 has finished for PR 27318 at commit ac1ef4b.

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

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-30603][SQL] move RESERVED_PROPERTIES from SupportsNamespaces and TableCatalog to CatalogV2Util [SPARK-30603][SQL] Move RESERVED_PROPERTIES from SupportsNamespaces and TableCatalog to CatalogV2Util Jan 23, 2020
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, LGTM. Merged to master. Thank you, @yaooqinn and @cloud-fan .

(cc @viirya. It seems that CRAN issue is fixed. Thank you always.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants