Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-48946][SQL] Fix redact method to avoid NPE when session is null #47419

Closed
wants to merge 1 commit into from

Conversation

mikoszilard
Copy link
Contributor

@mikoszilard mikoszilard commented Jul 19, 2024

What changes were proposed in this pull request?

If we call DataSourceV2ScanExecBase redact method from a thread that don't have a session in thread local we get an NPE. Getting stringRedactionPattern from conf could prevent this problem as conf checks if session is null or not. We also use this in DataSourceScanExec trait.
https://github.com/apache/spark/blob/master/sql/core/src/main/scala/org/apache/spark/sql/execution/DataSourceScanExec.scala#L93-L95

Why are the changes needed?

To prevent NPE when session is null.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Existing UTs.

Was this patch authored or co-authored using generative AI tooling?

No

@github-actions github-actions bot added the SQL label Jul 19, 2024
Copy link
Contributor

@peter-toth peter-toth left a comment

Choose a reason for hiding this comment

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

I think this is kind of a follow-up to my #32947.

Looks good, but can you please adjust the title to describe what the PR does and not the issue?

Copy link
Contributor

@attilapiros attilapiros left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, LGTM. Thank you, @mikoszilard and all.

@dongjoon-hyun
Copy link
Member

Merged to master for Apache Saprk 4.0.0-preview2.

Welcome to the Apache Spark community, @mikoszilard.
I added you to the Apache Spark contributor group and assigned SPARK-48946 to you.

@mikoszilard
Copy link
Contributor Author

Thank you very much @dongjoon-hyun. I'm really happy that I could start contributing.

@mikoszilard mikoszilard changed the title [SPARK-48946][SQL] NPE in redact method when session is null [SPARK-48946][SQL] Fix redact method to avoid NPE when session is null Jul 22, 2024
jingz-db pushed a commit to jingz-db/spark that referenced this pull request Jul 22, 2024
### What changes were proposed in this pull request?

If we call DataSourceV2ScanExecBase redact method from a thread that don't have a session in thread local we get an NPE. Getting stringRedactionPattern from conf could prevent this problem as conf checks if session is null or not. We also use this in DataSourceScanExec trait.
https://github.com/apache/spark/blob/master/sql/core/src/main/scala/org/apache/spark/sql/execution/DataSourceScanExec.scala#L93-L95

### Why are the changes needed?

To prevent NPE when session is null.

### Does this PR introduce _any_ user-facing change?

No

### How was this patch tested?

Existing UTs.

### Was this patch authored or co-authored using generative AI tooling?

No

Closes apache#47419 from mikoszilard/SPARK-48946.

Authored-by: Szilard Miko <smiko@cloudera.com>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
attilapiros pushed a commit to attilapiros/spark that referenced this pull request Oct 4, 2024
### What changes were proposed in this pull request?

If we call DataSourceV2ScanExecBase redact method from a thread that don't have a session in thread local we get an NPE. Getting stringRedactionPattern from conf could prevent this problem as conf checks if session is null or not. We also use this in DataSourceScanExec trait.
https://github.com/apache/spark/blob/master/sql/core/src/main/scala/org/apache/spark/sql/execution/DataSourceScanExec.scala#L93-L95

### Why are the changes needed?

To prevent NPE when session is null.

### Does this PR introduce _any_ user-facing change?

No

### How was this patch tested?

Existing UTs.

### Was this patch authored or co-authored using generative AI tooling?

No

Closes apache#47419 from mikoszilard/SPARK-48946.

Authored-by: Szilard Miko <smiko@cloudera.com>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants