Merge pull request #125 from aws/lyndon/aggregate-issue

Fixed aggregate issue and added unsupported exception for WHERE in JOIN

sql-gremlin/README.asciidoc

@@ -45,6 +45,9 @@ Identifier matching is case-sensitive and identifiers that match a reserved SQL
 === Joins
 Currently, the driver only support `INNER JOIN` on two vertices that are connected by an edge. When looking at vertices you will see `<edge_label>_IN_ID` or `<edge_label>_OUT_ID`.
 
+`WHERE` and `HAVING` are not currently supported in conjunction with a `JOIN` clause.
+Foreign keys are not generated and not exposed in JDBC metadata at this time.
+
 Vertices can be joined on columns that have the same edge label and one ends with `IN_ID` while the other ends with an `OUT_ID`.
 
 === Data Types
@@ -66,11 +69,13 @@ Supported operators are listed below. Arithmetic, string, conditional and date o
 
 ==== Comparison Operators
 * `value1 <op> value2` where `op` is one of : `=`, `<>`, `<`, `>`, `<=` or `>=`
+* `NOT` in conjunction with a comparison operator is not currently supported
 
 ==== Logical Operators
 * `boolean1` OR `boolean2`
 * `boolean1` AND `boolean2`
-* NOT `boolean2`
+* NOT `boolean2` is supported only when used with a boolean valued column in `WHERE`/`HAVING` filters
+* NOT `boolean2` is supported with expressions and boolean values in `SELECT` clause
 
 ==== Aggregate Functions
 * `AVG(numeric)`

sql-gremlin/src/main/java/org/twilmes/sql/gremlin/adapter/converter/SqlMetadata.java

@@ -33,7 +33,6 @@
 import org.twilmes.sql.gremlin.adapter.converter.schema.gremlin.GremlinTableBase;
 import org.twilmes.sql.gremlin.adapter.converter.schema.gremlin.GremlinVertexTable;
 import org.twilmes.sql.gremlin.adapter.util.SqlGremlinError;
-
 import java.sql.SQLException;
 import java.time.Instant;
 import java.util.ArrayList;
