-
Notifications
You must be signed in to change notification settings - Fork 21
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.
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
Procedural Constraints Database Support #1596
Draft
skovati
wants to merge
12
commits into
develop
Choose a base branch
from
feature/procedural-constraints-db-support
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+281
−60
Draft
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
691bac4
update DB schema to support procedural constraints
skovati 79435a6
update merlin-server to confirm to new procedural constraints schema …
skovati dda61d9
initial work to update e2e tests with constraint invocation ids
skovati d9c1319
change `goal_type` to `constraint_type`
skovati 2bf82c6
update e2e tests for constraint invocations
skovati 6e83813
add baseline Hasura permissions
skovati d6b6487
add hasura permissions for constraint specs
skovati 262a631
return constraint invocation id from violations action
skovati bc2e747
remove `arguments` from `constraint_run` PK
skovati 0748d83
fix constraint names in `constraint_definition`
skovati 6e671cd
add procudural constraint DB migrations
skovati 141cec3
delete invocation_id at end of migration
skovati File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
63 changes: 63 additions & 0 deletions
63
deployment/hasura/migrations/Aerie/12_procedural_constraints/down.sql
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
-- remove procedural constraints | ||
|
||
alter table merlin.constraint_run | ||
drop column arguments; | ||
|
||
alter table merlin.constraint_specification | ||
drop column arguments; | ||
|
||
-- delete all procedural constraints from specs (cascade is restricted) | ||
delete from merlin.constraint_specification cs | ||
using merlin.constraint_definition cd | ||
where cd.constraint_id = cs.constraint_id | ||
and cd.type = 'JAR'::merlin.constraint_type; | ||
|
||
-- delete all procedural constraint metadata and definitions | ||
delete from merlin.constraint_metadata cm | ||
using merlin.constraint_definition cd | ||
where cd.constraint_id = cm.id | ||
and cd.type = 'JAR'::merlin.constraint_type; | ||
|
||
alter table merlin.constraint_definition | ||
drop constraint constraint_procedure_has_uploaded_jar, | ||
drop constraint check_constraint_definition_type_consistency, | ||
|
||
alter column definition set not null, | ||
|
||
drop column type, | ||
drop column uploaded_jar_id, | ||
drop column parameter_schema; | ||
|
||
drop type merlin.constraint_type; | ||
|
||
-- revert constraint invocations | ||
|
||
-- remove all but the first invocation of a constraint | ||
-- we need to revert to a more constrained primary key, so we need to delete data | ||
-- keeping the first invocation is just a convention | ||
delete | ||
from merlin.constraint_specification | ||
where invocation_id not in (select min(invocation_id) | ||
from merlin.constraint_specification | ||
group by plan_id, constraint_id); | ||
|
||
alter table merlin.constraint_specification | ||
drop constraint constraint_specification_pkey, | ||
add constraint constraint_specification_pkey | ||
primary key (plan_id, constraint_id), | ||
|
||
drop column invocation_id; | ||
|
||
-- similar action with constraint_runs | ||
delete | ||
from merlin.constraint_run | ||
where constraint_run.constraint_invocation_id not in (select min(constraint_invocation_id) | ||
from merlin.constraint_run | ||
group by constraint_id, constraint_revision, simulation_dataset_id); | ||
|
||
alter table merlin.constraint_run | ||
drop constraint constraint_run_key, | ||
add constraint constraint_run_key | ||
primary key (constraint_id, constraint_revision, simulation_dataset_id), | ||
|
||
drop column constraint_invocation_id; |
63 changes: 63 additions & 0 deletions
63
deployment/hasura/migrations/Aerie/12_procedural_constraints/up.sql
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
-- constraint invocation support | ||
alter table merlin.constraint_specification | ||
add column invocation_id integer generated by default as identity, | ||
|
||
drop constraint constraint_specification_pkey, | ||
add constraint constraint_specification_pkey | ||
primary key (invocation_id); | ||
|
||
comment on column merlin.constraint_specification.invocation_id is e'' | ||
'The id of a specific constraint invocation in the specification. Primary key.'; | ||
|
||
-- update constraint_run PK | ||
alter table merlin.constraint_run | ||
-- temp set as nullable so we can insert, made explictly not null below | ||
add column constraint_invocation_id integer null, | ||
|
||
drop constraint constraint_run_key; | ||
|
||
-- copy invocation_ids from spec | ||
update merlin.constraint_run cr | ||
set constraint_invocation_id = cs.invocation_id | ||
from merlin.constraint_specification cs | ||
where cs.constraint_id = cr.constraint_id; | ||
|
||
-- delete cached constraint runs that aren't referenced by a spec anymore (this isn't ideal) | ||
delete from merlin.constraint_run where constraint_invocation_id is null; | ||
|
||
alter table merlin.constraint_run | ||
add constraint constraint_run_key | ||
primary key (constraint_invocation_id, simulation_dataset_id); | ||
|
||
|
||
-- procedural constraints support | ||
create type merlin.constraint_type as enum ('EDSL', 'JAR'); | ||
|
||
alter table merlin.constraint_definition | ||
add column type merlin.constraint_type not null default 'EDSL', | ||
add column uploaded_jar_id integer, | ||
add column parameter_schema jsonb, | ||
|
||
alter column definition drop not null, | ||
|
||
add constraint constraint_procedure_has_uploaded_jar | ||
foreign key (uploaded_jar_id) | ||
references merlin.uploaded_file | ||
on update cascade | ||
on delete restrict, | ||
|
||
add constraint check_constraint_definition_type_consistency | ||
check ( | ||
(type = 'EDSL' and definition is not null and uploaded_jar_id is null) | ||
or | ||
(type = 'JAR' and uploaded_jar_id is not null and definition is null) | ||
); | ||
|
||
alter table merlin.constraint_specification | ||
add column arguments jsonb not null default '{}'::jsonb; | ||
|
||
alter table merlin.constraint_run | ||
add column arguments jsonb not null; | ||
|
||
|
||
call migrations.mark_migration_applied('12'); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
4 changes: 3 additions & 1 deletion
4
deployment/postgres-init-db/sql/tables/merlin/constraints/constraint_specification.sql
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1 change: 1 addition & 0 deletions
1
deployment/postgres-init-db/sql/types/merlin/constraint_type.sql
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
create type merlin.constraint_type as enum ('EDSL', 'JAR'); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
3 changes: 3 additions & 0 deletions
3
e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/types/ConstraintInvocationId.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
package gov.nasa.jpl.aerie.e2e.types; | ||
|
||
public record ConstraintInvocationId(int id, int invocationId) {} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blocking: This pkey is not sufficient. If the user decides to check constraints twice, updating the definition of a constraint between checks, they will get a pkey conflict when the action attempts to cache the output of the second definition.
(This comment is here to prevent me from not fixing this)