Skip to content

Commit

Permalink
Delete and Insert queries are validated once rather than per answer (#…
Browse files Browse the repository at this point in the history 
…6369)

## What is the goal of this PR?

We refactor `Deleter`, `Inserter` and `Updater` in order to validate the queries once, before execution, rather than per-answer.


## What are the changes implemented in this PR?

* extract Validation for `Insert` and `Delete` operations into a static validation that runs before each query is executed
  • Loading branch information
@flyingsilverfin
flyingsilverfin committed Jun 11, 2021
1 parent 3cbe5d6 commit 7b7a57f
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 39 deletions.
2 changes: 1 addition & 1 deletion dependencies/vaticle/repositories.bzl
View file
Expand Up @@ -49,7 +49,7 @@ def vaticle_typedb_behaviour():
git_repository(
name = "vaticle_typedb_behaviour",
remote = "https://github.com/vaticle/typedb-behaviour",
commit = "f763ba70f081b73f7668d904de50b9cb9cfc8858", # sync-marker: do not remove this comment, this is used for sync-dependencies by @vaticle_typedb_behaviour
commit = "9581b8bee1403fd975d71ae95b04445eb7d1dbb4", # sync-marker: do not remove this comment, this is used for sync-dependencies by @vaticle_typedb_behaviour
)

