You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/08/26 23:36:29 UTC

[GitHub] [iceberg] sumeetgajjar opened a new pull request, #5645: Update drop table behavior in spark-ddl docs

sumeetgajjar opened a new pull request, #5645:
URL: https://github.com/apache/iceberg/pull/5645

   #3056 added support for `PURGE` flag to Spark.
   However, the corresponding `DROP TABLE` doc is outdated.
   This PR aims at documenting the updated behavior of the `DROP TABLE` DDL command.


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] sumeetgajjar commented on a diff in pull request #5645: [Docs] Update drop table behavior in spark-ddl docs

Posted by GitBox <gi...@apache.org>.
sumeetgajjar commented on code in PR #5645:
URL: https://github.com/apache/iceberg/pull/5645#discussion_r973492567


##########
docs/spark-ddl.md:
##########
@@ -132,12 +132,28 @@ The new table properties in the `REPLACE TABLE` command will be merged with any
 
 ## `DROP TABLE`
 
-To delete a table, run:
+The drop table behavior changed in 0.14.
+
+Prior to 0.14, running `DROP TABLE` would remove the table from the catalog and delete the table contents as well.
+
+From 0.14 onwards, `DROP TABLE` would only remove the table from the catalog.
+In order to delete the table contents `DROP TABLE PURGE` should be used.

Review Comment:
   I believe it is implied that if an operation involves removing any table files then `gc.enabled` should be true.
   With that said, I didn't find an occurrence of `gc.enabled` anywhere in the iceberg spark docs. Thus decided to follow the same pattern.



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] pvary commented on pull request #5645: [Docs] Update drop table behavior in spark-ddl docs

Posted by GitBox <gi...@apache.org>.
pvary commented on PR #5645:
URL: https://github.com/apache/iceberg/pull/5645#issuecomment-1263893581

   Thanks for the fix @sumeetgajjar and @wypoon and @samredai for the review!


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] samredai commented on a diff in pull request #5645: [Docs] Update drop table behavior in spark-ddl docs

Posted by GitBox <gi...@apache.org>.
samredai commented on code in PR #5645:
URL: https://github.com/apache/iceberg/pull/5645#discussion_r973539659


##########
docs/spark-ddl.md:
##########
@@ -132,12 +132,28 @@ The new table properties in the `REPLACE TABLE` command will be merged with any
 
 ## `DROP TABLE`
 
-To delete a table, run:
+The drop table behavior changed in 0.14.
+
+Prior to 0.14, running `DROP TABLE` would remove the table from the catalog and delete the table contents as well.
+
+From 0.14 onwards, `DROP TABLE` would only remove the table from the catalog.
+In order to delete the table contents `DROP TABLE PURGE` should be used.
+
+### `DROP TABLE`
+
+To drop the table from the catalog, run:
 
 ```sql
 DROP TABLE prod.db.sample
 ```
 
+### `DROP TABLE PURGE`
+
+To delete a table, run:

Review Comment:
   To make the distinction a bit clearer with `DROP TABLE`, how about rewording this to:
   
   > To drop the table from the catalog and delete the table's contents, run:



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] sumeetgajjar commented on pull request #5645: [Docs] Update drop table behavior in spark-ddl docs

Posted by GitBox <gi...@apache.org>.
sumeetgajjar commented on PR #5645:
URL: https://github.com/apache/iceberg/pull/5645#issuecomment-1232267477

   A gentle ping @Fokko @samredai 


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] pvary merged pull request #5645: [Docs] Update drop table behavior in spark-ddl docs

Posted by GitBox <gi...@apache.org>.
pvary merged PR #5645:
URL: https://github.com/apache/iceberg/pull/5645


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] sumeetgajjar commented on pull request #5645: [Docs] Update drop table behavior in spark-ddl docs

Posted by GitBox <gi...@apache.org>.
sumeetgajjar commented on PR #5645:
URL: https://github.com/apache/iceberg/pull/5645#issuecomment-1263897515

   Thanks @pvary for the prompt response!!!
   Should we merge the cherry-pick of this fix to the 0.14 branch as well https://github.com/apache/iceberg/pull/5647?


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] sumeetgajjar commented on a diff in pull request #5645: [Docs] Update drop table behavior in spark-ddl docs

