potiuk commented on a change in pull request #15581:
URL: https://github.com/apache/airflow/pull/15581#discussion_r624493306



##########
File path: airflow/providers/oracle/operators/oracle.py
##########
@@ -63,4 +68,25 @@ def __init__(
     def execute(self, context) -> None:
         self.log.info('Executing: %s', self.sql)
         hook = OracleHook(oracle_conn_id=self.oracle_conn_id)
-        hook.run(self.sql, autocommit=self.autocommit, 
parameters=self.parameters)
+
+        def handler(cur):
+            bindvars = cur.bindvars
+
+            if isinstance(bindvars, list):
+                bindvars = [v.getvalue() for v in bindvars]
+            elif isinstance(bindvars, dict):
+                bindvars = {n: v.getvalue() for (n, v) in bindvars.items()}
+            else:
+                raise TypeError(bindvars)
+
+            return {
+                "bindvars": bindvars,
+                "rows": cur.fetchall(),
+            }
+
+        return hook.run(

Review comment:
       There is a subtle problem with this approach. This would mean that the 
Oracle Provider will be only compatible with the new DBApi implementation, 
which is part of the Airflow itself. This means that either we make a breaking 
change (and add > 2.1 limitation to the Oracle provider, or (better) we 
implement it in backward-compatible way - honoring the fact that the run method 
might have no "handler" added.
   
   That would mean that bind-variable functionality will only be available in 
Airflow 2.1 and above




-- 
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:
[email protected]


Reply via email to