[SPARK-39258][TESTS] Fix `Hide credentials in show create table` #36637

LuciferYang · 2022-05-23T15:42:16Z

What changes were proposed in this pull request?

SPARK-35378-FOLLOWUP changes the return value of CommandResultExec.executeCollect() from InternalRow to UnsafeRow, this change causes the result of r.tostring in the following code:

spark/sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala

Lines 1143 to 1148 in de73753

    
           val show = ShowCreateTableCommand(TableIdentifier(tableName), ShowCreateTable.getoutputAttrs) 
        
           spark.sessionState.executePlan(show).executedPlan.executeCollect().foreach { r => 
        
             assert(!r.toString.contains(password)) 
        
             assert(r.toString.contains(dbTable)) 
        
             assert(r.toString.contains(userName)) 
        
           }

change from

[CREATE TABLE tab1 (
  NAME STRING,
  THEID INT)
USING org.apache.spark.sql.jdbc
OPTIONS (
  'dbtable' = 'TEST.PEOPLE',
  'password' = '*********(redacted)',
  'url' = '*********(redacted)',
  'user' = 'testUser')
]

to

[0,10000000d5,5420455441455243,62617420454c4241,414e20200a282031,4e4952545320454d,45485420200a2c47,a29544e49204449,726f20474e495355,6568636170612e67,732e6b726170732e,a6362646a2e6c71,20534e4f4954504f,7462642720200a28,203d2027656c6261,45502e5453455427,200a2c27454c504f,6f77737361702720,2a27203d20276472,2a2a2a2a2a2a2a2a,6574636164657228,2720200a2c272964,27203d20276c7275,2a2a2a2a2a2a2a2a,746361646572282a,20200a2c27296465,3d20277265737527,7355747365742720,a29277265]

and the UT JDBCSuite$Hide credentials in show create table failed in master branch.

This pr is change to use executeCollectPublic() instead of executeCollect() to fix this UT.

Why are the changes needed?

Fix UT failed in mater branch after SPARK-35378-FOLLOWUP

Does this PR introduce any user-facing change?

NO.

How was this patch tested?

GitHub Action pass
Manual test

Run mvn clean install -DskipTests -pl sql/core -am -Dtest=none -DwildcardSuites=org.apache.spark.sql.jdbc.JDBCSuite

Before

- Hide credentials in show create table *** FAILED ***
  "[0,10000000d5,5420455441455243,62617420454c4241,414e20200a282031,4e4952545320454d,45485420200a2c47,a29544e49204449,726f20474e495355,6568636170612e67,732e6b726170732e,a6362646a2e6c71,20534e4f4954504f,7462642720200a28,203d2027656c6261,45502e5453455427,200a2c27454c504f,6f77737361702720,2a27203d20276472,2a2a2a2a2a2a2a2a,6574636164657228,2720200a2c272964,27203d20276c7275,2a2a2a2a2a2a2a2a,746361646572282a,20200a2c27296465,3d20277265737527,7355747365742720,a29277265]" did not contain "TEST.PEOPLE" (JDBCSuite.scala:1146)

After

Run completed in 24 seconds, 868 milliseconds.
Total number of tests run: 93
Suites: completed 2, aborted 0
Tests: succeeded 93, failed 0, canceled 0, ignored 0, pending 0
All tests passed.

LuciferYang · 2022-05-23T16:08:44Z

cc @srowen

LuciferYang · 2022-05-23T16:09:39Z

also ping @cloud-fan for fix master UT

srowen · 2022-05-23T16:12:58Z

The original change went into 3.2, but do we need this only in master (3.4) or also 3.3?

LuciferYang · 2022-05-23T16:16:33Z

The original change went into 3.2, but do we need this only in master (3.4) or also 3.3?

Let me check this

LuciferYang · 2022-05-23T16:19:48Z

The original change went into 3.2, but do we need this only in master (3.4) or also 3.3?

