Skip to content

fix: prevent SQL keyword parameterization in MySQL bulk_load_custom - #63530

Merged
potiuk merged 6 commits into
apache:mainfrom
majiayu000:fix/issue-62506-mysql-bulk-load-parameterization
Apr 6, 2026
Merged

fix: prevent SQL keyword parameterization in MySQL bulk_load_custom#63530
potiuk merged 6 commits into
apache:mainfrom
majiayu000:fix/issue-62506-mysql-bulk-load-parameterization

Conversation

@majiayu000

Copy link
Copy Markdown
Contributor

Fixes incorrect SQL keyword parameterization in MySqlHook.bulk_load_custom() that causes MySQL drivers to quote IGNORE/REPLACE keywords as string literals, producing invalid SQL.

Problem

bulk_load_custom() passes duplicate_key_handling (e.g., IGNORE, REPLACE) and extra_options as parameterized query values via cursor.execute(sql, parameters). The MySQL driver treats parameterized values as data and quotes them as string literals, producing invalid SQL like:

LOAD DATA LOCAL INFILE '/tmp/file' 'IGNORE' INTO TABLE `my_table` 'FIELDS TERMINATED BY ...'

This was introduced in PR #33328 which changed from string concatenation to parameterization for these keywords.

Root Cause

In providers/mysql/src/airflow/providers/mysql/hooks/mysql.py, the bulk_load_custom method builds the SQL as:

sql_statement = f"LOAD DATA LOCAL INFILE %s %s INTO TABLE `{table}` %s"
parameters = (tmp_file, duplicate_key_handling, extra_options)

Both duplicate_key_handling and extra_options are SQL syntax keywords, not data values. Only tmp_file is actual data that should be parameterized.

Fix

Changed bulk_load_custom to interpolate duplicate_key_handling and extra_options directly into the SQL statement via f-string, while keeping tmp_file as the sole parameterized value:

sql_statement = f"LOAD DATA LOCAL INFILE %s {duplicate_key_handling} INTO TABLE `{table}` {extra_options}"
parameters = (tmp_file,)

Updated existing tests (test_bulk_load_custom, test_bulk_load_custom_hook_lineage) to assert the new SQL shape and parameter tuple. Added a new parametrized test test_bulk_load_custom_duplicate_key_not_parameterized that validates both IGNORE and REPLACE appear literally in the executed SQL and only tmp_file is parameterized.

closes: #62506


Was generative AI tooling used to co-author this PR?
  • Yes — Claude Opus 4.6

Generated-by: Claude Opus 4.6 following the guidelines

The bulk_load_custom method was incorrectly passing duplicate_key_handling
and extra_options as parameterized values to cursor.execute(). MySQL drivers
treat parameterized values as data and quote them as string literals, producing
invalid SQL like:

  LOAD DATA LOCAL INFILE '/tmp/file' 'IGNORE' INTO TABLE `table` 'FIELDS...'

This was introduced in PR apache#33328 which changed from string concatenation to
parameterization. However, duplicate_key_handling (IGNORE/REPLACE) and
extra_options are SQL syntax keywords, not data values.

Fixed by interpolating these keywords directly into the SQL statement via
f-string while keeping tmp_file as the sole parameterized value:

  LOAD DATA LOCAL INFILE %s IGNORE INTO TABLE `table` FIELDS...

Updated existing tests and added parametrized test to verify both IGNORE
and REPLACE keywords appear literally in SQL without being parameterized.

Closes: apache#62506

Signed-off-by: majiayu000 <1835304752@qq.com>
@boring-cyborg

boring-cyborg Bot commented Mar 13, 2026

Copy link
Copy Markdown

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
Here are some useful points:

  • Pay attention to the quality of your code (ruff, mypy and type annotations). Our prek-hooks will help you with that.
  • In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it.
  • Consider using Breeze environment for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
  • Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
  • Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
  • Be sure to read the Airflow Coding style.
  • Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
    Apache Airflow is a community-driven project and together we are making it better 🚀.
    In case of doubts contact the developers at:
    Mailing List: dev@airflow.apache.org
    Slack: https://s.apache.org/airflow-slack

@SameerMesiah97 SameerMesiah97 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix CI and possibly a minor spacing issue and this should be good enough to merge.

Comment thread providers/mysql/src/airflow/providers/mysql/hooks/mysql.py
Comment thread providers/mysql/tests/unit/mysql/hooks/test_mysql.py
@potiuk
potiuk merged commit 01603f2 into apache:main Apr 6, 2026
94 checks passed
@boring-cyborg

boring-cyborg Bot commented Apr 6, 2026

Copy link
Copy Markdown

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

shivaam pushed a commit to shivaam/airflow that referenced this pull request Apr 8, 2026
…pache#63530)

* fix: prevent SQL keyword parameterization in MySQL bulk_load_custom

The bulk_load_custom method was incorrectly passing duplicate_key_handling
and extra_options as parameterized values to cursor.execute(). MySQL drivers
treat parameterized values as data and quote them as string literals, producing
invalid SQL like:

  LOAD DATA LOCAL INFILE '/tmp/file' 'IGNORE' INTO TABLE `table` 'FIELDS...'

This was introduced in PR apache#33328 which changed from string concatenation to
parameterization. However, duplicate_key_handling (IGNORE/REPLACE) and
extra_options are SQL syntax keywords, not data values.

Fixed by interpolating these keywords directly into the SQL statement via
f-string while keeping tmp_file as the sole parameterized value:

  LOAD DATA LOCAL INFILE %s IGNORE INTO TABLE `table` FIELDS...

Updated existing tests and added parametrized test to verify both IGNORE
and REPLACE keywords appear literally in SQL without being parameterized.

Closes: apache#62506

Signed-off-by: majiayu000 <1835304752@qq.com>

* fix: address review feedback on test formatting and spacing

Signed-off-by: majiayu000 <1835304752@qq.com>

* fix: clean up spacing in bulk_load_custom test assertion

Signed-off-by: majiayu000 <1835304752@qq.com>

* fix: apply ruff format to mysql.py for CI compliance

Signed-off-by: majiayu000 <1835304752@qq.com>

---------

Signed-off-by: majiayu000 <1835304752@qq.com>
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The bulk_load_custom in MySQL hook incorrectly parameterized duplicate_key_handling

3 participants