Skip to content

Commit

Permalink
SQL: be lenient for tests involving comparison to H2 but strict for c…
Browse files Browse the repository at this point in the history
…sv spec tests (elastic#36498)

* Add a lenient parameter for floating point numbers and mark csv-spec
tests as not lenient vs. sql-spec tests to be lenient.
  • Loading branch information
astefan committed Dec 13, 2018
1 parent 692cff8 commit 1270d85
Show file tree
Hide file tree
Showing 8 changed files with 64 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ protected void assertResults(ResultSet expected, ResultSet elastic) throws SQLEx
// uncomment this to printout the result set and create new CSV tests
//
//JdbcTestUtils.logLikeCLI(elastic, log);
JdbcAssert.assertResultSets(expected, elastic, log, true);
JdbcAssert.assertResultSets(expected, elastic, log, true, false);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@

import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;

import org.apache.logging.log4j.Logger;
import org.elasticsearch.xpack.sql.qa.jdbc.CsvTestUtils.CsvTestCase;

import java.sql.Connection;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.List;

Expand Down Expand Up @@ -58,4 +60,10 @@ protected final void doTest() throws Throwable {
assertResults(expected, elasticResults);
}
}

@Override
protected void assertResults(ResultSet expected, ResultSet elastic) throws SQLException {
Logger log = logEsResultSet() ? logger : null;
JdbcAssert.assertResultSets(expected, elastic, log, false, false);
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/*
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
Expand Down Expand Up @@ -57,15 +58,29 @@ public static void assertResultSets(ResultSet expected, ResultSet actual, Logger

/**
* Assert the given result sets, potentially in a lenient way.
* When lenient is specified, the type comparison of a column is widden to reach a common, compatible ground.
* When lenientDataType is specified, the type comparison of a column is widden to reach a common, compatible ground.
* This means promoting integer types to long and floating types to double and comparing their values.
* For example in a non-lenient, strict case a comparison between an int and a tinyint would fail, with lenient it will succeed as
* long as the actual value is the same.
* For example in a non-lenient, strict case a comparison between an int and a tinyint would fail, with lenientDataType it will succeed
* as long as the actual value is the same.
*/
public static void assertResultSets(ResultSet expected, ResultSet actual, Logger logger, boolean lenient) throws SQLException {
public static void assertResultSets(ResultSet expected, ResultSet actual, Logger logger, boolean lenientDataType) throws SQLException {
assertResultSets(expected, actual, logger, lenientDataType, true);
}

/**
* Assert the given result sets, potentially in a lenient way.
* When lenientDataType is specified, the type comparison of a column is widden to reach a common, compatible ground.
* This means promoting integer types to long and floating types to double and comparing their values.
* For example in a non-lenient, strict case a comparison between an int and a tinyint would fail, with lenientDataType it will succeed
* as long as the actual value is the same.
* Also, has the option of treating the numeric results for floating point numbers in a leninent way, if chosen to. Usually,
* we would want lenient treatment for floating point numbers in sql-spec tests where the comparison is being made with H2.
*/
public static void assertResultSets(ResultSet expected, ResultSet actual, Logger logger, boolean lenientDataType,
boolean lenientFloatingNumbers) throws SQLException {
try (ResultSet ex = expected; ResultSet ac = actual) {
assertResultSetMetadata(ex, ac, logger, lenient);
assertResultSetData(ex, ac, logger, lenient);
assertResultSetMetadata(ex, ac, logger, lenientDataType);
assertResultSetData(ex, ac, logger, lenientDataType, lenientFloatingNumbers);
}
}

Expand All @@ -74,7 +89,8 @@ public static void assertResultSetMetadata(ResultSet expected, ResultSet actual,
}

// metadata doesn't consume a ResultSet thus it shouldn't close it
public static void assertResultSetMetadata(ResultSet expected, ResultSet actual, Logger logger, boolean lenient) throws SQLException {
public static void assertResultSetMetadata(ResultSet expected, ResultSet actual, Logger logger, boolean lenientDataType)
throws SQLException {
ResultSetMetaData expectedMeta = expected.getMetaData();
ResultSetMetaData actualMeta = actual.getMetaData();

Expand Down Expand Up @@ -116,8 +132,8 @@ public static void assertResultSetMetadata(ResultSet expected, ResultSet actual,
}

// use the type not the name (timestamp with timezone returns spaces for example)
int expectedType = typeOf(expectedMeta.getColumnType(column), lenient);
int actualType = typeOf(actualMeta.getColumnType(column), lenient);
int expectedType = typeOf(expectedMeta.getColumnType(column), lenientDataType);
int actualType = typeOf(actualMeta.getColumnType(column), lenientDataType);

// since H2 cannot use a fixed timezone, the data is stored in UTC (and thus with timezone)
if (expectedType == Types.TIMESTAMP_WITH_TIMEZONE) {
Expand Down Expand Up @@ -153,13 +169,20 @@ public static void assertResultSetData(ResultSet expected, ResultSet actual, Log
assertResultSetData(expected, actual, logger, false);
}

public static void assertResultSetData(ResultSet expected, ResultSet actual, Logger logger, boolean lenient) throws SQLException {
public static void assertResultSetData(ResultSet expected, ResultSet actual, Logger logger, boolean lenientDataType)
throws SQLException {
assertResultSetData(expected, actual, logger, lenientDataType, true);
}

public static void assertResultSetData(ResultSet expected, ResultSet actual, Logger logger, boolean lenientDataType,
boolean lenientFloatingNumbers) throws SQLException {
try (ResultSet ex = expected; ResultSet ac = actual) {
doAssertResultSetData(ex, ac, logger, lenient);
doAssertResultSetData(ex, ac, logger, lenientDataType, lenientFloatingNumbers);
}
}

private static void doAssertResultSetData(ResultSet expected, ResultSet actual, Logger logger, boolean lenient) throws SQLException {
private static void doAssertResultSetData(ResultSet expected, ResultSet actual, Logger logger, boolean lenientDataType,
boolean lenientFloatingNumbers) throws SQLException {
ResultSetMetaData metaData = expected.getMetaData();
int columns = metaData.getColumnCount();

Expand Down Expand Up @@ -199,7 +222,7 @@ private static void doAssertResultSetData(ResultSet expected, ResultSet actual,
}

Object expectedObject = expected.getObject(column);
Object actualObject = lenient ? actual.getObject(column, expectedColumnClass) : actual.getObject(column);
Object actualObject = lenientDataType ? actual.getObject(column, expectedColumnClass) : actual.getObject(column);

String msg = format(Locale.ROOT, "Different result for column [%s], entry [%d]",
metaData.getColumnName(column), count + 1);
Expand All @@ -219,10 +242,9 @@ else if (type == Types.TIMESTAMP || type == Types.TIMESTAMP_WITH_TIMEZONE) {
}
// and floats/doubles
else if (type == Types.DOUBLE) {
// the 1d/1f difference is used due to rounding/flooring
assertEquals(msg, (double) expectedObject, (double) actualObject, 1d);
assertEquals(msg, (double) expectedObject, (double) actualObject, lenientFloatingNumbers ? 1d : 0.0d);
} else if (type == Types.FLOAT) {
assertEquals(msg, (float) expectedObject, (float) actualObject, 1f);
assertEquals(msg, (float) expectedObject, (float) actualObject, lenientFloatingNumbers ? 1f : 0.0f);
}
// intervals
else if (type == Types.VARCHAR && actualObject instanceof TemporalAmount) {
Expand Down Expand Up @@ -251,8 +273,8 @@ else if (type == Types.VARCHAR && actualObject instanceof TemporalAmount) {
/**
* Returns the value of the given type either in a lenient fashion (widened) or strict.
*/
private static int typeOf(int columnType, boolean lenient) {
if (lenient) {
private static int typeOf(int columnType, boolean lenientDataType) {
if (lenientDataType) {
// integer upcast to long
if (columnType == TINYINT || columnType == SMALLINT || columnType == INTEGER || columnType == BIGINT) {
return BIGINT;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,4 +197,4 @@ public interface Parser {
public static InputStream readFromJarUrl(URL source) throws IOException {
return source.openStream();
}
}
}
4 changes: 2 additions & 2 deletions x-pack/plugin/sql/qa/src/main/resources/agg.csv-spec
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ SELECT gender, PERCENTILE(emp_no, 97.76) p1 FROM test_emp GROUP BY gender;

gender:s | p1:d
null |10019.0
F |10099.51
M |10095.789999999999
F |10099.7608
M |10096.2232
;

multiplePercentilesOneWithCommaOneWithout
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugin/sql/qa/src/main/resources/alias.csv-spec
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,6 @@ SELECT gender, PERCENTILE(emp_no, 97) p1 FROM "test_*" WHERE gender is NOT NULL

gender:s | p1:d

F | 10099.32
M | 10095.98
F | 10099.52
M | 10096.0
;
16 changes: 8 additions & 8 deletions x-pack/plugin/sql/qa/src/main/resources/docs.csv-spec
Original file line number Diff line number Diff line change
Expand Up @@ -669,7 +669,7 @@ SELECT MIN(salary) AS min, MAX(salary) AS max, AVG(salary) AS avg, COUNT(*) AS c

min:i | max:i | avg:d | count:l
---------------+---------------+---------------+---------------
25324 |74999 |48248 |100
25324 |74999 |48248.55 |100

// end::groupByImplicitMultipleAggs
;
Expand Down Expand Up @@ -724,9 +724,9 @@ SELECT SCORE(), * FROM library WHERE MATCH(name, 'dune') ORDER BY SCORE() DESC;

SCORE() | author | name | page_count | release_date
---------------+---------------+-------------------+---------------+--------------------
2.288635 |Frank Herbert |Dune |604 |1965-06-01T00:00:00Z
2.2886353 |Frank Herbert |Dune |604 |1965-06-01T00:00:00Z
1.8893257 |Frank Herbert |Dune Messiah |331 |1969-10-15T00:00:00Z
1.6086555 |Frank Herbert |Children of Dune |408 |1976-04-21T00:00:00Z
1.6086556 |Frank Herbert |Children of Dune |408 |1976-04-21T00:00:00Z
1.4005898 |Frank Herbert |God Emperor of Dune|454 |1981-05-28T00:00:00Z

// end::orderByScore
Expand All @@ -738,9 +738,9 @@ SELECT SCORE(), * FROM library WHERE MATCH(name, 'dune') ORDER BY page_count DES

SCORE() | author | name | page_count | release_date
---------------+---------------+-------------------+---------------+--------------------
2.288635 |Frank Herbert |Dune |604 |1965-06-01T00:00:00Z
2.2886353 |Frank Herbert |Dune |604 |1965-06-01T00:00:00Z
1.4005898 |Frank Herbert |God Emperor of Dune|454 |1981-05-28T00:00:00Z
1.6086555 |Frank Herbert |Children of Dune |408 |1976-04-21T00:00:00Z
1.6086556 |Frank Herbert |Children of Dune |408 |1976-04-21T00:00:00Z
1.8893257 |Frank Herbert |Dune Messiah |331 |1969-10-15T00:00:00Z

// end::orderByScoreWithMatch
Expand All @@ -753,9 +753,9 @@ SELECT SCORE() AS score, name, release_date FROM library WHERE QUERY('dune') ORD
score | name | release_date
---------------+-------------------+--------------------
1.4005898 |God Emperor of Dune|1981-05-28T00:00:00Z
1.6086555 |Children of Dune |1976-04-21T00:00:00Z
1.6086556 |Children of Dune |1976-04-21T00:00:00Z
1.8893257 |Dune Messiah |1969-10-15T00:00:00Z
2.288635 |Dune |1965-06-01T00:00:00Z
2.2886353 |Dune |1965-06-01T00:00:00Z
// end::scoreWithMatch
;

Expand Down Expand Up @@ -789,7 +789,7 @@ SELECT AVG(salary) AS avg FROM emp;

avg:d
---------------
48248
48248.55
// end::aggAvg
;

Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugin/sql/qa/src/main/resources/fulltext.csv-spec
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,12 @@ score
SELECT emp_no, first_name, SCORE() FROM test_emp WHERE MATCH(first_name, 'Erez') ORDER BY SCORE();

emp_no:i | first_name:s | SCORE():f
10076 |Erez |4.2096553
10076 |Erez |4.1053944
;

scoreAsSomething
SELECT emp_no, first_name, SCORE() as s FROM test_emp WHERE MATCH(first_name, 'Erez') ORDER BY SCORE();

emp_no:i | first_name:s | s:f
10076 |Erez |4.2096553
10076 |Erez |4.1053944
;

0 comments on commit 1270d85

Please sign in to comment.