Copilot commented on code in PR #12549:
URL: https://github.com/apache/gluten/pull/12549#discussion_r3948521688
##########
gluten-core/src/main/scala/org/apache/gluten/config/GlutenCoreConfig.scala:
##########
@@ -39,7 +39,7 @@ class GlutenCoreConfig(conf: SQLConf) extends Logging {
def offHeapMemorySize: Long = getConf(COLUMNAR_OFFHEAP_SIZE_IN_BYTES)
- def taskOffHeapMemorySize: Long =
getConf(COLUMNAR_TASK_OFFHEAP_SIZE_IN_BYTES)
+ def taskOffHeapMemorySize: Long =
getConf(COLUMNAR_TASK_OFFHEAP_SIZE_IN_BYTES).getOrElse(0L)
Review Comment:
This changes the JVM-facing semantics of `taskOffHeapMemorySize` when the
conf is unset: returning `0L` can be interpreted as ‘zero memory available’
rather than ‘unbounded/unknown’, which is explicitly called out elsewhere as an
unsafe placeholder. Prefer returning `Option[Long]` from the accessor
(propagating the optionality), or use a clearly documented sentinel consistent
with existing call sites (e.g., `Long.MaxValue`) so callers can preserve the
prior ‘absent means no bound / defer to defaults’ behavior.
##########
gluten-substrait/src/main/scala/org/apache/gluten/backendsapi/BackendSettingsApi.scala:
##########
@@ -170,16 +170,4 @@ trait BackendSettingsApi {
Review Comment:
Removing `extraNativeSessionConfKeys()` / `extraNativeBackendConfKeys()`
from `BackendSettingsApi` is a source/binary breaking change for any
downstream/out-of-tree backends implementing this trait. Consider keeping the
methods with default implementations and marking them deprecated (with a
pointer to the new `Component.confs()` + `passToNative()` mechanism), then
removing them in a later major/minor release.
##########
cpp/core/jni/JniWrapper.cc:
##########
@@ -845,6 +844,22 @@
Java_org_apache_gluten_vectorized_LocalPartitionWriterJniWrapper_createPartition
auto dataFile = jStringToCString(env, dataFileJstr);
auto localDirs = splitPaths(jStringToCString(env, localDirsJstr));
+ // `spark.shuffle.file.buffer` is declared with `bytesConf(ByteUnit.KiB)` on
the JVM side, matching
+ // Spark's own declaration, so the delivered value is a KiB count. Convert
it to bytes here, which
+ // is the unit every reader of `shuffleFileBufferSize` uses.
+ auto shuffleFileBufferSize = kDefaultShuffleFileBufferSize;
+ auto& conf = ctx->getConfMap();
+ if (auto it = conf.find(kShuffleFileBufferSize); it != conf.end()) {
+ try {
+ shuffleFileBufferSize = std::stoll(it->second) * 1024;
+ } catch (const std::exception&) {
+ // A malformed value should not fail shuffle writer creation when a sane
native default is at
+ // hand. Without this, `JNI_METHOD_END` would turn it into a
`GlutenException` and take the
+ // query down over a buffer size.
+ shuffleFileBufferSize = kDefaultShuffleFileBufferSize;
+ }
+ }
Review Comment:
The `std::stoll(...)*1024` conversion can overflow `long long` for
sufficiently large inputs, which is not caught by the current exception handler
(overflow on multiplication is UB and won’t throw). Consider guarding the
multiplication (e.g., checked multiply against `LLONG_MAX/1024`) and falling
back to `kDefaultShuffleFileBufferSize` (or clamping) when it would overflow.
##########
gluten-substrait/src/main/scala/org/apache/gluten/config/GlutenConfig.scala:
##########
@@ -489,110 +490,154 @@ object GlutenConfig extends ConfigRegistry {
def prefixOf(backendName: String): String =
s"spark.gluten.sql.columnar.backend.$backendName"
def prefixSessionOf(backendName: String): String =
s"spark.gluten.$backendName"
- private lazy val nativeKeys = Set(
- DEBUG_ENABLED.key,
- BENCHMARK_SAVE_DIR.key,
- GlutenCoreConfig.COLUMNAR_TASK_OFFHEAP_SIZE_IN_BYTES.key,
- COLUMNAR_MAX_BATCH_SIZE.key,
- SHUFFLE_WRITER_BUFFER_SIZE.key,
- COLUMNAR_CUDF_ENABLED.key,
- SQLConf.LEGACY_SIZE_OF_NULL.key,
- SQLConf.LEGACY_STATISTICAL_AGGREGATE.key,
- SQLConf.JSON_GENERATOR_IGNORE_NULL_FIELDS.key,
- SQLConf.RUNTIME_BLOOM_FILTER_EXPECTED_NUM_ITEMS.key,
- SQLConf.RUNTIME_BLOOM_FILTER_NUM_BITS.key,
- SQLConf.RUNTIME_BLOOM_FILTER_MAX_NUM_BITS.key,
- SQLConf.RUNTIME_BLOOM_FILTER_MAX_NUM_ITEMS.key,
- "spark.io.compression.codec",
- "spark.sql.decimalOperations.allowPrecisionLoss",
- "spark.sql.legacy.parquet.returnNullStructIfAllFieldsMissing",
- // s3 config
- SPARK_S3_ACCESS_KEY,
- SPARK_S3_SECRET_KEY,
- SPARK_S3_ENDPOINT,
- SPARK_S3_CONNECTION_SSL_ENABLED,
- SPARK_S3_PATH_STYLE_ACCESS,
- SPARK_S3_USE_INSTANCE_CREDENTIALS,
- SPARK_S3_IAM,
- SPARK_S3_IAM_SESSION_NAME,
- SPARK_S3_RETRY_MAX_ATTEMPTS,
- SPARK_S3_CONNECTION_MAXIMUM,
- SPARK_S3_ENDPOINT_REGION,
- SPARK_S3_AWS_IMDS_ENABLED,
- "spark.gluten.velox.fs.s3a.retry.mode",
- "spark.gluten.velox.awsSdkLogLevel",
- "spark.gluten.velox.s3UseProxyFromEnv",
- "spark.gluten.velox.s3PayloadSigningPolicy",
- "spark.gluten.velox.s3LogLocation",
- // gcs config
- SPARK_GCS_STORAGE_ROOT_URL,
- SPARK_GCS_AUTH_TYPE,
- SPARK_GCS_AUTH_SERVICE_ACCOUNT_JSON_KEYFILE,
- SPARK_REDACTION_REGEX,
- "spark.gluten.sql.columnar.backend.velox.queryTraceEnabled",
- "spark.gluten.sql.columnar.backend.velox.queryTraceDir",
- "spark.gluten.sql.columnar.backend.velox.queryTraceNodeIds",
- "spark.gluten.sql.columnar.backend.velox.queryTraceMaxBytes",
- "spark.gluten.sql.columnar.backend.velox.queryTraceTaskRegExp",
- "spark.gluten.sql.columnar.backend.velox.opTraceDirectoryCreateConfig",
- "spark.gluten.sql.columnar.backend.velox.enableUserExceptionStacktrace",
- "spark.gluten.sql.columnar.backend.velox.enableSystemExceptionStacktrace",
- "spark.gluten.sql.columnar.backend.velox.memoryUseHugePages",
- "spark.gluten.sql.columnar.backend.velox.cachePrefetchMinPct",
-
"spark.gluten.sql.columnar.backend.velox.memoryPoolCapacityTransferAcrossTasks",
- "spark.gluten.sql.columnar.backend.velox.preferredBatchBytes",
- "spark.gluten.sql.columnar.backend.velox.cudf.enableTableScan",
-
"spark.gluten.sql.columnar.backend.velox.columnarBatchSerializerCompression"
- )
+ // Declarations of non-Gluten configurations (Spark SQL / Spark core /
Hadoop keys that have no
+ // Gluten ConfigEntry) to be passed to native side. `registerConf` /
`registerStaticConf` declare
+ // only the native delivery: the key stays owned by Spark / Hadoop, so
nothing is registered as a
+ // Gluten config entry or to SQLConf. Gluten's own configurations declare
native passing via
+ // `ConfigBuilder.passToNative` at their definitions instead.
+ private def registerNativeConfs(): Unit = {
+ // Force GlutenCoreConfig's object initialization, so that its own
`passToNative`
+ // registrations are in place before native confs are selected.
+ GlutenCoreConfig.ensureRegistered()
+
+ // Spark SQL confs read by native. All of these rely on native's own
fallback matching Spark's
+ // default, so nothing is delivered when the key is unset - see
`ConfigBuilder.passToNative`.
+ // `spark.sql.legacy.sizeOfNull` is `passToNative()` for documentation
purposes only: it is
+ // never read from the conf map, since the value is baked as a substrait
literal at plan
+ // conversion (see `ExpressionConverter`).
+
registerConf(SQLConf.LEGACY_SIZE_OF_NULL.key).stringConf.passToNative().createOptional
Review Comment:
Several Spark SQL keys that are natively boolean-like (e.g.
`CASE_SENSITIVE`, `IGNORE_MISSING_FILES`, `ANSI_ENABLED`,
`DECIMAL_OPERATIONS_ALLOW_PREC_LOSS`) are registered as `stringConf`. This
bypasses the typed converter path that normalizes user input (e.g.
case-insensitive booleans) and can lead to delivering unexpected raw strings to
native. Register these with the matching typed builders (`booleanConf` /
`intConf` / etc.) where the Spark/Hadoop semantics are known, so values are
normalized consistently before being passed to native.
##########
gluten-substrait/src/main/scala/org/apache/gluten/config/GlutenConfig.scala:
##########
@@ -489,110 +490,154 @@ object GlutenConfig extends ConfigRegistry {
def prefixOf(backendName: String): String =
s"spark.gluten.sql.columnar.backend.$backendName"
def prefixSessionOf(backendName: String): String =
s"spark.gluten.$backendName"
- private lazy val nativeKeys = Set(
- DEBUG_ENABLED.key,
- BENCHMARK_SAVE_DIR.key,
- GlutenCoreConfig.COLUMNAR_TASK_OFFHEAP_SIZE_IN_BYTES.key,
- COLUMNAR_MAX_BATCH_SIZE.key,
- SHUFFLE_WRITER_BUFFER_SIZE.key,
- COLUMNAR_CUDF_ENABLED.key,
- SQLConf.LEGACY_SIZE_OF_NULL.key,
- SQLConf.LEGACY_STATISTICAL_AGGREGATE.key,
- SQLConf.JSON_GENERATOR_IGNORE_NULL_FIELDS.key,
- SQLConf.RUNTIME_BLOOM_FILTER_EXPECTED_NUM_ITEMS.key,
- SQLConf.RUNTIME_BLOOM_FILTER_NUM_BITS.key,
- SQLConf.RUNTIME_BLOOM_FILTER_MAX_NUM_BITS.key,
- SQLConf.RUNTIME_BLOOM_FILTER_MAX_NUM_ITEMS.key,
- "spark.io.compression.codec",
- "spark.sql.decimalOperations.allowPrecisionLoss",
- "spark.sql.legacy.parquet.returnNullStructIfAllFieldsMissing",
- // s3 config
- SPARK_S3_ACCESS_KEY,
- SPARK_S3_SECRET_KEY,
- SPARK_S3_ENDPOINT,
- SPARK_S3_CONNECTION_SSL_ENABLED,
- SPARK_S3_PATH_STYLE_ACCESS,
- SPARK_S3_USE_INSTANCE_CREDENTIALS,
- SPARK_S3_IAM,
- SPARK_S3_IAM_SESSION_NAME,
- SPARK_S3_RETRY_MAX_ATTEMPTS,
- SPARK_S3_CONNECTION_MAXIMUM,
- SPARK_S3_ENDPOINT_REGION,
- SPARK_S3_AWS_IMDS_ENABLED,
- "spark.gluten.velox.fs.s3a.retry.mode",
- "spark.gluten.velox.awsSdkLogLevel",
- "spark.gluten.velox.s3UseProxyFromEnv",
- "spark.gluten.velox.s3PayloadSigningPolicy",
- "spark.gluten.velox.s3LogLocation",
- // gcs config
- SPARK_GCS_STORAGE_ROOT_URL,
- SPARK_GCS_AUTH_TYPE,
- SPARK_GCS_AUTH_SERVICE_ACCOUNT_JSON_KEYFILE,
- SPARK_REDACTION_REGEX,
- "spark.gluten.sql.columnar.backend.velox.queryTraceEnabled",
- "spark.gluten.sql.columnar.backend.velox.queryTraceDir",
- "spark.gluten.sql.columnar.backend.velox.queryTraceNodeIds",
- "spark.gluten.sql.columnar.backend.velox.queryTraceMaxBytes",
- "spark.gluten.sql.columnar.backend.velox.queryTraceTaskRegExp",
- "spark.gluten.sql.columnar.backend.velox.opTraceDirectoryCreateConfig",
- "spark.gluten.sql.columnar.backend.velox.enableUserExceptionStacktrace",
- "spark.gluten.sql.columnar.backend.velox.enableSystemExceptionStacktrace",
- "spark.gluten.sql.columnar.backend.velox.memoryUseHugePages",
- "spark.gluten.sql.columnar.backend.velox.cachePrefetchMinPct",
-
"spark.gluten.sql.columnar.backend.velox.memoryPoolCapacityTransferAcrossTasks",
- "spark.gluten.sql.columnar.backend.velox.preferredBatchBytes",
- "spark.gluten.sql.columnar.backend.velox.cudf.enableTableScan",
-
"spark.gluten.sql.columnar.backend.velox.columnarBatchSerializerCompression"
- )
+ // Declarations of non-Gluten configurations (Spark SQL / Spark core /
Hadoop keys that have no
+ // Gluten ConfigEntry) to be passed to native side. `registerConf` /
`registerStaticConf` declare
+ // only the native delivery: the key stays owned by Spark / Hadoop, so
nothing is registered as a
+ // Gluten config entry or to SQLConf. Gluten's own configurations declare
native passing via
+ // `ConfigBuilder.passToNative` at their definitions instead.
+ private def registerNativeConfs(): Unit = {
+ // Force GlutenCoreConfig's object initialization, so that its own
`passToNative`
+ // registrations are in place before native confs are selected.
+ GlutenCoreConfig.ensureRegistered()
+
+ // Spark SQL confs read by native. All of these rely on native's own
fallback matching Spark's
+ // default, so nothing is delivered when the key is unset - see
`ConfigBuilder.passToNative`.
+ // `spark.sql.legacy.sizeOfNull` is `passToNative()` for documentation
purposes only: it is
+ // never read from the conf map, since the value is baked as a substrait
literal at plan
+ // conversion (see `ExpressionConverter`).
+
registerConf(SQLConf.LEGACY_SIZE_OF_NULL.key).stringConf.passToNative().createOptional
+ // Read by `ConfigExtractor` as a bool with its own fallback of `true`,
matching Spark's
+ // default. A string literal because not every supported Spark version has
the entry.
+ registerConf("spark.sql.legacy.parquet.returnNullStructIfAllFieldsMissing")
+ .booleanConf
+ .passToNative()
+ .createOptional
+ registerConf(SQLConf.JSON_GENERATOR_IGNORE_NULL_FIELDS.key)
+ .stringConf
+ .passToNative()
+ .createOptional
+ registerConf(SQLConf.RUNTIME_BLOOM_FILTER_EXPECTED_NUM_ITEMS.key)
+ .stringConf
+ .passToNative()
+ .createOptional
+ registerConf(SQLConf.RUNTIME_BLOOM_FILTER_NUM_BITS.key)
+ .stringConf
+ .passToNative()
+ .createOptional
+ registerConf(SQLConf.RUNTIME_BLOOM_FILTER_MAX_NUM_BITS.key)
+ .stringConf
+ .passToNative()
+ .createOptional
+ registerConf(SQLConf.RUNTIME_BLOOM_FILTER_MAX_NUM_ITEMS.key)
+ .stringConf
Review Comment:
Several Spark SQL keys that are natively boolean-like (e.g.
`CASE_SENSITIVE`, `IGNORE_MISSING_FILES`, `ANSI_ENABLED`,
`DECIMAL_OPERATIONS_ALLOW_PREC_LOSS`) are registered as `stringConf`. This
bypasses the typed converter path that normalizes user input (e.g.
case-insensitive booleans) and can lead to delivering unexpected raw strings to
native. Register these with the matching typed builders (`booleanConf` /
`intConf` / etc.) where the Spark/Hadoop semantics are known, so values are
normalized consistently before being passed to native.
##########
gluten-substrait/src/main/scala/org/apache/gluten/config/GlutenConfig.scala:
##########
@@ -489,110 +490,154 @@ object GlutenConfig extends ConfigRegistry {
def prefixOf(backendName: String): String =
s"spark.gluten.sql.columnar.backend.$backendName"
def prefixSessionOf(backendName: String): String =
s"spark.gluten.$backendName"
- private lazy val nativeKeys = Set(
- DEBUG_ENABLED.key,
- BENCHMARK_SAVE_DIR.key,
- GlutenCoreConfig.COLUMNAR_TASK_OFFHEAP_SIZE_IN_BYTES.key,
- COLUMNAR_MAX_BATCH_SIZE.key,
- SHUFFLE_WRITER_BUFFER_SIZE.key,
- COLUMNAR_CUDF_ENABLED.key,
- SQLConf.LEGACY_SIZE_OF_NULL.key,
- SQLConf.LEGACY_STATISTICAL_AGGREGATE.key,
- SQLConf.JSON_GENERATOR_IGNORE_NULL_FIELDS.key,
- SQLConf.RUNTIME_BLOOM_FILTER_EXPECTED_NUM_ITEMS.key,
- SQLConf.RUNTIME_BLOOM_FILTER_NUM_BITS.key,
- SQLConf.RUNTIME_BLOOM_FILTER_MAX_NUM_BITS.key,
- SQLConf.RUNTIME_BLOOM_FILTER_MAX_NUM_ITEMS.key,
- "spark.io.compression.codec",
- "spark.sql.decimalOperations.allowPrecisionLoss",
- "spark.sql.legacy.parquet.returnNullStructIfAllFieldsMissing",
- // s3 config
- SPARK_S3_ACCESS_KEY,
- SPARK_S3_SECRET_KEY,
- SPARK_S3_ENDPOINT,
- SPARK_S3_CONNECTION_SSL_ENABLED,
- SPARK_S3_PATH_STYLE_ACCESS,
- SPARK_S3_USE_INSTANCE_CREDENTIALS,
- SPARK_S3_IAM,
- SPARK_S3_IAM_SESSION_NAME,
- SPARK_S3_RETRY_MAX_ATTEMPTS,
- SPARK_S3_CONNECTION_MAXIMUM,
- SPARK_S3_ENDPOINT_REGION,
- SPARK_S3_AWS_IMDS_ENABLED,
- "spark.gluten.velox.fs.s3a.retry.mode",
- "spark.gluten.velox.awsSdkLogLevel",
- "spark.gluten.velox.s3UseProxyFromEnv",
- "spark.gluten.velox.s3PayloadSigningPolicy",
- "spark.gluten.velox.s3LogLocation",
- // gcs config
- SPARK_GCS_STORAGE_ROOT_URL,
- SPARK_GCS_AUTH_TYPE,
- SPARK_GCS_AUTH_SERVICE_ACCOUNT_JSON_KEYFILE,
- SPARK_REDACTION_REGEX,
- "spark.gluten.sql.columnar.backend.velox.queryTraceEnabled",
- "spark.gluten.sql.columnar.backend.velox.queryTraceDir",
- "spark.gluten.sql.columnar.backend.velox.queryTraceNodeIds",
- "spark.gluten.sql.columnar.backend.velox.queryTraceMaxBytes",
- "spark.gluten.sql.columnar.backend.velox.queryTraceTaskRegExp",
- "spark.gluten.sql.columnar.backend.velox.opTraceDirectoryCreateConfig",
- "spark.gluten.sql.columnar.backend.velox.enableUserExceptionStacktrace",
- "spark.gluten.sql.columnar.backend.velox.enableSystemExceptionStacktrace",
- "spark.gluten.sql.columnar.backend.velox.memoryUseHugePages",
- "spark.gluten.sql.columnar.backend.velox.cachePrefetchMinPct",
-
"spark.gluten.sql.columnar.backend.velox.memoryPoolCapacityTransferAcrossTasks",
- "spark.gluten.sql.columnar.backend.velox.preferredBatchBytes",
- "spark.gluten.sql.columnar.backend.velox.cudf.enableTableScan",
-
"spark.gluten.sql.columnar.backend.velox.columnarBatchSerializerCompression"
- )
+ // Declarations of non-Gluten configurations (Spark SQL / Spark core /
Hadoop keys that have no
+ // Gluten ConfigEntry) to be passed to native side. `registerConf` /
`registerStaticConf` declare
+ // only the native delivery: the key stays owned by Spark / Hadoop, so
nothing is registered as a
+ // Gluten config entry or to SQLConf. Gluten's own configurations declare
native passing via
+ // `ConfigBuilder.passToNative` at their definitions instead.
+ private def registerNativeConfs(): Unit = {
+ // Force GlutenCoreConfig's object initialization, so that its own
`passToNative`
+ // registrations are in place before native confs are selected.
+ GlutenCoreConfig.ensureRegistered()
+
+ // Spark SQL confs read by native. All of these rely on native's own
fallback matching Spark's
+ // default, so nothing is delivered when the key is unset - see
`ConfigBuilder.passToNative`.
+ // `spark.sql.legacy.sizeOfNull` is `passToNative()` for documentation
purposes only: it is
+ // never read from the conf map, since the value is baked as a substrait
literal at plan
+ // conversion (see `ExpressionConverter`).
+
registerConf(SQLConf.LEGACY_SIZE_OF_NULL.key).stringConf.passToNative().createOptional
+ // Read by `ConfigExtractor` as a bool with its own fallback of `true`,
matching Spark's
+ // default. A string literal because not every supported Spark version has
the entry.
+ registerConf("spark.sql.legacy.parquet.returnNullStructIfAllFieldsMissing")
+ .booleanConf
+ .passToNative()
+ .createOptional
+ registerConf(SQLConf.JSON_GENERATOR_IGNORE_NULL_FIELDS.key)
+ .stringConf
+ .passToNative()
+ .createOptional
+ registerConf(SQLConf.RUNTIME_BLOOM_FILTER_EXPECTED_NUM_ITEMS.key)
+ .stringConf
+ .passToNative()
+ .createOptional
+ registerConf(SQLConf.RUNTIME_BLOOM_FILTER_NUM_BITS.key)
+ .stringConf
+ .passToNative()
+ .createOptional
+ registerConf(SQLConf.RUNTIME_BLOOM_FILTER_MAX_NUM_BITS.key)
+ .stringConf
+ .passToNative()
+ .createOptional
+ registerConf(SQLConf.RUNTIME_BLOOM_FILTER_MAX_NUM_ITEMS.key)
+ .stringConf
+ .passToNative()
+ .createOptional
+
registerConf(SPARK_IO_COMPRESSION_CODEC).stringConf.passToNative().createOptional
+ // Velox compares the value against upper-cased literals; ClickHouse
lower-cases it itself.
+ // Declaring `transform(toUpperCase)` mirrors Spark's own entry which also
upper-cases.
+ registerConf(SQLConf.LEGACY_TIME_PARSER_POLICY.key)
+ .stringConf
+ .transform(_.toUpperCase(Locale.ROOT))
+ .passToNative()
+ .createOptional
+
registerConf(SQLConf.CASE_SENSITIVE.key).stringConf.passToNative().createOptional
+
registerConf(SQLConf.IGNORE_MISSING_FILES.key).stringConf.passToNative().createOptional
+ registerConf(SQLConf.LEGACY_STATISTICAL_AGGREGATE.key)
+ .stringConf
+ .passToNative()
+ .createOptional
+ registerConf(SQLConf.DECIMAL_OPERATIONS_ALLOW_PREC_LOSS.key)
+ .stringConf
+ .passToNative()
+ .createOptional
Review Comment:
Several Spark SQL keys that are natively boolean-like (e.g.
`CASE_SENSITIVE`, `IGNORE_MISSING_FILES`, `ANSI_ENABLED`,
`DECIMAL_OPERATIONS_ALLOW_PREC_LOSS`) are registered as `stringConf`. This
bypasses the typed converter path that normalizes user input (e.g.
case-insensitive booleans) and can lead to delivering unexpected raw strings to
native. Register these with the matching typed builders (`booleanConf` /
`intConf` / etc.) where the Spark/Hadoop semantics are known, so values are
normalized consistently before being passed to native.
##########
gluten-substrait/src/main/scala/org/apache/gluten/config/GlutenConfig.scala:
##########
@@ -489,110 +490,154 @@ object GlutenConfig extends ConfigRegistry {
def prefixOf(backendName: String): String =
s"spark.gluten.sql.columnar.backend.$backendName"
def prefixSessionOf(backendName: String): String =
s"spark.gluten.$backendName"
- private lazy val nativeKeys = Set(
- DEBUG_ENABLED.key,
- BENCHMARK_SAVE_DIR.key,
- GlutenCoreConfig.COLUMNAR_TASK_OFFHEAP_SIZE_IN_BYTES.key,
- COLUMNAR_MAX_BATCH_SIZE.key,
- SHUFFLE_WRITER_BUFFER_SIZE.key,
- COLUMNAR_CUDF_ENABLED.key,
- SQLConf.LEGACY_SIZE_OF_NULL.key,
- SQLConf.LEGACY_STATISTICAL_AGGREGATE.key,
- SQLConf.JSON_GENERATOR_IGNORE_NULL_FIELDS.key,
- SQLConf.RUNTIME_BLOOM_FILTER_EXPECTED_NUM_ITEMS.key,
- SQLConf.RUNTIME_BLOOM_FILTER_NUM_BITS.key,
- SQLConf.RUNTIME_BLOOM_FILTER_MAX_NUM_BITS.key,
- SQLConf.RUNTIME_BLOOM_FILTER_MAX_NUM_ITEMS.key,
- "spark.io.compression.codec",
- "spark.sql.decimalOperations.allowPrecisionLoss",
- "spark.sql.legacy.parquet.returnNullStructIfAllFieldsMissing",
- // s3 config
- SPARK_S3_ACCESS_KEY,
- SPARK_S3_SECRET_KEY,
- SPARK_S3_ENDPOINT,
- SPARK_S3_CONNECTION_SSL_ENABLED,
- SPARK_S3_PATH_STYLE_ACCESS,
- SPARK_S3_USE_INSTANCE_CREDENTIALS,
- SPARK_S3_IAM,
- SPARK_S3_IAM_SESSION_NAME,
- SPARK_S3_RETRY_MAX_ATTEMPTS,
- SPARK_S3_CONNECTION_MAXIMUM,
- SPARK_S3_ENDPOINT_REGION,
- SPARK_S3_AWS_IMDS_ENABLED,
- "spark.gluten.velox.fs.s3a.retry.mode",
- "spark.gluten.velox.awsSdkLogLevel",
- "spark.gluten.velox.s3UseProxyFromEnv",
- "spark.gluten.velox.s3PayloadSigningPolicy",
- "spark.gluten.velox.s3LogLocation",
- // gcs config
- SPARK_GCS_STORAGE_ROOT_URL,
- SPARK_GCS_AUTH_TYPE,
- SPARK_GCS_AUTH_SERVICE_ACCOUNT_JSON_KEYFILE,
- SPARK_REDACTION_REGEX,
- "spark.gluten.sql.columnar.backend.velox.queryTraceEnabled",
- "spark.gluten.sql.columnar.backend.velox.queryTraceDir",
- "spark.gluten.sql.columnar.backend.velox.queryTraceNodeIds",
- "spark.gluten.sql.columnar.backend.velox.queryTraceMaxBytes",
- "spark.gluten.sql.columnar.backend.velox.queryTraceTaskRegExp",
- "spark.gluten.sql.columnar.backend.velox.opTraceDirectoryCreateConfig",
- "spark.gluten.sql.columnar.backend.velox.enableUserExceptionStacktrace",
- "spark.gluten.sql.columnar.backend.velox.enableSystemExceptionStacktrace",
- "spark.gluten.sql.columnar.backend.velox.memoryUseHugePages",
- "spark.gluten.sql.columnar.backend.velox.cachePrefetchMinPct",
-
"spark.gluten.sql.columnar.backend.velox.memoryPoolCapacityTransferAcrossTasks",
- "spark.gluten.sql.columnar.backend.velox.preferredBatchBytes",
- "spark.gluten.sql.columnar.backend.velox.cudf.enableTableScan",
-
"spark.gluten.sql.columnar.backend.velox.columnarBatchSerializerCompression"
- )
+ // Declarations of non-Gluten configurations (Spark SQL / Spark core /
Hadoop keys that have no
+ // Gluten ConfigEntry) to be passed to native side. `registerConf` /
`registerStaticConf` declare
+ // only the native delivery: the key stays owned by Spark / Hadoop, so
nothing is registered as a
+ // Gluten config entry or to SQLConf. Gluten's own configurations declare
native passing via
+ // `ConfigBuilder.passToNative` at their definitions instead.
+ private def registerNativeConfs(): Unit = {
+ // Force GlutenCoreConfig's object initialization, so that its own
`passToNative`
+ // registrations are in place before native confs are selected.
+ GlutenCoreConfig.ensureRegistered()
+
+ // Spark SQL confs read by native. All of these rely on native's own
fallback matching Spark's
+ // default, so nothing is delivered when the key is unset - see
`ConfigBuilder.passToNative`.
+ // `spark.sql.legacy.sizeOfNull` is `passToNative()` for documentation
purposes only: it is
+ // never read from the conf map, since the value is baked as a substrait
literal at plan
+ // conversion (see `ExpressionConverter`).
+
registerConf(SQLConf.LEGACY_SIZE_OF_NULL.key).stringConf.passToNative().createOptional
+ // Read by `ConfigExtractor` as a bool with its own fallback of `true`,
matching Spark's
+ // default. A string literal because not every supported Spark version has
the entry.
+ registerConf("spark.sql.legacy.parquet.returnNullStructIfAllFieldsMissing")
+ .booleanConf
+ .passToNative()
+ .createOptional
+ registerConf(SQLConf.JSON_GENERATOR_IGNORE_NULL_FIELDS.key)
+ .stringConf
+ .passToNative()
+ .createOptional
+ registerConf(SQLConf.RUNTIME_BLOOM_FILTER_EXPECTED_NUM_ITEMS.key)
+ .stringConf
+ .passToNative()
+ .createOptional
+ registerConf(SQLConf.RUNTIME_BLOOM_FILTER_NUM_BITS.key)
+ .stringConf
+ .passToNative()
+ .createOptional
+ registerConf(SQLConf.RUNTIME_BLOOM_FILTER_MAX_NUM_BITS.key)
+ .stringConf
+ .passToNative()
+ .createOptional
+ registerConf(SQLConf.RUNTIME_BLOOM_FILTER_MAX_NUM_ITEMS.key)
+ .stringConf
+ .passToNative()
+ .createOptional
+
registerConf(SPARK_IO_COMPRESSION_CODEC).stringConf.passToNative().createOptional
+ // Velox compares the value against upper-cased literals; ClickHouse
lower-cases it itself.
+ // Declaring `transform(toUpperCase)` mirrors Spark's own entry which also
upper-cases.
+ registerConf(SQLConf.LEGACY_TIME_PARSER_POLICY.key)
+ .stringConf
+ .transform(_.toUpperCase(Locale.ROOT))
+ .passToNative()
+ .createOptional
+
registerConf(SQLConf.CASE_SENSITIVE.key).stringConf.passToNative().createOptional
+
registerConf(SQLConf.IGNORE_MISSING_FILES.key).stringConf.passToNative().createOptional
Review Comment:
Several Spark SQL keys that are natively boolean-like (e.g.
`CASE_SENSITIVE`, `IGNORE_MISSING_FILES`, `ANSI_ENABLED`,
`DECIMAL_OPERATIONS_ALLOW_PREC_LOSS`) are registered as `stringConf`. This
bypasses the typed converter path that normalizes user input (e.g.
case-insensitive booleans) and can lead to delivering unexpected raw strings to
native. Register these with the matching typed builders (`booleanConf` /
`intConf` / etc.) where the Spark/Hadoop semantics are known, so values are
normalized consistently before being passed to native.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
