Skip to content

Conversation

@iRakson
Copy link
Contributor

@iRakson iRakson commented Jan 3, 2020

What changes were proposed in this pull request?

Document updated for CACHE TABLE & UNCACHE TABLE

Why are the changes needed?

Cache table creates a temp view while caching data using CACHE TABLE name AS query. UNCACHE TABLE does not remove this temp view.

These things were not mentioned in the existing doc for CACHE TABLE & UNCACHE TABLE.

Does this PR introduce any user-facing change?

Document updated for CACHE TABLE & UNCACHE TABLE command.

How was this patch tested?

Manually

@iRakson iRakson changed the title SPARK-27545 [SPARK-27545][SQL] Temporary view is not removed while uncaching the table Jan 3, 2020
@iRakson
Copy link
Contributor Author

iRakson commented Jan 3, 2020

cc @cloud-fan @HyukjinKwon

@cloud-fan
Copy link
Contributor

think about this example

create temp view t as select ...
cache table t
uncache table t

it's weird if we drop the temp view after uncache table.

@iRakson
Copy link
Contributor Author

iRakson commented Jan 3, 2020

think about this example

create temp view t as select ...
cache table t
uncache table t

it's weird if we drop the temp view after uncache table.

Yeah. Here its not wise to drop the temp view after uncache table. I did not take into account the possibility when user creates a view of their own.
But Cache table do create temp view which should be removed when we are uncaching the table.

@cloud-fan
Copy link
Contributor

maybe we should clearly document that CACHE TABLE t1 as SELECT * FROM some_table; means create a temp view and cache it. cc @viirya @dongjoon-hyun

@viirya
Copy link
Member

viirya commented Jan 4, 2020

maybe we should clearly document that CACHE TABLE t1 as SELECT * FROM some_table; means create a temp view and cache it.

As shown above, UNCACHE TABLE t where t could be created by CACHE TABLE AS query or a temp view created separately. I think it is hard to make UNCACHE TABLE command know which one is its target.

Although UNCACHE TABLE cannot remove temp view if it is created by CACHE TABLE AS query, it seems not a serious issue. When trying to create another temp view with the same name, an error will be seen.

Documenting it is a good idea.


### Description
`CACHE TABLE` statement caches contents of a table or output of a query with the given storage level. This reduces scanning of the original files in future queries.
`CACHE TABLE` statement caches contents of a table or output of a query with the given storage level. It creates a temporary view with
Copy link
Contributor

Choose a reason for hiding this comment

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

If a query is cached, then a temp view will be created for this query.

### Description
`UNCACHE TABLE` removes the entries and associated data from the in-memory and/or on-disk cache for a given table or view. The
underlying entries should already have been brought to cache by previous `CACHE TABLE` operation. `UNCACHE TABLE` on a non-existent table throws Exception if `IF EXISTS` is not specified.
underlying entries should already have been brought to cache by previous `CACHE TABLE` operation. `UNCACHE TABLE` do not remove
Copy link
Contributor

Choose a reason for hiding this comment

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

does not remove

`UNCACHE TABLE` removes the entries and associated data from the in-memory and/or on-disk cache for a given table or view. The
underlying entries should already have been brought to cache by previous `CACHE TABLE` operation. `UNCACHE TABLE` on a non-existent table throws Exception if `IF EXISTS` is not specified.
underlying entries should already have been brought to cache by previous `CACHE TABLE` operation. `UNCACHE TABLE` do not remove
the temporary view if it is created by `CACHE TABLE AS query`. `UNCACHE TABLE` on a non-existent table throws Exception if `IF EXISTS`
Copy link
Contributor

Choose a reason for hiding this comment

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

CACHE TABLE name AS query

@iRakson iRakson requested a review from cloud-fan January 6, 2020 08:18
@cloud-fan
Copy link
Contributor

can we update PR title and description as well?

@iRakson iRakson changed the title [SPARK-27545][SQL] Temporary view is not removed while uncaching the table [SPARK-27545][SQL][DOC] Update the Documentation for CACHE TABLE and UNCACHE TABLE Jan 6, 2020
@iRakson
Copy link
Contributor Author

iRakson commented Jan 6, 2020

can we update PR title and description as well?

Done.

@cloud-fan
Copy link
Contributor

ok to test

@SparkQA
Copy link

SparkQA commented Jan 6, 2020

Test build #116161 has finished for PR 27090 at commit 56bc5af.

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