3.3 and 3.2 also need this pr

LuciferYang · 2022-05-23T16:22:28Z

https://github.com/apache/spark/commits/branch-3.2

https://github.com/apache/spark/commits/branch-3.3

Hide credentials in show create table also failed in branch-3.3 and branch-3.2

cloud-fan · 2022-05-23T16:23:47Z

Somehow I thought that PR passed tests and merged it... Probably I opened too many tabs for PRs at the same time and messed them up. Sorry about it and thanks for the fix!

LuciferYang · 2022-05-23T16:34:41Z

This is something I ought to do, haha :)

dongjoon-hyun

+1, LGTM. Thank you for the fix.

However, @LuciferYang , please use PR link instead of JIRA link to point the problematic commit in the PR description. The bug is not originated from the original SPARK-39258, isn't it?

LuciferYang · 2022-05-23T17:26:58Z

+1, LGTM. Thank you for the fix.

However, @LuciferYang , please use PR link instead of JIRA link to point the problematic commit in the PR description. The bug is not originated from the original SPARK-39258, isn't it?

Use the pr link now

dongjoon-hyun · 2022-05-23T21:27:31Z

PySpark GitHub Action job is still running over 5h 40minute. It's irrelevant to this test PR. I'll merge this PR.

### What changes were proposed in this pull request? [SPARK-35378-FOLLOWUP](#36632) changes the return value of `CommandResultExec.executeCollect()` from `InternalRow` to `UnsafeRow`, this change causes the result of `r.tostring` in the following code: https://github.com/apache/spark/blob/de73753bb2e5fd947f237e731ff05aa9f2711677/sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala#L1143-L1148 change from ``` [CREATE TABLE tab1 ( NAME STRING, THEID INT) USING org.apache.spark.sql.jdbc OPTIONS ( 'dbtable' = 'TEST.PEOPLE', 'password' = '*********(redacted)', 'url' = '*********(redacted)', 'user' = 'testUser') ] ``` to ``` [0,10000000d5,5420455441455243,62617420454c4241,414e20200a282031,4e4952545320454d,45485420200a2c47,a29544e49204449,726f20474e495355,6568636170612e67,732e6b726170732e,a6362646a2e6c71,20534e4f4954504f,7462642720200a28,203d2027656c6261,45502e5453455427,200a2c27454c504f,6f77737361702720,2a27203d20276472,2a2a2a2a2a2a2a2a,6574636164657228,2720200a2c272964,27203d20276c7275,2a2a2a2a2a2a2a2a,746361646572282a,20200a2c27296465,3d20277265737527,7355747365742720,a29277265] ``` and the UT `JDBCSuite$Hide credentials in show create table` failed in master branch. This pr is change to use `executeCollectPublic()` instead of `executeCollect()` to fix this UT. ### Why are the changes needed? Fix UT failed in mater branch after [SPARK-35378-FOLLOWUP](#36632) ### Does this PR introduce _any_ user-facing change? NO. ### How was this patch tested? - GitHub Action pass - Manual test Run `mvn clean install -DskipTests -pl sql/core -am -Dtest=none -DwildcardSuites=org.apache.spark.sql.jdbc.JDBCSuite` **Before** ``` - Hide credentials in show create table *** FAILED *** "[0,10000000d5,5420455441455243,62617420454c4241,414e20200a282031,4e4952545320454d,45485420200a2c47,a29544e49204449,726f20474e495355,6568636170612e67,732e6b726170732e,a6362646a2e6c71,20534e4f4954504f,7462642720200a28,203d2027656c6261,45502e5453455427,200a2c27454c504f,6f77737361702720,2a27203d20276472,2a2a2a2a2a2a2a2a,6574636164657228,2720200a2c272964,27203d20276c7275,2a2a2a2a2a2a2a2a,746361646572282a,20200a2c27296465,3d20277265737527,7355747365742720,a29277265]" did not contain "TEST.PEOPLE" (JDBCSuite.scala:1146) ``` **After** ``` Run completed in 24 seconds, 868 milliseconds. Total number of tests run: 93 Suites: completed 2, aborted 0 Tests: succeeded 93, failed 0, canceled 0, ignored 0, pending 0 All tests passed. ``` Closes #36637 from LuciferYang/SPARK-39258. Authored-by: yangjie01 <yangjie01@baidu.com> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit 6eb15d1) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

