Skip to content

Conversation

@cloud-fan
Copy link
Contributor

What changes were proposed in this pull request?

Currently HiveExternalCatalog will filter out the Spark SQL internal table properties, e.g. spark.sql.sources.provider, spark.sql.sources.schema, etc. This is reasonable for external users as they don't want to see these internal properties in DESC TABLE.

However, as a Spark developer, sometimes we do wanna see the raw table properties. This PR adds a new internal SQL conf, spark.sql.debug, to enable debug mode and keep these raw table properties.

This config can also be used in similar places where we wanna retain debug information in the future.

How was this patch tested?

new test in MetastoreDataSourcesSuite

@cloud-fan
Copy link
Contributor Author

cc @yhuai

@rxin
Copy link
Contributor

rxin commented Oct 13, 2016

LGTM pending Jenkins.

checkAnswer(
newSession.sql(s"SET ${SCHEMA_STRING_LENGTH_THRESHOLD.key}"),
Row(SCHEMA_STRING_LENGTH_THRESHOLD.key, "2000"))
test("static SQL conf comes from SparkConf") {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this change is not related to this PR.

SparkSession.builder()....getOrCreate() may not set the static SQL conf if there is an active or global default session. To make this test more robust, I explicitly create a new SparkSession.

This is the same approach I used in the new test, so I include this unrelated change in this PR.

@SparkQA
Copy link

SparkQA commented Oct 13, 2016

Test build #66871 has finished for PR 15458 at commit e821f1a.

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

@andrewor14
Copy link
Contributor

Cool beans. Merging into master 2.0.

@andrewor14
Copy link
Contributor

andrewor14 commented Oct 13, 2016

JK, actually it doesn't merge in 2.0. Probably OK to just leave it in master.

@asfgit asfgit closed this in db8784f Oct 13, 2016
@gatorsmile
Copy link
Member

If we turn this flag on, I am just afraid that some DDL statements might not work well. I assume this flag is really used for the our Spark SQL developers.

@andrewor14
Copy link
Contributor

Yes that's why it's internal

@gatorsmile
Copy link
Member

Just FYI. #15478 shows at least three DDL commands do not work properly when spark.sql.debug is set to true: ALTER TABLE, ANALYZE TABLE and CREATE TABLE LIKE

@andrewor14
Copy link
Contributor

I see. Then maybe we should add a comment above the config to note that several commands don't work (e.g. ALTER TABLE) if this is turned on, even if it's only internal.

val DEBUG_MODE = buildConf("spark.sql.debug")
.internal()
.booleanConf
.createWithDefault(false)
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add doc for this flag.

Copy link
Member

Choose a reason for hiding this comment

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

Sure, I can do a quick PR to fix it.

uzadude pushed a commit to uzadude/spark that referenced this pull request Jan 27, 2017
…iveExternalCatalog

## What changes were proposed in this pull request?

Currently `HiveExternalCatalog` will filter out the Spark SQL internal table properties, e.g. `spark.sql.sources.provider`, `spark.sql.sources.schema`, etc. This is reasonable for external users as they don't want to see these internal properties in `DESC TABLE`.

However, as a Spark developer, sometimes we do wanna see the raw table properties. This PR adds a new internal SQL conf, `spark.sql.debug`, to enable debug mode and keep these raw table properties.

This config can also be used in similar places where we wanna retain debug information in the future.

## How was this patch tested?

new test in MetastoreDataSourcesSuite

Author: Wenchen Fan <wenchen@databricks.com>

Closes apache#15458 from cloud-fan/debug.
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.

6 participants