Skip to content

Additional checks for valid db during db_execute #20560

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 1 commit into from
Jun 24, 2025

Conversation

stelfrag
Copy link
Collaborator

Summary
  • Ensure the database is valid before executing any commands.
  • Update db_execute calls to include an sqlite_rc parameter (currently not used). This will return the actual db error and in future improvements, help refine error handling from the caller.

…se and NULL the databases

Update db_execute calls to include NULL for sqlite_rc parameter. This will return the actual db error and help refine error handling from the caller
@github-actions github-actions bot added area/database area/aclk area/ml Machine Learning Related Issues labels Jun 24, 2025
@stelfrag stelfrag marked this pull request as ready for review June 24, 2025 09:15
(void) db_execute(db_meta,"DELETE FROM health_log WHERE host_id NOT IN (SELECT host_id FROM host)");
(void) db_execute(db_meta,"DELETE FROM health_log_detail WHERE health_log_id NOT IN (SELECT health_log_id FROM health_log)");
(void) db_execute(db_meta,"DELETE FROM alert_version WHERE health_log_id NOT IN (SELECT health_log_id FROM health_log)");
(void)db_execute(db_meta, "DELETE FROM health_log WHERE host_id NOT IN (SELECT host_id FROM host)", NULL);
Copy link
Contributor

Choose a reason for hiding this comment

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

I would like to propose a small improvement for future PRs. Currently, some files use define for queries, while in this file, they are passed directly into functions. To maintain a uniform pattern across the project, I suggest using define everywhere.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure will do in a future PR

Copy link
Contributor

@thiagoftsm thiagoftsm left a comment

Choose a reason for hiding this comment

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

No issues found during runtime, LGTM!

@stelfrag stelfrag merged commit d7809e1 into netdata:master Jun 24, 2025
108 checks passed
@stelfrag stelfrag deleted the check_valid_db branch June 24, 2025 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/aclk area/database area/ml Machine Learning Related Issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants