[SPARK-32827][SQL] Add spark.sql.maxMetadataStringLength config #29688

ulysses-you · 2020-09-09T04:32:51Z

What changes were proposed in this pull request?

Add a new config spark.sql.maxMetadataStringLength. This config aims to limit metadata value length, e.g. file location.

Why are the changes needed?

Some metadata have been abbreviated by ... when I tried to add some test in SQLQueryTestSuite. We need to replace such value to notIncludedMsg. That caused we can't replace that like location value by className since the className has been abbreviated.

Here is a case:

CREATE table  explain_temp1 (key int, val int) USING PARQUET;

EXPLAIN EXTENDED SELECT sum(distinct val) FROM explain_temp1;

-- ignore parsed,analyzed,optimized
-- The output like
== Physical Plan ==
*HashAggregate(keys=[], functions=[sum(distinct cast(val#x as bigint)#xL)], output=[sum(DISTINCT val)#xL])
+- Exchange SinglePartition, true, [id=#x]
   +- *HashAggregate(keys=[], functions=[partial_sum(distinct cast(val#x as bigint)#xL)], output=[sum#xL])
      +- *HashAggregate(keys=[cast(val#x as bigint)#xL], functions=[], output=[cast(val#x as bigint)#xL])
         +- Exchange hashpartitioning(cast(val#x as bigint)#xL, 4), true, [id=#x]
            +- *HashAggregate(keys=[cast(val#x as bigint) AS cast(val#x as bigint)#xL], functions=[], output=[cast(val#x as bigint)#xL])
               +- *ColumnarToRow
                  +- FileScan parquet default.explain_temp1[val#x] Batched: true, DataFilters: [], Format: Parquet, Location: InMemoryFileIndex[file:/home/runner/work/spark/spark/sql/core/spark-warehouse/org.apache.spark.sq...], PartitionFilters: ...

Does this PR introduce any user-facing change?

No, a new config.

How was this patch tested?

new test.

SparkQA · 2020-09-09T04:40:40Z

Test build #128434 has finished for PR 29688 at commit 7b676b8.

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

SparkQA · 2020-09-09T07:05:02Z

Test build #128435 has finished for PR 29688 at commit eb67ccd.

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

ulysses-you · 2020-09-09T07:08:52Z

@maropu @dongjoon-hyun @cloud-fan do you have time to look this ? thanks !

maropu · 2020-09-09T07:50:39Z

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala


+  val MAX_METADATA_STRING_LENGTH = buildConf("spark.sql.maxMetadataStringLength")
+    .doc("Maximum number of characters to output for a metadata string. e.g. " +
+      "`DataSourceScanExec`, every value will be abbreviated if exceed length.")


e.g. DataSourceScanExec => e.g. file location in DataSourceScanExec

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala

maropu · 2020-09-09T07:51:23Z

sql/core/src/test/scala/org/apache/spark/sql/FileBasedDataSourceSuite.scala

+
+  test("SPARK-32827: Add spark.sql.maxMetadataStringLength config") {
+    withTempDir { dir =>
+      val tableName = "t1"


nit: t1 -> t

maropu · 2020-09-09T07:52:14Z

sql/core/src/test/scala/org/apache/spark/sql/FileBasedDataSourceSuite.scala

+      val tableName = "t1"
+      val path = s"${dir.getCanonicalPath}/$tableName"
+      withTable(tableName) {
+        sql(s"create table t1(c int) using parquet location '$path'")


plz use uppercases for SQL keywords, e.g., CREATE TABLE

maropu · 2020-09-09T07:54:05Z

Adding the config looks okay to me.

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala

SparkQA · 2020-09-09T14:03:19Z

Test build #128444 has finished for PR 29688 at commit 681f84e.

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

SparkQA · 2020-09-09T14:40:51Z

Test build #128443 has finished for PR 29688 at commit 239f62b.

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

xkrogen · 2020-09-09T15:36:48Z

Glad to see this happening @ulysses-you ! We have a similar internal patch we added recently that we've been planning to open source. Nice to see that the changes are even simpler on the 3.x line.

gengliangwang · 2020-09-11T16:57:47Z

sql/core/src/test/scala/org/apache/spark/sql/FileBasedDataSourceSuite.scala

    }
  }
+
+  test("SPARK-32827: Add spark.sql.maxMetadataStringLength config") {


Nit: Add spark.sql.maxMetadataStringLength config => Set max metadata string length

gengliangwang · 2020-09-11T16:58:44Z

@ulysses-you thanks for the work.
It seems that the PR only changes the file source related metadata. Is there other places we can use the new config as well?

ulysses-you · 2020-09-13T02:13:05Z

Thanks @gengliangwang .The origin propose is for explain test what I said in pr description, related pr is #29586. Location value has been abbreviated by ... since default metadata length is 100. We can add the config at head of test file like --SET spark.sql.maxMetadataStringLength=500; if we have this config, then we can get the total location value.

I'm not sure if it's well to do these change in one pr. But I can merge them into one pr if you think it's ok.

SparkQA · 2020-09-13T07:05:01Z

Test build #128596 has finished for PR 29688 at commit 9c19245.

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

…2827

SparkQA · 2020-09-14T04:48:54Z

Test build #128611 has finished for PR 29688 at commit e83bf95.

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

AngersZhuuuu · 2020-09-15T02:40:59Z

@ulysses-you thanks for the work.
It seems that the PR only changes the file source related metadata. Is there other places we can use the new config as well?

I will use this config in my pr #29739.
Similar work in my pr and reverted waiting this.

cloud-fan · 2020-09-15T14:11:28Z

thanks, merging to master!

ulysses-you · 2020-09-15T23:57:00Z

Thanks for merging, thanks all !

init

7b676b8

probot-autolabeler bot added the SQL label Sep 9, 2020

style

eb67ccd

maropu reviewed Sep 9, 2020

View reviewed changes

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala Show resolved Hide resolved

maropu reviewed Sep 9, 2020

View reviewed changes

ulysses-you added 3 commits September 9, 2020 16:51

conf

8a14e58

test

239f62b

fix

681f84e

ulysses-you commented Sep 9, 2020

View reviewed changes

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala Show resolved Hide resolved

gengliangwang reviewed Sep 11, 2020

View reviewed changes

nit

9c19245

Merge branch 'master' of https://github.com/apache/spark into SPARK-3…

e83bf95

…2827

maropu mentioned this pull request Sep 15, 2020

[SPARK-32867][SQL] When explain, HiveTableRelation show limited message #29739

Closed

cloud-fan approved these changes Sep 15, 2020

View reviewed changes

cloud-fan closed this in 888b343 Sep 15, 2020

tgravescs mentioned this pull request Feb 19, 2021

[FEA] Spark 3.1.0 - pull in [SPARK-32827][SQL] Add spark.sql.maxMetadataStringLength config NVIDIA/spark-rapids#1460

Closed

[SPARK-32827][SQL] Add spark.sql.maxMetadataStringLength config #29688

[SPARK-32827][SQL] Add spark.sql.maxMetadataStringLength config #29688

Uh oh!

Conversation

ulysses-you commented Sep 9, 2020

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

SparkQA commented Sep 9, 2020

Uh oh!

SparkQA commented Sep 9, 2020

Uh oh!

ulysses-you commented Sep 9, 2020

Uh oh!

maropu Sep 9, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

maropu Sep 9, 2020

Choose a reason for hiding this comment

Uh oh!

maropu Sep 9, 2020

Choose a reason for hiding this comment

Uh oh!

ulysses-you Sep 9, 2020

Choose a reason for hiding this comment

Uh oh!

maropu commented Sep 9, 2020

Uh oh!

Uh oh!

SparkQA commented Sep 9, 2020

Uh oh!

SparkQA commented Sep 9, 2020

Uh oh!

xkrogen commented Sep 9, 2020

Uh oh!

gengliangwang Sep 11, 2020

Choose a reason for hiding this comment

Uh oh!

gengliangwang commented Sep 11, 2020

Uh oh!

ulysses-you commented Sep 13, 2020

Uh oh!

SparkQA commented Sep 13, 2020

Uh oh!

SparkQA commented Sep 14, 2020

Uh oh!

AngersZhuuuu commented Sep 15, 2020

Uh oh!

cloud-fan commented Sep 15, 2020

Uh oh!

ulysses-you commented Sep 15, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants