Skip to content

Commit 27c5818

Browse files
dandelanydandelany
authored
Merge pull request #1822 from NASA-AMMOS/fix/anchor-validation
Fix Edge Case in Anchor Validation
2 parents c8dab71 + d4f78bd commit 27c5818

6 files changed

Lines changed: 814 additions & 683 deletions

File tree

‎db-tests/src/test/java/gov/nasa/jpl/aerie/database/AnchorTests.java‎

Lines changed: 682 additions & 646 deletions
Large diffs are not rendered by default.

‎db-tests/src/test/java/gov/nasa/jpl/aerie/database/MerlinDatabaseTestHelper.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package gov.nasa.jpl.aerie.database;
22

3+
import org.postgresql.util.PGInterval;
4+
35
import java.sql.Connection;
46
import java.sql.SQLException;
57
import java.util.UUID;
@@ -130,6 +132,10 @@ int insertActivity(final int planId) throws SQLException {
130132
return insertActivity(planId, "00:00:00");
131133
}
132134

135+
int insertActivity(final int planId, PGInterval startOffset) throws SQLException {
136+
return insertActivity(planId, startOffset.toString());
137+
}
138+
133139
int insertActivity(final int planId, final String startOffset) throws SQLException {
134140
return insertActivity(planId, startOffset, "{}");
135141
}

‎deployment/hasura/migrations/Aerie/33_fix_anchor_validation/down.sql‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
create or replace procedure merlin.validate_nonegative_net_plan_start(_activity_id integer, _plan_id integer)
2+
security definer
3+
language plpgsql as $$
4+
declare
5+
net_offset interval;
6+
_anchor_id integer;
7+
_start_offset interval;
8+
_anchored_to_start boolean;
9+
begin
10+
select anchor_id, start_offset, anchored_to_start
11+
from merlin.activity_directive
12+
where (id, plan_id) = (_activity_id, _plan_id)
13+
into _anchor_id, _start_offset, _anchored_to_start;
14+
15+
if (_start_offset < '0' and _anchored_to_start) then -- only need to check if anchored to start or something with a negative offset
16+
with recursive anchors(activity_id, anchor_id, anchored_to_start, start_offset, total_offset) as (
17+
select _activity_id, _anchor_id, _anchored_to_start, _start_offset, _start_offset
18+
union
19+
select ad.id, ad.anchor_id, ad.anchored_to_start, ad.start_offset, anchors.total_offset + ad.start_offset
20+
from merlin.activity_directive ad, anchors
21+
where anchors.anchor_id is not null -- stop at plan
22+
and (ad.id, ad.plan_id) = (anchors.anchor_id, _plan_id)
23+
and anchors.anchored_to_start -- or, stop at end-time offset
24+
)
25+
select total_offset -- get the id of the activity that the selected activity is anchored to
26+
from anchors a
27+
where a.anchor_id is null
28+
and a.anchored_to_start
29+
limit 1
30+
into net_offset;
31+
32+
if(net_offset < '0') then
33+
raise notice 'Activity Directive % has a net negative offset relative to Plan Start.', _activity_id;
34+
35+
insert into merlin.anchor_validation_status (activity_id, plan_id, reason_invalid)
36+
values (_activity_id, _plan_id, 'Activity Directive ' || _activity_id || ' has a net negative offset relative to Plan Start.')
37+
on conflict (activity_id, plan_id) do update
38+
set reason_invalid = 'Activity Directive ' || excluded.activity_id || ' has a net negative offset relative to Plan Start.';
39+
end if;
40+
end if;
41+
end
42+
$$;
43+
call migrations.mark_migration_rolled_back(33);

‎deployment/hasura/migrations/Aerie/33_fix_anchor_validation/up.sql‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
create or replace procedure merlin.validate_nonegative_net_plan_start(_activity_id integer, _plan_id integer)
2+
security definer
3+
language plpgsql as $$
4+
declare
5+
net_offset interval;
6+
_anchor_id integer;
7+
_start_offset interval;
8+
_anchored_to_start boolean;
9+
begin
10+
select anchor_id, start_offset, anchored_to_start
11+
from merlin.activity_directive
12+
where (id, plan_id) = (_activity_id, _plan_id)
13+
into _anchor_id, _start_offset, _anchored_to_start;
14+
15+
if (_anchored_to_start) then -- only need to check if anchored to start
16+
with recursive anchors(activity_id, anchor_id, anchored_to_start, start_offset, total_offset) as (
17+
select _activity_id, _anchor_id, _anchored_to_start, _start_offset, _start_offset
18+
union
19+
select ad.id, ad.anchor_id, ad.anchored_to_start, ad.start_offset, anchors.total_offset + ad.start_offset
20+
from merlin.activity_directive ad, anchors
21+
where anchors.anchor_id is not null -- stop at plan
22+
and (ad.id, ad.plan_id) = (anchors.anchor_id, _plan_id)
23+
and anchors.anchored_to_start -- or, stop at end-time offset
24+
)
25+
select total_offset -- get the id of the activity that the selected activity is anchored to
26+
from anchors a
27+
where a.anchor_id is null
28+
and a.anchored_to_start
29+
limit 1
30+
into net_offset;
31+
32+
if(net_offset < '0') then
33+
raise notice 'Activity Directive % has a net negative offset relative to Plan Start.', _activity_id;
34+
35+
insert into merlin.anchor_validation_status (activity_id, plan_id, reason_invalid)
36+
values (_activity_id, _plan_id, 'Activity Directive ' || _activity_id || ' has a net negative offset relative to Plan Start.')
37+
on conflict (activity_id, plan_id) do update
38+
set reason_invalid = 'Activity Directive ' || excluded.activity_id || ' has a net negative offset relative to Plan Start.';
39+
end if;
40+
end if;
41+
end
42+
$$;
43+
44+
call migrations.mark_migration_applied(33);
45+

