From af3bcdd2e622779cbc115484e6f08954bf273969 Mon Sep 17 00:00:00 2001 From: Afroz Alam Date: Thu, 4 Sep 2025 12:57:35 -0700 Subject: [PATCH 1/4] Add option COPY GRANTS to create or replace view/dynamic table --- .../snowpark/_internal/analyzer/analyzer.py | 14 +++++---- .../_internal/analyzer/analyzer_utils.py | 11 +++++-- .../_internal/analyzer/snowflake_plan.py | 5 +++- .../_internal/analyzer/unary_plan_node.py | 4 +++ src/snowflake/snowpark/dataframe.py | 29 +++++++++++++++---- 5 files changed, 49 insertions(+), 14 deletions(-) diff --git a/src/snowflake/snowpark/_internal/analyzer/analyzer.py b/src/snowflake/snowpark/_internal/analyzer/analyzer.py index fced68784d..6749972980 100644 --- a/src/snowflake/snowpark/_internal/analyzer/analyzer.py +++ b/src/snowflake/snowpark/_internal/analyzer/analyzer.py @@ -1338,12 +1338,13 @@ def do_resolve_with_resolved_children( ) return self.plan_builder.create_or_replace_view( - logical_plan.name, - resolved_children[logical_plan.child], - is_temp, - logical_plan.comment, - logical_plan.replace, - logical_plan, + name=logical_plan.name, + child=resolved_children[logical_plan.child], + is_temp=is_temp, + comment=logical_plan.comment, + replace=logical_plan.replace, + copy_grants=logical_plan.copy_grants, + source_plan=logical_plan, ) if isinstance(logical_plan, CreateDynamicTableCommand): @@ -1365,6 +1366,7 @@ def do_resolve_with_resolved_children( child=resolved_children[logical_plan.child], source_plan=logical_plan, iceberg_config=logical_plan.iceberg_config, + copy_grants=logical_plan.copy_grants, ) if isinstance(logical_plan, ReadFileNode): diff --git a/src/snowflake/snowpark/_internal/analyzer/analyzer_utils.py b/src/snowflake/snowpark/_internal/analyzer/analyzer_utils.py index 9cb80ae63b..078c16ca74 100644 --- a/src/snowflake/snowpark/_internal/analyzer/analyzer_utils.py +++ b/src/snowflake/snowpark/_internal/analyzer/analyzer_utils.py @@ -1370,7 +1370,12 @@ def order_expression(name: str, direction: str, null_ordering: str) -> str: def create_or_replace_view_statement( - name: str, child: str, is_temp: bool, comment: Optional[str], replace: bool + name: str, + child: str, + is_temp: bool, + comment: Optional[str], + replace: bool, + copy_grants: bool, ) -> str: comment_sql = get_comment_sql(comment) return ( @@ -1380,6 +1385,7 @@ def create_or_replace_view_statement( + VIEW + name + comment_sql + + (COPY_GRANTS if copy_grants else EMPTY_STRING) + AS + project_statement([], child) ) @@ -1400,6 +1406,7 @@ def create_or_replace_dynamic_table_statement( max_data_extension_time: Optional[int], child: str, iceberg_config: Optional[dict] = None, + copy_grants: bool = False, ) -> str: cluster_by_sql = ( f"{CLUSTER_BY}{LEFT_PARENTHESIS}{COMMA.join(clustering_keys)}{RIGHT_PARENTHESIS}" @@ -1430,7 +1437,7 @@ def create_or_replace_dynamic_table_statement( f"{IF + NOT + EXISTS if if_not_exists else EMPTY_STRING}{name}{LAG}{EQUALS}" f"{convert_value_to_sql_option(lag)}{WAREHOUSE}{EQUALS}{warehouse}" f"{refresh_and_initialize_options}{cluster_by_sql}{data_retention_options}{iceberg_options}" - f"{comment_sql}{AS}{project_statement([], child)}" + f"{comment_sql}{COPY_GRANTS if copy_grants else EMPTY_STRING}{AS}{project_statement([], child)}" ) diff --git a/src/snowflake/snowpark/_internal/analyzer/snowflake_plan.py b/src/snowflake/snowpark/_internal/analyzer/snowflake_plan.py index 8be98e1b1d..d302700890 100644 --- a/src/snowflake/snowpark/_internal/analyzer/snowflake_plan.py +++ b/src/snowflake/snowpark/_internal/analyzer/snowflake_plan.py @@ -1548,6 +1548,7 @@ def create_or_replace_view( is_temp: bool, comment: Optional[str], replace: bool, + copy_grants: bool, source_plan: Optional[LogicalPlan], ) -> SnowflakePlan: if len(child.queries) != 1: @@ -1574,7 +1575,7 @@ def create_or_replace_view( return self.build( lambda x: create_or_replace_view_statement( - name, x, is_temp, comment, replace + name, x, is_temp, comment, replace, copy_grants ), child, source_plan, @@ -1666,6 +1667,7 @@ def create_or_replace_dynamic_table( child: SnowflakePlan, source_plan: Optional[LogicalPlan], iceberg_config: Optional[dict] = None, + copy_grants: bool = False, ) -> SnowflakePlan: child = self.find_and_update_table_function_plan(child) @@ -1705,6 +1707,7 @@ def create_or_replace_dynamic_table( max_data_extension_time=max_data_extension_time, child=x, iceberg_config=iceberg_config, + copy_grants=copy_grants, ), child, source_plan, diff --git a/src/snowflake/snowpark/_internal/analyzer/unary_plan_node.py b/src/snowflake/snowpark/_internal/analyzer/unary_plan_node.py index bab74476f2..7ee2038e90 100644 --- a/src/snowflake/snowpark/_internal/analyzer/unary_plan_node.py +++ b/src/snowflake/snowpark/_internal/analyzer/unary_plan_node.py @@ -322,6 +322,7 @@ def __init__( view_type: ViewType, comment: Optional[str], replace: bool, + copy_grants: bool, child: LogicalPlan, ) -> None: super().__init__(child) @@ -329,6 +330,7 @@ def __init__( self.view_type = view_type self.comment = comment self.replace = replace + self.copy_grants = copy_grants class CreateDynamicTableCommand(UnaryNode): @@ -347,6 +349,7 @@ def __init__( max_data_extension_time: Optional[int], child: LogicalPlan, iceberg_config: Optional[dict] = None, + copy_grants: bool = False, ) -> None: super().__init__(child) self.name = name @@ -361,3 +364,4 @@ def __init__( self.data_retention_time = data_retention_time self.max_data_extension_time = max_data_extension_time self.iceberg_config = iceberg_config + self.copy_grants = copy_grants diff --git a/src/snowflake/snowpark/dataframe.py b/src/snowflake/snowpark/dataframe.py index 4984404dc9..11ee5ea5c4 100644 --- a/src/snowflake/snowpark/dataframe.py +++ b/src/snowflake/snowpark/dataframe.py @@ -5244,6 +5244,7 @@ def create_or_replace_view( *, comment: Optional[str] = None, statement_params: Optional[Dict[str, str]] = None, + copy_grants: bool = False, _emit_ast: bool = True, ) -> List[Row]: """Creates a view that captures the computation expressed by this DataFrame. @@ -5259,6 +5260,8 @@ def create_or_replace_view( that specifies the database name, schema name, and view name. comment: Adds a comment for the created view. See `COMMENT `_. + copy_grants: A boolean value that specifies whether to retain the access permissions from the original view + when a new view is created. Defaults to False. statement_params: Dictionary of statement level parameters to be set while executing this action. """ @@ -5270,6 +5273,7 @@ def create_or_replace_view( stmt = self._session._ast_batch.bind() expr = with_src_position(stmt.expr.dataframe_create_or_replace_view, stmt) expr.is_temp = False + expr.copy_grants = copy_grants self._set_ast_ref(expr.df) build_view_name(expr.name, name) if comment is not None: @@ -5280,6 +5284,7 @@ def create_or_replace_view( formatted_name, PersistedView(), comment=comment, + copy_grants=copy_grants, _statement_params=create_or_update_statement_params_with_query_tag( statement_params or self._statement_params, self._session.query_tag, @@ -5309,6 +5314,7 @@ def create_or_replace_dynamic_table( max_data_extension_time: Optional[int] = None, statement_params: Optional[Dict[str, str]] = None, iceberg_config: Optional[dict] = None, + copy_grants: bool = False, _emit_ast: bool = True, ) -> List[Row]: """Creates a dynamic table that captures the computation expressed by this DataFrame. @@ -5352,6 +5358,8 @@ def create_or_replace_dynamic_table( - base_location: the base directory that snowflake can write iceberg metadata and files to. - catalog_sync: optionally sets the catalog integration configured for Polaris Catalog. - storage_serialization_policy: specifies the storage serialization policy for the table. + copy_grants: A boolean value that specifies whether to retain the access permissions from the original view + when a new view is created. Defaults to False. Note: @@ -5405,6 +5413,7 @@ def create_or_replace_dynamic_table( if statement_params is not None: build_expr_from_dict_str_str(expr.statement_params, statement_params) + expr.copy_grants = copy_grants # TODO: Support create_or_replace_dynamic_table in MockServerConnection. from snowflake.snowpark.mock._connection import MockServerConnection @@ -5441,6 +5450,7 @@ def create_or_replace_dynamic_table( ), ), iceberg_config=iceberg_config, + copy_grants=copy_grants, ) @df_collect_api_telemetry @@ -5451,6 +5461,7 @@ def create_or_replace_temp_view( *, comment: Optional[str] = None, statement_params: Optional[Dict[str, str]] = None, + copy_grants: bool = False, _emit_ast: bool = True, ) -> List[Row]: """Creates or replace a temporary view that returns the same results as this DataFrame. @@ -5470,6 +5481,8 @@ def create_or_replace_temp_view( that specifies the database name, schema name, and view name. comment: Adds a comment for the created view. See `COMMENT `_. + copy_grants: A boolean value that specifies whether to retain the access permissions from the original view + when a new view is created. Defaults to False. statement_params: Dictionary of statement level parameters to be set while executing this action. """ @@ -5487,11 +5500,13 @@ def create_or_replace_temp_view( expr.comment.value = comment if statement_params is not None: build_expr_from_dict_str_str(expr.statement_params, statement_params) + expr.copy_grants = copy_grants return self._do_create_or_replace_view( formatted_name, LocalTempView(), comment=comment, + copy_grants=copy_grants, _statement_params=create_or_update_statement_params_with_query_tag( statement_params or self._statement_params, self._session.query_tag, @@ -5571,16 +5586,18 @@ def _do_create_or_replace_view( view_type: ViewType, comment: Optional[str], replace: bool = True, + copy_grants: bool = False, _ast_stmt: Optional[proto.Bind] = None, **kwargs, ): validate_object_name(view_name) cmd = CreateViewCommand( - view_name, - view_type, - comment, - replace, - self._plan, + name=view_name, + view_type=view_type, + comment=comment, + replace=replace, + copy_grants=copy_grants, + child=self._plan, ) return self._session._conn.execute( @@ -5601,6 +5618,7 @@ def _do_create_or_replace_dynamic_table( data_retention_time: Optional[int] = None, max_data_extension_time: Optional[int] = None, iceberg_config: Optional[dict] = None, + copy_grants: bool = False, **kwargs, ): validate_object_name(name) @@ -5628,6 +5646,7 @@ def _do_create_or_replace_dynamic_table( max_data_extension_time=max_data_extension_time, child=self._plan, iceberg_config=iceberg_config, + copy_grants=copy_grants, ) return self._session._conn.execute( From 5be7e2e87de6146161120b3490d2772910c41a94 Mon Sep 17 00:00:00 2001 From: Afroz Alam Date: Fri, 5 Sep 2025 16:52:59 -0700 Subject: [PATCH 2/4] ast tests --- .../snowpark/_internal/proto/ast.proto | 40 ++++++++++--------- .../ast/data/DataFrame.create_or_replace.test | 25 +++++++----- 2 files changed, 35 insertions(+), 30 deletions(-) diff --git a/src/snowflake/snowpark/_internal/proto/ast.proto b/src/snowflake/snowpark/_internal/proto/ast.proto index 0f8b56b268..c3d9837f04 100644 --- a/src/snowflake/snowpark/_internal/proto/ast.proto +++ b/src/snowflake/snowpark/_internal/proto/ast.proto @@ -835,7 +835,7 @@ message DataframeCollect { repeated Tuple_String_String statement_params = 7; } -// dataframe-io.ir:163 +// dataframe-io.ir:165 message DataframeCopyIntoTable { repeated Tuple_String_Expr copy_options = 1; Expr df = 2; @@ -859,32 +859,34 @@ message DataframeCount { repeated Tuple_String_String statement_params = 4; } -// dataframe-io.ir:147 +// dataframe-io.ir:148 message DataframeCreateOrReplaceDynamicTable { repeated Expr clustering_keys = 1; google.protobuf.StringValue comment = 2; - google.protobuf.Int64Value data_retention_time = 3; - Expr df = 4; - google.protobuf.StringValue initialize = 5; - bool is_transient = 6; - string lag = 7; - google.protobuf.Int64Value max_data_extension_time = 8; - SaveMode mode = 9; - NameRef name = 10; - google.protobuf.StringValue refresh_mode = 11; - SrcPosition src = 12; - repeated Tuple_String_String statement_params = 13; - string warehouse = 14; + bool copy_grants = 3; + google.protobuf.Int64Value data_retention_time = 4; + Expr df = 5; + google.protobuf.StringValue initialize = 6; + bool is_transient = 7; + string lag = 8; + google.protobuf.Int64Value max_data_extension_time = 9; + SaveMode mode = 10; + NameRef name = 11; + google.protobuf.StringValue refresh_mode = 12; + SrcPosition src = 13; + repeated Tuple_String_String statement_params = 14; + string warehouse = 15; } // dataframe-io.ir:139 message DataframeCreateOrReplaceView { google.protobuf.StringValue comment = 1; - Expr df = 2; - bool is_temp = 3; - NameRef name = 4; - SrcPosition src = 5; - repeated Tuple_String_String statement_params = 6; + bool copy_grants = 2; + Expr df = 3; + bool is_temp = 4; + NameRef name = 5; + SrcPosition src = 6; + repeated Tuple_String_String statement_params = 7; } // dataframe.ir:185 diff --git a/tests/ast/data/DataFrame.create_or_replace.test b/tests/ast/data/DataFrame.create_or_replace.test index 70ef546827..cb67178208 100644 --- a/tests/ast/data/DataFrame.create_or_replace.test +++ b/tests/ast/data/DataFrame.create_or_replace.test @@ -4,11 +4,11 @@ df = session.table(tables.table1) df.create_or_replace_view(["test_db", "test_schema", "test_view"], comment="foo") -df.create_or_replace_view("test_view", statement_params={"foo": "bar"}) +df.create_or_replace_view("test_view", statement_params={"foo": "bar"}, copy_grants=True) df.create_or_replace_temp_view(["test_db", "test_schema", "test_view"], comment="foo") -df.create_or_replace_temp_view("test_view", statement_params={"foo": "bar"}) +df.create_or_replace_temp_view("test_view", statement_params={"foo": "bar"}, copy_grants=True) # TODO: remove the suppress check in `copy_into_table()`. # session.file.put(local_file_name="test.json", stage_location="test", auto_compress=False) @@ -30,19 +30,19 @@ df3 = df.cache_result() df4 = df.cache_result(statement_params={"foo": "bar"}) -df.create_or_replace_dynamic_table("test_dyn_table", warehouse="test_wh", lag="1 hour", comment="foo") +df.create_or_replace_dynamic_table("test_dyn_table", warehouse="test_wh", lag="1 hour", comment="foo", copy_grants=True) ## EXPECTED UNPARSER OUTPUT df = session.table("table1") -res1 = df.create_or_replace_view(["test_db", "test_schema", "test_view"], comment="foo") +res1 = df.create_or_replace_view(["test_db", "test_schema", "test_view"], comment="foo", copy_grants=False) -res2 = df.create_or_replace_view("test_view", statement_params={"foo": "bar"}) +res2 = df.create_or_replace_view("test_view", statement_params={"foo": "bar"}, copy_grants=True) -res3 = df.create_or_replace_temp_view(["test_db", "test_schema", "test_view"], comment="foo") +res3 = df.create_or_replace_temp_view(["test_db", "test_schema", "test_view"], comment="foo", copy_grants=False) -res4 = df.create_or_replace_temp_view("test_view", statement_params={"foo": "bar"}) +res4 = df.create_or_replace_temp_view("test_view", statement_params={"foo": "bar"}, copy_grants=True) df.copy_into_table(["test_db", "test_schema", "table2"], files=["file1", "file2"], pattern="[A-Z]+", validation_mode="RETURN_ERRORS", target_columns=["n", "str"], transformations=[col("n") * 10, col("str")], format_type_options={"COMPRESSION": "GZIP", "RECORD_DELIMITER": "|"}, statement_params={"foo": "bar"}, force=True) @@ -52,7 +52,7 @@ df3 = df.cache_result() df4 = df.cache_result(statement_params={"foo": "bar"}) -res6 = df.create_or_replace_dynamic_table("test_dyn_table", warehouse="test_wh", lag="1 hour", comment="foo", mode="overwrite") +res6 = df.create_or_replace_dynamic_table("test_dyn_table", warehouse="test_wh", lag="1 hour", comment="foo", mode="overwrite", copy_grants=True) ## EXPECTED ENCODED AST @@ -135,6 +135,7 @@ body { bind { expr { dataframe_create_or_replace_view { + copy_grants: true df { dataframe_ref { id: 1 @@ -148,7 +149,7 @@ body { } } src { - end_column: 79 + end_column: 97 end_line: 29 file: 2 start_column: 8 @@ -207,6 +208,7 @@ body { bind { expr { dataframe_create_or_replace_view { + copy_grants: true df { dataframe_ref { id: 1 @@ -221,7 +223,7 @@ body { } } src { - end_column: 84 + end_column: 102 end_line: 33 file: 2 start_column: 8 @@ -532,6 +534,7 @@ body { comment { value: "foo" } + copy_grants: true df { dataframe_ref { id: 1 @@ -549,7 +552,7 @@ body { } } src { - end_column: 110 + end_column: 128 end_line: 55 file: 2 start_column: 8 From 894ab2061f3ea62bdec3e5410461c44ddbd82e7e Mon Sep 17 00:00:00 2001 From: Afroz Alam Date: Fri, 5 Sep 2025 17:10:51 -0700 Subject: [PATCH 3/4] add test + changelog --- CHANGELOG.md | 6 +++- tests/unit/test_analyzer_util_suite.py | 38 +++++++++++++++++++++++++- 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8366a11330..09c65b9b3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,10 @@ - Added a new function `interval_year_month_from_parts` that allows users to easily create `YearMonthIntervalType` without using SQL. - Added support for `FileOperation.list` to list files in a stage with metadata. - Added support for `FileOperation.remove` to remove files in a stage. +- Added an option to specify `copy_grants` for the following `DataFrame` APIs: + - `create_or_replace_view` + - `create_or_replace_temp_view` + - `create_or_replace_dynamic_table` #### Bug Fixes @@ -121,7 +125,7 @@ - Raised `NotImplementedError` instead of `AttributeError` on attempting to call Snowflake extension functions/methods `to_dynamic_table()`, `cache_result()`, `to_view()`, `create_or_replace_dynamic_table()`, and - `create_or_replace_view()` on dataframes or series using the pandas or ray + `create_or_replace_view()` on dataframes or series using the pandas or ray backends. ## 1.37.0 (2025-08-18) diff --git a/tests/unit/test_analyzer_util_suite.py b/tests/unit/test_analyzer_util_suite.py index 2a110f3d95..9ea886067f 100644 --- a/tests/unit/test_analyzer_util_suite.py +++ b/tests/unit/test_analyzer_util_suite.py @@ -17,6 +17,7 @@ NOT, OR, REPLACE, + create_or_replace_view_statement, format_uuid, convert_value_to_sql_option, create_file_format_statement, @@ -280,6 +281,40 @@ def test_create_table_statement( assert if_not_exists_sql in create_table_stmt +def test_create_or_replace_view_statement(): + assert create_or_replace_view_statement( + name="my_view", + child="select * from foo", + is_temp=False, + comment=None, + replace=True, + copy_grants=True, + ) == "\n".join( + [ + " CREATE OR REPLACE VIEW my_view COPY GRANTS AS SELECT * ", + " FROM (", + "select * from foo", + ")", + ] + ) + + assert create_or_replace_view_statement( + name="my_view", + child="select * from foo", + is_temp=True, + comment="A frosty winter wonderland with glistening snowflakes and icy views", + replace=False, + copy_grants=False, + ) == "\n".join( + [ + " CREATE TEMPORARY VIEW my_view COMMENT = 'A frosty winter wonderland with glistening snowflakes and icy views' AS SELECT * ", + " FROM (", + "select * from foo", + ")", + ] + ) + + def test_create_or_replace_dynamic_table_statement(): dt_name = "my_dt" warehouse = "my_warehouse" @@ -303,9 +338,10 @@ def test_create_or_replace_dynamic_table_statement(): data_retention_time=None, max_data_extension_time=None, child="select * from foo", + copy_grants=True, ) == ( f" CREATE OR REPLACE DYNAMIC TABLE {dt_name} LAG = '1 minute' WAREHOUSE = {warehouse} " - "AS SELECT * \n FROM (\nselect * from foo\n)" + "COPY GRANTS AS SELECT * \n FROM (\nselect * from foo\n)" ) assert create_or_replace_dynamic_table_statement( From eb2ddfe98cd09a4ff81c70c9ef3832a0660967e3 Mon Sep 17 00:00:00 2001 From: Afroz Alam Date: Fri, 5 Sep 2025 17:35:33 -0700 Subject: [PATCH 4/4] fix test --- tests/integ/compiler/test_query_generator.py | 4 +++- tests/integ/test_deepcopy.py | 11 ++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/integ/compiler/test_query_generator.py b/tests/integ/compiler/test_query_generator.py index bb462ee255..22986784b7 100644 --- a/tests/integ/compiler/test_query_generator.py +++ b/tests/integ/compiler/test_query_generator.py @@ -429,7 +429,9 @@ def test_multiple_plan_query_generation(session): clustering_exprs=None, comment=None, ), - lambda df, name: CreateViewCommand(name, PersistedView(), None, True, df._plan), + lambda df, name: CreateViewCommand( + name, PersistedView(), None, True, True, df._plan + ), lambda df, name: CopyIntoLocationNode( df._plan, name, diff --git a/tests/integ/test_deepcopy.py b/tests/integ/test_deepcopy.py index 0d2cf8235c..8b8f2aff9d 100644 --- a/tests/integ/test_deepcopy.py +++ b/tests/integ/test_deepcopy.py @@ -353,11 +353,12 @@ def test_table_creation(session, mode): def test_create_or_replace_view(session): df = session.create_dataframe([[1, 2], [3, 4]], schema=["a", "b"]) create_view_logical_plan = CreateViewCommand( - random_name_for_temp_object(TempObjectType.VIEW), - LocalTempView(), - None, - True, - df._plan, + name=random_name_for_temp_object(TempObjectType.VIEW), + view_type=LocalTempView(), + comment=None, + replace=True, + copy_grants=True, + child=df._plan, ) snowflake_plan = session._analyzer.resolve(create_view_logical_plan)