Copilot commented on code in PR #3039:
URL: https://github.com/apache/hugegraph/pull/3039#discussion_r3340413748
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/traversal/optimize/TraversalUtil.java:
##########
@@ -181,6 +204,156 @@ public static void extractHasContainer(HugeGraphStep<?,
?> newStep,
}
}
+ private static boolean followedByMatchStep(Step<?, ?> step) {
+ Step<?, ?> next = step.getNextStep();
+ while (next instanceof HasStep ||
+ next instanceof NoOpBarrierStep ||
+ next instanceof IdentityStep) {
+ next = next.getNextStep();
+ }
+ return next instanceof MatchStep;
+ }
+
+ private static boolean hasUnusableMatchPredicate(HugeGraphStep<?, ?> step,
+ HasContainerHolder
holder) {
+ HugeGraph graph = tryGetGraph(step);
+ for (HasContainer has : holder.getHasContainers()) {
+ if (!hasMatchIndexSensitivePredicate(has)) {
+ continue;
+ }
+ if (graph == null || !hasUsableMatchIndex(graph, step, has)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ private static List<HasContainer> extractUsableHasContainers(
+ HugeGraphStep<?, ?> step, HasContainerHolder holder) {
+ List<HasContainer> extracted = new ArrayList<>();
+ HugeGraph graph = tryGetGraph(step);
+ for (HasContainer has : holder.getHasContainers()) {
+ if (!canExtractHasContainer(graph, has)) {
+ continue;
+ }
+ if (hasMatchIndexSensitivePredicate(has) &&
+ !hasUsableMatchIndex(graph, step, has)) {
+ continue;
+ }
+ if (!GraphStep.processHasContainerIds(step, has)) {
+ step.addHasContainer(has);
+ }
+ extracted.add(has);
+ }
+ return extracted;
+ }
+
+ private static boolean hasMatchIndexSensitivePredicate(HasContainer has) {
+ List<P<Object>> predicates = new ArrayList<>();
+ collectPredicates(predicates, ImmutableList.of(has.getPredicate()));
+ for (P<Object> pred : predicates) {
+ BiPredicate<?, ?> bp = pred.getBiPredicate();
+ if (bp == Compare.neq ||
+ bp == Compare.gt || bp == Compare.gte ||
+ bp == Compare.lt || bp == Compare.lte) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ private static boolean hasUsableMatchIndex(HugeGraph graph,
+ HugeGraphStep<?, ?> step,
+ HasContainer has) {
+ if (isSysProp(has.getKey())) {
+ return false;
+ }
+ if (!canExtractHasContainer(graph, has)) {
+ return false;
+ }
+
+ PropertyKey pkey;
+ try {
+ pkey = graph.propertyKey(has.getKey());
+ } catch (NotFoundException e) {
+ return false;
+ }
+
+ Collection<? extends SchemaLabel> schemaLabels = step.returnsVertex() ?
+ graph.vertexLabels() :
+ graph.edgeLabels();
+ boolean seen = false;
+ for (SchemaLabel schemaLabel : schemaLabels) {
+ if (!schemaLabel.properties().contains(pkey.id())) {
+ continue;
+ }
+ seen = true;
+ if (pkey.dataType() == DataType.BOOLEAN &&
+ !hasBooleanIndex(graph, schemaLabel, pkey)) {
+ return false;
+ }
+ if (pkey.dataType().isNumber() &&
+ (!hasOnlyRangePredicates(has) ||
+ !hasRangeIndex(graph, schemaLabel, pkey))) {
+ return false;
+ }
+ if (pkey.dataType() != DataType.BOOLEAN &&
+ !pkey.dataType().isNumber()) {
+ return false;
+ }
+ }
+ return seen;
+ }
+
+ private static boolean hasBooleanIndex(HugeGraph graph,
+ SchemaLabel schemaLabel,
+ PropertyKey pkey) {
+ for (Id id : schemaLabel.indexLabels()) {
+ IndexLabel indexLabel = graph.indexLabel(id);
+ if (!matchSingleFieldIndex(indexLabel, pkey)) {
+ continue;
+ }
+ if (indexLabel.indexType().isSecondary()) {
+ return true;
+ }
+ }
+ return false;
Review Comment:
hasBooleanIndex() calls graph.indexLabel(id), which throws
IllegalArgumentException if schemaTransaction#getIndexLabel(id) returns null
(e.g., during index-label creation, GraphIndexTransaction explicitly expects
getIndexLabel() may return null). This can break traversal strategy application
with an unexpected exception; consider tolerating missing index labels and just
skipping them like GraphIndexTransaction does.
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/traversal/optimize/TraversalUtil.java:
##########
@@ -181,6 +204,156 @@ public static void extractHasContainer(HugeGraphStep<?,
?> newStep,
}
}
+ private static boolean followedByMatchStep(Step<?, ?> step) {
+ Step<?, ?> next = step.getNextStep();
+ while (next instanceof HasStep ||
+ next instanceof NoOpBarrierStep ||
+ next instanceof IdentityStep) {
+ next = next.getNextStep();
+ }
+ return next instanceof MatchStep;
+ }
+
+ private static boolean hasUnusableMatchPredicate(HugeGraphStep<?, ?> step,
+ HasContainerHolder
holder) {
+ HugeGraph graph = tryGetGraph(step);
+ for (HasContainer has : holder.getHasContainers()) {
+ if (!hasMatchIndexSensitivePredicate(has)) {
+ continue;
+ }
+ if (graph == null || !hasUsableMatchIndex(graph, step, has)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ private static List<HasContainer> extractUsableHasContainers(
+ HugeGraphStep<?, ?> step, HasContainerHolder holder) {
+ List<HasContainer> extracted = new ArrayList<>();
+ HugeGraph graph = tryGetGraph(step);
+ for (HasContainer has : holder.getHasContainers()) {
+ if (!canExtractHasContainer(graph, has)) {
+ continue;
+ }
+ if (hasMatchIndexSensitivePredicate(has) &&
+ !hasUsableMatchIndex(graph, step, has)) {
+ continue;
+ }
+ if (!GraphStep.processHasContainerIds(step, has)) {
+ step.addHasContainer(has);
+ }
+ extracted.add(has);
+ }
+ return extracted;
+ }
+
+ private static boolean hasMatchIndexSensitivePredicate(HasContainer has) {
+ List<P<Object>> predicates = new ArrayList<>();
+ collectPredicates(predicates, ImmutableList.of(has.getPredicate()));
+ for (P<Object> pred : predicates) {
+ BiPredicate<?, ?> bp = pred.getBiPredicate();
+ if (bp == Compare.neq ||
+ bp == Compare.gt || bp == Compare.gte ||
+ bp == Compare.lt || bp == Compare.lte) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ private static boolean hasUsableMatchIndex(HugeGraph graph,
+ HugeGraphStep<?, ?> step,
+ HasContainer has) {
+ if (isSysProp(has.getKey())) {
+ return false;
+ }
+ if (!canExtractHasContainer(graph, has)) {
+ return false;
+ }
+
+ PropertyKey pkey;
+ try {
+ pkey = graph.propertyKey(has.getKey());
+ } catch (NotFoundException e) {
+ return false;
+ }
+
+ Collection<? extends SchemaLabel> schemaLabels = step.returnsVertex() ?
+ graph.vertexLabels() :
+ graph.edgeLabels();
+ boolean seen = false;
+ for (SchemaLabel schemaLabel : schemaLabels) {
+ if (!schemaLabel.properties().contains(pkey.id())) {
+ continue;
+ }
+ seen = true;
+ if (pkey.dataType() == DataType.BOOLEAN &&
+ !hasBooleanIndex(graph, schemaLabel, pkey)) {
+ return false;
+ }
+ if (pkey.dataType().isNumber() &&
+ (!hasOnlyRangePredicates(has) ||
+ !hasRangeIndex(graph, schemaLabel, pkey))) {
+ return false;
+ }
+ if (pkey.dataType() != DataType.BOOLEAN &&
+ !pkey.dataType().isNumber()) {
+ return false;
+ }
+ }
+ return seen;
+ }
+
+ private static boolean hasBooleanIndex(HugeGraph graph,
+ SchemaLabel schemaLabel,
+ PropertyKey pkey) {
+ for (Id id : schemaLabel.indexLabels()) {
+ IndexLabel indexLabel = graph.indexLabel(id);
+ if (!matchSingleFieldIndex(indexLabel, pkey)) {
+ continue;
+ }
+ if (indexLabel.indexType().isSecondary()) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ private static boolean hasRangeIndex(HugeGraph graph,
+ SchemaLabel schemaLabel,
+ PropertyKey pkey) {
+ for (Id id : schemaLabel.indexLabels()) {
+ IndexLabel indexLabel = graph.indexLabel(id);
+ if (!matchSingleFieldIndex(indexLabel, pkey)) {
+ continue;
+ }
+ if (indexLabel.indexType().isRange()) {
+ return true;
+ }
+ }
+ return false;
Review Comment:
hasRangeIndex() also calls graph.indexLabel(id) and can throw
IllegalArgumentException if the index label isn't visible yet
(schemaTransaction#getIndexLabel may return null while an index is being
created). Skipping missing index labels avoids failing traversal optimization
with an unrelated exception.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]