Skip to content

[SPARK-24166][SQL] InMemoryTableScanExec should not access SQLConf at executor side - #21223

Closed
cloud-fan wants to merge 1 commit into
apache:masterfrom
cloud-fan:minor1
Closed

[SPARK-24166][SQL] InMemoryTableScanExec should not access SQLConf at executor side#21223
cloud-fan wants to merge 1 commit into
apache:masterfrom
cloud-fan:minor1

Conversation

@cloud-fan

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

This PR is extracted from #21190 , to make it easier to backport.

InMemoryTableScanExec#createAndDecompressColumn is executed inside rdd.map, we can't access conf.offHeapColumnVectorEnabled there.

How was this patch tested?

it's tested in #21190

@cloud-fan

Copy link
Copy Markdown
ContributorAuthor

cc @kiszk@viirya

@kiszk

kiszk commented May 3, 2018

Copy link
Copy Markdown
Member

LGTM

@dongjoon-hyundongjoon-hyun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1, LGTM.

@SparkQA

Copy link
Copy Markdown

Test build #90095 has finished for PR 21223 at commit d900b4c.

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

@cloud-fan

Copy link
Copy Markdown
ContributorAuthor

retest this please

@viiryaviirya left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@SparkQA

Copy link
Copy Markdown

Test build #90107 has finished for PR 21223 at commit d900b4c.

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

@adrian-wangadrian-wang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1 LGTM

asfgit pushed a commit that referenced this pull request May 3, 2018
… executor side
## What changes were proposed in this pull request?
This PR is extracted from #21190 , to make it easier to backport.
`InMemoryTableScanExec#createAndDecompressColumn` is executed inside `rdd.map`, we can't access `conf.offHeapColumnVectorEnabled` there.
## How was this patch tested?
it's tested in #21190
Author: Wenchen Fan <wenchen@databricks.com>
Closes#21223 from cloud-fan/minor1.
(cherry picked from commit 991b526)
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
@asfgitasfgit closed this in 991b526May 3, 2018
@cloud-fan

Copy link
Copy Markdown
ContributorAuthor

thanks, merging to master/2.3!

zzcclp pushed a commit to zzcclp/spark that referenced this pull request May 11, 2018
… on the driver
## What changes were proposed in this pull request?
This is a followup of apache#20136 . apache#20136 didn't really work because in the test, we are using local backend, which shares the driver side `SparkEnv`, so `SparkEnv.get.executorId == SparkContext.DRIVER_IDENTIFIER` doesn't work.
This PR changes the check to `TaskContext.get != null`, and move the check to `SQLConf.get`, and fix all the places that violate this check:
* `InMemoryTableScanExec#createAndDecompressColumn` is executed inside `rdd.map`, we can't access `conf.offHeapColumnVectorEnabled` there. apache#21223 merged
* `DataType#sameType` may be executed in executor side, for things like json schema inference, so we can't call `conf.caseSensitiveAnalysis` there. This contributes to most of the code changes, as we need to add `caseSensitive` parameter to a lot of methods.
* `ParquetFilters` is used in the file scan function, which is executed in executor side, so we can't can't call `conf.parquetFilterPushDownDate` there. apache#21224 merged
* `WindowExec#createBoundOrdering` is called on executor side, so we can't use `conf.sessionLocalTimezone` there. apache#21225 merged
* `JsonToStructs` can be serialized to executors and evaluate, we should not call `SQLConf.get.getConf(SQLConf.FROM_JSON_FORCE_NULLABLE_SCHEMA)` in the body. apache#21226 merged
## How was this patch tested?
existing test
Author: Wenchen Fan <wenchen@databricks.com>
Closesapache#21190 from cloud-fan/minor.
robert3005 pushed a commit to palantir/spark that referenced this pull request Jun 24, 2018
… on the driver
## What changes were proposed in this pull request?
This is a followup of apache#20136 . apache#20136 didn't really work because in the test, we are using local backend, which shares the driver side `SparkEnv`, so `SparkEnv.get.executorId == SparkContext.DRIVER_IDENTIFIER` doesn't work.
This PR changes the check to `TaskContext.get != null`, and move the check to `SQLConf.get`, and fix all the places that violate this check:
* `InMemoryTableScanExec#createAndDecompressColumn` is executed inside `rdd.map`, we can't access `conf.offHeapColumnVectorEnabled` there. apache#21223 merged
* `DataType#sameType` may be executed in executor side, for things like json schema inference, so we can't call `conf.caseSensitiveAnalysis` there. This contributes to most of the code changes, as we need to add `caseSensitive` parameter to a lot of methods.
* `ParquetFilters` is used in the file scan function, which is executed in executor side, so we can't can't call `conf.parquetFilterPushDownDate` there. apache#21224 merged
* `WindowExec#createBoundOrdering` is called on executor side, so we can't use `conf.sessionLocalTimezone` there. apache#21225 merged
* `JsonToStructs` can be serialized to executors and evaluate, we should not call `SQLConf.get.getConf(SQLConf.FROM_JSON_FORCE_NULLABLE_SCHEMA)` in the body. apache#21226 merged
## How was this patch tested?
existing test
Author: Wenchen Fan <wenchen@databricks.com>
Closesapache#21190 from cloud-fan/minor.
Sign up for freeto 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

@cloud-fan@kiszk@SparkQA@viirya@adrian-wang@dongjoon-hyun