Skip to content

Fix fatal PRIMARY KEY (Array) on fresh install from array primary key - #276

Merged
TheWitness merged 1 commit into
developfrom
fix/create-path-array-primary
Oct 15, 2026
Merged

TheWitness merged 1 commit into
developfrom
fix/create-path-array-primary

Conversation

@TheWitness

Copy link
Copy Markdown
Member

Problem

On a fresh install (and whenever the upgrade path creates a missing table), monitor hands api_plugin_db_table_create() table definitions whose primary is declared in the array form, e.g.:

$data['primary'] = ['id'];      // notify/reboot history, dashboards
$data['primary'] = ['host_id']; // uptime

Down to this plugin''s compat = 1.2.29 floor, api_plugin_db_table_create() (lib/plugins.php) interpolates the primary key straight into the DDL string:

$sql .= ",\n PRIMARY KEY (`" . $data['primary'] . '`)';

With an array, PHP renders it as PRIMARY KEY (`Array`), so the CREATE TABLE fails and the table is never created on Cacti 1.2.29-1.2.31.

This is the fresh-install mirror of the upgrade-path fatal fixed in plugin_routerconfigs #191: api_plugin_db_table_create() requires the scalar primary form, while db_update_table() requires the array form.

Fix

Add a monitor_create_definition() helper that flattens an array primary to the backtick-joined scalar form, and wrap every api_plugin_db_table_create() call with it. The canonical *_table_data() definitions keep the array form that db_update_table() still requires on the upgrade path, so both paths get the form they need. This mirrors the helpers already present in plugin_maint, plugin_apcupsd, plugin_exlist, plugin_lsflog and plugin_stream.

Validation

  • php -l includes/database.php - no syntax errors.
  • Added regression coverage asserting the create API receives a scalar primary while the shared definitions keep the array form.

@TheWitness
TheWitness requested review from bmfmancini and xmacan and a balanced review from Copilot October 15, 2026 13:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The localized conversion fixes table creation while preserving existing-table upgrade behavior, with no identified blocking issues.

0 open findings

What changed in this PR

Fixes table creation on supported Cacti versions by converting primary keys to the scalar format required by the create API.

Changes:

  • Wraps fresh-install and missing-table creation calls with a conversion helper.
  • Preserves array definitions for upgrades and adds regression coverage.
File Description
tests/​Unit/​MonitorSetupTableAndPollerTest.php Verifies scalar creation keys and preserved array definitions.
includes/​database.php Adds and applies primary-key conversion for table creation.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@TheWitness
TheWitness requested a review from xmacan October 15, 2026 13:37
@TheWitness
TheWitness merged commit 487f9b3 into develop Oct 15, 2026
6 checks passed
@TheWitness
TheWitness deleted the fix/create-path-array-primary branch October 15, 2026 13:37
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.

3 participants