feat: add copy_grants to write_pandas #2050
Open
Conversation
This file contains 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
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
sfc-gh-aalam
requested changes
Sep 17, 2024
Comment on lines
-447
to
+467
| drop_object(original_table_location, "table") | ||
| rename_table_sql = f"ALTER TABLE {target_table_location} RENAME TO {original_table_location} /* Python:snowflake.connector.pandas_tools.write_pandas() */" | ||
| logger.debug(f"rename table with '{rename_table_sql}'") | ||
| cursor.execute(rename_table_sql, _is_internal=True) | ||
| clone_table_sql = ( | ||
| f"CREATE OR REPLACE TABLE {original_table_location} " | ||
| f"CLONE {target_table_location} " | ||
| f"{'COPY GRANTS' if copy_grants else ''}" | ||
| f" /* Python:snowflake.connector.pandas_tools.write_pandas() */ " | ||
| ) | ||
| logger.debug(f"clone table with '{clone_table_sql}'") | ||
| cursor.execute(clone_table_sql, _is_internal=True) | ||
| drop_object(target_table_location, "table") |
There was a problem hiding this comment.
this could be a breaking change since not all users my have create table privileges. Can you refactor the code like so
if clone_table_sql:
clone_table_sql = "create or replace ..."
logger.debug()
cursor.execute()
drop_object()
else:
rename_table_sql = "alter table ..."
logger.debug()
cursor.execute()
Comment on lines
+249
to
+251
| copy_grants: When true and when both overwrite is true and auto_create_table is true, the grants of the table | ||
| being overwritten will be copied to the new table. Otherwise, the new table will be created with only the | ||
| default/future grants defined on the schema/database. |
There was a problem hiding this comment.
please add the fact that create table privileges are required with this option.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Please answer these questions before submitting your pull requests. Thanks!
What GitHub issue is this PR addressing? Make sure that there is an accompanying issue to your PR.
Fixes SNOW-1658882: Please provide the option to copy grants when overwriting table using write_pandas #2049
Fill out the following pre-review checklist:
Please describe how your code solves the related issue.
Replaces the DROP + RENAME logic into a REPLACE logic, which I believe is anyways better because it is an atomic statement.
Adds COPY GRANTS if copy_grants is True.