Merge pull request #219 from LinneaAndersson/4.0-plan-table-details-column

Render Details column in plan table

Author: LinneaAndersson

cypher-shell/src/integration-test/java/org/neo4j/shell/commands/CypherShellVerboseIntegrationTest.java

@@ -13,6 +13,7 @@
 import org.neo4j.shell.cli.Format;
 import org.neo4j.shell.exception.CommandException;
 import org.neo4j.shell.prettyprint.PrettyConfig;
+import org.neo4j.shell.prettyprint.TablePlanFormatter;
 
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.equalTo;
@@ -192,7 +193,7 @@ public void paramsAndListVariablesWithSpecialCharacters() throws EvaluationExcep
     public void cypherWithOrder() throws CommandException {
         // given
         String serverVersion = shell.getServerVersion();
-        assumeTrue(minorVersion(serverVersion) == 6 || majorVersion(serverVersion) == 4);
+        assumeTrue((minorVersion(serverVersion) == 6 && majorVersion(serverVersion) == 3) || majorVersion(serverVersion) > 3);
 
         shell.execute( "CREATE INDEX ON :Person(age)" );
         shell.execute( "CALL db.awaitIndexes()" );
@@ -206,6 +207,37 @@ public void cypherWithOrder() throws CommandException {
         assertThat( actual, containsString( "n.age ASC" ) );
     }
 
+    @Test
+    public void cypherWithQueryDetails() throws CommandException {
+        // given
+        String serverVersion = shell.getServerVersion();
+        assumeTrue((minorVersion(serverVersion) > 0 && majorVersion(serverVersion) == 4) || majorVersion(serverVersion) > 4);
+
+        //when
+        shell.execute("EXPLAIN MATCH (n) with n.age AS age RETURN age");
+
+        //then
+        String actual = linePrinter.output();
+        assertThat( actual, containsString( TablePlanFormatter.DETAILS ) );
+        assertThat( actual, containsString( "n.age AS age" ) );
+        assertThat( actual, not( containsString( TablePlanFormatter.IDENTIFIERS ) ) );
+    }
+
+    @Test
+    public void cypherWithoutQueryDetails() throws CommandException {
+        // given
+        String serverVersion = shell.getServerVersion();
+        assumeTrue((minorVersion(serverVersion) == 0 && majorVersion(serverVersion) == 4) || majorVersion(serverVersion) < 4);
+
+        //when
+        shell.execute("EXPLAIN MATCH (n) with n.age AS age RETURN age");
+
+        //then
+        String actual = linePrinter.output();
+        assertThat( actual, not( containsString( TablePlanFormatter.DETAILS ) ) );
+        assertThat( actual, containsString( TablePlanFormatter.IDENTIFIERS ) );
+    }
+
     @Test
     public void cypherWithExplainAndRulePlanner() throws CommandException {
         //given (there is no rule planner in neo4j 4.0)

cypher-shell/src/main/java/org/neo4j/shell/prettyprint/TablePlanFormatter.java

@@ -26,9 +26,9 @@
 import static org.neo4j.shell.prettyprint.OutputFormatter.NEWLINE;
 import static org.neo4j.shell.prettyprint.OutputFormatter.repeat;
 
-class TablePlanFormatter {
+public class TablePlanFormatter {
 
-    private static final String UNNAMED_PATTERN_STRING = "  (UNNAMED|FRESHID|AGGREGATION)(\\d+)";
+    private static final String UNNAMED_PATTERN_STRING = "  (UNNAMED|FRESHID|AGGREGATION|NODE|REL)(\\d+)";
     private static final Pattern UNNAMED_PATTERN = Pattern.compile(UNNAMED_PATTERN_STRING);
     private static final String OPERATOR = "Operator";
     private static final String ESTIMATED_ROWS = "Estimated Rows";
@@ -37,16 +37,18 @@ class TablePlanFormatter {
     private static final String PAGE_CACHE = "Cache H/M";
     private static final String TIME = "Time (ms)";
     private static final String ORDER = "Ordered by";
-    private static final String IDENTIFIERS = "Identifiers";
+    public static final String IDENTIFIERS = "Identifiers";
     private static final String OTHER = "Other";
+    public static final String DETAILS = "Details";
     private static final String SEPARATOR = ", ";
     private static final Pattern DEDUP_PATTERN = Pattern.compile("\\s*(\\S+)@\\d+");
+    public static final int MAX_DETAILS_COLUMN_WIDTH = 100;
 
-    private static final List<String> HEADERS = asList(OPERATOR, ESTIMATED_ROWS, ROWS, HITS, PAGE_CACHE, TIME, IDENTIFIERS, ORDER, OTHER);
+    private static final List<String> HEADERS = asList(OPERATOR, DETAILS, ESTIMATED_ROWS, ROWS, HITS, PAGE_CACHE, TIME, IDENTIFIERS, ORDER, OTHER);
 
     private static final Set<String> IGNORED_ARGUMENTS = new LinkedHashSet<>(
             asList( "Rows", "DbHits", "EstimatedRows", "planner", "planner-impl", "planner-version", "version", "runtime", "runtime-impl", "runtime-version",
-                    "time", "source-code", "PageCacheMisses", "PageCacheHits", "PageCacheHitRatio", "Order" ) );
+                    "time", "source-code", "PageCacheMisses", "PageCacheHits", "PageCacheHitRatio", "Order", "Details" ) );
     public static final Value ZERO_VALUE = Values.value(0);
 
     private int width(@Nonnull String header, @Nonnull Map<String, Integer> columns) {
@@ -78,7 +80,8 @@ String formatPlan(@Nonnull Plan plan) {
         Map<String, Integer> columns = new HashMap<>();
         List<Line> lines = accumulate(plan, new Root(), columns);
 
-        List<String> headers = HEADERS.stream().filter(columns::containsKey).collect(Collectors.toList());
+        // Remove Identifiers column if we have a Details column
+        List<String> headers = HEADERS.stream().filter(header -> columns.containsKey(header) && !(header.equals(IDENTIFIERS) && columns.containsKey(DETAILS))).collect(Collectors.toList());
 
         StringBuilder result = new StringBuilder((2 + NEWLINE.length() + headers.stream().mapToInt(h -> width(h, columns)).sum()) * (lines.size() * 2 + 3));
 
@@ -160,6 +163,8 @@ private String serialize(@Nonnull String key, @Nonnull Value v) {
                 return v.asString();
             case "PageCacheMisses":
                 return v.asNumber().toString();
+            case "Details":
+                return v.asString();
             default:
                 return v.asObject().toString();
         }
@@ -207,6 +212,8 @@ private Map<String, Justified> details(@Nonnull Plan plan, @Nonnull Map<String,
                     return mapping(TIME, new Right(String.format("%.3f", value.asLong() / 1000000.0d)), columns);
                 case "Order":
                     return mapping( ORDER, new Left( String.format( "%s", value.asString() ) ), columns );
+                case "Details":
+                    return mapping( DETAILS, new Left( truncate(value.asString()) ), columns );
                 default:
                     return Optional.empty();
             }
@@ -436,4 +443,12 @@ public static <T1, T2> Pair<T1, T2> of(T1 _1, T2 _2) {
             return new Pair<>(_1, _2);
         }
     }
+
+    private String truncate( String original ) {
+        if(original.length() <= MAX_DETAILS_COLUMN_WIDTH){
+            return original;
+        }
+
+        return original.substring( 0, MAX_DETAILS_COLUMN_WIDTH - 3 ) + "...";
+    }
 }

cypher-shell/src/test/java/org/neo4j/shell/prettyprint/TablePlanFormatterTest.java

@@ -0,0 +1,69 @@
+package org.neo4j.shell.prettyprint;
+
+import org.junit.Test;
+
+import java.util.Collections;
+import java.util.Map;
+
+import org.neo4j.driver.Value;
+import org.neo4j.driver.internal.value.StringValue;
+import org.neo4j.driver.summary.Plan;
+
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+import static org.neo4j.shell.prettyprint.OutputFormatter.NEWLINE;
+
+public class TablePlanFormatterTest
+{
+    TablePlanFormatter tablePlanFormatter = new TablePlanFormatter();
+
+    @Test
+    public void renderShortDetails() {
+        Plan plan = mock(Plan.class);
+        Map<String, Value> args = Collections.singletonMap("Details", new StringValue("x.prop AS prop"));
+        when(plan.arguments()).thenReturn(args);
+        when(plan.operatorType()).thenReturn("Projection");
+
+        assertThat(tablePlanFormatter.formatPlan( plan ), is(String.join(NEWLINE,
+                                         "+-------------+----------------+",
+                                         "| Operator    | Details        |",
+                                         "+-------------+----------------+",
+                                         "| +Projection | x.prop AS prop |",
+                                         "+-------------+----------------+", "")));
+    }
+
+    @Test
+    public void renderExactMaxLengthDetails() {
+        Plan plan = mock(Plan.class);
+        String details = stringOfLength(TablePlanFormatter.MAX_DETAILS_COLUMN_WIDTH);
+        Map<String, Value> args = Collections.singletonMap("Details", new StringValue(details));
+        when(plan.arguments()).thenReturn(args);
+        when(plan.operatorType()).thenReturn("Projection");
+
+        assertThat(tablePlanFormatter.formatPlan( plan ), containsString("| +Projection | " + details + " |"));
+    }
+
+    @Test
+    public void truncateTooLongDetails() {
+        Plan plan = mock(Plan.class);
+        String details = stringOfLength(TablePlanFormatter.MAX_DETAILS_COLUMN_WIDTH + 1);
+        Map<String, Value> args = Collections.singletonMap("Details", new StringValue(details));
+        when(plan.arguments()).thenReturn(args);
+        when(plan.operatorType()).thenReturn("Projection");
+
+        assertThat(tablePlanFormatter.formatPlan( plan ), containsString("| +Projection | " + details.substring( 0, TablePlanFormatter.MAX_DETAILS_COLUMN_WIDTH - 3 ) + "... |"));
+    }
+
+    private String stringOfLength(int length) {
+        StringBuilder strBuilder = new StringBuilder();
+
+        for(int i=0; i<length; i++) {
+            strBuilder.append('a');
+        }
+
+        return strBuilder.toString();
+    }
+}