Skip to content

[Bug] ApplicationBackUpCleanTask Runtime Error#4253

Merged
wolfboys merged 2 commits intoapache:devfrom
Gianzie:mysql_sql_mode_error
Jun 8, 2025
Merged

[Bug] ApplicationBackUpCleanTask Runtime Error#4253
wolfboys merged 2 commits intoapache:devfrom
Gianzie:mysql_sql_mode_error

Conversation

@Gianzie
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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!

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Jun 8, 2025

Copy link
Copy Markdown
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