Rewrite the common.sql connections guide and fix the dialect extra name - #73605
Conversation
The connections page said nothing beyond 'pass a connection ID' and told users to pass hook arguments as operator kwargs, which does not work. It now explains that common.sql reuses database provider connections, how the hook is resolved, the database and hook_params overrides, and the connection extras every DbApiHook reads. The dialects page told users to set dialect_name in the extras, but the hook reads dialect.
dabla
left a comment
There was a problem hiding this comment.
The rewrite is accurate on everything I checked against the source: no connection type of its own, hook resolved from the connection type with the DbApiHook isinstance check and its error message, database overriding the schema (Postgres sets hook.database, everything else hook.schema, same effect), extras merged into hook_params with hook_params winning, SQLSensor and GenericTransfer parameter names, the placeholder whitelist and warning, the SQLite/Presto/Trino ? default, the three statement templates and both escape settings with their defaults, and the dialect_name → dialect key fix on the dialects page. The :doc: cross-references (supported-database-types, apache-airflow-providers-postgres:connections/postgres) resolve with the existing intersphinx mapping.
One accuracy issue remains, and it is the same kind the PR sets out to fix: the dialect extra is documented as the knob for ODBC and JDBC, but DbApiHook.dialect_name prefers the parsed URI and then sqlalchemy_scheme, and OdbcHook always produces a parseable URI (mssql+pyodbc by default). So dialect is ignored for ODBC connections and the ODBC example's "dialect": "mssql" does nothing. Suggested wording is in comment [1]; comment [2] covers the example and the MSSQL provider dependency for MERGE upserts. Both are doc-only changes to lines already in the diff.
This is a diff-based review; deeper cross-file checks were limited to the hook, operator, sensor, dialect and ODBC/JDBC hook sources named above.
DbApiHook takes the dialect from the parsed connection URI first, then sqlalchemy_scheme, and only then the dialect extra. OdbcHook always builds a parseable URI from sqlalchemy_scheme, so the dialect extra is never read for ODBC connections.
The common.sql connections page was two paragraphs that said to pass a connection ID and that other parameters "can be passed as key word args to the operators". Those arguments actually go through
hook_params. The page also never said that common.sql has no connection type of its own, which is the first thing someone reading it needs to know.The rewrite covers what the operators actually do, all taken from
BaseSQLOperatorandDbApiHook:DbApiHook.airflow providers hooksshows what a type resolves to.databaseandhook_paramsoverride settings per task. Connection extras are merged intohook_params, andhook_paramswins on conflict.DbApiHookreads (placeholder,dialect,sqlalchemy_scheme, the insert/replace statement formats and the escape settings) with their defaults. These matter most for ODBC and JDBC connections, where the database can't be inferred from the URI.The dialects page had a real bug. It told users to set
dialect_namein the connection extras, butDbApiHook.dialect_namereads thedialectkey. Anyone who followed the docs got the default dialect with no error.{pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.