[GitHub] [airflow] dstandish commented on a change in pull request #13017: optional schema configuration of S3ToSnowflakeTransferOperator

[GitBox <gi...@apache.org>]

dstandish commented on a change in pull request #13017:
URL: https://github.com/apache/airflow/pull/13017#discussion_r549970545



##########
File path: airflow/providers/snowflake/transfers/s3_to_snowflake.py
##########
@@ -91,16 +91,18 @@ def execute(self, context: Any) -> None:
         )
 
         if self.columns_array:
-            copy_query = """
-                COPY INTO {schema}.{table}({columns}) {base_sql}
-            """.format(
-                schema=self.schema, table=self.table, columns=",".join(self.columns_array), base_sql=base_sql
+            columns = ','.join(self.columns_array)
+            copy_query = (
+                f"COPY INTO {self.schema}.{self.table}({columns}) {base_sql}"
+                if self.schema
+                else f"COPY INTO {self.table} ({columns}) {base_sql}"
             )
+
         else:

Review comment:
       ```suggestion
           columns = '(' + ','.join(self.columns_array) + ')' if self.columns_array else ''
           table_name = f"{self.schema}.{self.table}" if self.schema else self.table
           copy_query = f"COPY INTO {table_name} {columns} {from_sql}"
   ```
   this whole if / else block could be condensed to this (but the platform isn't letting me apply the suggestion correctly)
   
   even if maybe you don't go with my exact suggestion, it's just more verbose than it needs to be ( e.g. COPY INTO appears 4 times) and could be cleaned up
   
   note here i'm also suggesting renaming `base_sql` to `from_sql` because that's more descriptive -- it's the FROM clause




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

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