`UNCACHE TABLE` removes the entries and associated data from the in-memory and/or on-disk cache for a given table or view. The
underlying entries should already have been brought to cache by previous `CACHE TABLE` operation. `UNCACHE TABLE` on a non-existent table throws Exception if `IF EXISTS` is not specified.
underlying entries should already have been brought to cache by previous `CACHE TABLE` operation. `UNCACHE TABLE` does not remove
the temporary view if it is created by `CACHE TABLE name AS query`. `UNCACHE TABLE` on a non-existent table throws Exception if `IF EXISTS`
Copy link
Member

Choose a reason for hiding this comment

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

UNCACHE TABLE does not remove the temporary view if it is created by CACHE TABLE name AS query -> it sounds like it will remove a temporary view that is not created by CACHE TABLE name AS query.

Or just say UNCACHE TABLE does not remove a temporary view?

`UNCACHE TABLE` removes the entries and associated data from the in-memory and/or on-disk cache for a given table or view. The
underlying entries should already have been brought to cache by previous `CACHE TABLE` operation. `UNCACHE TABLE` on a non-existent table throws Exception if `IF EXISTS` is not specified.
underlying entries should already have been brought to cache by previous `CACHE TABLE` operation. `UNCACHE TABLE` does not remove
the temporary view if it is created by `CACHE TABLE name AS query`. `UNCACHE TABLE` on a non-existent table throws Exception if `IF EXISTS`
Copy link
Member

Choose a reason for hiding this comment

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

Exception -> an exception

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@iRakson iRakson requested review from HyukjinKwon and viirya January 7, 2020 05:31
@SparkQA
Copy link

SparkQA commented Jan 7, 2020

Test build #116214 has finished for PR 27090 at commit b842d4f.

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

@iRakson
Copy link
Contributor Author

iRakson commented Jan 13, 2020

@HyukjinKwon Any more changes required for this one?

@iRakson
Copy link
Contributor Author

iRakson commented Feb 11, 2020

gentle ping @cloud-fan @HyukjinKwon

`UNCACHE TABLE` removes the entries and associated data from the in-memory and/or on-disk cache for a given table or view. The
underlying entries should already have been brought to cache by previous `CACHE TABLE` operation. `UNCACHE TABLE` on a non-existent table throws Exception if `IF EXISTS` is not specified.
underlying entries should already have been brought to cache by previous `CACHE TABLE` operation. `UNCACHE TABLE` does not remove
a temporary view. `UNCACHE TABLE` on a non-existent table throws an exception if `IF EXISTS` is not specified.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to highlight UNCACHE TABLE does not remove a temporary view.? It's intuitive that UNCACHE won't drop temp view.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. Make sense. I will update the PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Please review @cloud-fan

@iRakson iRakson requested a review from cloud-fan February 11, 2020 12:02
@SparkQA
Copy link

SparkQA commented Feb 11, 2020

Test build #118235 has finished for PR 27090 at commit f7bca7b.

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

@cloud-fan
Copy link
Contributor

thanks, merging to master/3.0!

@cloud-fan cloud-fan closed this in b20754d Feb 11, 2020
cloud-fan pushed a commit that referenced this pull request Feb 11, 2020
…UNCACHE TABLE

### What changes were proposed in this pull request?
Document updated for `CACHE TABLE` & `UNCACHE TABLE`

### Why are the changes needed?
Cache table creates a temp view while caching data using `CACHE TABLE name AS query`. `UNCACHE TABLE` does not remove this temp view.

These things were not mentioned in the existing doc for `CACHE TABLE` & `UNCACHE TABLE`.

### Does this PR introduce any user-facing change?
Document updated for `CACHE TABLE` & `UNCACHE TABLE` command.

### How was this patch tested?
Manually

Closes #27090 from iRakson/SPARK-27545.

Lead-authored-by: root1 <raksonrakesh@gmail.com>
Co-authored-by: iRakson <raksonrakesh@gmail.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
(cherry picked from commit b20754d)
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
@cloud-fan
Copy link
Contributor

@iRakson can you leave a message in the JIRA ticket so that I can assign it to you?

sjincho pushed a commit to sjincho/spark that referenced this pull request Apr 15, 2020
…UNCACHE TABLE

### What changes were proposed in this pull request?
Document updated for `CACHE TABLE` & `UNCACHE TABLE`

### Why are the changes needed?
Cache table creates a temp view while caching data using `CACHE TABLE name AS query`. `UNCACHE TABLE` does not remove this temp view.

These things were not mentioned in the existing doc for `CACHE TABLE` & `UNCACHE TABLE`.

### Does this PR introduce any user-facing change?
Document updated for `CACHE TABLE` & `UNCACHE TABLE` command.

### How was this patch tested?
Manually

Closes apache#27090 from iRakson/SPARK-27545.

Lead-authored-by: root1 <raksonrakesh@gmail.com>
Co-authored-by: iRakson <raksonrakesh@gmail.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants