Skip to content

Commit

Permalink
Merge pull request #73 from aws/lyndon/AN-878-limit-1-error
Browse files Browse the repository at this point in the history 
Fixed exception in DBVisualizer for LIMIT 1 queries.
  • Loading branch information
@lyndonbauto
lyndonbauto authored Nov 26, 2021
2 parents c7e021d + fbe7396 commit 6b62826
Show file tree
Hide file tree
Showing 6 changed files with 89 additions and 82 deletions.
3 changes: 2 additions & 1 deletion build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ task writeProperties(type: WriteProperties) {
jacoco {
toolVersion = "0.8.5"
}

jacocoTestReport {
reports {
html.enabled true
Expand Down Expand Up @@ -259,7 +260,7 @@ dependencies {

// Testing
testImplementation group: 'com.googlecode.json-simple', name: 'json-simple', version: '1.1.1'
testImplementation group: 'junit', name: 'junit', version: '4.13.2'
testImplementation group: 'junit', name: 'junit', version: '4.13.2'
testImplementation group: 'org.junit.jupiter', name: 'junit-jupiter-api', version: jupiterVersion
testImplementation group: 'org.junit.jupiter', name: 'junit-jupiter-engine', version: jupiterVersion
testImplementation group: 'org.mockito', name: 'mockito-core', version: '4.1.0'
Expand Down
69 changes: 22 additions & 47 deletions sql-gremlin/src/main/java/org/twilmes/sql/gremlin/adapter/results/SqlGremlinQueryResult.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@

import lombok.Getter;
import org.twilmes.sql.gremlin.adapter.converter.SqlMetadata;
import org.twilmes.sql.gremlin.adapter.converter.schema.gremlin.GremlinProperty;
import org.twilmes.sql.gremlin.adapter.converter.schema.gremlin.GremlinTableBase;
import java.sql.SQLException;
import java.util.ArrayList;
Expand All @@ -30,16 +29,13 @@
import java.util.concurrent.LinkedBlockingQueue;

@Getter
public class SqlGremlinQueryResult {
public class SqlGremlinQueryResult implements AutoCloseable {
public static final String EMPTY_MESSAGE = "No more results.";
public static final String NULL_VALUE = "$%#NULL#%$";
private final List<String> columns;
private final List<String> columnTypes = new ArrayList<>();
private final Object assertEmptyLock = new Object();
private final BlockingQueue<List<Object>> blockingQueueRows = new LinkedBlockingQueue<>();
private boolean isEmpty = false;
private SQLException paginationException = null;
private Thread currThread = null;

public SqlGremlinQueryResult(final List<String> columns, final List<GremlinTableBase> gremlinTableBases,
final SqlMetadata sqlMetadata) throws SQLException {
Expand All @@ -49,7 +45,8 @@ public SqlGremlinQueryResult(final List<String> columns, final List<GremlinTable
}
}

private String getType(final String column, final SqlMetadata sqlMetadata, final List<GremlinTableBase> gremlinTableBases) throws SQLException {
private String getType(final String column, final SqlMetadata sqlMetadata,
final List<GremlinTableBase> gremlinTableBases) throws SQLException {
if (sqlMetadata.aggregateTypeExists(column)) {
return sqlMetadata.getOutputType(column, "string");
}
Expand All @@ -67,60 +64,38 @@ private String getType(final String column, final SqlMetadata sqlMetadata, final
}

public void setPaginationException(final SQLException e) {
synchronized (assertEmptyLock) {
paginationException = e;
if (currThread != null && blockingQueueRows.size() == 0) {
currThread.interrupt();
}
}
}

public boolean getIsEmpty() throws SQLException {
if (paginationException == null) {
return isEmpty;
}
throw paginationException;
paginationException = e;
close();
}

public void assertIsEmpty() {
synchronized (assertEmptyLock) {
if (currThread != null && blockingQueueRows.size() == 0) {
currThread.interrupt();
}
isEmpty = true;
}
@Override
public void close() {
blockingQueueRows.add(new EmptyResult());
}

public void addResults(final List<List<Object>> rows) {
for (final List<Object> row : rows) {
for (int i = 0; i < row.size(); i++) {
if (row.get(i) instanceof String && row.get(i).toString().equals(NULL_VALUE)) {
row.set(i, null);
}
}
}
// This is a workaround for Gremlin null support not being in any version of Gremlin that is
// widely supported by database vendors.
rows.forEach(row -> row.replaceAll(col -> (col instanceof String && col.equals(NULL_VALUE) ? null : col)));
blockingQueueRows.addAll(rows);
}

private boolean getShouldExit() throws SQLException {
synchronized (assertEmptyLock) {
return (getIsEmpty() && blockingQueueRows.size() == 0);
}
}

public Object getResult() throws SQLException {
synchronized (assertEmptyLock) {
this.currThread = Thread.currentThread();
}
while (!getShouldExit()) {
public List<Object> getResult() throws SQLException {
while (true) {
try {
return this.blockingQueueRows.take();
} catch (final InterruptedException ignored) {
final List<Object> result = blockingQueueRows.take();

// If a pagination exception occurs, an EmptyResult Object will be inserted into the BlockingQueue.
// The pagination exception needs to be checked before returning.
if (paginationException != null) {
throw paginationException;
}
return result;
} catch (final InterruptedException ignored) {
}
}
throw new SQLException(EMPTY_MESSAGE);
}

public static class EmptyResult extends ArrayList<Object> {
}
}
2 changes: 1 addition & 1 deletion sql-gremlin/src/main/java/org/twilmes/sql/gremlin/adapter/results/pagination/Pagination.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public void run() {
convertAndInsertResult(sqlGremlinQueryResult, rows);
}
// If we run out of traversal data (or hit our limit), stop and signal to the result that it is done.
sqlGremlinQueryResult.assertIsEmpty();
sqlGremlinQueryResult.close();
} catch (final Exception e) {
final StringWriter sw = new StringWriter();
final PrintWriter pw = new PrintWriter(sw);
Expand Down
64 changes: 55 additions & 9 deletions sql-gremlin/src/test/java/org/twilmes/sql/gremlin/adapter/GremlinSqlAdvancedSelectTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,36 +241,82 @@ void testMisc() throws SQLException {
rows(r("Tom"), r("Susan"), r("Phil"), r("Pavel"), r("Patty"), r("Juanita")));

// NULLS FIRST predicate is not currently supported.
runQueryTestThrows("SELECT name FROM \"gremlin\".\"person\" ORDER BY name NULLS FIRST", "Error, no appropriate order for GremlinSqlPostFixOperator of NULLS_FIRST.");
runQueryTestThrows("SELECT name FROM \"gremlin\".\"person\" ORDER BY name NULLS FIRST",
"Error, no appropriate order for GremlinSqlPostFixOperator of NULLS_FIRST.");
}

@Test
void testAggregateLiteralHavingNoGroupBy() throws SQLException {
// Tableau was sending queries like this for the preview in 2021.3
runQueryTestResults("SELECT SUM(1) AS \"cnt:airport_03C2E834E28942D3AA2423AC01F4B33D:ok\" FROM gremlin.person AS person HAVING COUNT(1) > 0",
runQueryTestResults(
"SELECT SUM(1) AS \"cnt:airport_03C2E834E28942D3AA2423AC01F4B33D:ok\" FROM gremlin.person AS person HAVING COUNT(1) > 0",
columns("cnt:airport_03C2E834E28942D3AA2423AC01F4B33D:ok"),
rows(r(new BigDecimal(6))));
runQueryTestResults("SELECT MIN(1) AS \"cnt:airport_03C2E834E28942D3AA2423AC01F4B33D:ok\" FROM gremlin.person AS person HAVING COUNT(1) > 0",
runQueryTestResults(
"SELECT MIN(1) AS \"cnt:airport_03C2E834E28942D3AA2423AC01F4B33D:ok\" FROM gremlin.person AS person HAVING COUNT(1) > 0",
columns("cnt:airport_03C2E834E28942D3AA2423AC01F4B33D:ok"),
rows(r(new BigDecimal(1))));
runQueryTestResults("SELECT MAX(1) AS \"cnt:airport_03C2E834E28942D3AA2423AC01F4B33D:ok\" FROM gremlin.person AS person HAVING COUNT(1) > 0",
runQueryTestResults(
"SELECT MAX(1) AS \"cnt:airport_03C2E834E28942D3AA2423AC01F4B33D:ok\" FROM gremlin.person AS person HAVING COUNT(1) > 0",
columns("cnt:airport_03C2E834E28942D3AA2423AC01F4B33D:ok"),
rows(r(new BigDecimal(1))));
runQueryTestResults("SELECT AVG(1) AS \"cnt:airport_03C2E834E28942D3AA2423AC01F4B33D:ok\" FROM gremlin.person AS person HAVING COUNT(1) > 0",
runQueryTestResults(
"SELECT AVG(1) AS \"cnt:airport_03C2E834E28942D3AA2423AC01F4B33D:ok\" FROM gremlin.person AS person HAVING COUNT(1) > 0",
columns("cnt:airport_03C2E834E28942D3AA2423AC01F4B33D:ok"),
rows(r(new BigDecimal(1))));

runQueryTestResults("SELECT SUM(2) AS \"cnt:airport_03C2E834E28942D3AA2423AC01F4B33D:ok\" FROM gremlin.person AS person HAVING COUNT(1) > 0",
runQueryTestResults(
"SELECT SUM(2) AS \"cnt:airport_03C2E834E28942D3AA2423AC01F4B33D:ok\" FROM gremlin.person AS person HAVING COUNT(1) > 0",
columns("cnt:airport_03C2E834E28942D3AA2423AC01F4B33D:ok"),
rows(r(new BigDecimal(12))));
runQueryTestResults("SELECT MIN(2) AS \"cnt:airport_03C2E834E28942D3AA2423AC01F4B33D:ok\" FROM gremlin.person AS person HAVING COUNT(1) > 0",
runQueryTestResults(
"SELECT MIN(2) AS \"cnt:airport_03C2E834E28942D3AA2423AC01F4B33D:ok\" FROM gremlin.person AS person HAVING COUNT(1) > 0",
columns("cnt:airport_03C2E834E28942D3AA2423AC01F4B33D:ok"),
rows(r(new BigDecimal(2))));
runQueryTestResults("SELECT MAX(2) AS \"cnt:airport_03C2E834E28942D3AA2423AC01F4B33D:ok\" FROM gremlin.person AS person HAVING COUNT(1) > 0",
runQueryTestResults(
"SELECT MAX(2) AS \"cnt:airport_03C2E834E28942D3AA2423AC01F4B33D:ok\" FROM gremlin.person AS person HAVING COUNT(1) > 0",
columns("cnt:airport_03C2E834E28942D3AA2423AC01F4B33D:ok"),
rows(r(new BigDecimal(2))));
runQueryTestResults("SELECT AVG(2) AS \"cnt:airport_03C2E834E28942D3AA2423AC01F4B33D:ok\" FROM gremlin.person AS person HAVING COUNT(1) > 0",
runQueryTestResults(
"SELECT AVG(2) AS \"cnt:airport_03C2E834E28942D3AA2423AC01F4B33D:ok\" FROM gremlin.person AS person HAVING COUNT(1) > 0",
columns("cnt:airport_03C2E834E28942D3AA2423AC01F4B33D:ok"),
rows(r(new BigDecimal(2))));
}

@Test
void testLimit() throws SQLException {
// LIMIT 1 tests.
// Single result query.
runQueryTestResults("SELECT name, age FROM person WHERE name = 'Tom' ORDER BY age LIMIT 1",
columns("name", "age"),
rows(r("Tom", 35)));
// Multi result query.
runQueryTestResults("SELECT name, age FROM person ORDER BY age LIMIT 1",
columns("name", "age"),
rows(r("Patty", 29)));

// LIMIT > 1 tests.
runQueryTestResults("SELECT name, age FROM person WHERE name <> 'Tom' ORDER BY age LIMIT 2",
columns("name", "age"),
rows(r("Patty", 29), r("Pavel", 30)));
runQueryTestResults("SELECT name, age FROM person WHERE name <> 'Tom' ORDER BY age LIMIT 3",
columns("name", "age"),
rows(r("Patty", 29), r("Pavel", 30), r("Phil", 31)));
runQueryTestResults("SELECT name, age FROM person WHERE name <> 'Tom' ORDER BY age LIMIT 4",
columns("name", "age"),
rows(r("Patty", 29), r("Pavel", 30), r("Phil", 31), r("Susan", 45)));
runQueryTestResults("SELECT name, age FROM person WHERE name <> 'Tom' ORDER BY age LIMIT 5",
columns("name", "age"),
rows(r("Patty", 29), r("Pavel", 30), r("Phil", 31), r("Susan", 45), r("Juanita", 50)));
runQueryTestResults("SELECT name, age FROM person WHERE name <> 'Tom' ORDER BY age LIMIT 6",
columns("name", "age"),
rows(r("Patty", 29), r("Pavel", 30), r("Phil", 31), r("Susan", 45), r("Juanita", 50)));
runQueryTestResults("SELECT name, age FROM person WHERE name <> 'Tom' ORDER BY age LIMIT 1000",
columns("name", "age"),
rows(r("Patty", 29), r("Pavel", 30), r("Phil", 31), r("Susan", 45), r("Juanita", 50)));
runQueryTestResults(
String.format("SELECT name, age FROM person WHERE name <> 'Tom' ORDER BY age LIMIT %d", Long.MAX_VALUE),
columns("name", "age"),
rows(r("Patty", 29), r("Pavel", 30), r("Phil", 31), r("Susan", 45), r("Juanita", 50)));
}
}
18 changes: 5 additions & 13 deletions sql-gremlin/src/test/java/org/twilmes/sql/gremlin/adapter/GremlinSqlBaseTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,21 +151,13 @@ static class SqlGremlinTestResult {

SqlGremlinTestResult(final SqlGremlinQueryResult sqlGremlinQueryResult) throws SQLException {
columns = sqlGremlinQueryResult.getColumns();
Object res;
List<?> res;
do {
try {
res = sqlGremlinQueryResult.getResult();
} catch (final SQLException e) {
if (e.getMessage().equals(SqlGremlinQueryResult.EMPTY_MESSAGE)) {
break;
} else {
throw e;
}
res = sqlGremlinQueryResult.getResult();
if (!(res instanceof SqlGremlinQueryResult.EmptyResult)) {
this.rows.add(res);
}
if (res instanceof List<?>) {
this.rows.add((List<?>) res);
}
} while (true);
} while (!(res instanceof SqlGremlinQueryResult.EmptyResult));
}
}
}
15 changes: 4 additions & 11 deletions src/main/java/software/aws/neptune/gremlin/sql/SqlGremlinResultSet.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import org.twilmes.sql.gremlin.adapter.results.SqlGremlinQueryResult;
import software.aws.neptune.common.gremlindatamodel.resultset.ResultSetGetColumns;
import software.aws.neptune.gremlin.GremlinTypeMapping;
import software.aws.neptune.gremlin.resultset.GremlinResultSet;
import software.aws.neptune.gremlin.resultset.GremlinResultSetMetadata;
import software.aws.neptune.jdbc.ResultSet;
import software.aws.neptune.jdbc.utilities.SqlError;
Expand All @@ -33,7 +32,7 @@
import java.util.Optional;

public class SqlGremlinResultSet extends ResultSet implements java.sql.ResultSet {
private static final Logger LOGGER = LoggerFactory.getLogger(GremlinResultSet.class);
private static final Logger LOGGER = LoggerFactory.getLogger(SqlGremlinResultSet.class);

private final List<String> columns;
private final List<String> columnTypes;
Expand Down Expand Up @@ -79,15 +78,9 @@ protected void doClose() throws SQLException {
@Override
public boolean next() throws SQLException {
final Object res;
try {
res = sqlQueryResult.getResult();
} catch (final SQLException e) {
if (e.getMessage().equals(SqlGremlinQueryResult.EMPTY_MESSAGE)) {
LOGGER.trace(SqlGremlinQueryResult.EMPTY_MESSAGE);
return false;
} else {
throw e;
}
res = sqlQueryResult.getResult();
if (res instanceof SqlGremlinQueryResult.EmptyResult) {
return false;
}
this.row = (List<Object>) res;
return true;
Expand Down

0 comments on commit 6b62826

Please sign in to comment.