You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "dabla (via GitHub)" <gi...@apache.org> on 2025/01/07 09:25:48 UTC

[PR] Added ADR document describing why the notion of dialects was introduced in the common sql provider [airflow]

dabla opened a new pull request, #45456:
URL: https://github.com/apache/airflow/pull/45456

   <!--
    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: [#41327](https://github.com/apache/airflow/pull/41327)
   
   How to write a good git commit message:
   http://chris.beams.io/posts/git-commit/
   -->
   
   Added ADR document describing why the notion of dialects was introduced in the common sql provider.
   
   <!-- 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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added ADR document describing why the notion of dialects was introduced in the common sql provider [airflow]

Posted by "dabla (via GitHub)" <gi...@apache.org>.
dabla commented on PR #45456:
URL: https://github.com/apache/airflow/pull/45456#issuecomment-2593313183

   > Sorry, "late to the party" now digging through my backlog... wanted to review earlier.
   > 
   > I really like this and the description.
   > 
   > For the docs build problem I see two options: (1) [not sure if this really works] outside the TOC create a small RST just with the heading and pointing to the other doctree - or (2) if you want to have exactly the same doc in each provider then use a symlink in GIT such that the RST is maintained in one place and each doctree has the same. (As long as we not move the providers the symlink is working in Sphinx)
   > 
   > Can you add some example the the `dialects.rst` or is this already contained in the implementation PR?
   
   Not that found of symlinks, dunno how git will handle those.  Aren't http references an option?  I saw it's also used within rst.


-- 
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


Re: [PR] Added ADR document describing why the notion of dialects was introduced in the common sql provider [airflow]

Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal merged PR #45456:
URL: https://github.com/apache/airflow/pull/45456


-- 
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


Re: [PR] Added ADR document describing why the notion of dialects was introduced in the common sql provider [airflow]

Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on code in PR #45456:
URL: https://github.com/apache/airflow/pull/45456#discussion_r1914851492


##########
docs/apache-airflow-providers-common-sql/dialects.rst:
##########
@@ -0,0 +1,55 @@
+ .. 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.
+
+SQL Dialects
+=============
+
+The :class:`~airflow.providers.common.sql.dialects.dialect.Dialect` offers an abstraction layer between the
+:class:`~airflow.providers.common.sql.hooks.sql.DbApiHook` implementation and the database.  For some database multiple
+connection types are available, like native, ODBC and or JDBC.  As the :class:`~airflow.providers.odbc.hooks.odbc.OdbcHook`
+and the :class:`~airflow.providers.jdbc.hooks.jdbc.JdbcHook` are generic hooks which allows you to interact with any
+database that has a driver for it, it needed an abstraction layer which allows us to run specialized queries
+depending of the database to which we connect and that's why dialects where introduced.
+
+The default :class:`~airflow.providers.common.sql.dialects.dialect.Dialect` class has following operations
+available which underneath use SQLAlchemy to execute, but can be overloaded with specialized implementations
+per database:
+
+- ``placeholder`` specifies the database specific placeholder used in prepared statements (default: ``%s``);
+- ``inspector`` returns the SQLAlchemy inspector which allows us to retrieve database metadata;
+- ``extract_schema_from_table`` allows us to extract the schema name from a string.
+- ``get_column_names`` returns the column names for the given table and schema (optional) using the SQLAlchemy inspector.
+- ``get_primary_keys`` returns the primary keys for the given table and schema (optional) using the SQLAlchemy inspector.
+- ``get_target_fields`` returns the columns names that aren't identity or auto incremented columns, this will be used by the insert_rows method of the :class:`~airflow.providers.common.sql.hooks.sql.DbApiHook` if the target_fields parameter wasn't specified and the Airflow property ``core.dbapihook_resolve_target_fields`` is set to True (default: False).
+- ``reserved_words`` returns the reserved words in SQL for the target database using the SQLAlchemy inspector.
+- ``generate_insert_sql`` generates the insert SQL statement for the target database.
+- ``generate_replace_sql`` generates the upsert SQL statement for the target database.
+
+At the moment there are only 3 dialects available:
+
+- ``default`` :class:`~airflow.providers.common.sql.dialects.dialect.Dialect` reuses the generic functionality that was already available in the :class:`~airflow.providers.common.sql.hooks.sql.DbApiHook`;
+- ``mssql`` :class:`~airflow.providers.microsoft.mssql.dialects.mssql.MsSqlDialect` specialized for Microsoft SQL Server;
+- ``postgresql`` :class:`~airflow.providers.postgres.dialects.postgres.PostgresDialect` specialized for PostgreSQL;
+
+The dialect to be used will be derived from the connection string, which sometimes won't be possible.  There is always
+the possibility to specify the dialect name through the extra options of the connection:
+
+.. code-block::
+
+  dialect_name: 'mssql'

Review Comment:
   I don't think this can pass the doc build... i think you have to create a separate doc for each one but if the doc build passes I am fine with it



-- 
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


Re: [PR] Added ADR document describing why the notion of dialects was introduced in the common sql provider [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #45456:
URL: https://github.com/apache/airflow/pull/45456#discussion_r1913258478


##########
docs/apache-airflow-providers-common-sql/dialects.rst:
##########
@@ -0,0 +1,55 @@
+ .. 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.
+
+SQL Dialects
+=============
+
+The :class:`~airflow.providers.common.sql.dialects.dialect.Dialect` offers an abstraction layer between the
+:class:`~airflow.providers.common.sql.hooks.sql.DbApiHook` implementation and the database.  For some database multiple
+connection types are available, like native, ODBC and or JDBC.  As the :class:`~airflow.providers.odbc.hooks.odbc.OdbcHook`
+and the :class:`~airflow.providers.jdbc.hooks.jdbc.JdbcHook` are generic hooks which allows you to interact with any
+database that has a driver for it, it needed an abstraction layer which allows us to run specialized queries
+depending of the database to which we connect and that's why dialects where introduced.
+
+The default :class:`~airflow.providers.common.sql.dialects.dialect.Dialect` class has following operations
+available which underneath use SQLAlchemy to execute, but can be overloaded with specialized implementations
+per database:
+
+- ``placeholder`` specifies the database specific placeholder used in prepared statements (default: ``%s``);
+- ``inspector`` returns the SQLAlchemy inspector which allows us to retrieve database metadata;
+- ``extract_schema_from_table`` allows us to extract the schema name from a string.
+- ``get_column_names`` returns the column names for the given table and schema (optional) using the SQLAlchemy inspector.
+- ``get_primary_keys`` returns the primary keys for the given table and schema (optional) using the SQLAlchemy inspector.
+- ``get_target_fields`` returns the columns names that aren't identity or auto incremented columns, this will be used by the insert_rows method of the :class:`~airflow.providers.common.sql.hooks.sql.DbApiHook` if the target_fields parameter wasn't specified and the Airflow property ``core.dbapihook_resolve_target_fields`` is set to True (default: False).
+- ``reserved_words`` returns the reserved words in SQL for the target database using the SQLAlchemy inspector.
+- ``generate_insert_sql`` generates the insert SQL statement for the target database.
+- ``generate_replace_sql`` generates the upsert SQL statement for the target database.
+
+At the moment there are only 3 dialects available:
+
+- ``default`` :class:`~airflow.providers.common.sql.dialects.dialect.Dialect` reuses the generic functionality that was already available in the :class:`~airflow.providers.common.sql.hooks.sql.DbApiHook`;
+- ``mssql`` :class:`~airflow.providers.microsoft.mssql.dialects.mssql.MsSqlDialect` specialized for Microsoft SQL Server;
+- ``postgresql`` :class:`~airflow.providers.postgres.dialects.postgres.PostgresDialect` specialized for PostgreSQL;
+
+The dialect to be used will be derived from the connection string, which sometimes won't be possible.  There is always
+the possibility to specify the dialect name through the extra options of the connection:
+
+.. code-block::
+
+  dialect_name: 'mssql'

Review Comment:
   I will look at it shortly. Been kinda busy with refactors and stuff :)



-- 
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


Re: [PR] Added ADR document describing why the notion of dialects was introduced in the common sql provider [airflow]

Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on PR #45456:
URL: https://github.com/apache/airflow/pull/45456#issuecomment-2582244947

   @dabla do you intend to add the docs in this 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


Re: [PR] Added ADR document describing why the notion of dialects was introduced in the common sql provider [airflow]

Posted by "dabla (via GitHub)" <gi...@apache.org>.
dabla commented on PR #45456:
URL: https://github.com/apache/airflow/pull/45456#issuecomment-2593116034

   > Sorry, "late to the party" now digging through my backlog... wanted to review earlier.
   > 
   > I really like this and the description.
   > 
   > For the docs build problem I see two options: (1) [not sure if this really works] outside the TOC create a small RST just with the heading and pointing to the other doctree - or (2) if you want to have exactly the same doc in each provider then use a symlink in GIT such that the RST is maintained in one place and each doctree has the same. (As long as we not move the providers the symlink is working in Sphinx)
   > 
   > Can you add some example the the `dialects.rst` or is this already contained in the implementation PR?
   
   Will look into it.  I also opened another [PR](https://github.com/apache/airflow/pull/45640) related to this which fixes an issue with reserverd words and special characters using the dialects.


-- 
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


Re: [PR] Added ADR document describing why the notion of dialects was introduced in the common sql provider [airflow]

Posted by "dabla (via GitHub)" <gi...@apache.org>.
dabla commented on PR #45456:
URL: https://github.com/apache/airflow/pull/45456#issuecomment-2611789814

   @eladkal I've duplicated the dialects document in each provider, we can sort it out later as you mentioned


-- 
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


Re: [PR] Added ADR document describing why the notion of dialects was introduced in the common sql provider [airflow]

Posted by "dabla (via GitHub)" <gi...@apache.org>.
dabla commented on PR #45456:
URL: https://github.com/apache/airflow/pull/45456#issuecomment-2574792606

   @potiuk @jscheffl As promised a document describing the dialects in common sql provider.  If any remarks or improvements are necessary please let me know.


-- 
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


Re: [PR] Added ADR document describing why the notion of dialects was introduced in the common sql provider [airflow]

Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on PR #45456:
URL: https://github.com/apache/airflow/pull/45456#issuecomment-2575552411

   > Where should I put those user facing docs? Is that under docs/apache-airflow-providers-common-sql? Should I create a new dialects.rst document?
   
   Yes. You dont have to create new .rst 
   Depends on the scope of what you write. If its 1-2 pargraphs it might fit in the current rst we have


-- 
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


Re: [PR] Added ADR document describing why the notion of dialects was introduced in the common sql provider [airflow]

Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on code in PR #45456:
URL: https://github.com/apache/airflow/pull/45456#discussion_r1912837609


##########
docs/apache-airflow-providers-common-sql/dialects.rst:
##########
@@ -0,0 +1,55 @@
+ .. 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.
+
+SQL Dialects
+=============
+
+The :class:`~airflow.providers.common.sql.dialects.dialect.Dialect` offers an abstraction layer between the
+:class:`~airflow.providers.common.sql.hooks.sql.DbApiHook` implementation and the database.  For some database multiple
+connection types are available, like native, ODBC and or JDBC.  As the :class:`~airflow.providers.odbc.hooks.odbc.OdbcHook`
+and the :class:`~airflow.providers.jdbc.hooks.jdbc.JdbcHook` are generic hooks which allows you to interact with any
+database that has a driver for it, it needed an abstraction layer which allows us to run specialized queries
+depending of the database to which we connect and that's why dialects where introduced.
+
+The default :class:`~airflow.providers.common.sql.dialects.dialect.Dialect` class has following operations
+available which underneath use SQLAlchemy to execute, but can be overloaded with specialized implementations
+per database:
+
+- ``placeholder`` specifies the database specific placeholder used in prepared statements (default: ``%s``);
+- ``inspector`` returns the SQLAlchemy inspector which allows us to retrieve database metadata;
+- ``extract_schema_from_table`` allows us to extract the schema name from a string.
+- ``get_column_names`` returns the column names for the given table and schema (optional) using the SQLAlchemy inspector.
+- ``get_primary_keys`` returns the primary keys for the given table and schema (optional) using the SQLAlchemy inspector.
+- ``get_target_fields`` returns the columns names that aren't identity or auto incremented columns, this will be used by the insert_rows method of the :class:`~airflow.providers.common.sql.hooks.sql.DbApiHook` if the target_fields parameter wasn't specified and the Airflow property ``core.dbapihook_resolve_target_fields`` is set to True (default: False).
+- ``reserved_words`` returns the reserved words in SQL for the target database using the SQLAlchemy inspector.
+- ``generate_insert_sql`` generates the insert SQL statement for the target database.
+- ``generate_replace_sql`` generates the upsert SQL statement for the target database.
+
+At the moment there are only 3 dialects available:
+
+- ``default`` :class:`~airflow.providers.common.sql.dialects.dialect.Dialect` reuses the generic functionality that was already available in the :class:`~airflow.providers.common.sql.hooks.sql.DbApiHook`;
+- ``mssql`` :class:`~airflow.providers.microsoft.mssql.dialects.mssql.MsSqlDialect` specialized for Microsoft SQL Server;
+- ``postgresql`` :class:`~airflow.providers.postgres.dialects.postgres.PostgresDialect` specialized for PostgreSQL;
+
+The dialect to be used will be derived from the connection string, which sometimes won't be possible.  There is always
+the possibility to specify the dialect name through the extra options of the connection:
+
+.. code-block::
+
+  dialect_name: 'mssql'

Review Comment:
   This is nice!
   I think we better also have dialects.rst in the docs of `mssql` and `postgres`  and reference the guide from here.
   Some users may land directly in the relevant docs rather than in the common.sql doc



-- 
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


Re: [PR] Added ADR document describing why the notion of dialects was introduced in the common sql provider [airflow]

Posted by "dabla (via GitHub)" <gi...@apache.org>.
dabla commented on PR #45456:
URL: https://github.com/apache/airflow/pull/45456#issuecomment-2582273539

   > @dabla do you intend to add the docs in this PR?
   
   I just [committed ](https://github.com/apache/airflow/pull/45456/commits/3b78ec318097f90f03d65ea3ebe5d0ae1a3a8e5f) a new dialects.rst and adapted the index.rst this morning, what do you think about it?


-- 
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


Re: [PR] Added ADR document describing why the notion of dialects was introduced in the common sql provider [airflow]

Posted by "dabla (via GitHub)" <gi...@apache.org>.
dabla commented on code in PR #45456:
URL: https://github.com/apache/airflow/pull/45456#discussion_r1914712330


##########
docs/apache-airflow-providers-common-sql/dialects.rst:
##########
@@ -0,0 +1,55 @@
+ .. 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.
+
+SQL Dialects
+=============
+
+The :class:`~airflow.providers.common.sql.dialects.dialect.Dialect` offers an abstraction layer between the
+:class:`~airflow.providers.common.sql.hooks.sql.DbApiHook` implementation and the database.  For some database multiple
+connection types are available, like native, ODBC and or JDBC.  As the :class:`~airflow.providers.odbc.hooks.odbc.OdbcHook`
+and the :class:`~airflow.providers.jdbc.hooks.jdbc.JdbcHook` are generic hooks which allows you to interact with any
+database that has a driver for it, it needed an abstraction layer which allows us to run specialized queries
+depending of the database to which we connect and that's why dialects where introduced.
+
+The default :class:`~airflow.providers.common.sql.dialects.dialect.Dialect` class has following operations
+available which underneath use SQLAlchemy to execute, but can be overloaded with specialized implementations
+per database:
+
+- ``placeholder`` specifies the database specific placeholder used in prepared statements (default: ``%s``);
+- ``inspector`` returns the SQLAlchemy inspector which allows us to retrieve database metadata;
+- ``extract_schema_from_table`` allows us to extract the schema name from a string.
+- ``get_column_names`` returns the column names for the given table and schema (optional) using the SQLAlchemy inspector.
+- ``get_primary_keys`` returns the primary keys for the given table and schema (optional) using the SQLAlchemy inspector.
+- ``get_target_fields`` returns the columns names that aren't identity or auto incremented columns, this will be used by the insert_rows method of the :class:`~airflow.providers.common.sql.hooks.sql.DbApiHook` if the target_fields parameter wasn't specified and the Airflow property ``core.dbapihook_resolve_target_fields`` is set to True (default: False).
+- ``reserved_words`` returns the reserved words in SQL for the target database using the SQLAlchemy inspector.
+- ``generate_insert_sql`` generates the insert SQL statement for the target database.
+- ``generate_replace_sql`` generates the upsert SQL statement for the target database.
+
+At the moment there are only 3 dialects available:
+
+- ``default`` :class:`~airflow.providers.common.sql.dialects.dialect.Dialect` reuses the generic functionality that was already available in the :class:`~airflow.providers.common.sql.hooks.sql.DbApiHook`;
+- ``mssql`` :class:`~airflow.providers.microsoft.mssql.dialects.mssql.MsSqlDialect` specialized for Microsoft SQL Server;
+- ``postgresql`` :class:`~airflow.providers.postgres.dialects.postgres.PostgresDialect` specialized for PostgreSQL;
+
+The dialect to be used will be derived from the connection string, which sometimes won't be possible.  There is always
+the possibility to specify the dialect name through the extra options of the connection:
+
+.. code-block::
+
+  dialect_name: 'mssql'

Review Comment:
   > @dabla can you add `dialects.rst` also in mysql and postgres providers?
   
   I've added the reference to dialects in both indexes.



-- 
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


Re: [PR] Added ADR document describing why the notion of dialects was introduced in the common sql provider [airflow]

Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on PR #45456:
URL: https://github.com/apache/airflow/pull/45456#issuecomment-2574906432

   NIce! I think we are also missing user facing docs about what dialects are and how to use them (ADR is for Airflow developers to document our design choices but it's not really a user facing doc)


-- 
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


Re: [PR] Added ADR document describing why the notion of dialects was introduced in the common sql provider [airflow]

Posted by "dabla (via GitHub)" <gi...@apache.org>.
dabla commented on PR #45456:
URL: https://github.com/apache/airflow/pull/45456#issuecomment-2575030319

   > NIce! I think we are also missing user facing docs about what dialects are and how to use them (ADR is for Airflow developers to document our design choices but it's not really a user facing doc)
   
   Where should I put those user facing docs?  Is that under docs/apache-airflow-providers-common-sql?  Should I create a new dialects.rst document?


-- 
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


Re: [PR] Added ADR document describing why the notion of dialects was introduced in the common sql provider [airflow]

Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on code in PR #45456:
URL: https://github.com/apache/airflow/pull/45456#discussion_r1914526259


##########
docs/apache-airflow-providers-common-sql/dialects.rst:
##########
@@ -0,0 +1,55 @@
+ .. 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.
+
+SQL Dialects
+=============
+
+The :class:`~airflow.providers.common.sql.dialects.dialect.Dialect` offers an abstraction layer between the
+:class:`~airflow.providers.common.sql.hooks.sql.DbApiHook` implementation and the database.  For some database multiple
+connection types are available, like native, ODBC and or JDBC.  As the :class:`~airflow.providers.odbc.hooks.odbc.OdbcHook`
+and the :class:`~airflow.providers.jdbc.hooks.jdbc.JdbcHook` are generic hooks which allows you to interact with any
+database that has a driver for it, it needed an abstraction layer which allows us to run specialized queries
+depending of the database to which we connect and that's why dialects where introduced.
+
+The default :class:`~airflow.providers.common.sql.dialects.dialect.Dialect` class has following operations
+available which underneath use SQLAlchemy to execute, but can be overloaded with specialized implementations
+per database:
+
+- ``placeholder`` specifies the database specific placeholder used in prepared statements (default: ``%s``);
+- ``inspector`` returns the SQLAlchemy inspector which allows us to retrieve database metadata;
+- ``extract_schema_from_table`` allows us to extract the schema name from a string.
+- ``get_column_names`` returns the column names for the given table and schema (optional) using the SQLAlchemy inspector.
+- ``get_primary_keys`` returns the primary keys for the given table and schema (optional) using the SQLAlchemy inspector.
+- ``get_target_fields`` returns the columns names that aren't identity or auto incremented columns, this will be used by the insert_rows method of the :class:`~airflow.providers.common.sql.hooks.sql.DbApiHook` if the target_fields parameter wasn't specified and the Airflow property ``core.dbapihook_resolve_target_fields`` is set to True (default: False).
+- ``reserved_words`` returns the reserved words in SQL for the target database using the SQLAlchemy inspector.
+- ``generate_insert_sql`` generates the insert SQL statement for the target database.
+- ``generate_replace_sql`` generates the upsert SQL statement for the target database.
+
+At the moment there are only 3 dialects available:
+
+- ``default`` :class:`~airflow.providers.common.sql.dialects.dialect.Dialect` reuses the generic functionality that was already available in the :class:`~airflow.providers.common.sql.hooks.sql.DbApiHook`;
+- ``mssql`` :class:`~airflow.providers.microsoft.mssql.dialects.mssql.MsSqlDialect` specialized for Microsoft SQL Server;
+- ``postgresql`` :class:`~airflow.providers.postgres.dialects.postgres.PostgresDialect` specialized for PostgreSQL;
+
+The dialect to be used will be derived from the connection string, which sometimes won't be possible.  There is always
+the possibility to specify the dialect name through the extra options of the connection:
+
+.. code-block::
+
+  dialect_name: 'mssql'

Review Comment:
   @dabla can you add `dialects.rst` also in mysql and postgres providers?



-- 
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