eladkal commented on code in PR #23971:
URL: https://github.com/apache/airflow/pull/23971#discussion_r924331554
##########
airflow/providers/presto/hooks/presto.py:
##########
@@ -241,32 +237,38 @@ def get_pandas_df(self, sql: str = "", parameters=None,
hql: str = "", **kwargs)
@overload
def run(
self,
- sql: str = "",
+ sql: Union[str, Iterable[str]],
autocommit: bool = False,
- parameters: Optional[dict] = None,
+ parameters: Optional[Union[Iterable, Mapping]] = None,
handler: Optional[Callable] = None,
- ) -> None:
+ split_statements: bool = False,
+ return_last: bool = True,
+ ) -> Optional[Union[Any, List[Any]]]:
"""Execute the statement against Presto. Can be used to create
views."""
@overload
def run(
self,
- sql: str = "",
+ sql: Union[str, Iterable[str]],
autocommit: bool = False,
- parameters: Optional[dict] = None,
+ parameters: Optional[Union[Iterable, Mapping]] = None,
handler: Optional[Callable] = None,
+ split_statements: bool = False,
+ return_last: bool = True,
hql: str = "",
- ) -> None:
+ ) -> Optional[Union[Any, List[Any]]]:
""":sphinx-autoapi-skip:"""
def run(
self,
- sql: str = "",
+ sql: Union[str, Iterable[str]],
autocommit: bool = False,
- parameters: Optional[dict] = None,
+ parameters: Optional[Union[Iterable, Mapping]] = None,
handler: Optional[Callable] = None,
+ split_statements: bool = False,
Review Comment:
Same Q as TrinoHook
##########
airflow/providers/trino/hooks/trino.py:
##########
@@ -255,32 +257,38 @@ def get_pandas_df(
@overload
def run(
self,
- sql,
+ sql: Union[str, Iterable[str]],
autocommit: bool = False,
- parameters: Optional[Tuple] = None,
+ parameters: Optional[Union[Iterable, Mapping]] = None,
handler: Optional[Callable] = None,
- ) -> None:
+ split_statements: bool = False,
+ return_last: bool = True,
+ ) -> Optional[Union[Any, List[Any]]]:
"""Execute the statement against Trino. Can be used to create views."""
@overload
def run(
self,
- sql,
+ sql: Union[str, Iterable[str]],
autocommit: bool = False,
- parameters: Optional[Tuple] = None,
+ parameters: Optional[Union[Iterable, Mapping]] = None,
handler: Optional[Callable] = None,
+ split_statements: bool = False,
+ return_last: bool = True,
hql: str = "",
- ) -> None:
+ ) -> Optional[Union[Any, List[Any]]]:
""":sphinx-autoapi-skip:"""
def run(
self,
- sql,
+ sql: Union[str, Iterable[str]],
autocommit: bool = False,
- parameters: Optional[Tuple] = None,
+ parameters: Optional[Union[Iterable, Mapping]] = None,
handler: Optional[Callable] = None,
+ split_statements: bool = False,
Review Comment:
Something to consider...
I suggest to set it to True
```suggestion
split_statements: bool = True,
```
I think in trino this should be default True.
Trino doesn't support multiple statement in single query so we need to split
them.
Currently Train Operator works only with a single statement. This will make
the operator also work with multiple statements
https://github.com/apache/airflow/blob/52cf5793523d3acab480b1933fb7cb33db1c15c7/airflow/providers/trino/operators/trino.py#L81-L83
Alternatively we can do that in a different PR on the Operator level but I
think that for all the DBs that need split statment behavior it would be best
to set the default as True for them. It just makes everything easier
WDYT?
##########
airflow/providers/presto/hooks/presto.py:
##########
@@ -276,7 +278,16 @@ def run(
)
sql = hql
- return super().run(sql=self._strip_sql(sql), parameters=parameters,
handler=handler)
+ if isinstance(sql, str):
+ sql = self.strip_sql_string(sql)
Review Comment:
Why do we need this here?
It's handled in dbapi hook isn't it?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