dongjoon-hyun · 2022-05-23T21:29:16Z

Merged to master/3.3/3.2.

cc @MaxGekk

LuciferYang · 2022-05-24T01:21:54Z

thanks all ~

### What changes were proposed in this pull request? [SPARK-35378-FOLLOWUP](apache#36632) changes the return value of `CommandResultExec.executeCollect()` from `InternalRow` to `UnsafeRow`, this change causes the result of `r.tostring` in the following code: https://github.com/apache/spark/blob/de73753bb2e5fd947f237e731ff05aa9f2711677/sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala#L1143-L1148 change from ``` [CREATE TABLE tab1 ( NAME STRING, THEID INT) USING org.apache.spark.sql.jdbc OPTIONS ( 'dbtable' = 'TEST.PEOPLE', 'password' = '*********(redacted)', 'url' = '*********(redacted)', 'user' = 'testUser') ] ``` to ``` [0,10000000d5,5420455441455243,62617420454c4241,414e20200a282031,4e4952545320454d,45485420200a2c47,a29544e49204449,726f20474e495355,6568636170612e67,732e6b726170732e,a6362646a2e6c71,20534e4f4954504f,7462642720200a28,203d2027656c6261,45502e5453455427,200a2c27454c504f,6f77737361702720,2a27203d20276472,2a2a2a2a2a2a2a2a,6574636164657228,2720200a2c272964,27203d20276c7275,2a2a2a2a2a2a2a2a,746361646572282a,20200a2c27296465,3d20277265737527,7355747365742720,a29277265] ``` and the UT `JDBCSuite$Hide credentials in show create table` failed in master branch. This pr is change to use `executeCollectPublic()` instead of `executeCollect()` to fix this UT. ### Why are the changes needed? Fix UT failed in mater branch after [SPARK-35378-FOLLOWUP](apache#36632) ### Does this PR introduce _any_ user-facing change? NO. ### How was this patch tested? - GitHub Action pass - Manual test Run `mvn clean install -DskipTests -pl sql/core -am -Dtest=none -DwildcardSuites=org.apache.spark.sql.jdbc.JDBCSuite` **Before** ``` - Hide credentials in show create table *** FAILED *** "[0,10000000d5,5420455441455243,62617420454c4241,414e20200a282031,4e4952545320454d,45485420200a2c47,a29544e49204449,726f20474e495355,6568636170612e67,732e6b726170732e,a6362646a2e6c71,20534e4f4954504f,7462642720200a28,203d2027656c6261,45502e5453455427,200a2c27454c504f,6f77737361702720,2a27203d20276472,2a2a2a2a2a2a2a2a,6574636164657228,2720200a2c272964,27203d20276c7275,2a2a2a2a2a2a2a2a,746361646572282a,20200a2c27296465,3d20277265737527,7355747365742720,a29277265]" did not contain "TEST.PEOPLE" (JDBCSuite.scala:1146) ``` **After** ``` Run completed in 24 seconds, 868 milliseconds. Total number of tests run: 93 Suites: completed 2, aborted 0 Tests: succeeded 93, failed 0, canceled 0, ignored 0, pending 0 All tests passed. ``` Closes apache#36637 from LuciferYang/SPARK-39258. Authored-by: yangjie01 <yangjie01@baidu.com> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit 6eb15d1) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

fix ut

ff9ad1d

github-actions bot added the SQL label May 23, 2022

