Skip to content

Conversation

@amoghrajesh
Copy link
Contributor

@amoghrajesh amoghrajesh commented Mar 1, 2025

Adding regex validation for table names


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

Copy link
Contributor

@bugraoz93 bugraoz93 left a comment

Choose a reason for hiding this comment

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

This would definitely cause problems. Nice catch Amogh!
I think there are a few more places to add the ticks that you might be already aware

f"SELECT * INTO OUTFILE %s FROM {table}",

f"LOAD DATA LOCAL INFILE %s %s INTO TABLE {table} %s",

@amoghrajesh
Copy link
Contributor Author

Hey @bugraoz93, thank you. I will update those backticks in a follow up PR. I wanted to do soemthing else here :)

@amoghrajesh amoghrajesh changed the title Adding double ticks to tablename in bulk_load Add regex validation for table names in 'bulk_load' Mar 1, 2025
@amoghrajesh amoghrajesh requested a review from potiuk March 1, 2025 14:33
@bugraoz93
Copy link
Contributor

Hey @bugraoz93, thank you. I will update those backticks in a follow up PR. I wanted to do soemthing else here :)

Great, thanks for the context! I didn't know the context, so I just threw my thoughts on it :)

@amoghrajesh
Copy link
Contributor Author

No worries, entirely valid!

@potiuk potiuk merged commit 4e8fa92 into apache:main Mar 1, 2025
60 checks passed
shahar1 pushed a commit to shahar1/airflow that referenced this pull request Mar 5, 2025
nailo2c pushed a commit to nailo2c/airflow that referenced this pull request Apr 4, 2025
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.

3 participants