def vaticle_factory_tracing():
Expand Down
44 changes: 27 additions & 17 deletions query/Deleter.java
View file
Expand Up @@ -31,6 +31,8 @@
import com.vaticle.typedb.core.concept.type.ThingType;
import com.vaticle.typedb.core.pattern.constraint.thing.HasConstraint;
import com.vaticle.typedb.core.pattern.variable.ThingVariable;
import com.vaticle.typedb.core.pattern.variable.TypeVariable;
import com.vaticle.typedb.core.pattern.variable.Variable;
import com.vaticle.typedb.core.pattern.variable.VariableRegistry;
import com.vaticle.typedb.core.reasoner.Reasoner;
import com.vaticle.typeql.lang.pattern.variable.Reference;
Expand Down Expand Up @@ -73,16 +75,38 @@ public Deleter(Matcher matcher, Set<ThingVariable> variables, Context.Query cont
public static Deleter create(Reasoner reasoner, TypeQLDelete query, Context.Query context) {
try (FactoryTracingThreadStatic.ThreadTrace ignored = traceOnThread(TRACE_PREFIX + "create")) {
VariableRegistry registry = VariableRegistry.createFromThings(query.variables(), false);
iterate(registry.types()).filter(t -> !t.reference().isLabel()).forEachRemaining(t -> {
throw TypeDBException.of(ILLEGAL_TYPE_VARIABLE_IN_DELETE, t.reference());
});
registry.variables().forEach(Deleter::validate);

assert query.match().namedVariablesUnbound().containsAll(query.namedVariablesUnbound());
Matcher matcher = Matcher.create(reasoner, query.match().get(query.namedVariablesUnbound()));
return new Deleter(matcher, registry.things(), context);
}
}

public static void validate(Variable var) {
try (FactoryTracingThreadStatic.ThreadTrace ignored = traceOnThread(TRACE_PREFIX + "validate")) {
if (var.isType()) validate(var.asType());
else validate(var.asThing());
}
}

private static void validate(TypeVariable var) {
if (!var.reference().isLabel()) throw TypeDBException.of(ILLEGAL_TYPE_VARIABLE_IN_DELETE, var.reference());
}

private static void validate(ThingVariable var) {
if (!var.reference().isName()) {
ErrorMessage.ThingWrite msg = var.relation().isPresent()
? ILLEGAL_ANONYMOUS_RELATION_IN_DELETE
: ILLEGAL_ANONYMOUS_VARIABLE_IN_DELETE;
throw TypeDBException.of(msg, var);
} else if (var.iid().isPresent()) {
throw TypeDBException.of(THING_IID_NOT_INSERTABLE, var.reference(), var.iid().get());
} else if (!var.is().isEmpty()) {
throw TypeDBException.of(ILLEGAL_IS_CONSTRAINT, var, var.is().iterator().next());
}
}

public void execute() {
try (FactoryTracingThreadStatic.ThreadTrace ignored = traceOnThread(TRACE_PREFIX + "execute")) {
List<ConceptMap> matches = matcher.execute(context).toList();
Expand Down Expand Up @@ -121,20 +145,6 @@ private void delete(ThingVariable var) {
}
}

private void validate(ThingVariable var) {
try (FactoryTracingThreadStatic.ThreadTrace ignored = traceOnThread(TRACE_PREFIX + "validate")) {
if (!var.reference().isName()) {
ErrorMessage.ThingWrite msg = var.relation().isPresent()
? ILLEGAL_ANONYMOUS_RELATION_IN_DELETE
: ILLEGAL_ANONYMOUS_VARIABLE_IN_DELETE;
throw TypeDBException.of(msg, var);
} else if (var.iid().isPresent()) {
throw TypeDBException.of(THING_IID_NOT_INSERTABLE, var.reference(), var.iid().get());
} else if (!var.is().isEmpty()) {
throw TypeDBException.of(ILLEGAL_IS_CONSTRAINT, var, var.is().iterator().next());
}
}
}

private void deleteHas(ThingVariable var, Thing thing) {
try (FactoryTracingThreadStatic.ThreadTrace ignored = traceOnThread(TRACE_PREFIX + "delete_has")) {
Expand Down
38 changes: 23 additions & 15 deletions query/Inserter.java
View file
Expand Up @@ -36,6 +36,8 @@
import com.vaticle.typedb.core.pattern.constraint.thing.ValueConstraint;
import com.vaticle.typedb.core.pattern.constraint.type.LabelConstraint;
import com.vaticle.typedb.core.pattern.variable.ThingVariable;
import com.vaticle.typedb.core.pattern.variable.TypeVariable;
import com.vaticle.typedb.core.pattern.variable.Variable;
import com.vaticle.typedb.core.pattern.variable.VariableRegistry;
import com.vaticle.typedb.core.reasoner.Reasoner;
import com.vaticle.typedb.core.traversal.common.Identifier;
Expand Down Expand Up @@ -94,9 +96,7 @@ public Inserter(@Nullable Matcher matcher, ConceptManager conceptMgr,
public static Inserter create(Reasoner reasoner, ConceptManager conceptMgr, TypeQLInsert query, Context.Query context) {
try (FactoryTracingThreadStatic.ThreadTrace ignored = traceOnThread(TRACE_PREFIX + "create")) {
VariableRegistry registry = VariableRegistry.createFromThings(query.variables());
iterate(registry.types()).filter(t -> !t.reference().isLabel()).forEachRemaining(t -> {
throw TypeDBException.of(ILLEGAL_TYPE_VARIABLE_IN_INSERT, t.reference());
});
registry.variables().forEach(Inserter::validate);

Matcher matcher = null;
if (query.match().isPresent()) {
Expand All @@ -111,6 +111,26 @@ public static Inserter create(Reasoner reasoner, ConceptManager conceptMgr, Type
}
}

public static void validate(Variable var) {
try (FactoryTracingThreadStatic.ThreadTrace ignored = traceOnThread(TRACE_PREFIX + "validate")) {
if (var.isType()) validate(var.asType());
else validate(var.asThing());
}
}

private static void validate(TypeVariable var) {
if (!var.reference().isLabel()) throw TypeDBException.of(ILLEGAL_TYPE_VARIABLE_IN_INSERT, var.reference());
}

private static void validate(ThingVariable var) {
Identifier id = var.id();
if (var.iid().isPresent()) {
throw TypeDBException.of(THING_IID_NOT_INSERTABLE, id, var.iid().get());
} else if (!var.is().isEmpty()) {
throw TypeDBException.of(ILLEGAL_IS_CONSTRAINT, var, var.is().iterator().next());
}
}

public FunctionalIterator<ConceptMap> execute() {
try (FactoryTracingThreadStatic.ThreadTrace ignored = traceOnThread(TRACE_PREFIX + "execute")) {
if (matcher != null) return context.options().parallel() ? executeParallel() : executeSerial();
Expand Down Expand Up @@ -176,7 +196,6 @@ private Thing insert(ThingVariable var) {

if (id.isName() && (thing = inserted.get(id)) != null) return thing;
else if (matchedContains(var) && var.constraints().isEmpty()) return matchedGet(var);
else validate(var);

if (matchedContains(var)) {
thing = matchedGet(var);
Expand All @@ -194,17 +213,6 @@ private Thing insert(ThingVariable var) {
}
}

private void validate(ThingVariable var) {
try (FactoryTracingThreadStatic.ThreadTrace ignored = traceOnThread(TRACE_PREFIX + "validate")) {
Identifier id = var.id();
if (var.iid().isPresent()) {
throw TypeDBException.of(THING_IID_NOT_INSERTABLE, id, var.iid().get());
} else if (!var.is().isEmpty()) {
throw TypeDBException.of(ILLEGAL_IS_CONSTRAINT, var, var.is().iterator().next());
}
}
}

public ThingType getThingType(LabelConstraint labelConstraint) {
try (FactoryTracingThreadStatic.ThreadTrace ignored = traceOnThread(TRACE_PREFIX + "get_thing_type")) {
ThingType thingType = conceptMgr.getThingType(labelConstraint.label());
Expand Down
9 changes: 3 additions & 6 deletions query/Updater.java
View file
Expand Up @@ -38,6 +38,7 @@

import static com.vaticle.factory.tracing.client.FactoryTracingThreadStatic.traceOnThread;
import static com.vaticle.typedb.common.collection.Collections.list;
import static com.vaticle.typedb.core.common.exception.ErrorMessage.ThingWrite.ILLEGAL_ANONYMOUS_VARIABLE_IN_DELETE;
import static com.vaticle.typedb.core.common.exception.ErrorMessage.ThingWrite.ILLEGAL_TYPE_VARIABLE_IN_DELETE;
import static com.vaticle.typedb.core.common.exception.ErrorMessage.ThingWrite.ILLEGAL_TYPE_VARIABLE_IN_INSERT;
import static com.vaticle.typedb.core.common.iterator.Iterators.iterate;
Expand Down Expand Up @@ -69,14 +70,10 @@ public Updater(Matcher matcher, ConceptManager conceptMgr, Set<ThingVariable> de
public static Updater create(Reasoner reasoner, ConceptManager conceptMgr, TypeQLUpdate query, Context.Query context) {
try (FactoryTracingThreadStatic.ThreadTrace ignored = traceOnThread(TRACE_PREFIX + "create")) {
VariableRegistry deleteRegistry = VariableRegistry.createFromThings(query.deleteVariables(), false);
iterate(deleteRegistry.types()).filter(t -> !t.reference().isLabel()).forEachRemaining(t -> {
throw TypeDBException.of(ILLEGAL_TYPE_VARIABLE_IN_DELETE, t.reference());
});
deleteRegistry.variables().forEach(Deleter::validate);

VariableRegistry insertRegistry = VariableRegistry.createFromThings(query.insertVariables());
iterate(insertRegistry.types()).filter(t -> !t.reference().isLabel()).forEachRemaining(t -> {
throw TypeDBException.of(ILLEGAL_TYPE_VARIABLE_IN_INSERT, t.reference());
});
insertRegistry.variables().forEach(Inserter::validate);

assert query.match().namedVariablesUnbound().containsAll(query.namedDeleteVariablesUnbound());
HashSet<UnboundVariable> filter = new HashSet<>(query.namedDeleteVariablesUnbound());
Expand Down

0 comments on commit 7b7a57f

Please sign in to comment.