Triggers: support managing columns for data iteration#7541
Triggers: support managing columns for data iteration#7541labkey-nicka wants to merge 10 commits intodevelopfrom
Conversation
| for (String col : managedColumns) | ||
| { | ||
| if (row.get(col) == COLUMN_SENTINEL) | ||
| row.remove(col); |
There was a problem hiding this comment.
Not sure this is the correct thing to do. Looking to effectively no-op the column in this case. It might be better (and actually correct) to null the value here instead of removing from the map.
There was a problem hiding this comment.
Do we need clearManagedColumns() if we require all managedColumns to be set?
There was a problem hiding this comment.
I've removed clearManagedColumns() but it has been replaced with similar methods, setInsertManageColumns() and setUpdateManageColumns() that assist in filling out the row with the managed columns.
| var triggerColumns = _target.getTriggerManagedColumns(_c); | ||
| if (triggerColumns != null && !triggerColumns.isEmpty()) | ||
| { | ||
| var columns = triggerColumns.stream().map(_target::getColumn).filter(Objects::nonNull).toList(); |
There was a problem hiding this comment.
I think this should probably be a failure.
| if (errors.hasErrors()) | ||
| break; | ||
|
|
||
| if (newRow != null && before) |
There was a problem hiding this comment.
validate that all managed columns are set for update trigger
There was a problem hiding this comment.
I've switched it to only validate managed columns for update.
labkey-matthewb
left a comment
labkey-matthewb
left a comment
There was a problem hiding this comment.
Considerations for additional features (do we want to do now or never)?
- Support different managed columns for insert/update
- Support in JavaScript triggers
| for (String col : managedColumns) | ||
| { | ||
| if (row.get(col) == COLUMN_SENTINEL) | ||
| row.remove(col); |
There was a problem hiding this comment.
Do we need clearManagedColumns() if we require all managedColumns to be set?
labkey-nicka
force-pushed
the
fb_trigger_columns
branch
from
1d28e80 to
3e9e24f
Compare
Implemented both of these. |
labkey-nicka
force-pushed
the
fb_trigger_columns
branch
from
7e514e3 to
7096977
Compare
labkey-nicka
force-pushed
the
fb_trigger_columns
branch
from
7096977 to
5ff3f06
Compare
2 participants
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
TableInfo.getTriggerManagedColumns()to allow for data iterator to gather all managed columnsTasks