potiuk commented on code in PR #27548:
URL: https://github.com/apache/airflow/pull/27548#discussion_r1021018783
##########
airflow/providers/common/sql/hooks/sql.py:
##########
@@ -273,7 +273,7 @@ def run(
if not self.get_autocommit(conn):
conn.commit()
- if handler is None:
+ if handler is not None:
Review Comment:
This is based on misunderstanding on how it works.
Even If you look at your code, it's not only backwards incompatible (which
would be enough to reject this change "as is") but also wrong. When you look
closer, the PR when implemened always return either None or empty array
(because results are never appended to when handler is None).
And this is as intended. Because we do NOT want to return whatever cursor
interation returns.
Queries that are run via DBAPIHook are mostly DML and DDL queries, not DQL
queries. It usually makes little sense to use "plain" hook to receive the
resutls of the DQL queries because you would have to return array of cursor
results - which in most cases will be quite big of an array of row
dictionaries. And when you iterate over all those rows returned by DBApi - the
last thing you want to do is to store the full dictionary in an array.
The idea with a handler was that you can use hook to run DQL and Handlers
were precisely introduced to handle that. The cursor rows are NOT "processed"
by the handlers. They are converted to a form that can be returned as array
(and allows you to slim them down to a digestible size and format to put them
as array of results - for example just return an ID of the row - and the
handler can store the content of such row along the way of iteration in a file
or whatever way that will not require storing all output in memory.
You can write an identity handler of course (if you write the query in the
way you know it will return one or few rows), but if you won't have any handler
the None value is absolutely expected.
--
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]
