dabla opened a new pull request, #41327:
URL: https://github.com/apache/airflow/pull/41327
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
This PR introduces the notion of an SQL Dialect class in the DbApiHook.
Let me first elaborate why I did such an approach, this PR is a propopition
and can of course be changed where need but gives an idea on how it could be
implemented.
The reason why I wanted to introduce this is because I experienced that the
_insert_statement_format and _replace_statement_format string formatting
properties in
the DbApiHook are a bit of a naïve approache. Yes this approache is generic
and doesn't ty the code to a specific database, hence why I want to introduce
some kind of Dialect,
but it's limited as the parameters passed to the string format are
hard-coded and aren't always sufficant for certain databases.
We for example are using MsSQL and SAP Hanba a lot in our DAG's, and since
we are using the insert_rows methods as much as possible for inserting/updating
data,
we liked the approach of the replace parameter which allows you to specify
if you want to insert or replace data. That way all our inserts/replace tasks
in our DAG's are using
the same generic code independantly of which database we want to write to,
we even use the GenericTransfer operator accross multiple databases that way
which is great.
Beside that, we also implemented a custom SQLInsertRowsOperator which does
the same as the GenericTransfer operator but the get's the data from an XCom
instead of another database,
I'll also propose a PR for that once we have a good solution for the current
problem we try to solve in this PR.
So the issue with the current approach for automatically generating the
replace statement is when we want to use MsSQL. For SAP Hana it was easy, we
just had to override the
replace_statement_format parameter in the connection extra field and we were
done. For MsSQL that was not possible, that's why I created another PR that is
already merged in Airflow
for the MsSqlHook, in which I override the _generate_insert_sql method of
the DbApiHook to allow a more complex replace statement generation to support
the replace option.
The problem with this approach is that this functionality will only work
when using the MsSQL connection type (which underneath uses the pymssql
library). Unfortunately, we
experienced a lot of connection issues and even deadlocks (later one is
maybe unrelated) when writing data to MsSQL using the MsSQLHook. Our DBA
suggested using the OdbcHook,
unfortunately the OdbcHook doesn't have the enhanced _generate_insert_sql
for MsSQL as OdbcHook is agnostic of which database you connect to. So in
order to check if the OdbcHook
would solve our connection issues with MsSQL, I patched the
_generate_insert_sql method of the OdbcHook to test if the issues are solved
using the OdbcHook. So we did a load test
and indeed we did not experience any connection issues nor deadlocks and the
performance was also a lot better compared to the MsSqlHook.
So that's why I started thinking about a more generic opproach of this
problem, which would work independantly of which Connection type you use, and
move all the logic to the DbApiHook,
hence why I wanted to introduce the notion of a Dialect which can then be
specialized where needed per database. Ofc we would need to think how we want
it to be implemented, here I just
did a bit of a naïve implemented to show the idea. We are already using
this in our Airflow environment through patched code and it works great, now we
have to find a way to implement this
idea (if accepted) in a good/clean and furute proof approach.
<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request
Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)**
for more information.
In case of fundamental code changes, an Airflow Improvement Proposal
([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals))
is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party
License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a
newsfragment file, named `{pr_number}.significant.rst` or
`{issue_number}.significant.rst`, in
[newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
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]
