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/05/11 03:51:51 UTC

[GitHub] [iceberg] felixYyu opened a new pull request, #4746: Docs: Add syntax description for deleting empty tables

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

   Close #4735 
   
   When `DROP TABLE prod.db.sample` Non empty table, the table directory can be deleted at the same time, but when the table is empty, the table directory left after being dropped, it shoule use `DROP TABLE prod.db.sample PURGE`,But the description is missing in spark-ddl/#drop-table.


-- 
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] kbendick commented on a diff in pull request #4746: Docs: Add syntax description for deleting empty tables

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


##########
docs/spark/spark-ddl.md:
##########
@@ -134,6 +134,11 @@ To delete a table, run:
 DROP TABLE prod.db.sample
 ```
 
+To delete an empty table use `PURGE`, run:
+
+```sql
+DROP TABLE prod.db.sample PURGE
+```

Review Comment:
   Nit: I understand what you mean, but i think the saying "to delete an empty table" might be somewhat confusing for relatively new users.
   
   How about something like the following?
   
   ```
   To  delete a table as well as its underlying data, use the `PURGE` flag. 
   
   The `PURGE` flag can also be used to remove an empty table from its corresponding catalog if the table's metadata files have been removed by some other process.
   ```
   
   Not sure the best wording for the second statement .  The second statement might fit better somewhere else if we're only addressing the edge case of a table's metadata files being deleted by some other process.
   
   But please correct me if we're concerned with more than that edge case for "an empty table".
   
   Tagging @samredai for his thoughts.



-- 
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] kbendick commented on a diff in pull request #4746: Docs: Add syntax description for deleting empty tables

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


##########
docs/spark/spark-ddl.md:
##########
@@ -134,6 +134,11 @@ To delete a table, run:
 DROP TABLE prod.db.sample
 ```
 
+To delete an empty table use `PURGE`, run:
+
+```sql
+DROP TABLE prod.db.sample PURGE
+```

Review Comment:
   Nit: I understand what you mean, but i think the saying "to delete an empty table" might be somewhat confusing for relatively new users.
   
   How about something like the following?
   
   ```
   To delete a table and its underlying data, use the `PURGE` flag. 
   
   The `PURGE` flag can also be used to remove an empty table from its corresponding catalog if the table's metadata files have been removed by some other process and cannot be recovered.
   ```
   
   Not sure the best wording for the second statement .  The second statement might fit better somewhere else if we're only addressing the edge case of a table's metadata files being deleted by some other process.
   
   But please correct me if we're concerned with more than that edge case for "an empty table".
   
   Tagging @samredai for his thoughts.



-- 
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 #4746: Docs: Add syntax description for deleting empty tables

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


##########
docs/spark/spark-ddl.md:
##########
@@ -134,6 +134,11 @@ To delete a table, run:
 DROP TABLE prod.db.sample
 ```
 
+To delete an empty table use `PURGE`, run:
+
+```sql
+DROP TABLE prod.db.sample PURGE
+```

Review Comment:
   I like that wording. I think the additional statement could go in a hint box though. What do ya'll think of this?
   
   > To delete a table and its underlying data, use the `PURGE keyword.
   > ```sql
   > DROP TABLE prod.db.sample PURGE
   > ```
   > {{< hint info >}}
   > The `PURGE` flag can also be used to remove an empty table from its corresponding catalog if the table's metadata files have been removed by some other process and cannot be recovered.
   > {{< /hint >}}



-- 
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] felixYyu commented on a diff in pull request #4746: Docs: To delete a table and its underlying data, use the `PURGE` keyword

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


##########
docs/spark/spark-ddl.md:
##########
@@ -134,6 +134,11 @@ To delete a table, run:
 DROP TABLE prod.db.sample
 ```
 
+To delete an empty table use `PURGE`, run:
+
+```sql
+DROP TABLE prod.db.sample PURGE
+```

Review Comment:
   +1, fixed,thanks @samredai @kbendick 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] felixYyu commented on pull request #4746: Docs: Add syntax description for deleting empty tables

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

   ref #4698


-- 
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] kbendick commented on a diff in pull request #4746: Docs: Add syntax description for deleting empty tables

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


##########
docs/spark/spark-ddl.md:
##########
@@ -134,6 +134,11 @@ To delete a table, run:
 DROP TABLE prod.db.sample
 ```
 
+To delete an empty table use `PURGE`, run:
+
+```sql
+DROP TABLE prod.db.sample PURGE
+```

Review Comment:
   Nit: I understand what you mean, but i think the saying "to delete an empty table" might be somewhat confusiong for relatively new users.
   
   We should try to provide a little bit more context on the problem you're mentioning, as well as simply document `PURGE`.
   
   How about something like the following?
   
   ```
   To  delete a table as well as its underlying data, use the `PURGE` flag. 
   
   The `PURGE` flag can also be used to remove an empty table from its corresponding catalog if the table's metadata files have been removed by some other process.
   ```
   
   Not sure the best wording for the second statement (or if the second statement is most relevant here).
   
   Tagging @samredai for his thoughts.



-- 
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