Skip to content

Triggers: support managing columns for data iteration#7541

Open
labkey-nicka wants to merge 14 commits intodevelopfrom
fb_trigger_columns
Open

Triggers: support managing columns for data iteration#7541
labkey-nicka wants to merge 14 commits intodevelopfrom
fb_trigger_columns

Conversation

@labkey-nicka
Copy link
Copy Markdown
Contributor

@labkey-nicka labkey-nicka commented Apr 2, 2026

Rationale

When query triggers are called within data iterators they are allowed to manipulate the data of the row that is being procured. However, if a trigger modifies the columns (i.e., the keys of the row map) it can result in data being mismanaged due to the columns in the row not aligning with what the data iterator specified.

This introduces the notion of "managed" columns to triggers. Triggers can, at the outset, declare which columns they are going to manage. Any managed columns are then picked up by the data iterator during initialization to ensure they are persisted correctly. To enforce this management more strictly, and to avoid mismanagement of null values, triggers are required to fulfill all managed column values by either clearing the value or setting the value. After the trigger has fired the resulting row will be validated and checked. The result is consistent enforcement and acknowledgement of triggers that manipulate the shape of the data.

Related Pull Requests

Changes

  • Declare TableInfo.getTriggerManagedColumns() to allow for data iterator to gather all managed columns
  • Add difference checks for non-managed columns added or removed by triggers
  • Support managed columns in server-side JS trigger scripts
  • Add utilities to ensure managed column values are set (setInsertManagedColumns() and setUpdateManagedColumns())
  • Add experimental feature flag which when enabled will disable the managed trigger columns feature

Tasks

  • Initial review @labkey-matthewb
  • Hook up server-side JS triggers
  • Only throw errors in development for non-data iterator triggers
  • Add experimental feature flag to disable
  • Optimize row checks
  • Needs automation
  • Code review @mbellew
  • Manual testing (needs volunteer) 📍
  • Verify fix

@labkey-nicka labkey-nicka self-assigned this Apr 2, 2026
var triggerColumns = _target.getTriggerManagedColumns(_c);
if (triggerColumns != null && !triggerColumns.isEmpty())
{
var columns = triggerColumns.stream().map(_target::getColumn).filter(Objects::nonNull).toList();
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.

I think this should probably be a failure.

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.

I think we want to just resolve the known columns and not error on unknown columns as the script may contain "columns" that are not in the underlying table (e.g., "annotations").

if (errors.hasErrors())
break;

if (newRow != null && before)
Copy link
Copy Markdown
Contributor

@labkey-matthewb labkey-matthewb Apr 2, 2026

Choose a reason for hiding this comment

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

validate that all managed columns are set for update trigger

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.

I've switched it to only validate managed columns for update.

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.

I've re-reverted this to check managed columns for both insert/update.

Copy link
Copy Markdown
Contributor

@labkey-matthewb labkey-matthewb left a comment

Choose a reason for hiding this comment

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

Considerations for additional features (do we want to do now or never)?

  • Support different managed columns for insert/update
  • Support in JavaScript triggers

@labkey-nicka labkey-nicka modified the milestones: 26.04, 26.05 Apr 3, 2026
@labkey-nicka labkey-nicka marked this pull request as ready for review April 3, 2026 17:15
@labkey-nicka
Copy link
Copy Markdown
Contributor Author

Support different managed columns for insert/update
Support in JavaScript triggers

Implemented both of these.

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.

2 participants