Skip to content

Commit 1ba967b

Browse files
vinodkcgatorsmile
authored andcommitted
[SPARK-21588][SQL] SQLContext.getConf(key, null) should return null
## What changes were proposed in this pull request? In SQLContext.get(key,null) for a key that is not defined in the conf, and doesn't have a default value defined, throws a NPE. Int happens only when conf has a value converter Added null check on defaultValue inside SQLConf.getConfString to avoid calling entry.valueConverter(defaultValue) ## How was this patch tested? Added unit test Author: vinodkc <[email protected]> Closes #18852 from vinodkc/br_Fix_SPARK-21588.
1 parent 990efad commit 1ba967b

File tree

2 files changed

+17
-4
lines changed

2 files changed

+17
-4
lines changed

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1228,10 +1228,12 @@ class SQLConf extends Serializable with Logging {
12281228
* not set yet, return `defaultValue`.
12291229
*/
12301230
def getConfString(key: String, defaultValue: String): String = {
1231-
val entry = sqlConfEntries.get(key)
1232-
if (entry != null && defaultValue != "<undefined>") {
1233-
// Only verify configs in the SQLConf object
1234-
entry.valueConverter(defaultValue)
1231+
if (defaultValue != null && defaultValue != "<undefined>") {
1232+
val entry = sqlConfEntries.get(key)
1233+
if (entry != null) {
1234+
// Only verify configs in the SQLConf object
1235+
entry.valueConverter(defaultValue)
1236+
}
12351237
}
12361238
Option(settings.get(key)).getOrElse(defaultValue)
12371239
}

sql/core/src/test/scala/org/apache/spark/sql/internal/SQLConfSuite.scala

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,4 +270,15 @@ class SQLConfSuite extends QueryTest with SharedSQLContext {
270270
val e2 = intercept[AnalysisException](spark.conf.unset(SCHEMA_STRING_LENGTH_THRESHOLD.key))
271271
assert(e2.message.contains("Cannot modify the value of a static config"))
272272
}
273+
274+
test("SPARK-21588 SQLContext.getConf(key, null) should return null") {
275+
withSQLConf(SQLConf.SHUFFLE_PARTITIONS.key -> "1") {
276+
assert("1" == spark.conf.get(SQLConf.SHUFFLE_PARTITIONS.key, null))
277+
assert("1" == spark.conf.get(SQLConf.SHUFFLE_PARTITIONS.key, "<undefined>"))
278+
}
279+
280+
assert(spark.conf.getOption("spark.sql.nonexistent").isEmpty)
281+
assert(null == spark.conf.get("spark.sql.nonexistent", null))
282+
assert("<undefined>" == spark.conf.get("spark.sql.nonexistent", "<undefined>"))
283+
}
273284
}

0 commit comments

Comments
 (0)