Skip to content

[Bug] ApplicationBackUpCleanTask Runtime Error #4253

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 8, 2025

Conversation

Gianzie
Copy link
Contributor

@Gianzie Gianzie commented Jun 6, 2025

What changes were proposed in this pull request

In the current code, the table "t_flink_app_backup" will be cleaned at 01:00 everyday, and the sql is "SELECT id,app_id,sql_id,config_id,path,description,version,create_time FROM t_flink_app_backup GROUP BY app_id HAVING count(*) > 5"

But mysql8.0 sql_mode is "ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION", so this clean task is always error.

So I think we can just get "FlinkApplicationBackup::getAppId" column.

Brief change log

Verifying this change

This change is a trivial rework / code cleanup without any test coverage.

(or)

This change is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

Does this pull request potentially affect one of the following parts

  • Dependencies (does it add or upgrade a dependency): no

@Gianzie
Copy link
Contributor Author

Gianzie commented Jun 6, 2025

error log:
Snipaste_2025-06-06_15-46-46

@GOODBOY008 GOODBOY008 self-requested a review June 6, 2025 09:02
GOODBOY008
GOODBOY008 previously approved these changes Jun 8, 2025
Copy link
Member

@GOODBOY008 GOODBOY008 left a comment

Choose a reason for hiding this comment

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

@Gianzie I left a comment.

Copy link
Member

@GOODBOY008 GOODBOY008 left a comment

Choose a reason for hiding this comment

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

@

backUpService.lambdaQuery()
.select(FlinkApplicationBackup::getAppId)
.groupBy(FlinkApplicationBackup::getAppId)
.having("count(*) > " + maxBackupNum)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.having("count(*) > " + maxBackupNum)
.having("COUNT(*) > {0}", maxBackupNum)

Copy link
Member

Choose a reason for hiding this comment

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

@Gianzie Use params to pass value will be better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, you are right, this way of writing can prevent SQL injection, I have made the changes, thx!

Copy link

sonarqubecloud bot commented Jun 8, 2025

Copy link
Member

@wolfboys wolfboys left a comment

Choose a reason for hiding this comment

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

LGTM

@wolfboys wolfboys merged commit 9738a5d into apache:dev Jun 8, 2025
42 checks passed
@Gianzie Gianzie deleted the mysql_sql_mode_error branch June 8, 2025 14:14
Gianzie added a commit to Gianzie/streampark that referenced this pull request Jun 9, 2025
* [Bug] ApplicationBackUpCleanTask Runtime Error

* [Improve] Prevent SQL injection
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants