Skip to content

fix: data_input_data column-expansion bugs in template copy and source duplicate#7132

Merged
TheWitness merged 7 commits into
Cacti:developfrom
somethingwithproof:fix/data-input-data-template-copy-develop
May 13, 2026
Merged

fix: data_input_data column-expansion bugs in template copy and source duplicate#7132
TheWitness merged 7 commits into
Cacti:developfrom
somethingwithproof:fix/data-input-data-template-copy-develop

Conversation

@somethingwithproof
Copy link
Copy Markdown
Contributor

The develop schema added data_template_id, local_data_id, host_id columns to data_input_data. Two write paths were left in inconsistent states.

change_data_template() in lib/template.php copied \$item['data_template_id'], \$item['local_data_id'], \$item['host_id'] straight from the parent template row. Template rows hold 0 for those columns, so child data_input_data rows ended up orphaned from the new data source. Replace with the function-scope \$data_template_id, \$local_data_id, and a fresh host_id lookup against data_local for the new row.

api_data_source_duplicate() in lib/api_data_source.php expanded the INSERT IGNORE column list to seven columns but kept only four ? placeholders against seven bound values, so db_execute_prepared() rejected every duplicate operation. Match the placeholder count to the column list.

Both bugs cascade into update_poller_cache() seeing empty/orphaned data_input_data and producing no poller_item rows. Matches what TheWitness reported on Slack about manual data source creation in advanced mode and graph template + localhost not populating poller_item — though that thread was about 1.2.31, so this PR targets only the develop bugs (1.2.x is unaffected; its data_input_data schema still has only the four columns and both call sites already match).

Test in tests/Unit/DataInputDataTemplateCopyTest.php extracts both function bodies via regex and asserts column count == placeholder count, and that the change_data_template() bind list uses the function-scope identity tuple instead of the parent row's \$item[...] readback.

Closes #7131

…d api_data_source_duplicate

The develop schema added data_template_id, local_data_id, host_id columns
to data_input_data. Two write paths were left in inconsistent states:

change_data_template() copied $item['data_template_id'], $item['local_data_id'],
$item['host_id'] from the parent template row (always 0 for templates),
orphaning the child data_input_data rows from the new data source. Replace
with the function's own $data_template_id, $local_data_id, and a fresh
host_id lookup against data_local for the new row.

api_data_source_duplicate() expanded the INSERT IGNORE column list to seven
columns but kept only four ? placeholders against seven bound values, so
db_execute_prepared() rejected every duplicate operation. Match the
placeholder count to the column list.

Both bugs cascade into update_poller_cache() seeing empty/orphaned
data_input_data and producing no poller_item rows. 1.2.x is unaffected
(its schema still has only four columns and both call sites already match).

Closes Cacti#7131

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Copy link
Copy Markdown
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 develop-branch regressions after data_input_data gained denormalized identity columns (data_template_id, local_data_id, host_id), ensuring template application and data source duplication correctly persist data_input_data so downstream poller cache generation can work.

Changes:

  • change_data_template() now stamps copied data_input_data rows with the child data source’s identity tuple (and looks up host_id from data_local).
  • api_data_source_duplicate() fixes the INSERT IGNORE placeholder count to match the expanded 7-column insert.
  • Adds a source-level unit test to guard against future column/placeholder and identity-tuple regressions; updates CHANGELOG.

Reviewed changes

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

File Description
tests/Unit/DataInputDataTemplateCopyTest.php Adds regression tests that scan source for correct data_input_data insert shapes/binds.
lib/template.php Fixes change_data_template() to bind the new child identity columns (and fetch host_id).
lib/api_data_source.php Fixes placeholder count for the 7-column data_input_data insert during duplication.
CHANGELOG Notes the fix under 1.3.0-dev.
Comments suppressed due to low confidence (1)

lib/api_data_source.php:814

  • In api_data_source_duplicate(), the INSERT now has 7 placeholders (good), but the bound values for the new identity columns still come from the source row ($data_input_data['data_template_id'], ['local_data_id'], ['host_id']). After duplication, those columns should match the newly-created row identity (new $data_template_data_id’s data_template_id/local_data_id/host_id), consistent with the 1.3.0 upgrade backfill that sets did.local_data_id/data_template_id from data_template_data and did.host_id from data_local. As-is, the duplicate can insert data_input_data rows whose (data_template_id, local_data_id, host_id) point at the original object (or 0 for template-derived rows), leaving the duplicate effectively without usable input mappings for code paths that use the new indexes.
			db_execute_prepared('INSERT IGNORE INTO data_input_data
				(data_input_field_id, data_template_data_id, data_template_id, local_data_id, host_id, t_value, value)
				VALUES (?, ?, ?, ?, ?, ?, ?)',
				[
					$data_input_data['data_input_field_id'],
					$data_template_data_id,
					$data_input_data['data_template_id'],
					$data_input_data['local_data_id'],
					$data_input_data['host_id'],
					$data_input_data['t_value'],
					$data_input_data['value']
				]

…h write paths

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
…y, not source

Pre-fix: the bind list copied $data_input_data['data_template_id'],
['local_data_id'], ['host_id'] from the SOURCE row. For a duplicated data
source the new rows still pointed at the original source/host; for a
duplicated template the new rows carried the source's local_data_id and
host_id (templates have no device, so both should be 0).

Branch on $_local_data_id: data-source dup uses the new $local_data_id and
$data_local['host_id']; template dup uses 0/0 and the new $data_template_id.
Mirrors the data_template_data save above which already branched the
data_template_id this way.

Refs Cacti#7131

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
…ixes

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
…ock CI

Develop CI gates every PR through PHPStan Level 6, and the eleven errors
in aggregate_graphs.php, color_templates.php, graph_templates.php,
graphs.php and lib/html.php block this PR even though they are unrelated
to the data_input_data column-expansion fix that this PR owns.

Cherry-pick only the 5 source-file changes from Cacti#7138 (61f19b7). Tests
and CHANGELOG entry remain owned by Cacti#7138; once that PR merges, the
overlap will resolve cleanly on rebase.

Refs Cacti#7137

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
TheWitness
TheWitness previously approved these changes May 13, 2026
@TheWitness TheWitness merged commit 4769892 into Cacti:develop May 13, 2026
5 checks passed
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.

data_input_data column-expansion bugs in change_data_template and api_data_source_duplicate (develop)

5 participants