SelfJoinLikeExecutor.mergeSubstitutions(...) improved.

core/optimization/src/main/java/it/unibz/inf/ontop/iq/executor/join/RedundantSelfJoinExecutor.java

@@ -3,22 +3,28 @@
 import com.google.common.collect.ImmutableCollection;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMultimap;
+import com.google.common.collect.ImmutableSet;
 import it.unibz.inf.ontop.injection.IntermediateQueryFactory;
 import it.unibz.inf.ontop.iq.IntermediateQuery;
 import it.unibz.inf.ontop.iq.exception.EmptyQueryException;
 import it.unibz.inf.ontop.iq.exception.InvalidQueryOptimizationProposalException;
 import it.unibz.inf.ontop.iq.impl.QueryTreeComponent;
+import it.unibz.inf.ontop.iq.node.DataNode;
 import it.unibz.inf.ontop.iq.node.EmptyNode;
 import it.unibz.inf.ontop.iq.node.ExtensionalDataNode;
 import it.unibz.inf.ontop.iq.node.InnerJoinNode;
 import it.unibz.inf.ontop.iq.proposal.InnerJoinOptimizationProposal;
 import it.unibz.inf.ontop.iq.proposal.NodeCentricOptimizationResults;
 import it.unibz.inf.ontop.iq.proposal.impl.NodeCentricOptimizationResultsImpl;
 import it.unibz.inf.ontop.model.atom.RelationPredicate;
+import it.unibz.inf.ontop.model.term.ImmutableExpression;
 import it.unibz.inf.ontop.model.term.TermFactory;
 import it.unibz.inf.ontop.model.term.Variable;
+import it.unibz.inf.ontop.model.term.VariableOrGroundTerm;
+import it.unibz.inf.ontop.substitution.ImmutableSubstitution;
 import it.unibz.inf.ontop.substitution.SubstitutionFactory;
 import it.unibz.inf.ontop.substitution.impl.ImmutableUnificationTools;
+import it.unibz.inf.ontop.utils.ImmutableCollectors;
 
 import java.util.Optional;
 
@@ -37,12 +43,14 @@ public abstract class RedundantSelfJoinExecutor extends SelfJoinLikeExecutor imp
      */
     private static final int MAX_ITERATIONS = 100;
     private final IntermediateQueryFactory iqFactory;
+    private final TermFactory termFactory;
 
     protected RedundantSelfJoinExecutor(IntermediateQueryFactory iqFactory,
                                         SubstitutionFactory substitutionFactory,
                                         ImmutableUnificationTools unificationTools, TermFactory termFactory) {
         super(substitutionFactory, unificationTools, termFactory);
         this.iqFactory = iqFactory;
+        this.termFactory = termFactory;
     }
 
 
@@ -137,7 +145,38 @@ private Optional<ConcreteProposal> propose(InnerJoinNode joinNode, ImmutableMult
             predicateProposal.ifPresent(proposalListBuilder::add);
         }
 