Posted by GitBox <gi...@apache.org>.
sumeetgajjar commented on code in PR #5645:
URL: https://github.com/apache/iceberg/pull/5645#discussion_r973540360


##########
docs/spark-ddl.md:
##########
@@ -132,12 +132,28 @@ The new table properties in the `REPLACE TABLE` command will be merged with any
 
 ## `DROP TABLE`
 
-To delete a table, run:
+The drop table behavior changed in 0.14.
+
+Prior to 0.14, running `DROP TABLE` would remove the table from the catalog and delete the table contents as well.
+
+From 0.14 onwards, `DROP TABLE` would only remove the table from the catalog.
+In order to delete the table contents `DROP TABLE PURGE` should be used.
+
+### `DROP TABLE`
+
+To drop the table from the catalog, run:
 
 ```sql
 DROP TABLE prod.db.sample
 ```
 
+### `DROP TABLE PURGE`
+
+To delete a table, run:

Review Comment:
   Done.



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] sumeetgajjar commented on pull request #5645: [Docs] Update drop table behavior in spark-ddl docs

Posted by GitBox <gi...@apache.org>.
sumeetgajjar commented on PR #5645:
URL: https://github.com/apache/iceberg/pull/5645#issuecomment-1263875434

   @pvary @flyrain @Fokko 
   Since this PR is already reviewed, can you please help us merge this it?


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] wypoon commented on a diff in pull request #5645: [Docs] Update drop table behavior in spark-ddl docs

Posted by GitBox <gi...@apache.org>.
wypoon commented on code in PR #5645:
URL: https://github.com/apache/iceberg/pull/5645#discussion_r973468131


##########
docs/spark-ddl.md:
##########
@@ -132,12 +132,28 @@ The new table properties in the `REPLACE TABLE` command will be merged with any
 
 ## `DROP TABLE`
 
-To delete a table, run:
+The drop table behavior changed in 0.14.
+
+Prior to 0.14, running `DROP TABLE` would remove the table from the catalog and delete the table contents as well.
+
+From 0.14 onwards, `DROP TABLE` would only remove the table from the catalog.
+In order to delete the table contents `DROP TABLE PURGE` should be used.

Review Comment:
   Is it worth mentioning that gc.enabled has to be true for the contents to be deleted?



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] sumeetgajjar commented on a diff in pull request #5645: [Docs] Update drop table behavior in spark-ddl docs

Posted by GitBox <gi...@apache.org>.
sumeetgajjar commented on code in PR #5645:
URL: https://github.com/apache/iceberg/pull/5645#discussion_r973540516


##########
docs/spark-ddl.md:
##########
@@ -132,12 +132,28 @@ The new table properties in the `REPLACE TABLE` command will be merged with any
 
 ## `DROP TABLE`
 
-To delete a table, run:
+The drop table behavior changed in 0.14.
+
+Prior to 0.14, running `DROP TABLE` would remove the table from the catalog and delete the table contents as well.
+
+From 0.14 onwards, `DROP TABLE` would only remove the table from the catalog.
+In order to delete the table contents `DROP TABLE PURGE` should be used.
+
+### `DROP TABLE`
+
+To drop the table from the catalog, run:
 
 ```sql
 DROP TABLE prod.db.sample
 ```
 
+### `DROP TABLE PURGE`
+
+To delete a table, run:

Review Comment:
   Hi @samredai, thank you for your comment, took care of it in the latest commit.



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] sumeetgajjar commented on pull request #5645: [Docs] Update drop table behavior in spark-ddl docs

Posted by GitBox <gi...@apache.org>.
sumeetgajjar commented on PR #5645:
URL: https://github.com/apache/iceberg/pull/5645#issuecomment-1250000732

   Hi @wypoon @samredai, thank you for your comments and review on this PR.
   can you also review the exact same change for the 0.14 branch: https://github.com/apache/iceberg/pull/5647?


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org