You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/07/10 23:01:36 UTC
[GitHub] [airflow] kolfild26 opened a new issue, #24953: oracle hook _map_param() incorrect
kolfild26 opened a new issue, #24953:
URL: https://github.com/apache/airflow/issues/24953
### Apache Airflow Provider(s)
oracle
### Versions of Apache Airflow Providers
_No response_
### Apache Airflow version
2.3.3 (latest released)
### Operating System
OEL 7.6
### Deployment
Virtualenv installation
### Deployment details
_No response_
### What happened
[_map_param()](https://github.com/apache/airflow/blob/main/airflow/providers/oracle/hooks/oracle.py#L36) function from Oracle hook has an incorrect check of types:
```
PARAM_TYPES = {bool, float, int, str}
def _map_param(value):
if value in PARAM_TYPES:
# In this branch, value is a Python type; calling it produces
# an instance of the type which is understood by the Oracle driver
# in the out parameter mapping mechanism.
value = value()
return value
```
`if value in PARAM_TYPES` never gets True for all the mentioned variables types:
```
PARAM_TYPES = {bool, float, int, str}
>>> "abc" in PARAM_TYPES
False
>>> 123 in PARAM_TYPES
False
>>> True in PARAM_TYPES
False
>>> float(5.5) in PARAM_TYPES
False
```
The correct condition would be `if type(value) in PARAM_TYPES`
**But**, if we only fix this condition, next in positive case (type(value) in PARAM_TYPES = True) one more issue occurs with `value = value()`
`bool`, `float`, `int` or `str` are not callable
`TypeError: 'int' object is not callable`
This line is probaby here for passing a python callable into sql statement of procedure params in tasks, is it? If so, need to correct:
`if type(value) not in PARAM_TYPES`
Here is the full fix:
```
def _map_param(value):
if type(value) not in PARAM_TYPES:
value = value()
return value
```
Next casses are tested:
```
def oracle_callable(n=123):
return n
def oracle_pass():
return 123
task1 = OracleStoredProcedureOperator( task_id='task1', oracle_conn_id='oracle_conn', procedure='AIRFLOW_TEST',
parameters={'var':oracle_callable} )
task2 = OracleStoredProcedureOperator( task_id='task2', oracle_conn_id='oracle_conn', procedure='AIRFLOW_TEST',
parameters={'var':oracle_callable()} )
task3 = OracleStoredProcedureOperator( task_id='task3', oracle_conn_id='oracle_conn', procedure='AIRFLOW_TEST',
parameters={'var':oracle_callable(456)} )
task4 = OracleStoredProcedureOperator( task_id='task4', oracle_conn_id='oracle_conn', procedure='AIRFLOW_TEST',
parameters={'var':oacle_pass} )
```
### What you think should happen instead
_No response_
### How to reproduce
_No response_
### Anything else
_No response_
### Are you willing to submit PR?
- [X] Yes I am willing to submit a PR!
### Code of Conduct
- [X] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md)
--
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: commits-unsubscribe@airflow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk closed issue #24953: oracle hook _map_param() incorrect
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk closed issue #24953: oracle hook _map_param() incorrect
URL: https://github.com/apache/airflow/issues/24953
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #24953: oracle hook _map_param() incorrect
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #24953:
URL: https://github.com/apache/airflow/issues/24953#issuecomment-1180294085
Assigned you - feel free to make PR
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kolfild26 commented on issue #24953: oracle hook _map_param() incorrect
Posted by "kolfild26 (via GitHub)" <gi...@apache.org>.
kolfild26 commented on issue #24953:
URL: https://github.com/apache/airflow/issues/24953#issuecomment-1529218190
@vchiapaikeo
> eladkal asked me to take a look at this. I think this is actually working as expected.
Yes, I checked again that time, after posting, and found it's ok. Sorry, just forgot to close the issue.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] vchiapaikeo commented on issue #24953: oracle hook _map_param() incorrect
Posted by "vchiapaikeo (via GitHub)" <gi...@apache.org>.
vchiapaikeo commented on issue #24953:
URL: https://github.com/apache/airflow/issues/24953#issuecomment-1529201976
@eladkal asked me to take a look at this. I think this is actually working as expected. It took me awhile to install an Oracle db and test this E2E. I followed the instructions for the [express version of the db here to do so](https://container-registry.oracle.com/).
Docker run command:
```bash
docker run --name oracle -e ORACLE_PWD=admin -p 25500:5500 -p 21521:1521 container-registry.oracle.com/database/express:21.3.0-xe
```
With the db up, I was able to test a successful Oracle connection (I had to use my personal IP as the hostname instead of localhost for some reason, not sure why). I then created the following stored procedure based on [this documentation](https://python-oracledb.readthedocs.io/en/latest/user_guide/plsql_execution.html#pl-sql-stored-procedures).
```sql
CREATE OR REPLACE PROCEDURE
TEST_PROCEDURE (val_in IN INT,
val_out OUT INT) AS
BEGIN
val_out := val_in * 2
;
END;
/
```
Next, I created the following test dag to test the functionality:
```py
from airflow import DAG
from airflow.providers.oracle.operators.oracle import OracleOperator, OracleStoredProcedureOperator
DEFAULT_TASK_ARGS = {
"owner": "gcp-data-platform",
"retries": 1,
"retry_delay": 60,
"start_date": "2022-08-01",
}
with DAG(
max_active_runs=1,
max_active_tasks=2,
catchup=False,
schedule_interval="@daily",
dag_id="test_oracle",
default_args=DEFAULT_TASK_ARGS,
) as dag:
opr_sql = OracleOperator(
task_id='task_sql',
oracle_conn_id='oracle',
sql='SELECT 1 FROM DUAL',
autocommit='True')
opr_stored_procedure = OracleStoredProcedureOperator(
task_id='task_stored_procedure',
oracle_conn_id='oracle',
procedure='TEST_PROCEDURE',
# parameters=[3, int]
parameters={"val_in": 3, "val_out": int}
)
```
While the logs do not show the correct output value (6) based on the given input value (3), xcom does show the correct values. This is expected behavior however because it is simply printing what is being executed before the run.
<img width="1440" alt="image" src="https://user-images.githubusercontent.com/9200263/235381958-0dbb98a3-eb54-42e1-804e-fd4f41fa9318.png">
<img width="764" alt="image" src="https://user-images.githubusercontent.com/9200263/235381981-a6b42fad-6869-4ab2-807e-33c13dcb97a0.png">
The way to explain this is that output parameters need to be initialized. That is what calling int(), bool(), float(), etc. does. The cursor then mutates the initialized value after the stored procedure is run as described in [this documentation](https://python-oracledb.readthedocs.io/en/latest/user_guide/plsql_execution.html#pl-sql-stored-procedures).
I think this issue should be closed. As a follow-up, I will try to better describe this behavior in documentation --> https://github.com/apache/airflow/pull/30979
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org