fix explain cross join (#5725)

Author: hmottestad

core/pom.xml

@@ -40,13 +40,19 @@
 						<artifactId>maven-surefire-plugin</artifactId>
 						<configuration>
 							<groups>slow</groups>
+							<properties>
+								<includeTags>slow</includeTags>
+							</properties>
 						</configuration>
 					</plugin>
 					<plugin>
 						<groupId>org.apache.maven.plugins</groupId>
 						<artifactId>maven-failsafe-plugin</artifactId>
 						<configuration>
 							<groups>slow</groups>
+							<properties>
+								<includeTags>slow</includeTags>
+							</properties>
 						</configuration>
 					</plugin>
 				</plugins>
@@ -65,13 +71,19 @@
 						<artifactId>maven-surefire-plugin</artifactId>
 						<configuration>
 							<excludedGroups>slow</excludedGroups>
+							<properties>
+								<excludeTags>slow</excludeTags>
+							</properties>
 						</configuration>
 					</plugin>
 					<plugin>
 						<groupId>org.apache.maven.plugins</groupId>
 						<artifactId>maven-failsafe-plugin</artifactId>
 						<configuration>
 							<excludedGroups>slow</excludedGroups>
+							<properties>
+								<excludeTags>slow</excludeTags>
+							</properties>
 						</configuration>
 					</plugin>
 				</plugins>

core/query/src/main/java/org/eclipse/rdf4j/query/explanation/GenericPlanNode.java

@@ -14,6 +14,7 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -169,7 +170,8 @@ public void setType(String type) {
 	}
 
 	public List<GenericPlanNode> getPlans() {
-		return plans.isEmpty() ? null : plans; // for simplified json
+		List<GenericPlanNode> orderedPlans = orderedPlansForDisplay();
+		return orderedPlans.isEmpty() ? null : orderedPlans; // for simplified json
 	}
 
 	public void setPlans(List<GenericPlanNode> plans) {
@@ -575,16 +577,18 @@ public void setAlgorithm(String algorithm) {
 	 */
 	@Override
 	public String toString() {
-		return getHumanReadable(0);
+		return getHumanReadable(0, isProjectionElemListNode());
 	}
 
 	/**
 	 * @param prettyBoxDrawingType for deciding if we should use single or double walled character for drawing the
 	 *                             connectors between nodes in the query plan. Eg. ├ or ╠ and ─ o
+	 * @param ordered
 	 * @return
 	 */
-	private String getHumanReadable(int prettyBoxDrawingType) {
+	private String getHumanReadable(int prettyBoxDrawingType, boolean ordered) {
 		StringBuilder sb = new StringBuilder();
+		List<GenericPlanNode> displayPlans = ordered ? orderedPlansForDisplay() : plans;
 
 		if (timedOut != null && timedOut) {
 			sb.append("Timed out while retrieving explanation! Explanation may be incomplete!").append(newLine);
@@ -607,7 +611,7 @@ private String getHumanReadable(int prettyBoxDrawingType) {
 		// we use box-drawing characters to "group" nodes in the plan visually when there are exactly two child plans
 		// and
 		// the child plans contain child plans
-		if (plans.size() == 2 && plans.stream().anyMatch(p -> !p.plans.isEmpty())) {
+		if (displayPlans.size() == 2 && displayPlans.stream().anyMatch(p -> !p.plans.isEmpty())) {
 
 			String start;
 			String horizontal;
@@ -626,8 +630,8 @@ private String getHumanReadable(int prettyBoxDrawingType) {
 				end = "└";
 			}
 
-			String left = plans.get(0).getHumanReadable(prettyBoxDrawingType + 1);
-			String right = plans.get(1).getHumanReadable(prettyBoxDrawingType + 1);
+			String left = displayPlans.get(0).getHumanReadable(prettyBoxDrawingType + 1, false);
+			String right = displayPlans.get(1).getHumanReadable(prettyBoxDrawingType + 1, false);
 			boolean join = type.contains("Join");
 
 			{
@@ -655,10 +659,14 @@ private String getHumanReadable(int prettyBoxDrawingType) {
 
 		} else {
 
-			for (int i = 0; i < plans.size(); i++) {
-				GenericPlanNode child = plans.get(i);
+			for (int i = 0; i < displayPlans.size(); i++) {
+				GenericPlanNode child = displayPlans.get(i);
 				int j = i;
-				sb.append(Arrays.stream(child.getHumanReadable(prettyBoxDrawingType + 1).split(newLine))
+				sb.append(Arrays
+						.stream(child
+								.getHumanReadable(prettyBoxDrawingType + 1,
+										child.isProjectionElemListNode() && !isMultiProjectionNode())
+								.split(newLine))
 						.map(c -> {
 							if (type.startsWith("StatementPattern") && child.type.startsWith("Var")) {
 								return spoc[j] + ": " + c;
@@ -674,6 +682,23 @@ private String getHumanReadable(int prettyBoxDrawingType) {
 		return sb.toString();
 	}
 
+	private List<GenericPlanNode> orderedPlansForDisplay() {
+		if (!isProjectionElemListNode() || plans.size() < 2) {
+			return plans;
+		}
+		List<GenericPlanNode> orderedPlans = new ArrayList<>(plans);
+		orderedPlans.sort(Comparator.comparing(plan -> plan.getType() == null ? "" : plan.getType()));
+		return orderedPlans;
+	}
+
+	private boolean isProjectionElemListNode() {
+		return type != null && type.startsWith("ProjectionElemList");
+	}
+
+	private boolean isMultiProjectionNode() {
+		return type != null && type.startsWith("MultiProjection");
+	}
+
 	/**
 	 * @return Human readable number. Eg. 12.1M for 1212213.4 and UNKNOWN for -1.
 	 */
@@ -1336,6 +1361,7 @@ private static double getMaxResultSizeActual(GenericPlanNode genericPlanNode) {
 	private String toDotInternal(double maxResultSizeActual, double maxTotalTime, double maxSelfTime) {
 		StringBuilder sb = new StringBuilder();
 		sb.append("   ");
+		List<GenericPlanNode> displayPlans = orderedPlansForDisplay();
 
 		if (newScope != null && newScope) {
 			sb.append("subgraph cluster_")
@@ -1377,13 +1403,13 @@ private String toDotInternal(double maxResultSizeActual, double maxTotalTime, do
 				.orElse(""));
 
 		sb.append("</table>>").append(" shape=plaintext];").append(newLine);
-		for (int i = 0; i < plans.size(); i++) {
-			GenericPlanNode p = plans.get(i);
+		for (int i = 0; i < displayPlans.size(); i++) {
+			GenericPlanNode p = displayPlans.get(i);
 			String linkLabel = "index " + i;
 
-			if (plans.size() == 2) {
+			if (displayPlans.size() == 2) {
 				linkLabel = i == 0 ? "left" : "right";
-			} else if (plans.size() == 1) {
+			} else if (displayPlans.size() == 1) {
 				linkLabel = "";
 			}
 
@@ -1398,7 +1424,7 @@ private String toDotInternal(double maxResultSizeActual, double maxTotalTime, do
 					.append(newLine);
 		}
 
-		plans.forEach(p -> sb.append(p.toDotInternal(maxResultSizeActual, maxTotalTime, maxSelfTime)));
+		displayPlans.forEach(p -> sb.append(p.toDotInternal(maxResultSizeActual, maxTotalTime, maxSelfTime)));
 
 		if (newScope != null && newScope) {
 			sb.append(newLine).append("}").append(newLine);

core/query/src/test/java/org/eclipse/rdf4j/query/explanation/GenericPlanNodeTest.java

@@ -199,6 +199,20 @@ void toStringOrdersExplainAnnotationsIndependentlyOfInsertionOrder() {
 		assertTrue(actual.indexOf("bindingState=bound") < actual.indexOf("indexNames=spoc, posc"), actual);
 	}
 
+	@Test
+	void toStringSortsProjectionElemListChildrenAlphabetically() {
+		GenericPlanNode projectionElemList = new GenericPlanNode("ProjectionElemList");
+		projectionElemList.addPlans(
+				new GenericPlanNode("ProjectionElem \"d\""),
+				new GenericPlanNode("ProjectionElem \"a\""),
+				new GenericPlanNode("ProjectionElem \"code\""));
+
+		String actual = projectionElemList.toString();
+
+		assertTrue(actual.indexOf("ProjectionElem \"a\"") < actual.indexOf("ProjectionElem \"code\""), actual);
+		assertTrue(actual.indexOf("ProjectionElem \"code\"") < actual.indexOf("ProjectionElem \"d\""), actual);
+	}
+
 	@Test
 	void toDotIncludesExplainAnnotationRows() {
 		GenericPlanNode join = new GenericPlanNode("Join");