Skip to content

Fix Edge Case in Anchor Validation #1822

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dandelany merged 3 commits into from
May 8, 2026
Merged

Fix Edge Case in Anchor Validation #1822

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8549c55
Fix edge case in anchor validation
Mythicaeda Apr 27, 2026
a244fb2
Refactor AnchorTests
Mythicaeda Apr 30, 2026
d4f78bd
update comments on a few db tests, add new test for grandchild with n…
dandelany May 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1,328 changes: 682 additions & 646 deletions db-tests/src/test/java/gov/nasa/jpl/aerie/database/AnchorTests.java
View file
Open in desktop

Large diffs are not rendered by default.

6 changes: 6 additions & 0 deletions db-tests/src/test/java/gov/nasa/jpl/aerie/database/MerlinDatabaseTestHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package gov.nasa.jpl.aerie.database;

import org.postgresql.util.PGInterval;

import java.sql.Connection;
import java.sql.SQLException;
import java.util.UUID;
Expand Down Expand Up @@ -130,6 +132,10 @@ int insertActivity(final int planId) throws SQLException {
return insertActivity(planId, "00:00:00");
}

int insertActivity(final int planId, PGInterval startOffset) throws SQLException {
return insertActivity(planId, startOffset.toString());
}

int insertActivity(final int planId, final String startOffset) throws SQLException {
return insertActivity(planId, startOffset, "{}");
}
Expand Down
43 changes: 43 additions & 0 deletions deployment/hasura/migrations/Aerie/33_fix_anchor_validation/down.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
create or replace procedure merlin.validate_nonegative_net_plan_start(_activity_id integer, _plan_id integer)
security definer
language plpgsql as $$
declare
net_offset interval;
_anchor_id integer;
_start_offset interval;
_anchored_to_start boolean;
begin
select anchor_id, start_offset, anchored_to_start
from merlin.activity_directive
where (id, plan_id) = (_activity_id, _plan_id)
into _anchor_id, _start_offset, _anchored_to_start;

if (_start_offset < '0' and _anchored_to_start) then -- only need to check if anchored to start or something with a negative offset
with recursive anchors(activity_id, anchor_id, anchored_to_start, start_offset, total_offset) as (
select _activity_id, _anchor_id, _anchored_to_start, _start_offset, _start_offset
union
select ad.id, ad.anchor_id, ad.anchored_to_start, ad.start_offset, anchors.total_offset + ad.start_offset
from merlin.activity_directive ad, anchors
where anchors.anchor_id is not null -- stop at plan
and (ad.id, ad.plan_id) = (anchors.anchor_id, _plan_id)
and anchors.anchored_to_start -- or, stop at end-time offset
)
select total_offset -- get the id of the activity that the selected activity is anchored to
from anchors a
where a.anchor_id is null
and a.anchored_to_start
limit 1
into net_offset;

if(net_offset < '0') then
raise notice 'Activity Directive % has a net negative offset relative to Plan Start.', _activity_id;

insert into merlin.anchor_validation_status (activity_id, plan_id, reason_invalid)
values (_activity_id, _plan_id, 'Activity Directive ' || _activity_id || ' has a net negative offset relative to Plan Start.')
on conflict (activity_id, plan_id) do update
set reason_invalid = 'Activity Directive ' || excluded.activity_id || ' has a net negative offset relative to Plan Start.';
end if;
end if;
end
$$;
call migrations.mark_migration_rolled_back(33);
45 changes: 45 additions & 0 deletions deployment/hasura/migrations/Aerie/33_fix_anchor_validation/up.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
create or replace procedure merlin.validate_nonegative_net_plan_start(_activity_id integer, _plan_id integer)
security definer
language plpgsql as $$
declare
net_offset interval;
_anchor_id integer;
_start_offset interval;
_anchored_to_start boolean;
begin
select anchor_id, start_offset, anchored_to_start
from merlin.activity_directive
where (id, plan_id) = (_activity_id, _plan_id)
into _anchor_id, _start_offset, _anchored_to_start;

if (_anchored_to_start) then -- only need to check if anchored to start
with recursive anchors(activity_id, anchor_id, anchored_to_start, start_offset, total_offset) as (
select _activity_id, _anchor_id, _anchored_to_start, _start_offset, _start_offset
union
select ad.id, ad.anchor_id, ad.anchored_to_start, ad.start_offset, anchors.total_offset + ad.start_offset
from merlin.activity_directive ad, anchors
where anchors.anchor_id is not null -- stop at plan
and (ad.id, ad.plan_id) = (anchors.anchor_id, _plan_id)
and anchors.anchored_to_start -- or, stop at end-time offset
)
select total_offset -- get the id of the activity that the selected activity is anchored to
from anchors a
where a.anchor_id is null
and a.anchored_to_start
limit 1
into net_offset;