@@ -218,8 +217,14 @@ public String getRenameFromActual(final String actual) {
     }
 
     public String getActualColumnName(final GremlinTableBase table, final String column) throws SQLException {
-        final String actualColumnName = getRenamedColumn(column);
-        return table.getColumn(actualColumnName).getName();
+        if (table.hasColumn(column)) {
+            return table.getColumn(column).getName();
+        } else if (columnRenameMap.containsKey(column)) {
+            return table.getColumn(getRenamedColumn(column)).getName();
+        }
+        final Optional<String> actualName = columnRenameMap.entrySet().stream().
+                filter(entry -> entry.getValue().equals(column)).map(Map.Entry::getKey).findFirst();
+        return table.getColumn(actualName.orElse(column)).getName();
     }
 
     public boolean getTableHasColumn(final GremlinTableBase table, final String column) {

sql-gremlin/src/main/java/org/twilmes/sql/gremlin/adapter/converter/SqlTraversalEngine.java

@@ -29,7 +29,6 @@
 import org.twilmes.sql.gremlin.adapter.converter.schema.gremlin.GremlinTableBase;
 import org.twilmes.sql.gremlin.adapter.results.SqlGremlinQueryResult;
 import org.twilmes.sql.gremlin.adapter.util.SqlGremlinError;
-
 import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.List;
@@ -100,9 +99,16 @@ public static void addProjection(final List<GremlinSqlIdentifier> gremlinSqlIden
     }
 
 
-    public static void applyTraversal(final GraphTraversal<?, ?> graphTraversal,
-                                      final GraphTraversal<?, ?> subGraphTraversal) {
-        graphTraversal.by(subGraphTraversal);
+    public static void applyTraversal(GraphTraversal graphTraversal,
+                                      final GraphTraversal subGraphTraversal,
+                                      final boolean apply) {
+            graphTraversal.by((apply ? __.coalesce(subGraphTraversal, __.constant(SqlGremlinQueryResult.NULL_VALUE)) : subGraphTraversal));
+    }
+
+
+    public static void applyTraversal(final GraphTraversal graphTraversal,
+                                      final GraphTraversal subGraphTraversal) {
+        applyTraversal(graphTraversal, subGraphTraversal, false);
     }
 
     public static void applySqlIdentifier(final GremlinSqlIdentifier sqlIdentifier,
@@ -115,7 +121,8 @@ public static void applySqlIdentifier(final GremlinSqlIdentifier sqlIdentifier,
             graphTraversal.constant(1);
         } else if (sqlIdentifier.getNameCount() == 2) {
             // With size 2 format of identifier is 'table'.'column' => ['table', 'column']
-            appendGraphTraversal(sqlIdentifier.getName(0), sqlMetadata.getRenamedColumn(sqlIdentifier.getName(1)), sqlMetadata, graphTraversal);
+            appendGraphTraversal(sqlIdentifier.getName(0), sqlMetadata.getRenamedColumn(sqlIdentifier.getName(1)),
+                    sqlMetadata, graphTraversal);
         } else {
             // With size 1, format of identifier is 'column'.
             appendGraphTraversal(sqlMetadata.getRenamedColumn(sqlIdentifier.getName(0)), sqlMetadata, graphTraversal);
@@ -128,7 +135,8 @@ public static void applySqlIdentifier(final GremlinSqlIdentifier sqlIdentifier,
         return __.project(firstColumn, remaining);
     }
 
-    private static void appendColumnSelect(final SqlMetadata sqlMetadata, final String column, final GraphTraversal<?, ?> graphTraversal) {
+    private static void appendColumnSelect(final SqlMetadata sqlMetadata, final String column,
+                                           final GraphTraversal<?, ?> graphTraversal) {
         if (sqlMetadata.isDoneFilters()) {
             graphTraversal.choose(__.has(column), __.values(column), __.constant(SqlGremlinQueryResult.NULL_VALUE));
         } else {
@@ -151,7 +159,8 @@ private static void appendGraphTraversal(final String table, final String column
             }
         } else {
             // It's this vertex/edge.
-            if (columnName.toLowerCase(Locale.getDefault()).startsWith(gremlinTableBase.getLabel().toLowerCase(Locale.getDefault()))) {
+            if (columnName.toLowerCase(Locale.getDefault())
+                    .startsWith(gremlinTableBase.getLabel().toLowerCase(Locale.getDefault()))) {
                 graphTraversal.id();
             } else {
                 if (columnName.endsWith(IN_ID)) {

sql-gremlin/src/main/java/org/twilmes/sql/gremlin/adapter/converter/ast/nodes/operator/logic/GremlinSqlBinaryOperator.java

@@ -187,18 +187,20 @@ void handleEmbeddedGremlinSqlBasicCall(final GremlinSqlBasicCall gremlinSqlBasic
         if (operands.size() != 2) {
             throw SqlGremlinError.create(SqlGremlinError.BINARY_AND_PREFIX_OPERAND_COUNT);
         }
-        final GraphTraversal<?, ?>[] graphTraversals = new GraphTraversal[2];
+        final GraphTraversal[] graphTraversals = new GraphTraversal[2];
         for (int i = 0; i < operands.size(); i++) {
             graphTraversals[i] = __.unfold();
             if (operands.get(i) instanceof GremlinSqlIdentifier) {
-                GremlinSqlIdentifier gremlinSqlIdentifier = (GremlinSqlIdentifier) operands.get(i);
-                GraphTraversal subtraversal = __.unfold();
+                final GremlinSqlIdentifier gremlinSqlIdentifier = (GremlinSqlIdentifier) operands.get(i);
+                final GraphTraversal subtraversal = __.unfold();
                 SqlTraversalEngine.applySqlIdentifier(gremlinSqlIdentifier, sqlMetadata, subtraversal);
                 graphTraversals[i] = __.coalesce(subtraversal,
                         __.constant(sqlMetadata.getDefaultCoalesceValue(gremlinSqlIdentifier.getColumn())));
             } else if (operands.get(i) instanceof GremlinSqlBasicCall) {
-                GremlinSqlBasicCall gremlinSqlBasicCall = ((GremlinSqlBasicCall) operands.get(i));
+                final GremlinSqlBasicCall gremlinSqlBasicCall = ((GremlinSqlBasicCall) operands.get(i));
                 gremlinSqlBasicCall.generateTraversal(graphTraversals[i]);
+                graphTraversals[i] = __.coalesce(graphTraversals[i],
+                        __.constant(sqlMetadata.getDefaultCoalesceValue(gremlinSqlBasicCall.getActual())));
             } else if (operands.get(i) instanceof GremlinSqlLiteral) {
                 ((GremlinSqlLiteral) operands.get(i)).appendTraversal(graphTraversals[i]);
             }

sql-gremlin/src/main/java/org/twilmes/sql/gremlin/adapter/converter/ast/nodes/select/GremlinSqlSelect.java

@@ -130,12 +130,12 @@ protected void applyColumnRetrieval(final GraphTraversal<?, ?> graphTraversal, f
                         SqlTraversalEngine.getEmptyTraversal(stepDirection, sqlMetadata);
                 SqlTraversalEngine
                         .applySqlIdentifier((GremlinSqlIdentifier) gremlinSqlNode, sqlMetadata, subSubGraphTraversal);
-                SqlTraversalEngine.applyTraversal(subGraphTraversal, subSubGraphTraversal);
+                SqlTraversalEngine.applyTraversal(subGraphTraversal, subSubGraphTraversal, true);
             } else if (gremlinSqlNode instanceof GremlinSqlBasicCall) {
                 final GraphTraversal<?, ?> subSubGraphTraversal =
                         SqlTraversalEngine.getEmptyTraversal(stepDirection, sqlMetadata);
                 ((GremlinSqlBasicCall) gremlinSqlNode).generateTraversal(subSubGraphTraversal);
-                SqlTraversalEngine.applyTraversal(subGraphTraversal, subSubGraphTraversal);
+                SqlTraversalEngine.applyTraversal(subGraphTraversal, subSubGraphTraversal, true);
             } else {
                 throw SqlGremlinError.create(SqlGremlinError.UNKNOWN_NODE_SELECTLIST, gremlinSqlNode.getClass().getName());
             }

sql-gremlin/src/main/java/org/twilmes/sql/gremlin/adapter/converter/ast/nodes/select/GremlinSqlSelectMulti.java

@@ -209,6 +209,7 @@ protected void runTraversalExecutor(final GraphTraversal<?, ?> graphTraversal,
             graphTraversal = g.E().hasLabel(edgeLabel)
                     .where(__.inV().hasLabel(inVLabel))
                     .where(__.outV().hasLabel(outVLabel));
+            applyWhere(graphTraversal);
             applyGroupBy(graphTraversal, edgeLabel, inVRename, outVRename);
             applySelectValues(graphTraversal);
             applyOrderBy(graphTraversal, edgeLabel, inVRename, outVRename);
@@ -311,4 +312,11 @@ protected void applyHaving(final GraphTraversal<?, ?> graphTraversal) throws SQL
         }
         throw SqlGremlinError.createNotSupported(SqlGremlinError.JOIN_HAVING_UNSUPPORTED);
     }
+
+    protected void applyWhere(final GraphTraversal<?, ?> graphTraversal) throws SQLException {
+        if (sqlSelect.getWhere() == null) {
+            return;
+        }
+        throw SqlGremlinError.createNotSupported(SqlGremlinError.JOIN_WHERE_UNSUPPORTED);
+    }
 }

sql-gremlin/src/main/java/org/twilmes/sql/gremlin/adapter/util/SqlGremlinError.java

@@ -73,6 +73,7 @@ public enum SqlGremlinError {
     CANNOT_GROUP_TABLE,
     CANNOT_GROUP_COLUMN,
     JOIN_HAVING_UNSUPPORTED,
+    JOIN_WHERE_UNSUPPORTED,
     SINGLE_SELECT_MULTI_RETURN,
     SELECT_NO_LIST,
     UNEXPECTED_FROM_FORMAT,

sql-gremlin/src/main/resources/error-messages_en.properties

@@ -59,6 +59,7 @@ CANNOT_GROUP_EDGES=Error: Cannot group by edges.
 CANNOT_GROUP_TABLE=Error: Unable to group table %s.
 CANNOT_GROUP_COLUMN=Error: Unable to group column %s.
 JOIN_HAVING_UNSUPPORTED=Unsupported: HAVING is not currently supported for JOIN.
+JOIN_WHERE_UNSUPPORTED=Unsupported: WHERE is not currently supported for JOIN.
 SINGLE_SELECT_MULTI_RETURN=Error: Single select has multi-table return.
 SELECT_NO_LIST=Error: GremlinSqlSelect expects select list component.
 UNEXPECTED_FROM_FORMAT=Unexpected format for FROM.

sql-gremlin/src/test/java/org/twilmes/sql/gremlin/adapter/GremlinSqlAdvancedSelectTest.java

@@ -147,7 +147,7 @@ public void testWhere() throws SQLException {
                 rows(r("Tom", 35), r("Juanita", 50)));
     }
 
-    // TODO: Support NOT on operators
+    // TODO #127: Support NOT in WHERE filter.
     @Test
     @Disabled
     public void testWhereNot() throws SQLException {
@@ -385,6 +385,9 @@ void testMultiComparisonOperator() throws SQLException {
         runQueryTestResults(
                 "SELECT NOT age > 0 FROM person",
                 columns("NOT age > 0"), rows(r(false), r(false), r(false), r(false), r(false), r(false)));
+        runQueryTestResults(
+                "SELECT NOT name = 'Tom' FROM person",
+                columns("NOT name = Tom"), rows(r(false), r(true), r(true), r(true), r(true), r(true)));
     }
 
     @Test
@@ -410,5 +413,8 @@ void testComparisonWithAsOperator() throws SQLException {
         runQueryTestResults(
                 "SELECT NOT age > 0 AS a FROM person",
                 columns("a"), rows(r(false), r(false), r(false), r(false), r(false), r(false)));
+        runQueryTestResults(
+                "SELECT NOT name = 'Tom' AS a FROM person",
+                columns("a"), rows(r(false), r(true), r(true), r(true), r(true), r(true)));
     }
 }

sql-gremlin/src/test/java/org/twilmes/sql/gremlin/adapter/GremlinSqlBaseTest.java

@@ -123,13 +123,13 @@ protected void runJoinQueryTestResults(final String query, final List<String> co
 
     protected void runQueryTestThrows(final String query, final SqlGremlinError messageKey,
                                       final Object... formatArgs) {
-        Throwable t = Assertions.assertThrows(SQLException.class, () -> converter.executeQuery(g, query));
+        final Throwable t = Assertions.assertThrows(SQLException.class, () -> converter.executeQuery(g, query));
         Assert.assertEquals(SqlGremlinError.getMessage(messageKey, formatArgs), t.getMessage());
     }
 
     protected void runNotSupportedQueryTestThrows(final String query, final SqlGremlinError messageKey,
                                                   final Object... formatArgs) {
-        Throwable t = Assertions.assertThrows(SQLNotSupportedException.class, () -> converter.executeQuery(g, query));
+        final Throwable t = Assertions.assertThrows(SQLNotSupportedException.class, () -> converter.executeQuery(g, query));
         Assert.assertEquals(SqlGremlinError.getMessage(messageKey, formatArgs), t.getMessage());
     }
 

sql-gremlin/src/test/java/org/twilmes/sql/gremlin/adapter/GremlinSqlIncompleteSelectTest.java

@@ -19,11 +19,7 @@
 
 package org.twilmes.sql.gremlin.adapter;
 
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
-import org.twilmes.sql.gremlin.adapter.util.SqlGremlinError;
-
-import java.math.BigDecimal;
 import java.sql.SQLException;
 
 public class GremlinSqlIncompleteSelectTest extends GremlinSqlBaseTest {
@@ -44,7 +40,8 @@ public void testProject() throws SQLException {
 
     @Test
     public void testEdges() throws SQLException {
-        runQueryTestResults("select * from worksFor where yearsWorked = 9", columns("person_OUT_ID", "company_IN_ID", "yearsWorked", "worksFor_ID"),
+        runQueryTestResults("select * from worksFor where yearsWorked = 9",
+                columns("person_OUT_ID", "company_IN_ID", "yearsWorked", "worksFor_ID"),
                 rows(r(25L, 2L, 9, 61L)));
     }
 
@@ -66,7 +63,7 @@ public void testOrderWithNull() throws SQLException {
         runQueryTestResults("SELECT name, age AS a FROM person ORDER BY a", columns("name", "a"),
                 rows(r("Patty", 29), r(null, 30), r("Phil", 31), r("Susan", 45), r("Juanita", 50), r("Tom", null)));
         runQueryTestResults("SELECT name, age AS a FROM person ORDER BY a DESC", columns("name", "a"),
-                rows( r("Tom", null), r("Juanita", 50), r("Susan", 45), r("Phil", 31), r(null, 30), r("Patty", 29)));
+                rows(r("Tom", null), r("Juanita", 50), r("Susan", 45), r("Phil", 31), r(null, 30), r("Patty", 29)));
 
         // ORDER with string column.
         runQueryTestResults("SELECT name, age FROM person ORDER BY name", columns("name", "age"),
@@ -90,66 +87,70 @@ public void testWhereNull() throws SQLException {
 
     // TODO: Support aggregates for null values
     @Test
-    @Disabled
     public void testHavingNull() throws SQLException {
-        // HAVING with aggregate literal.
+        runQueryTestResults("SELECT MIN(age) AS age FROM person GROUP BY name HAVING MIN(age) > 1",
+               columns("age"),
+               rows(r((Object) null), r(29), r(31), r(45), r(50), r(30)));
         runQueryTestResults("SELECT wentToSpace, SUM(age) FROM person GROUP BY wentToSpace HAVING SUM(age) > 1000",
-                columns("wentToSpace", "SUM(age)"),
-                rows());
+               columns("wentToSpace", "SUM(age)"),
+               rows());
         runQueryTestResults("SELECT wentToSpace, COUNT(age) FROM person GROUP BY wentToSpace HAVING COUNT(age) < 1000",
                 columns("wentToSpace", "COUNT(age)"),
-                rows(r(false, 3L), r(true, 3L)));
-        runQueryTestResults("SELECT wentToSpace, COUNT(age) FROM person GROUP BY wentToSpace HAVING COUNT(age) <> 3",
+                rows(r(false, 2L), r(true, 2L), r(null, 1L)));
+        runQueryTestResults("SELECT wentToSpace, COUNT(age) FROM person GROUP BY wentToSpace HAVING COUNT(age) <> 2",
                 columns("wentToSpace", "COUNT(age)"),
-                rows());
+                rows(r(null, 1L)));
         runQueryTestResults("SELECT COUNT(age), SUM(age) FROM person GROUP BY wentToSpace HAVING SUM(age) > 100",
                 columns("COUNT(age)", "SUM(age)"),
-                rows(r(3L, 125L)));
+                rows());
+        runQueryTestResults("SELECT COUNT(age), SUM(age) FROM person GROUP BY wentToSpace HAVING SUM(age) > 80",
+                columns("COUNT(age)", "SUM(age)"),
+                rows(r(2L, 95L)));
         runQueryTestResults("SELECT COUNT(age), SUM(age) FROM person GROUP BY wentToSpace HAVING SUM(age) < 100",
                 columns("COUNT(age)", "SUM(age)"),
-                rows(r(3L, 95L)));
+                rows(r(2L, 60L), r(2L, 95L), r(1L, 30L)));
         runQueryTestResults("SELECT COUNT(age), SUM(age) FROM person GROUP BY age HAVING SUM(age) <> 1000",
                 columns("COUNT(age)", "SUM(age)"),
-                rows(r(1L, 35L), r(1L, 29L), r(1L, 31L), r(1L, 45L), r(1L, 50L), r(1L, 30L)));
-        runQueryTestResults("SELECT COUNT(age), SUM(age) FROM person GROUP BY age HAVING SUM(age) = 35",
+                rows(r(0L, null), r(1L, 29L), r(1L, 31L), r(1L, 45L), r(1L, 50L), r(1L, 30L)));
+        runQueryTestResults("SELECT COUNT(age), SUM(age) FROM person GROUP BY age HAVING SUM(age) = 29",
                 columns("COUNT(age)", "SUM(age)"),
-                rows(r(1L, 35L)));
+                rows(r(1L, 29L)));
 
         // Having with AND.
         runQueryTestResults(
                 "SELECT COUNT(age), SUM(age) FROM person GROUP BY wentToSpace HAVING SUM(age) <> 1000 AND COUNT(age) = 3",
                 columns("COUNT(age)", "SUM(age)"),
-                rows(r(3L, 95L), r(3L, 125L)));
+                rows());
         runQueryTestResults(
                 "SELECT COUNT(age), SUM(age) FROM person GROUP BY wentToSpace HAVING SUM(age) = 125 AND COUNT(age) = 3",
                 columns("COUNT(age)", "SUM(age)"),
-                rows(r(3L, 125L)));
+                rows());
         runQueryTestResults(
                 "SELECT COUNT(age), SUM(age) FROM person GROUP BY wentToSpace HAVING SUM(age) <> 125 AND COUNT(age) = 3",
                 columns("COUNT(age)", "SUM(age)"),
-                rows(r(3L, 95L)));
+                rows());
         runQueryTestResults(
                 "SELECT COUNT(age), SUM(age) FROM person GROUP BY wentToSpace HAVING SUM(age) <> 125 AND COUNT(age) <> 3",
                 columns("COUNT(age)", "SUM(age)"),
-                rows());
+                rows(r(2L, 60L), r(2L, 95L), r(1L, 30L)));
 
         // Having with OR.
         runQueryTestResults(
                 "SELECT COUNT(age), SUM(age) FROM person GROUP BY wentToSpace HAVING SUM(age) = 1000 OR COUNT(age) = 3",
                 columns("COUNT(age)", "SUM(age)"),
-                rows(r(3L, 95L), r(3L, 125L)));
+                rows());
         runQueryTestResults(
                 "SELECT COUNT(age), SUM(age) FROM person GROUP BY wentToSpace HAVING SUM(age) = 1000 OR COUNT(age) <> 3",
                 columns("COUNT(age)", "SUM(age)"),
-                rows());
+                rows(r(2L, 60L), r(2L, 95L), r(1L, 30L)));
         runQueryTestResults(
                 "SELECT COUNT(age), SUM(age) FROM person GROUP BY wentToSpace HAVING SUM(age) = 125 OR COUNT(age) <> 3",
                 columns("COUNT(age)", "SUM(age)"),
-                rows(r(3L, 125L)));
+                rows(r(2L, 60L), r(2L, 95L), r(1L, 30L)));
         runQueryTestResults(
                 "SELECT COUNT(age), SUM(age) FROM person GROUP BY wentToSpace HAVING SUM(age) <> 125 OR COUNT(age) <> 3",
                 columns("COUNT(age)", "SUM(age)"),
-                rows(r(3L, 95L)));
+                rows(r(2L, 60L), r(2L, 95L), r(1L, 30L)));
     }
 
     @Test