Skip to content

Conversation

@dabla
Copy link
Contributor

@dabla dabla commented Jan 14, 2025

Not only fixes escaping of reserved words or special characters as column names but also fixes un-escaping of column names or table names.

Unit tests have been added accordingly.

Also allow "escape_word_format", "insert_statement_format" and "replace_statement_format" to be defined in extra of connection, this is handy when using Jdbc or Odbc based hooks.


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
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.

davidblain-infrabel and others added 2 commits January 14, 2025 19:19
@dabla dabla changed the title Fixes escaping of reserved words as column names in MSSQLDialect Fixes escaping of reserved words as column names in dialects Jan 14, 2025
@dabla
Copy link
Contributor Author

dabla commented Jan 14, 2025

@eladkal Would it hurt if some methods in Dialects are being renamed? I don't think it's actually used yet by any released provider right?

I'm thinking of renaming following 2 methods:

escape_colmun_name -> escape_word
remove_quotes -> unescape_word

@dabla dabla changed the title Fixes escaping of reserved words as column names in dialects Fix escaping of reserved words as column names in dialects of common sql provider Jan 14, 2025
@dabla dabla changed the title Fix escaping of reserved words as column names in dialects of common sql provider Fix escaping of special characters or reserved words as column names in dialects of common sql provider Jan 15, 2025
@dabla dabla marked this pull request as ready for review January 15, 2025 15:07
@dabla dabla marked this pull request as draft January 23, 2025 12:26
@dabla dabla marked this pull request as ready for review January 24, 2025 07:06
@eladkal eladkal requested a review from potiuk January 24, 2025 10:33
@eladkal
Copy link
Contributor

eladkal commented Jan 24, 2025

cc @potiuk any comments on this one? If not happy to merge it

@eladkal
Copy link
Contributor

eladkal commented Jan 26, 2025

@dabla can you rebase and resolve conflicts? I will merge after

@dabla
Copy link
Contributor Author

dabla commented Jan 26, 2025

@dabla can you rebase and resolve conflicts? I will merge after

just rebased and merged with main

@eladkal eladkal merged commit 17d3a60 into apache:main Jan 26, 2025
61 checks passed
@utkarsharma2 utkarsharma2 added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Jan 27, 2025
got686-yandex pushed a commit to got686-yandex/airflow that referenced this pull request Jan 30, 2025
…in dialects of common sql provider (apache#45640)

* refactor: Make sure reserved words in column names are all escaped in the generate_replace_sql method of MSSQLDialect

---------

Co-authored-by: David Blain <david.blain@infrabel.be>
niklasr22 pushed a commit to niklasr22/airflow that referenced this pull request Feb 8, 2025
…in dialects of common sql provider (apache#45640)

* refactor: Make sure reserved words in column names are all escaped in the generate_replace_sql method of MSSQLDialect

---------

Co-authored-by: David Blain <david.blain@infrabel.be>
ambika-garg pushed a commit to ambika-garg/airflow that referenced this pull request Feb 17, 2025
…in dialects of common sql provider (apache#45640)

* refactor: Make sure reserved words in column names are all escaped in the generate_replace_sql method of MSSQLDialect

---------

Co-authored-by: David Blain <david.blain@infrabel.be>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providers changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) provider:common-sql provider:microsoft-mssql

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants