diff --git a/snowcap/blueprint.py b/snowcap/blueprint.py index dc5d3e5..a3ba8a8 100644 --- a/snowcap/blueprint.py +++ b/snowcap/blueprint.py @@ -1748,7 +1748,7 @@ def process_commands(commands, roles, available_roles): session_ctx = data_provider.fetch_session(session) _raise_if_plan_would_drop_session_user(session_ctx, plan) - sql_commands_per_change = compile_plan_to_sql(session_ctx, plan) + sql_commands_per_change, available_roles = compile_plan_to_sql(session_ctx, plan) roles_list: list[Any] = [] additive_commands = [] destructive_commands = [] @@ -1763,11 +1763,11 @@ def process_commands(commands, roles, available_roles): # Suppress SQL execution logs during apply (plan details already shown above) logging.getLogger("snowcap").setLevel(logging.WARNING) - # Process additive changes - process_commands(additive_commands, roles_set, session_ctx["available_roles"]) + # Process additive changes (use available_roles which includes roles being created) + process_commands(additive_commands, roles_set, available_roles) # Process destructive changes - process_commands(destructive_commands, roles_set, session_ctx["available_roles"]) + process_commands(destructive_commands, roles_set, available_roles) # Restore logging level logging.getLogger("snowcap").setLevel(logging.INFO) @@ -1998,8 +1998,15 @@ def sql_commands_for_change( return execution_role, [cmd for cmd in all_cmds if cmd is not None] -def compile_plan_to_sql(session_ctx: SessionContext, plan: Plan) -> list[dict]: - """Compile the plan into a list of SQL command lists, one per change.""" +def compile_plan_to_sql( + session_ctx: SessionContext, plan: Plan +) -> tuple[list[dict], list[ResourceName]]: + """Compile the plan into a list of SQL command lists, one per change. + + Returns: + A tuple of (sql_commands_per_change, available_roles) where available_roles + includes any roles being created in this plan. + """ sql_commands_per_change = [] available_roles = session_ctx["available_roles"].copy() default_role = session_ctx["role"] @@ -2013,7 +2020,7 @@ def compile_plan_to_sql(session_ctx: SessionContext, plan: Plan) -> list[dict]: for change in plan: role, commands = sql_commands_for_change(change, available_roles, default_role) sql_commands_per_change.append({"role": role, "commands": commands, "change": change}) - return sql_commands_per_change + return sql_commands_per_change, available_roles def compute_levels(resource_set: Set[URN], references: Set[tuple[URN, URN]]) -> dict[URN, int]: @@ -2143,9 +2150,12 @@ def _diff_resource_data(lhs: dict, rhs: dict) -> dict: for field_name in lhs.keys(): lhs_value = lhs[field_name] rhs_value = rhs[field_name] - # Skip fields where manifest value is None - means "use Snowflake default/inherit" - if rhs_value is None: + # Skip fields where manifest value is None or empty string - means "use Snowflake default/inherit" + if rhs_value is None or rhs_value == "": continue + # Normalize empty strings to None for comparison (Snowflake returns None for unset fields) + if lhs_value == "": + lhs_value = None if lhs_value != rhs_value: delta[field_name] = rhs_value return delta diff --git a/snowcap/client.py b/snowcap/client.py index 6b3d82c..ad87cf5 100644 --- a/snowcap/client.py +++ b/snowcap/client.py @@ -91,6 +91,7 @@ def execute( session_header = f"[{session.user}:{session.role}] > {sql_text}" # Thread-safe cache check with pending query deduplication + cache_key: tuple[str, str] | None = None if cacheable: cache_key = (session.role, sql_text) with _EXECUTION_CACHE_LOCK: @@ -132,40 +133,31 @@ def execute( # Log execution details logger.info(f"{session_header} \033[94m({len(result)} rows, {runtime:.2f}s)\033[0m") if cacheable: - cache_key = (session.role, sql_text) with _EXECUTION_CACHE_LOCK: if session.role not in _EXECUTION_CACHE: _EXECUTION_CACHE[session.role] = {} _EXECUTION_CACHE[session.role][sql_text] = result - # Signal waiting threads and cleanup - if cache_key in _PENDING_QUERIES: - _PENDING_QUERIES[cache_key].set() - del _PENDING_QUERIES[cache_key] return result except ProgrammingError as err: if empty_response_codes and err.errno in empty_response_codes: runtime = time.time() - start logger.info(f"{session_header} \033[94m(empty, {runtime:.2f}s)\033[0m") if cacheable: - cache_key = (session.role, sql_text) with _EXECUTION_CACHE_LOCK: if session.role not in _EXECUTION_CACHE: _EXECUTION_CACHE[session.role] = {} _EXECUTION_CACHE[session.role][sql_text] = [] - # Signal waiting threads and cleanup - if cache_key in _PENDING_QUERIES: - _PENDING_QUERIES[cache_key].set() - del _PENDING_QUERIES[cache_key] return [] - # On error, also cleanup pending query marker - if cacheable: - cache_key = (session.role, sql_text) + logger.error(f"{session_header} \033[31m(err {err.errno}, {time.time() - start:.2f}s)\033[0m") + raise ProgrammingError(f"{err} on {sql_text}", errno=err.errno) from err + finally: + # Always signal waiting threads and cleanup pending query marker + # This must happen regardless of success, failure, or exception type + if cache_key is not None: with _EXECUTION_CACHE_LOCK: if cache_key in _PENDING_QUERIES: _PENDING_QUERIES[cache_key].set() del _PENDING_QUERIES[cache_key] - logger.error(f"{session_header} \033[31m(err {err.errno}, {time.time() - start:.2f}s)\033[0m") - raise ProgrammingError(f"{err} on {sql_text}", errno=err.errno) from err def execute_in_parallel( diff --git a/snowcap/resources/user.py b/snowcap/resources/user.py index db27a25..10a48a7 100644 --- a/snowcap/resources/user.py +++ b/snowcap/resources/user.py @@ -79,7 +79,8 @@ def __post_init__(self): if self.login_name is None or self.login_name == "": self.login_name = ResourceName(str(self.name).upper()) if self.display_name is None: - self.display_name = self.name._name + # Snowflake stores display_name as uppercase for unquoted identifiers + self.display_name = self.name._name.upper() if self.must_change_password is None: self.must_change_password = False diff --git a/tests/helpers.py b/tests/helpers.py index d95d2e4..5d41e7a 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -157,8 +157,18 @@ def safe_fetch(cursor, urn): return data -def flatten_sql_commands(sql_commands: list[dict]) -> list[str]: - """Flatten compile_plan_to_sql output to a list of SQL strings for testing.""" +def flatten_sql_commands(sql_commands_result) -> list[str]: + """Flatten compile_plan_to_sql output to a list of SQL strings for testing. + + Args: + sql_commands_result: Either a list[dict] (legacy) or a tuple of (list[dict], available_roles) + """ + # Handle both old and new return formats from compile_plan_to_sql + if isinstance(sql_commands_result, tuple): + sql_commands = sql_commands_result[0] + else: + sql_commands = sql_commands_result + result = ["USE SECONDARY ROLES ALL"] last_role = None for cmd in sql_commands: diff --git a/tests/test_blueprint.py b/tests/test_blueprint.py index 473cc63..683c1ad 100644 --- a/tests/test_blueprint.py +++ b/tests/test_blueprint.py @@ -20,8 +20,18 @@ def find_change_by_urn(plan, urn): return None -def flatten_sql_commands(sql_commands: list[dict]) -> list[str]: - """Flatten compile_plan_to_sql output to a list of SQL strings for testing.""" +def flatten_sql_commands(sql_commands_result) -> list[str]: + """Flatten compile_plan_to_sql output to a list of SQL strings for testing. + + Args: + sql_commands_result: Either a list[dict] (legacy) or a tuple of (list[dict], available_roles) + """ + # Handle both old and new return formats from compile_plan_to_sql + if isinstance(sql_commands_result, tuple): + sql_commands = sql_commands_result[0] + else: + sql_commands = sql_commands_result + result = ["USE SECONDARY ROLES ALL"] last_role = None for cmd in sql_commands: diff --git a/tests/test_blueprint_ownership.py b/tests/test_blueprint_ownership.py index 5d2df21..eb4788a 100644 --- a/tests/test_blueprint_ownership.py +++ b/tests/test_blueprint_ownership.py @@ -15,8 +15,18 @@ from snowcap.resource_name import ResourceName -def flatten_sql_commands(sql_commands: list[dict]) -> list[str]: - """Flatten compile_plan_to_sql output to a list of SQL strings for testing.""" +def flatten_sql_commands(sql_commands_result) -> list[str]: + """Flatten compile_plan_to_sql output to a list of SQL strings for testing. + + Args: + sql_commands_result: Either a list[dict] (legacy) or a tuple of (list[dict], available_roles) + """ + # Handle both old and new return formats from compile_plan_to_sql + if isinstance(sql_commands_result, tuple): + sql_commands = sql_commands_result[0] + else: + sql_commands = sql_commands_result + result = ["USE SECONDARY ROLES ALL"] last_role = None for cmd in sql_commands: