[SPARK-37098][SQL] Alter table properties should invalidate cache #34365
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
cloud-fan
approved these changes
Oct 22, 2021
|
Test build #144535 has finished for PR 34365 at commit
|
viirya
approved these changes
Oct 22, 2021
sunchao
reviewed
Oct 22, 2021
| properties = table.properties ++ properties, | ||
| comment = properties.get(TableCatalog.PROP_COMMENT).orElse(table.comment)) | ||
| catalog.alterTable(newTable) | ||
| catalog.invalidateCachedTable(tableName) |
Member
There was a problem hiding this comment.
would it be better to do this inside SessionCatalog.alterTable?
Contributor
Author
There was a problem hiding this comment.
thank you @sunchao . I think it's a code improvement, and if we decide to do that, it should consider more methods include alterTableDataSchema, alterPartition, etc. So for now in this PR, I prefer do the simple fix which is straightforward.
Member
There was a problem hiding this comment.
Thanks. This looks good to me. I'm just curious whether other alter table variants have the same issue.
HyukjinKwon
approved these changes
Oct 23, 2021
yaooqinn
pushed a commit
that referenced
this pull request
Oct 25, 2021
### What changes were proposed in this pull request?
Invalidate the table cache after alter table properties (set and unset).
### Why are the changes needed?
The table properties can change the behavior of wriing. e.g. the parquet table with `parquet.compression`.
If you execute the following SQL, we will get the file with snappy compression rather than zstd.
```
CREATE TABLE t (c int) STORED AS PARQUET;
// cache table metadata
SELECT * FROM t;
ALTER TABLE t SET TBLPROPERTIES('parquet.compression'='zstd');
INSERT INTO TABLE t values(1);
```
So we should invalidate the table cache after alter table properties.
### Does this PR introduce _any_ user-facing change?
yes, bug fix
### How was this patch tested?
Add test
Closes #34365 from ulysses-you/SPARK-37098.
Authored-by: ulysses-you <[email protected]>
Signed-off-by: Kent Yao <[email protected]>
(cherry picked from commit 02d3b3b)
Signed-off-by: Kent Yao <[email protected]>
yaooqinn
pushed a commit
that referenced
this pull request
Oct 25, 2021
### What changes were proposed in this pull request?
Invalidate the table cache after alter table properties (set and unset).
### Why are the changes needed?
The table properties can change the behavior of wriing. e.g. the parquet table with `parquet.compression`.
If you execute the following SQL, we will get the file with snappy compression rather than zstd.
```
CREATE TABLE t (c int) STORED AS PARQUET;
// cache table metadata
SELECT * FROM t;
ALTER TABLE t SET TBLPROPERTIES('parquet.compression'='zstd');
INSERT INTO TABLE t values(1);
```
So we should invalidate the table cache after alter table properties.
### Does this PR introduce _any_ user-facing change?
yes, bug fix
### How was this patch tested?
Add test
Closes #34365 from ulysses-you/SPARK-37098.
Authored-by: ulysses-you <[email protected]>
Signed-off-by: Kent Yao <[email protected]>
(cherry picked from commit 02d3b3b)
Signed-off-by: Kent Yao <[email protected]>
Member
|
thanks, merged to master/3.2/3.1. |
Member
|
It conflicts 3.0, would you please backport it @ulysses-you ? |
Contributor
Author
|
thank you all, created #34379 |
Member
|
+1, LGTM. Thank you, @ulysses-you and all. |
Contributor
This pr can't be merged into branch-3.1 directly too, there is an error below: in |
Member
|
@zzcclp Thanks for reporting this. I created 05aea97 to revert it from branch-3.1. @ulysses-you would you please also send a PR target 3.1 again? |
Contributor
Author
|
created #34390 for banchr-3.1 |
dongjoon-hyun
pushed a commit
that referenced
this pull request
Oct 26, 2021
This PR backport #34365 to branch-3.1 ### What changes were proposed in this pull request? Invalidate the table cache after alter table properties (set and unset). ### Why are the changes needed? The table properties can change the behavior of wriing. e.g. the parquet table with `parquet.compression`. If you execute the following SQL, we will get the file with snappy compression rather than zstd. ``` CREATE TABLE t (c int) STORED AS PARQUET; // cache table metadata SELECT * FROM t; ALTER TABLE t SET TBLPROPERTIES('parquet.compression'='zstd'); INSERT INTO TABLE t values(1); ``` So we should invalidate the table cache after alter table properties. ### Does this PR introduce _any_ user-facing change? yes, bug fix ### How was this patch tested? Add test Closes #34390 from ulysses-you/SOARK-37098-3.1. Authored-by: ulysses-you <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
dongjoon-hyun
pushed a commit
that referenced
this pull request
Oct 26, 2021
This PR backport #34365 to branch-3.0 ### What changes were proposed in this pull request? Invalidate the table cache after alter table properties (set and unset). ### Why are the changes needed? The table properties can change the behavior of wriing. e.g. the parquet table with `parquet.compression`. If you execute the following SQL, we will get the file with snappy compression rather than zstd. ``` CREATE TABLE t (c int) STORED AS PARQUET; // cache table metadata SELECT * FROM t; ALTER TABLE t SET TBLPROPERTIES('parquet.compression'='zstd'); INSERT INTO TABLE t values(1); ``` So we should invalidate the table cache after alter table properties. ### Does this PR introduce _any_ user-facing change? yes, bug fix ### How was this patch tested? Add test Closes #34379 from ulysses-you/SPARK-37098-3.0. Authored-by: ulysses-you <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
sunchao
pushed a commit
to sunchao/spark
that referenced
this pull request
Dec 8, 2021
### What changes were proposed in this pull request?
Invalidate the table cache after alter table properties (set and unset).
### Why are the changes needed?
The table properties can change the behavior of wriing. e.g. the parquet table with `parquet.compression`.
If you execute the following SQL, we will get the file with snappy compression rather than zstd.
```
CREATE TABLE t (c int) STORED AS PARQUET;
// cache table metadata
SELECT * FROM t;
ALTER TABLE t SET TBLPROPERTIES('parquet.compression'='zstd');
INSERT INTO TABLE t values(1);
```
So we should invalidate the table cache after alter table properties.
### Does this PR introduce _any_ user-facing change?
yes, bug fix
### How was this patch tested?
Add test
Closes apache#34365 from ulysses-you/SPARK-37098.
Authored-by: ulysses-you <[email protected]>
Signed-off-by: Kent Yao <[email protected]>
(cherry picked from commit 02d3b3b)
Signed-off-by: Kent Yao <[email protected]>
(cherry picked from commit b7948ee)
fishcus
pushed a commit
to fishcus/spark
that referenced
this pull request
Jan 12, 2022
This PR backport apache#34365 to branch-3.1 ### What changes were proposed in this pull request? Invalidate the table cache after alter table properties (set and unset). ### Why are the changes needed? The table properties can change the behavior of wriing. e.g. the parquet table with `parquet.compression`. If you execute the following SQL, we will get the file with snappy compression rather than zstd. ``` CREATE TABLE t (c int) STORED AS PARQUET; // cache table metadata SELECT * FROM t; ALTER TABLE t SET TBLPROPERTIES('parquet.compression'='zstd'); INSERT INTO TABLE t values(1); ``` So we should invalidate the table cache after alter table properties. ### Does this PR introduce _any_ user-facing change? yes, bug fix ### How was this patch tested? Add test Closes apache#34390 from ulysses-you/SOARK-37098-3.1. Authored-by: ulysses-you <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
catalinii
pushed a commit
to lyft/spark
that referenced
this pull request
Feb 22, 2022
### What changes were proposed in this pull request?
Invalidate the table cache after alter table properties (set and unset).
### Why are the changes needed?
The table properties can change the behavior of wriing. e.g. the parquet table with `parquet.compression`.
If you execute the following SQL, we will get the file with snappy compression rather than zstd.
```
CREATE TABLE t (c int) STORED AS PARQUET;
// cache table metadata
SELECT * FROM t;
ALTER TABLE t SET TBLPROPERTIES('parquet.compression'='zstd');
INSERT INTO TABLE t values(1);
```
So we should invalidate the table cache after alter table properties.
### Does this PR introduce _any_ user-facing change?
yes, bug fix
### How was this patch tested?
Add test
Closes apache#34365 from ulysses-you/SPARK-37098.
Authored-by: ulysses-you <[email protected]>
Signed-off-by: Kent Yao <[email protected]>
(cherry picked from commit 02d3b3b)
Signed-off-by: Kent Yao <[email protected]>
catalinii
pushed a commit
to lyft/spark
that referenced
this pull request
Mar 4, 2022
### What changes were proposed in this pull request?
Invalidate the table cache after alter table properties (set and unset).
### Why are the changes needed?
The table properties can change the behavior of wriing. e.g. the parquet table with `parquet.compression`.
If you execute the following SQL, we will get the file with snappy compression rather than zstd.
```
CREATE TABLE t (c int) STORED AS PARQUET;
// cache table metadata
SELECT * FROM t;
ALTER TABLE t SET TBLPROPERTIES('parquet.compression'='zstd');
INSERT INTO TABLE t values(1);
```
So we should invalidate the table cache after alter table properties.
### Does this PR introduce _any_ user-facing change?
yes, bug fix
### How was this patch tested?
Add test
Closes apache#34365 from ulysses-you/SPARK-37098.
Authored-by: ulysses-you <[email protected]>
Signed-off-by: Kent Yao <[email protected]>
(cherry picked from commit 02d3b3b)
Signed-off-by: Kent Yao <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Invalidate the table cache after alter table properties (set and unset).
Why are the changes needed?
The table properties can change the behavior of wriing. e.g. the parquet table with
parquet.compression.If you execute the following SQL, we will get the file with snappy compression rather than zstd.
So we should invalidate the table cache after alter table properties.
Does this PR introduce any user-facing change?
yes, bug fix
How was this patch tested?
Add test