Skip to content

fix: execute replication CREATE TABLE SQL from SHOW CREATE output#267

Open
somethingwithproof wants to merge 2 commits intoCacti:developfrom
somethingwithproof:bugfix/replication-create-sql-variable
Open

fix: execute replication CREATE TABLE SQL from SHOW CREATE output#267
somethingwithproof wants to merge 2 commits intoCacti:developfrom
somethingwithproof:bugfix/replication-create-sql-variable

Conversation

@somethingwithproof
Copy link

@somethingwithproof somethingwithproof commented Mar 6, 2026

Summary

  • execute the CREATE TABLE statement text ($create_sql) when a replicated rules table is missing
  • initialize and guard create_sql before execution in syslog_replace_data()
  • if CREATE SQL cannot be derived, log a replication warning and return safely
  • keep the existing truncate + bulk upsert replication flow unchanged
  • add regression coverage for syslog_replace_data() table bootstrap path
  • harden regression test isolation to avoid accidental real DB execution
  • run regression scripts from CI when tests/regression/*.php exist
  • add changelog entry for issue bug: syslog_replace_data executes wrong variable when creating missing table #258

Validation

  • php -l setup.php
  • php -l tests/regression/issue258_replication_create_sql_test.php
  • php tests/regression/issue258_replication_create_sql_test.php

Fixes #258

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 6, 2026 22:31
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes syslog rule replication bootstrap when the destination rules table is missing by executing the actual CREATE TABLE statement obtained from SHOW CREATE TABLE, and adds a regression test + CI coverage to prevent regressions.

Changes:

  • Fix syslog_replace_data() to execute $create_sql (from SHOW CREATE TABLE) when creating a missing replicated rules table.
  • Add a standalone regression script validating the “missing table => CREATE + TRUNCATE + bulk upsert” path.
  • Run regression scripts in CI when tests/regression exists, and add a changelog entry for issue #258.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
setup.php Fixes CREATE TABLE execution during replication bootstrap for missing rules tables.
tests/regression/issue258_replication_create_sql_test.php Adds regression coverage for the missing-table bootstrap path.
.github/workflows/plugin-ci-workflow.yml Runs regression scripts in CI when present.
CHANGELOG.md Documents the fix for issue #258.

Comment on lines 830 to 832
if (!syslog_db_table_exists($table)) {
syslog_db_execute($create);
syslog_db_execute($create_sql);
syslog_db_execute("TRUNCATE TABLE $table");
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

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

$create_sql is only set when the SHOW CREATE TABLE result contains expected keys. If db_fetch_row() returns an empty/false value (e.g., permissions issue, table missing, or unexpected column label), this will call syslog_db_execute($create_sql) with an undefined variable and fail to create the destination table. Initialize $create_sql (e.g., to an empty string) and add a guard that logs/returns early when the CREATE statement cannot be derived before executing it.

Copilot uses AI. Check for mistakes.
Comment on lines +16 to +33
if (!function_exists('db_fetch_row')) {
function db_fetch_row($sql) {
return array('Create Table' => 'CREATE TABLE `syslog_alert` (`id` INT NOT NULL)');
}
}

if (!function_exists('syslog_db_table_exists')) {
function syslog_db_table_exists($table) {
return false;
}
}

if (!function_exists('syslog_db_execute')) {
function syslog_db_execute($sql) {
$GLOBALS['syslog_replace_data_execute_calls'][] = $sql;

return true;
}
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

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

This regression script conditionally defines DB wrapper stubs using function_exists(). If someone runs it in an environment where Cacti/syslog functions are already loaded (so the stubs are skipped), the script will call the real syslog_replace_data() against a real DB and may create/modify syslog rule tables before failing its assertions. Consider adding an explicit safety check up front (e.g., abort when core functions like db_fetch_row/syslog_db_execute already exist) to ensure the test only runs in an isolated/stubbed context.

Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@somethingwithproof
Copy link
Author

Addressed review feedback in the latest commit:

  • initialized and guarded create_sql before execution in syslog_replace_data()
  • when SHOW CREATE TABLE output cannot be derived, log warning and return early instead of executing undefined SQL
  • hardened regression test isolation to abort if DB wrapper functions already exist
  • extended regression coverage to verify safe no-op behavior when CREATE SQL is unavailable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: syslog_replace_data executes wrong variable when creating missing table

2 participants