LuciferYang mentioned this pull request May 23, 2022

[SPARK-35378][SQL][FOLLOW-UP] Fix incorrect return type in CommandResultExec.executeCollect() #36632

Closed

srowen approved these changes May 23, 2022

View reviewed changes

cloud-fan approved these changes May 23, 2022

View reviewed changes

dongjoon-hyun approved these changes May 23, 2022

View reviewed changes

LuciferYang changed the title ~~[SPARK-39258][TESTS] Fix Hide credentials in show create table after SPARK-35378~~ [SPARK-39258][TESTS] Fix Hide credentials in show create table after SPARK-35378-FOLLOWUP May 23, 2022

LuciferYang changed the title ~~[SPARK-39258][TESTS] Fix Hide credentials in show create table after SPARK-35378-FOLLOWUP~~ [SPARK-39258][TESTS] Fix Hide credentials in show create table May 23, 2022

yaooqinn approved these changes May 23, 2022

View reviewed changes

dongjoon-hyun closed this in 6eb15d1 May 23, 2022

dongjoon-hyun mentioned this pull request May 24, 2022

[SPARK-39250][BUILD] Upgrade Jackson to 2.13.3 #36627

Closed

zzzzming95 mentioned this pull request May 21, 2023

[SPARK-43327][CORE][3.3] Trigger committer.setupJob before plan execute in FileFormatWriter#write #41154

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-39258][TESTS] Fix `Hide credentials in show create table` #36637

[SPARK-39258][TESTS] Fix `Hide credentials in show create table` #36637

LuciferYang commented May 23, 2022 •

edited by dongjoon-hyun

Loading

LuciferYang commented May 23, 2022

LuciferYang commented May 23, 2022

srowen commented May 23, 2022

LuciferYang commented May 23, 2022

LuciferYang commented May 23, 2022

LuciferYang commented May 23, 2022 •

edited

Loading

cloud-fan commented May 23, 2022 •

edited

Loading

LuciferYang commented May 23, 2022

dongjoon-hyun left a comment

LuciferYang commented May 23, 2022

dongjoon-hyun commented May 23, 2022

dongjoon-hyun commented May 23, 2022 •

edited

Loading

LuciferYang commented May 24, 2022

	val show = ShowCreateTableCommand(TableIdentifier(tableName), ShowCreateTable.getoutputAttrs)
	spark.sessionState.executePlan(show).executedPlan.executeCollect().foreach { r =>
	assert(!r.toString.contains(password))
	assert(r.toString.contains(dbTable))
	assert(r.toString.contains(userName))
	}

[SPARK-39258][TESTS] Fix Hide credentials in show create table #36637

[SPARK-39258][TESTS] Fix Hide credentials in show create table #36637

Conversation

LuciferYang commented May 23, 2022 • edited by dongjoon-hyun Loading

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?

LuciferYang commented May 23, 2022

LuciferYang commented May 23, 2022

srowen commented May 23, 2022

LuciferYang commented May 23, 2022

LuciferYang commented May 23, 2022

LuciferYang commented May 23, 2022 • edited Loading

cloud-fan commented May 23, 2022 • edited Loading

LuciferYang commented May 23, 2022

dongjoon-hyun left a comment

Choose a reason for hiding this comment

LuciferYang commented May 23, 2022

dongjoon-hyun commented May 23, 2022

dongjoon-hyun commented May 23, 2022 • edited Loading

LuciferYang commented May 24, 2022

[SPARK-39258][TESTS] Fix `Hide credentials in show create table` #36637

[SPARK-39258][TESTS] Fix `Hide credentials in show create table` #36637

LuciferYang commented May 23, 2022 •

edited by dongjoon-hyun

Loading

LuciferYang commented May 23, 2022 •

edited

Loading

cloud-fan commented May 23, 2022 •

edited

Loading

dongjoon-hyun commented May 23, 2022 •

edited

Loading