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/12/07 15:21:33 UTC
[GitHub] [airflow] jon-evergreen opened a new issue, #28195: SQLTableCheckOperator doesn't correctly handle templated partition clause
jon-evergreen opened a new issue, #28195:
URL: https://github.com/apache/airflow/issues/28195
### Apache Airflow Provider(s)
common-sql
### Versions of Apache Airflow Providers
apache-airflow-providers-common-sql==1.3.1
### Apache Airflow version
2.5.0
### Operating System
Debian GNU/Linux 11 (bullseye)
### Deployment
Other Docker-based deployment
### Deployment details
Docker image based on the official image (couple of tools added) deployed on AWS ECS Fargate
### What happened
I have a task which uses the table check operator to run some table stakes data validation. But I don't want it failing every time it runs in our test environments which do not get records every day. So there's a templated switch in the sql:
```python
test_data_changed_yesterday = SQLTableCheckOperator(
task_id="test_data_changed_yesterday",
table="reporting.events",
conn_id="pg_conn",
checks={"changed_record_count": {"check_statement": "count(*) > 0"}},
partition_clause="""
{% if var.value.get('is_test_env', False) %}
modifieddate >= '2015-12-01T01:01:01.000Z'
{% else %}
modifieddate >= '{{ data_interval_start.isoformat() }}' and
modifieddate < '{{ data_interval_end.isoformat() }}'
{% endif %}
""",
)
```
This shows correctly in the rendered field for the task. Not pretty, but it works:
```
('\n'
' \n'
" modifieddate >= '2015-12-01T01:01:01.000Z'\n"
' \n'
' ')
```
However, in the logs I see it's running this query:
```
[2022-12-07, 11:49:34 UTC] {sql.py:364} INFO - Running statement: SELECT check_name, check_result FROM (
SELECT 'changed_record_count' AS check_name, MIN(changed_record_count) AS check_result
FROM (SELECT CASE WHEN count(*) > 0 THEN 1 ELSE 0 END AS changed_record_count
FROM reporting.events WHERE
{% if var.value.get('is_test_env', False) %}
modifieddate >= '2015-12-01T01:01:01.000Z'
{% else %}
modifieddate >= '{{ data_interval_start.isoformat() }}' and
modifieddate < '{{ data_interval_end.isoformat() }}'
{% endif %}
) AS sq
) AS check_table, parameters: None
```
Which unsurprisingly the DB rejects as invalid sql!
### What you think should happen instead
The rendered code should be used in the sql which is run!
I think this error comes about through this line: https://github.com/apache/airflow/blob/main/airflow/providers/common/sql/operators/sql.py#L576 which is run in the `__init__` method of the operator. i.e. before templating is applied in the build up to calling `execute(context)`.
### How to reproduce
Use the operator with a templated `partition_clause`
### Anything else
Happens every time
### 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] jon-evergreen commented on issue #28195: SQLTableCheckOperator doesn't correctly handle templated partition clause
Posted by GitBox <gi...@apache.org>.
jon-evergreen commented on issue #28195:
URL: https://github.com/apache/airflow/issues/28195#issuecomment-1341255378
I'll try and work on that. While I'm sure how to _fix_ the code, I'm less sure how to test it. Some exploration of breeze for me I guess.
Also, in slight scope creep, would it acceptable to also add in `table` to the templated fields? The dags I write tend to override `templated_fields` to include that as often the DAG will write to different schemas (postgres sense) when it's run in different environments
--
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 closed issue #28195: SQLTableCheckOperator doesn't correctly handle templated partition clause
Posted by GitBox <gi...@apache.org>.
potiuk closed issue #28195: SQLTableCheckOperator doesn't correctly handle templated partition clause
URL: https://github.com/apache/airflow/issues/28195
--
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 #28195: SQLTableCheckOperator doesn't correctly handle templated partition clause
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #28195:
URL: https://github.com/apache/airflow/issues/28195#issuecomment-1341302283
yeah. Feel free also to add more template_fields. I thik the way to test it would be to add a unit test instantiating the operator and running a render_template_fields() (or similar - can't remember) method and then running the execute method and mocking the db_cursor to see if the right sql is called. I am pretty sure there are similar unit tests in other parts of the code. If you won't be able to find them - i might help finding one.
--
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 #28195: SQLTableCheckOperator doesn't correctly handle templated partition clause
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #28195:
URL: https://github.com/apache/airflow/issues/28195#issuecomment-1341165302
Nice. Indeed the __init__ is the culprit there. Assigned you. Should be an easy fix.
--
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