if(net_offset < '0') then
raise notice 'Activity Directive % has a net negative offset relative to Plan Start.', _activity_id;

insert into merlin.anchor_validation_status (activity_id, plan_id, reason_invalid)
values (_activity_id, _plan_id, 'Activity Directive ' || _activity_id || ' has a net negative offset relative to Plan Start.')
on conflict (activity_id, plan_id) do update
set reason_invalid = 'Activity Directive ' || excluded.activity_id || ' has a net negative offset relative to Plan Start.';
end if;
end if;
end
$$;

call migrations.mark_migration_applied(33);

1 change: 1 addition & 0 deletions deployment/postgres-init-db/sql/applied_migrations.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,4 @@ call migrations.mark_migration_applied(29);
call migrations.mark_migration_applied(30);
call migrations.mark_migration_applied(31);
call migrations.mark_migration_applied(32);
call migrations.mark_migration_applied(33);
74 changes: 37 additions & 37 deletions ...oyment/postgres-init-db/sql/tables/merlin/activity_directive/anchor_validation_status.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,45 +107,45 @@ comment on procedure merlin.validate_nonnegative_net_end_offset(_activity_id int
create procedure merlin.validate_nonegative_net_plan_start(_activity_id integer, _plan_id integer)
security definer
language plpgsql as $$
declare
net_offset interval;
_anchor_id integer;
_start_offset interval;
_anchored_to_start boolean;
begin
select anchor_id, start_offset, anchored_to_start
from merlin.activity_directive
where (id, plan_id) = (_activity_id, _plan_id)
into _anchor_id, _start_offset, _anchored_to_start;

if (_start_offset < '0' and _anchored_to_start) then -- only need to check if anchored to start or something with a negative offset
with recursive anchors(activity_id, anchor_id, anchored_to_start, start_offset, total_offset) as (
select _activity_id, _anchor_id, _anchored_to_start, _start_offset, _start_offset
union
select ad.id, ad.anchor_id, ad.anchored_to_start, ad.start_offset, anchors.total_offset + ad.start_offset
from merlin.activity_directive ad, anchors
where anchors.anchor_id is not null -- stop at plan
and (ad.id, ad.plan_id) = (anchors.anchor_id, _plan_id)
and anchors.anchored_to_start -- or, stop at end-time offset
)
select total_offset -- get the id of the activity that the selected activity is anchored to
from anchors a
where a.anchor_id is null
and a.anchored_to_start
limit 1
into net_offset;
declare
net_offset interval;
_anchor_id integer;
_start_offset interval;
_anchored_to_start boolean;
begin
select anchor_id, start_offset, anchored_to_start
from merlin.activity_directive
where (id, plan_id) = (_activity_id, _plan_id)
into _anchor_id, _start_offset, _anchored_to_start;

if(net_offset < '0') then
raise notice 'Activity Directive % has a net negative offset relative to Plan Start.', _activity_id;
if (_anchored_to_start) then -- only need to check if anchored to start
with recursive anchors(activity_id, anchor_id, anchored_to_start, start_offset, total_offset) as (
select _activity_id, _anchor_id, _anchored_to_start, _start_offset, _start_offset
union
select ad.id, ad.anchor_id, ad.anchored_to_start, ad.start_offset, anchors.total_offset + ad.start_offset
from merlin.activity_directive ad, anchors
where anchors.anchor_id is not null -- stop at plan
and (ad.id, ad.plan_id) = (anchors.anchor_id, _plan_id)
and anchors.anchored_to_start -- or, stop at end-time offset
)
select total_offset -- get the id of the activity that the selected activity is anchored to
from anchors a
where a.anchor_id is null
and a.anchored_to_start
limit 1
into net_offset;

if(net_offset < '0') then
raise notice 'Activity Directive % has a net negative offset relative to Plan Start.', _activity_id;

insert into merlin.anchor_validation_status (activity_id, plan_id, reason_invalid)
values (_activity_id, _plan_id, 'Activity Directive ' || _activity_id || ' has a net negative offset relative to Plan Start.')
on conflict (activity_id, plan_id) do update
set reason_invalid = 'Activity Directive ' || excluded.activity_id || ' has a net negative offset relative to Plan Start.';
end if;
end if;
end
$$;
insert into merlin.anchor_validation_status (activity_id, plan_id, reason_invalid)
values (_activity_id, _plan_id, 'Activity Directive ' || _activity_id || ' has a net negative offset relative to Plan Start.')
on conflict (activity_id, plan_id) do update
set reason_invalid = 'Activity Directive ' || excluded.activity_id || ' has a net negative offset relative to Plan Start.';
end if;
end if;
end
$$;
comment on procedure merlin.validate_nonegative_net_plan_start(_activity_id integer, _plan_id integer) is e''
'Returns true if the specified activity has a net negative offset from plan start. Otherwise, returns false.\n'
'If true, writes to anchor_validation_status.';
Expand Down
Loading