-        return createConcreteProposal(proposalListBuilder.build(), priorityVariables);
+        return createConcreteProposal(proposalListBuilder.build(), initialDataNodeMap, priorityVariables);
+    }
+
+    protected Optional<ConcreteProposal> createConcreteProposal(
+            ImmutableList<PredicateLevelProposal> predicateProposals,
+            ImmutableMultimap<RelationPredicate, ExtensionalDataNode> initialDataNodeMap, ImmutableList<Variable> priorityVariables) {
+
+
+
+        Optional<ImmutableSubstitution<VariableOrGroundTerm>> optionalMergedSubstitution;
+        try {
+            optionalMergedSubstitution = mergeSubstitutions(extractSubstitutions(predicateProposals), initialDataNodeMap, priorityVariables);
+        } catch (AtomUnificationException e) {
+            return Optional.empty();
+        }
+
+        ImmutableSet<DataNode> removedDataNodes =predicateProposals.stream()
+                .flatMap(p -> p.getRemovedDataNodes().stream())
+                .collect(ImmutableCollectors.toSet());
+
+        if (removedDataNodes.isEmpty()
+                && (! optionalMergedSubstitution.isPresent()))
+            return Optional.empty();
+
+        Optional<ImmutableExpression> isNotConjunction = termFactory.getConjunction(predicateProposals.stream()
+                .map(PredicateLevelProposal::getIsNotNullConjunction)
+                .filter(Optional::isPresent)
+                .map(Optional::get)
+                .flatMap(ImmutableExpression::flattenAND)
+                .distinct());
+
+        return Optional.of(new ConcreteProposal(optionalMergedSubstitution, removedDataNodes, isNotConjunction));
     }
 
     protected abstract Optional<PredicateLevelProposal> proposePerPredicate(InnerJoinNode joinNode, ImmutableCollection<ExtensionalDataNode> initialNodes,

core/optimization/src/main/java/it/unibz/inf/ontop/iq/executor/join/SelfJoinLikeExecutor.java

@@ -212,7 +212,6 @@ protected PredicateLevelProposal proposeForGroupingMap(
             /*
              * Collection of unifying substitutions
              */
-            long beforeUnifying = System.currentTimeMillis();
             ImmutableSet<ImmutableSubstitution<VariableOrGroundTerm>> unifyingSubstitutions =
                     dataNodeGroups.stream()
                             .filter(g -> g.size() > 1)
@@ -225,7 +224,6 @@ protected PredicateLevelProposal proposeForGroupingMap(
                             })
                             .filter(s -> !s.isEmpty())
                             .collect(ImmutableCollectors.toSet());
-            LOGGER.debug(String.format("Self-join-like unification took %d ms", System.currentTimeMillis() - beforeUnifying));
 
             /*
              * All the nodes that have been at least once dominated (--> could thus be removed).
@@ -257,34 +255,6 @@ protected PredicateLevelProposal proposeForGroupingMap(
         }
     }
 
-    protected Optional<ConcreteProposal> createConcreteProposal(
-            ImmutableList<PredicateLevelProposal> predicateProposals,
-            ImmutableList<Variable> priorityVariables) {
-        Optional<ImmutableSubstitution<VariableOrGroundTerm>> optionalMergedSubstitution;
-        try {
-            optionalMergedSubstitution = mergeSubstitutions(extractSubstitutions(predicateProposals), priorityVariables);
-        } catch (AtomUnificationException e) {
-            return Optional.empty();
-        }
-
-        ImmutableSet<DataNode> removedDataNodes =predicateProposals.stream()
-                .flatMap(p -> p.getRemovedDataNodes().stream())
-                .collect(ImmutableCollectors.toSet());
-
-        if (removedDataNodes.isEmpty()
-                && (! optionalMergedSubstitution.isPresent()))
-            return Optional.empty();
-
-        Optional<ImmutableExpression> isNotConjunction = termFactory.getConjunction(predicateProposals.stream()
-                .map(PredicateLevelProposal::getIsNotNullConjunction)
-                .filter(Optional::isPresent)
-                .map(Optional::get)
-                .flatMap(ImmutableExpression::flattenAND)
-                .distinct());
-
-        return Optional.of(new ConcreteProposal(optionalMergedSubstitution, removedDataNodes, isNotConjunction));
-    }
-
 
     protected ImmutableSubstitution<VariableOrGroundTerm> unifyRedundantNodes(
             Collection<ExtensionalDataNode> redundantNodes) throws AtomUnificationException {
@@ -353,17 +323,55 @@ protected ImmutableSubstitution<VariableOrGroundTerm> unifyRedundantNodes(
     }
 
     protected Optional<ImmutableSubstitution<VariableOrGroundTerm>> mergeSubstitutions(
-            ImmutableList<ImmutableSubstitution<VariableOrGroundTerm>> substitutions, ImmutableList<Variable> priorityVariables)
+            ImmutableList<ImmutableSubstitution<VariableOrGroundTerm>> substitutions,
+            ImmutableMultimap<RelationPredicate, ExtensionalDataNode> initialDataNodeMap,
+            ImmutableList<Variable> priorityVariables)
             throws AtomUnificationException {
 
+        ImmutableMap<Variable, Collection<RelationPredicate>> occurrenceVariableMap = initialDataNodeMap.asMap().entrySet().stream()
+                .flatMap(e -> e.getValue().stream()
+                        .flatMap(n -> n.getVariables().stream())
+                        .map(v -> Maps.immutableEntry(v, e.getKey())))
+                .collect(ImmutableCollectors.toMultimap()).asMap();
+
+        // Variables appearing for for more one relation predicate
+        ImmutableSet<Variable> sharedVariables = occurrenceVariableMap.entrySet().stream()
+                .filter(e -> ImmutableSet.copyOf(e.getValue()).size() > 1)
+                .map(Map.Entry::getKey)
+                .collect(ImmutableCollectors.toSet());
+        ImmutableSet<Variable> nonSharedVariables = Sets.difference(occurrenceVariableMap.keySet(), sharedVariables)
+                .immutableCopy();
+
+        /*
+         * For performance purposes, we can detach some fragments from the substitution to be "unified" with the following atom.
+         */
+        ImmutableList.Builder<ImmutableSubstitution<VariableOrGroundTerm>> nonSharedSubstitutionListBuilder = ImmutableList.builder();
+
         // Non-final
         Optional<ImmutableSubstitution<VariableOrGroundTerm>> optionalAccumulatedSubstitution = Optional.empty();
 
         for (ImmutableSubstitution<VariableOrGroundTerm> substitution : substitutions) {
             if (!substitution.isEmpty()) {
                 if (optionalAccumulatedSubstitution.isPresent()) {
+
+                    ImmutableSubstitution<VariableOrGroundTerm> accumulatedSubstitution = optionalAccumulatedSubstitution.get();
+
+                    /*
+                     * Before the following unification, we detach a fragment about non-shared variables from the accumulated substitution
+                     *
+                     * Particularly useful when dealing with tables with a large number of columns (e.g. views after collapsing some JSON objects)
+                     *
+                     */
+                    ImmutableSubstitution<VariableOrGroundTerm> nonSharedSubstitution = accumulatedSubstitution.reduceDomainToIntersectionWith(nonSharedVariables);
+                    if (!nonSharedSubstitution.isEmpty())
+                        nonSharedSubstitutionListBuilder.add(nonSharedSubstitution);
+
+                    ImmutableSubstitution<VariableOrGroundTerm> substitutionToUnify = nonSharedSubstitution.isEmpty()
+                            ? accumulatedSubstitution
+                            : accumulatedSubstitution.reduceDomainToIntersectionWith(sharedVariables);
+
                     Optional<ImmutableSubstitution<VariableOrGroundTerm>> optionalMGUS = unificationTools.computeAtomMGUS(
-                            optionalAccumulatedSubstitution.get(), substitution);
+                            substitutionToUnify, substitution);
                     if (optionalMGUS.isPresent()) {
                         optionalAccumulatedSubstitution = optionalMGUS;
                     }
@@ -379,6 +387,11 @@ protected Optional<ImmutableSubstitution<VariableOrGroundTerm>> mergeSubstitutio
         }
 
         return optionalAccumulatedSubstitution
+                .map(s -> Stream.concat(
+                            nonSharedSubstitutionListBuilder.build().stream(),
+                            Stream.of(s))
+                        .reduce((v1, v2) -> v2.composeWith2(v1))
+                        .orElseThrow(() -> new MinorOntopInternalBugException("At least one substitution was expected")))
                 .map(s -> s.orientate(priorityVariables));
     }