‎deployment/postgres-init-db/sql/applied_migrations.sql‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,3 +34,4 @@ call migrations.mark_migration_applied(29);
3434
call migrations.mark_migration_applied(30);
3535
call migrations.mark_migration_applied(31);
3636
call migrations.mark_migration_applied(32);
37+
call migrations.mark_migration_applied(33);

‎deployment/postgres-init-db/sql/tables/merlin/activity_directive/anchor_validation_status.sql‎

Lines changed: 37 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -107,45 +107,45 @@ comment on procedure merlin.validate_nonnegative_net_end_offset(_activity_id int
107107
create procedure merlin.validate_nonegative_net_plan_start(_activity_id integer, _plan_id integer)
108108
security definer
109109
language plpgsql as $$
110-
declare
111-
net_offset interval;
112-
_anchor_id integer;
113-
_start_offset interval;
114-
_anchored_to_start boolean;
115-
begin
116-
select anchor_id, start_offset, anchored_to_start
117-
from merlin.activity_directive
118-
where (id, plan_id) = (_activity_id, _plan_id)
119-
into _anchor_id, _start_offset, _anchored_to_start;
120-
121-
if (_start_offset < '0' and _anchored_to_start) then -- only need to check if anchored to start or something with a negative offset
122-
with recursive anchors(activity_id, anchor_id, anchored_to_start, start_offset, total_offset) as (
123-
select _activity_id, _anchor_id, _anchored_to_start, _start_offset, _start_offset
124-
union
125-
select ad.id, ad.anchor_id, ad.anchored_to_start, ad.start_offset, anchors.total_offset + ad.start_offset
126-
from merlin.activity_directive ad, anchors
127-
where anchors.anchor_id is not null -- stop at plan
128-
and (ad.id, ad.plan_id) = (anchors.anchor_id, _plan_id)
129-
and anchors.anchored_to_start -- or, stop at end-time offset
130-
)
131-
select total_offset -- get the id of the activity that the selected activity is anchored to
132-
from anchors a
133-
where a.anchor_id is null
134-
and a.anchored_to_start
135-
limit 1
136-
into net_offset;
110+
declare
111+
net_offset interval;
112+
_anchor_id integer;
113+
_start_offset interval;
114+
_anchored_to_start boolean;
115+
begin
116+
select anchor_id, start_offset, anchored_to_start
117+
from merlin.activity_directive
118+
where (id, plan_id) = (_activity_id, _plan_id)
119+
into _anchor_id, _start_offset, _anchored_to_start;
137120

138-
if(net_offset < '0') then
139-
raise notice 'Activity Directive % has a net negative offset relative to Plan Start.', _activity_id;
121+
if (_anchored_to_start) then -- only need to check if anchored to start
122+
with recursive anchors(activity_id, anchor_id, anchored_to_start, start_offset, total_offset) as (
123+
select _activity_id, _anchor_id, _anchored_to_start, _start_offset, _start_offset
124+
union
125+
select ad.id, ad.anchor_id, ad.anchored_to_start, ad.start_offset, anchors.total_offset + ad.start_offset
126+
from merlin.activity_directive ad, anchors
127+
where anchors.anchor_id is not null -- stop at plan
128+
and (ad.id, ad.plan_id) = (anchors.anchor_id, _plan_id)
129+
and anchors.anchored_to_start -- or, stop at end-time offset
130+
)
131+
select total_offset -- get the id of the activity that the selected activity is anchored to
132+
from anchors a
133+
where a.anchor_id is null
134+
and a.anchored_to_start
135+
limit 1
136+
into net_offset;
137+
138+
if(net_offset < '0') then
139+
raise notice 'Activity Directive % has a net negative offset relative to Plan Start.', _activity_id;
140140

141-
insert into merlin.anchor_validation_status (activity_id, plan_id, reason_invalid)
142-
values (_activity_id, _plan_id, 'Activity Directive ' || _activity_id || ' has a net negative offset relative to Plan Start.')
143-
on conflict (activity_id, plan_id) do update
144-
set reason_invalid = 'Activity Directive ' || excluded.activity_id || ' has a net negative offset relative to Plan Start.';
145-
end if;
146-
end if;
147-
end
148-
$$;
141+
insert into merlin.anchor_validation_status (activity_id, plan_id, reason_invalid)
142+
values (_activity_id, _plan_id, 'Activity Directive ' || _activity_id || ' has a net negative offset relative to Plan Start.')
143+
on conflict (activity_id, plan_id) do update
144+
set reason_invalid = 'Activity Directive ' || excluded.activity_id || ' has a net negative offset relative to Plan Start.';
145+
end if;
146+
end if;
147+
end
148+
$$;
149149
comment on procedure merlin.validate_nonegative_net_plan_start(_activity_id integer, _plan_id integer) is e''
150150
'Returns true if the specified activity has a net negative offset from plan start. Otherwise, returns false.\n'
151151
'If true, writes to anchor_validation_status.';

0 commit comments

Comments
 (0)