From d12cdf14f5ed84176614908ebf7765928625f1a0 Mon Sep 17 00:00:00 2001 From: Will Noble <78444363+wnob@users.noreply.github.com> Date: Mon, 11 Dec 2023 15:28:14 -0800 Subject: [PATCH 01/19] Use recommended semantics for getObject of date/time types (#181) --- .../clouddb/jdbc/BQForwardOnlyResultSet.java | 160 ++++++------------ .../clouddb/jdbc/BQScrollableResultSet.java | 31 ++-- .../clouddb/jdbc/DateTimeUtils.java | 156 +++++++++++++++++ .../BQForwardOnlyResultSetFunctionTest.java | 113 +++++++++++-- 4 files changed, 322 insertions(+), 138 deletions(-) create mode 100644 src/main/java/net/starschema/clouddb/jdbc/DateTimeUtils.java diff --git a/src/main/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSet.java b/src/main/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSet.java index de1664d..9000b05 100644 --- a/src/main/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSet.java +++ b/src/main/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSet.java @@ -61,10 +61,6 @@ import java.sql.Statement; import java.sql.Time; import java.sql.Timestamp; -import java.text.SimpleDateFormat; -import java.time.Instant; -import java.time.ZoneId; -import java.time.format.DateTimeFormatter; import java.util.*; import javax.annotation.Nullable; import org.slf4j.Logger; @@ -127,9 +123,6 @@ public class BQForwardOnlyResultSet implements java.sql.ResultSet { */ private int Cursor = -1; - private final DateTimeFormatter TIMESTAMP_FORMATTER = - DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss.SSS").withZone(ZoneId.of("UTC")); - public BQForwardOnlyResultSet( Bigquery bigquery, String projectId, @@ -248,7 +241,7 @@ public BQForwardOnlyResultSet( */ public Object getObject(int columnIndex) throws SQLException { - String result = getString(columnIndex, false); + String result = rawString(columnIndex); if (this.wasNull()) { return null; @@ -274,16 +267,26 @@ public Object getObject(int columnIndex) throws SQLException { if (Columntype.equals("INTEGER")) { return toLong(result); } + if (Columntype.equals("DATETIME")) { + // A BigQuery DATETIME is essentially defined by its string representation; + // the "clock-calendar parameters" comprising year, month, day, hour, minute, etc. + // On the other hand, a [java.sql.Timestamp] object is defined as a global instant, similar + // to BQ TIMESTAMP. It has a [toString] method that interprets that instant in the system + // default time zone. Thus, in order to produce a [Timestamp] object whose [toString] method + // has the correct result, we must adjust the value of the instant according to the system + // default time zone (passing a null Calendar uses the system default). + return DateTimeUtils.parseDateTime(result, null); + } if (Columntype.equals("TIMESTAMP")) { - return toTimestamp(result, null); + // A BigQuery TIMESTAMP is defined as a global instant in time, so when we create the + // [java.sql.Timestamp] object to represent it, we must not make any time zone adjustment. + return DateTimeUtils.parseTimestamp(result); } if (Columntype.equals("DATE")) { - return toDate(result, null); + return DateTimeUtils.parseDate(result, null); } - if (Columntype.equals("DATETIME")) { - // Date time represents a "clock face" time and so should NOT be processed into an actual - // time - return result; + if (Columntype.equals("TIME")) { + return DateTimeUtils.parseTime(result, null); } if (Columntype.equals("NUMERIC")) { return toBigDecimal(result); @@ -321,49 +324,6 @@ private Long toLong(String value) throws SQLException { } } - /** Parse date/time types */ - private Timestamp toTimestamp(String value, Calendar cal) throws SQLException { - try { - long dbValue = - new BigDecimal(value) - .movePointRight(3) - .longValue(); // movePointRight(3) = *1000 (done before rounding) - from seconds - // (BigQuery specifications) to milliseconds (required by java). - // Precision under millisecond is discarded (BigQuery supports - // micro-seconds) - if (cal == null) { - cal = - Calendar.getInstance( - TimeZone.getTimeZone( - "UTC")); // The time zone of the server that host the JVM should NOT impact the - // results. Use UTC calendar instead (which wont apply any correction, - // whatever the time zone of the data) - } - - Calendar dbCal = Calendar.getInstance(TimeZone.getTimeZone("UTC")); - dbCal.setTimeInMillis(dbValue); - cal.set(Calendar.YEAR, dbCal.get(Calendar.YEAR)); - cal.set(Calendar.MONTH, dbCal.get(Calendar.MONTH)); - cal.set(Calendar.DAY_OF_MONTH, dbCal.get(Calendar.DAY_OF_MONTH)); - cal.set(Calendar.HOUR_OF_DAY, dbCal.get(Calendar.HOUR_OF_DAY)); - cal.set(Calendar.MINUTE, dbCal.get(Calendar.MINUTE)); - cal.set(Calendar.SECOND, dbCal.get(Calendar.SECOND)); - cal.set(Calendar.MILLISECOND, dbCal.get(Calendar.MILLISECOND)); - return new Timestamp(cal.getTime().getTime()); - } catch (NumberFormatException e) { - // before giving up, check to see if we've been given a 'time' value without a - // date, e.g. from current_time(), and if we have, try to parse it - try { - SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss"); - String dateStringFromValue = ("1970-01-01 " + value).substring(0, 19); - java.util.Date parsedDate = dateFormat.parse(dateStringFromValue); - return new java.sql.Timestamp(parsedDate.getTime()); - } catch (Exception e2) { - throw new BQSQLException(e); - } - } - } - // Secondary converters /** Parse integral or floating types with (virtually) infinite precision */ @@ -371,21 +331,6 @@ private BigDecimal toBigDecimal(String value) { return new BigDecimal(value); } - static Date toDate(String value, Calendar cal) throws SQLException { - // Dates in BigQuery come back in the YYYY-MM-DD format - SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd"); - try { - java.util.Date date = sdf.parse(value); - return new java.sql.Date(date.getTime()); - } catch (java.text.ParseException e) { - throw new BQSQLException(e); - } - } - - private Time toTime(String value, Calendar cal) throws SQLException { - return new java.sql.Time(toTimestamp(value, cal).getTime()); - } - @Override /** * Returns the current rows Data at the given index as String @@ -396,10 +341,18 @@ private Time toTime(String value, Calendar cal) throws SQLException { * is unsupported */ public String getString(int columnIndex) throws SQLException { - return getString(columnIndex, true); + String rawString = rawString(columnIndex); + + BQResultsetMetaData metadata = getBQResultsetMetaData(); + if (metadata.getColumnTypeName(columnIndex).equals("TIMESTAMP") + && !metadata.getColumnMode(columnIndex).equals("REPEATED")) { + return DateTimeUtils.formatTimestamp(rawString); + } + + return rawString; } - private String getString(int columnIndex, boolean formatTimestamps) throws SQLException { + private String rawString(int columnIndex) throws SQLException { // to make the logfiles smaller! // logger.debug("Function call getString columnIndex is: " + String.valueOf(columnIndex)); this.closestrm(); @@ -410,21 +363,21 @@ private String getString(int columnIndex, boolean formatTimestamps) throws SQLEx throw new BQSQLException("ColumnIndex is not valid"); } - if (this.rowsofResult == null) throw new BQSQLException("Invalid position!"); + if (this.rowsofResult == null) { + throw new BQSQLException("Invalid position!"); + } + + // Since the results came from BigQuery's JSON API, + // the only types we'll ever see for resultObject are JSON-supported types, + // i.e. strings, numbers, arrays, booleans, or null. + // Many standard Java types, like timestamps, will be represented as strings. Object resultObject = this.rowsofResult.get(this.Cursor).getF().get(columnIndex - 1).getV(); + if (Data.isNull(resultObject)) { this.wasnull = true; return null; } this.wasnull = false; - if (!this.getBQResultsetMetaData().getColumnMode(columnIndex).equals("REPEATED") - && formatTimestamps - && getMetaData().getColumnTypeName(columnIndex).equals("TIMESTAMP")) { - Instant instant = - Instant.ofEpochMilli( - (new BigDecimal((String) resultObject).movePointRight(3)).longValue()); - return TIMESTAMP_FORMATTER.format(instant); - } if (resultObject instanceof List || resultObject instanceof Map) { Object resultTransformedWithSchema = smartTransformResult(resultObject, schema.getFields().get(columnIndex - 1)); @@ -913,11 +866,7 @@ public String getCursorName() throws SQLException { /** {@inheritDoc} */ @Override public Date getDate(int columnIndex) throws SQLException { - String value = this.getString(columnIndex); - if (this.wasNull()) { - return null; - } - return toDate(value, null); + return getDate(columnIndex, null); } /** {@inheritDoc} */ @@ -927,14 +876,13 @@ public Date getDate(int columnIndex, Calendar cal) throws SQLException { if (this.wasNull()) { return null; } - return toDate(value, cal); + return DateTimeUtils.parseDate(value, cal); } /** {@inheritDoc} */ @Override public Date getDate(String columnLabel) throws SQLException { - int columnIndex = this.findColumn(columnLabel); - return this.getDate(columnIndex); + return getDate(columnLabel, null); } /** {@inheritDoc} */ @@ -1336,11 +1284,7 @@ public String getString(String columnLabel) throws SQLException { /** {@inheritDoc} */ @Override public Time getTime(int columnIndex) throws SQLException { - String value = this.getString(columnIndex); - if (this.wasNull()) { - return null; - } - return toTime(value, null); + return getTime(columnIndex, null); } /** {@inheritDoc} */ @@ -1350,14 +1294,13 @@ public Time getTime(int columnIndex, Calendar cal) throws SQLException { if (this.wasNull()) { return null; } - return toTime(value, cal); + return DateTimeUtils.parseTime(value, cal); } /** {@inheritDoc} */ @Override public Time getTime(String columnLabel) throws SQLException { - int columnIndex = this.findColumn(columnLabel); - return this.getTime(columnIndex); + return getTime(columnLabel, null); } /** {@inheritDoc} */ @@ -1370,28 +1313,29 @@ public Time getTime(String columnLabel, Calendar cal) throws SQLException { /** {@inheritDoc} */ @Override public Timestamp getTimestamp(int columnIndex) throws SQLException { - String value = this.getString(columnIndex); - if (this.wasNull()) { - return null; - } - return toTimestamp(value, null); + return getTimestamp(columnIndex, null); } /** {@inheritDoc} */ @Override public Timestamp getTimestamp(int columnIndex, Calendar cal) throws SQLException { - String value = this.getString(columnIndex); + String value = rawString(columnIndex); if (this.wasNull()) { return null; } - return toTimestamp(value, cal); + // Both the TIMESTAMP and DATETIME objects support JDBC's getTimestamp method. + // DATETIME in BigQuery is analogous to TIMESTAMP in ISO SQL. + // TIMESTAMP in BigQuery is analogous to TIMESTAMP WITH LOCAL TIME ZONE in ISO SQL. + String columnType = this.schema.getFields().get(columnIndex - 1).getType(); + return "TIMESTAMP".equals(columnType) + ? DateTimeUtils.parseTimestamp(value) + : DateTimeUtils.parseDateTime(value, cal); } /** {@inheritDoc} */ @Override public Timestamp getTimestamp(String columnLabel) throws SQLException { - int columnIndex = this.findColumn(columnLabel); - return this.getTimestamp(columnIndex); + return getTimestamp(columnLabel, null); } /** {@inheritDoc} */ diff --git a/src/main/java/net/starschema/clouddb/jdbc/BQScrollableResultSet.java b/src/main/java/net/starschema/clouddb/jdbc/BQScrollableResultSet.java index 816d0b4..b938852 100644 --- a/src/main/java/net/starschema/clouddb/jdbc/BQScrollableResultSet.java +++ b/src/main/java/net/starschema/clouddb/jdbc/BQScrollableResultSet.java @@ -22,8 +22,6 @@ */ package net.starschema.clouddb.jdbc; -import static net.starschema.clouddb.jdbc.BQForwardOnlyResultSet.toDate; - import com.google.api.client.util.Data; import com.google.api.services.bigquery.model.BiEngineReason; import com.google.api.services.bigquery.model.GetQueryResultsResponse; @@ -36,7 +34,6 @@ import java.sql.ResultSetMetaData; import java.sql.SQLException; import java.sql.Statement; -import java.sql.Timestamp; import java.util.List; import javax.annotation.Nullable; @@ -204,21 +201,31 @@ public Object getObject(int columnIndex) throws SQLException { if (Columntype.equals("INTEGER")) { return Long.parseLong(result); } + if (Columntype.equals("DATETIME")) { + // A BigQuery DATETIME is essentially defined by its string representation; + // the "clock-calendar parameters" comprising year, month, day, hour, minute, etc. + // On the other hand, a [java.sql.Timestamp] object is defined as a global instant, + // similar to BQ TIMESTAMP. It has a [toString] method that interprets that instant in the + // system default time zone. Thus, in order to produce a [Timestamp] object whose + // [toString] method has the correct result, we must adjust the value of the instant + // according to the system default time zone (passing a null Calendar uses the system + // default). + return DateTimeUtils.parseDateTime(result, null); + } if (Columntype.equals("TIMESTAMP")) { - long val = new BigDecimal(result).longValue() * 1000; - return new Timestamp(val); + // A BigQuery TIMESTAMP is defined as a global instant in time, so when we create the + // [java.sql.Timestamp] object to represent it, we must not make any time zone adjustment. + return DateTimeUtils.parseTimestamp(result); } - if (Columntype.equals("DATETIME")) { - // Date time represents a "clock face" time and so should NOT be processed into an actual - // time - return result; + if (Columntype.equals("DATE")) { + return DateTimeUtils.parseDate(result, null); + } + if (Columntype.equals("TIME")) { + return DateTimeUtils.parseTime(result, null); } if (Columntype.equals("NUMERIC")) { return new BigDecimal(result); } - if (Columntype.equals("DATE")) { - return toDate(result, null); - } throw new BQSQLException("Unsupported Type"); } catch (NumberFormatException e) { throw new BQSQLException(e); diff --git a/src/main/java/net/starschema/clouddb/jdbc/DateTimeUtils.java b/src/main/java/net/starschema/clouddb/jdbc/DateTimeUtils.java new file mode 100644 index 0000000..e54a65d --- /dev/null +++ b/src/main/java/net/starschema/clouddb/jdbc/DateTimeUtils.java @@ -0,0 +1,156 @@ +package net.starschema.clouddb.jdbc; + +import java.math.BigDecimal; +import java.sql.Date; +import java.sql.SQLException; +import java.sql.Time; +import java.sql.Timestamp; +import java.text.SimpleDateFormat; +import java.time.Instant; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.time.LocalTime; +import java.time.OffsetDateTime; +import java.time.ZoneId; +import java.time.ZoneOffset; +import java.time.format.DateTimeFormatter; +import java.time.format.DateTimeFormatterBuilder; +import java.time.format.DateTimeParseException; +import java.time.temporal.ChronoField; +import java.util.Calendar; +import java.util.GregorianCalendar; +import java.util.Locale; +import java.util.TimeZone; + +class DateTimeUtils { + + private static final ZoneId UTC_ZONE = ZoneId.of("UTC"); + private static final Calendar DEFAULT_CALENDAR = new GregorianCalendar(TimeZone.getDefault()); + + /** Formatter used to parse DATETIME literals. */ + private static final DateTimeFormatter DATETIME_FORMATTER = + new DateTimeFormatterBuilder() + // The date part is always YYYY-MM-DD. + .appendValue(ChronoField.YEAR, 4) + .appendLiteral('-') + .appendValue(ChronoField.MONTH_OF_YEAR, 2) + .appendLiteral('-') + .appendValue(ChronoField.DAY_OF_MONTH, 2) + // Accept either a literal 'T' or a space to separate the date from the time. + // Make the space optional but pad with 'T' if it's omitted, so that parsing accepts both, + // but formatting defaults to using the space. + .padNext(1, 'T') + .optionalStart() + .appendLiteral(' ') + .optionalEnd() + // The whole-second time part is always HH:MM:SS. + .appendValue(ChronoField.HOUR_OF_DAY, 2) + .appendLiteral(':') + .appendValue(ChronoField.MINUTE_OF_HOUR, 2) + .appendLiteral(':') + .appendValue(ChronoField.SECOND_OF_MINUTE, 2) + // BigQuery has optional microsecond precision. + .optionalStart() + .appendFraction(ChronoField.NANO_OF_SECOND, 0, 6, true) + .optionalEnd() + .toFormatter(Locale.ROOT); + + /** Formatter used to parse TIME literals. */ + private static final DateTimeFormatter TIME_FORMATTER = + new DateTimeFormatterBuilder() + // The whole-second time part is always HH:MM:SS. + .appendValue(ChronoField.HOUR_OF_DAY, 2) + .appendLiteral(':') + .appendValue(ChronoField.MINUTE_OF_HOUR, 2) + .appendLiteral(':') + .appendValue(ChronoField.SECOND_OF_MINUTE, 2) + // BigQuery only has microsecond precision, but this parser supports up to nanosecond + // precision after the decimal point. + .optionalStart() + .appendFraction(ChronoField.NANO_OF_SECOND, 0, 9, true) + .optionalEnd() + .toFormatter(Locale.ROOT); + + private static final LocalDate TIME_EPOCH = LocalDate.of(1970, 1, 1); + + /** Parse a BigQuery DATETIME represented as a String. */ + static Timestamp parseDateTime(String value, Calendar cal) throws SQLException { + if (cal == null) { + cal = DEFAULT_CALENDAR; + } + try { + // BigQuery DATETIME has a string representation that looks like e.g. "2010-10-26 02:49:35". + LocalDateTime localDateTime = LocalDateTime.parse(value, DATETIME_FORMATTER); + // In order to represent it as a [java.sql.Timestamp], which is defined by an instant, + // we must subtract the offset from the calendar, so that the [toString] method of the + // resulting [Timestamp] has the correct result. Since time zones can have different offsets + // at different times of year (e.g. due to Daylight Saving), first calculate the instant + // assuming no offset, and use that to calculate the offset. + long utcInstant = localDateTime.toInstant(ZoneOffset.UTC).toEpochMilli(); + ZoneOffset offset = ZoneOffset.ofTotalSeconds(cal.getTimeZone().getOffset(utcInstant) / 1000); + Instant instant = localDateTime.atOffset(offset).toInstant(); + Timestamp timestamp = new Timestamp(instant.toEpochMilli()); + timestamp.setNanos(instant.getNano()); + return timestamp; + } catch (DateTimeParseException e) { + throw new BQSQLException(e); + } + } + + /** Parse a BigQuery TIMESTAMP literal, represented as the number of seconds since epoch. */ + static Timestamp parseTimestamp(String value) throws SQLException { + try { + // BigQuery TIMESTAMP has a string representation that looks like e.g. "1.288061375E9" + // for 2010-10-26 02:49:35 UTC. + // It is the (possibly fractional) number of seconds since the epoch. + BigDecimal secondsSinceEpoch = new BigDecimal(value); + // https://stackoverflow.com/questions/5839411/java-sql-timestamp-way-of-storing-nanoseconds + // In order to support sub-millisecond precision, we need to first initialize the timestamp + // with the correct number of whole seconds (expressed in milliseconds), + // then set the nanosecond value, which overrides the initial milliseconds. + long wholeSeconds = secondsSinceEpoch.longValue(); + Timestamp timestamp = new Timestamp(wholeSeconds * 1000L); + int nanoSeconds = secondsSinceEpoch.remainder(BigDecimal.ONE).movePointRight(9).intValue(); + timestamp.setNanos(nanoSeconds); + return timestamp; + } catch (NumberFormatException e) { + throw new BQSQLException(e); + } + } + + static String formatTimestamp(String rawString) throws SQLException { + Timestamp timestamp = parseTimestamp(rawString); + return DATETIME_FORMATTER.format(OffsetDateTime.ofInstant(timestamp.toInstant(), UTC_ZONE)) + + " UTC"; + } + + static Date parseDate(String value, Calendar cal) throws SQLException { + // Dates in BigQuery come back in the YYYY-MM-DD format + SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd"); + try { + java.util.Date date = sdf.parse(value); + return new java.sql.Date(date.getTime()); + } catch (java.text.ParseException e) { + throw new BQSQLException(e); + } + } + + static Time parseTime(String value, Calendar cal) throws SQLException { + if (cal == null) { + cal = DEFAULT_CALENDAR; + } + try { + // BigQuery DATETIME has a string representation that looks like e.g. "2010-10-26 02:49:35". + LocalTime localTime = LocalTime.parse(value, TIME_FORMATTER); + // In order to represent it as a [java.sql.Time], which is defined by an instant, + // we must subtract the offset from the calendar, so that the [toString] method of the + // resulting [Time] has the correct result. Since time values do not have date components, + // assume the standard offset should be used. + ZoneOffset offset = ZoneOffset.ofTotalSeconds(cal.getTimeZone().getRawOffset() / 1000); + Instant instant = localTime.atDate(TIME_EPOCH).atOffset(offset).toInstant(); + return new Time(instant.toEpochMilli()); + } catch (DateTimeParseException e) { + throw new BQSQLException(e); + } + } +} diff --git a/src/test/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSetFunctionTest.java b/src/test/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSetFunctionTest.java index f51be8e..41195c8 100644 --- a/src/test/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSetFunctionTest.java +++ b/src/test/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSetFunctionTest.java @@ -29,12 +29,16 @@ import java.io.IOException; import java.math.BigDecimal; import java.sql.*; +import java.text.DateFormat; import java.text.ParseException; import java.text.SimpleDateFormat; +import java.util.Calendar; import java.util.Date; +import java.util.GregorianCalendar; import java.util.HashMap; import java.util.Map; import java.util.Properties; +import java.util.TimeZone; import junit.framework.Assert; import org.junit.Before; import org.junit.Test; @@ -432,7 +436,7 @@ public void testResultSetTypesInGetString() throws SQLException { + "['a', 'b', 'c'], " + "[STRUCT(1 as a, 'hello' as b), STRUCT(2 as a, 'goodbye' as b)], " + "STRUCT(1 as a, ['an', 'array'] as b)," - + "TIMESTAMP('2012-01-01 00:00:03.032') as t"; + + "TIMESTAMP('2012-01-01 00:00:03.0000') as t"; this.NewConnection(false); java.sql.ResultSet result = null; @@ -486,15 +490,79 @@ public void testResultSetTypesInGetString() throws SQLException { new Gson().toJson(new String[] {"an", "array"}), new Gson().toJson(mixedBagActual.get("b"))); - Assert.assertEquals("2012-01-01 00:00:03.032", result.getString(5)); + Assert.assertEquals("2012-01-01 00:00:03 UTC", result.getString(5)); + } + + @Test + public void testResultSetDateTimeType() throws SQLException, ParseException { + final String sql = "SELECT DATETIME('2012-01-01 01:02:03.04567')"; + final Calendar istCalendar = new GregorianCalendar(TimeZone.getTimeZone("Asia/Kolkata")); + final DateFormat utcDateFormatter = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSS"); + utcDateFormatter.setTimeZone(TimeZone.getTimeZone("UTC")); + + this.NewConnection(false); + Statement stmt = + BQForwardOnlyResultSetFunctionTest.con.createStatement( + ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY); + stmt.setQueryTimeout(500); + ResultSet result = stmt.executeQuery(sql); + Assert.assertTrue(result.next()); + + Timestamp resultTimestamp = result.getTimestamp(1); + Object resultObject = result.getObject(1); + String resultString = result.getString(1); + + // getObject() and getTimestamp() should behave the same for DATETIME. + Assert.assertEquals(resultTimestamp, resultObject); + + // getTimestamp().toString() should be equivalent to getString(), with full microsecond support. + Assert.assertEquals("2012-01-01 01:02:03.04567", resultTimestamp.toString()); + Assert.assertEquals("2012-01-01T01:02:03.045670", resultString); + + // If a different calendar is used, the string representation should be adjusted. + Timestamp adjustedTimestamp = result.getTimestamp(1, istCalendar); + // Render it from the perspective of UTC. + // Since it was created for IST, it should be adjusted by -5:30. + String adjustedString = utcDateFormatter.format(adjustedTimestamp); + Assert.assertEquals("2011-12-31 19:32:03.045", adjustedString); + // SimpleDateFormat does not support microseconds, + // but they should be correct on the adjusted timestamp. + Assert.assertEquals(45670000, adjustedTimestamp.getNanos()); + } + + @Test + public void testResultSetTimestampType() throws SQLException, ParseException { + final String sql = "SELECT TIMESTAMP('2012-01-01 01:02:03.04567')"; + + this.NewConnection(false); + Statement stmt = + BQForwardOnlyResultSetFunctionTest.con.createStatement( + ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY); + stmt.setQueryTimeout(500); + ResultSet result = stmt.executeQuery(sql); + Assert.assertTrue(result.next()); + + Timestamp resultTimestamp = result.getTimestamp(1); + Object resultObject = result.getObject(1); + String resultString = result.getString(1); + + // getObject() and getTimestamp() should behave the same for TIMESTAMP. + Assert.assertEquals(resultTimestamp, resultObject); + + // getString() should be the string representation in UTC+0. + Assert.assertEquals("2012-01-01 01:02:03.04567 UTC", resultString); + + // getTimestamp() should have the right number of milliseconds elapsed since epoch. + // 1325379723045 milliseconds after epoch, the time is 2012-01-01 01:02:03.045 in UTC+0. + Assert.assertEquals(1325379723045L, resultTimestamp.getTime()); + // The microseconds should also be correct, but nanoseconds are not supported by BigQuery. + Assert.assertEquals(45670000, resultTimestamp.getNanos()); } @Test public void testResultSetTypesInGetObject() throws SQLException, ParseException { final String sql = "SELECT " - + "DATETIME('2012-01-01 00:00:02'), " - + "TIMESTAMP('2012-01-01 00:00:03'), " + "CAST('2312412432423423334.234234234' AS NUMERIC), " + "CAST('2011-04-03' AS DATE), " + "CAST('nan' AS FLOAT)"; @@ -513,19 +581,14 @@ public void testResultSetTypesInGetObject() throws SQLException, ParseException } Assert.assertNotNull(result); Assert.assertTrue(result.next()); - Assert.assertEquals("2012-01-01T00:00:02", result.getObject(1)); - SimpleDateFormat timestampDateFormat = new SimpleDateFormat("yyyy-MM-dd hh:mm:ss z"); - Date parsedDate = timestampDateFormat.parse("2012-01-01 00:00:03 UTC"); - Timestamp timestamp = new java.sql.Timestamp(parsedDate.getTime()); - Assert.assertEquals(timestamp, result.getObject(2)); + Assert.assertEquals(new BigDecimal("2312412432423423334.234234234"), result.getObject(1)); - Assert.assertEquals(new BigDecimal("2312412432423423334.234234234"), result.getObject(3)); SimpleDateFormat dateDateFormat = new SimpleDateFormat("yyyy-MM-dd"); Date parsedDateDate = new java.sql.Date(dateDateFormat.parse("2011-04-03").getTime()); - Assert.assertEquals(parsedDateDate, result.getObject(4)); + Assert.assertEquals(parsedDateDate, result.getObject(2)); - Assert.assertEquals(Double.NaN, result.getObject(5)); + Assert.assertEquals(Double.NaN, result.getObject(3)); } @Test @@ -565,7 +628,7 @@ public void testResultSetArraysInGetObject() throws SQLException, ParseException @Test public void testResultSetTimeType() throws SQLException, ParseException { - final String sql = "select current_time(), CAST('00:00:02.123455' AS TIME)"; + final String sql = "select current_time(), CAST('00:00:02.12345' AS TIME)"; this.NewConnection(false); java.sql.ResultSet result = null; try { @@ -585,14 +648,28 @@ public void testResultSetTimeType() throws SQLException, ParseException { Object resultObject = result.getObject(1); String resultString = result.getString(1); - // getObject() and getString() should behave the same for TIME - Assert.assertEquals(resultString, (String) resultObject); + // getObject() and getTime() should behave the same for TIME. + Assert.assertEquals(resultTime, resultObject); - // getTime() will return a 'time' without milliseconds + // getTime().toString() should be equivalent to getString() without milliseconds. Assert.assertTrue(resultString.startsWith(resultTime.toString())); - // also check that explicit casts to TIME work as expected - Assert.assertEquals(result.getTime(2).toString(), "00:00:02"); + // getTime() should have milliseconds, though. They're just not included in toString(). + // Get whole milliseconds (modulo whole seconds) from resultTime. + long timeMillis = resultTime.getTime() % 1000; + // Get whole milliseconds from resultString. + int decimalPlace = resultString.lastIndexOf('.'); + long stringMillis = Long.parseLong(resultString.substring(decimalPlace + 1, decimalPlace + 4)); + Assert.assertEquals(timeMillis, stringMillis); + + // Check that explicit casts to TIME work as expected. + Time fixedTime = result.getTime(2); + Assert.assertEquals("00:00:02", fixedTime.toString()); + // The object should have milliseconds even though they're hidden by toString(). + // AFAICT [java.sql.Time] does not support microseconds. + Assert.assertEquals(123, fixedTime.getTime() % 1000); + // getString() should show microseconds. + Assert.assertEquals("00:00:02.123450", result.getString(2)); } @Test From a0e412ade62bd70fb20ed5b16a8267868424ef79 Mon Sep 17 00:00:00 2001 From: Will Noble Date: Mon, 11 Dec 2023 15:33:30 -0800 Subject: [PATCH 02/19] Prepare for 3.0.0-SNAPSHOT development release --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index bc791fe..6dd464e 100644 --- a/pom.xml +++ b/pom.xml @@ -4,7 +4,7 @@ to a groupId that we have ownership over --> com.google.looker-open-source bqjdbc - 2.3.30-SNAPSHOT + 3.0.0-SNAPSHOT Big Query over JDBC A simple JDBC driver, to reach Google's BigQuery From ae704cd2d7c60a5d173a617ff426f759a2992686 Mon Sep 17 00:00:00 2001 From: Will Noble Date: Mon, 11 Dec 2023 15:37:00 -0800 Subject: [PATCH 03/19] Remove SNAPSHOT --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 6dd464e..5e248e8 100644 --- a/pom.xml +++ b/pom.xml @@ -4,7 +4,7 @@ to a groupId that we have ownership over --> com.google.looker-open-source bqjdbc - 3.0.0-SNAPSHOT + 3.0.0 Big Query over JDBC A simple JDBC driver, to reach Google's BigQuery From efef4cc8d905fa43316092c9595510a01a87faf5 Mon Sep 17 00:00:00 2001 From: Mark Williams Date: Wed, 13 Dec 2023 18:52:04 +0000 Subject: [PATCH 04/19] Support stateless queries. From https://cloud.google.com/java/docs/reference/google-cloud-bigquery/2.34.1/com.google.cloud.bigquery.BigQuery > Stateless queries: query execution without corresponding job metadata Stateless queries are currently in preview. This PR introduces support for stateless queries via a new JDBC query parameter. Set `jobcreationmode` to a string that matches one of the standard `JobCreationMode` enum values: - `JOB_CREATION_MODE_UNSPECIFIED`: Unspecified JobCreationMode, defaults to `JOB_CREATION_REQUIRED`. - `JOB_CREATION_REQUIRED`: Default. Job creation is always required. - `JOB_CREATION_OPTIONAL`: Job creation is optional. Returning immediate results is prioritized. BigQuery will automatically determine if a Job needs to be created. The conditions under which BigQuery can decide to not create a Job are subject to change. If Job creation is required, JOB_CREATION_REQUIRED mode should be used, which is the default. See https://github.com/googleapis/java-bigquery/blob/v2.34.0/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/QueryJobConfiguration.java#L98-L111 For example, the following create a connection with optional job creation: DriverManager.getConnection( "jdbc:BQDriver:my-project?jobcreationmode=JOB_CREATION_OPTIONAL", driverProperties); Note that this will cause queries to fail if the provided project does not have stateless queries preview enabled. --- .../starschema/clouddb/jdbc/BQConnection.java | 48 +++++++++++++ .../clouddb/jdbc/BQForwardOnlyResultSet.java | 10 +++ .../clouddb/jdbc/BQPreparedStatement.java | 2 +- .../clouddb/jdbc/BQScrollableResultSet.java | 16 ++++- .../starschema/clouddb/jdbc/BQStatement.java | 9 ++- .../clouddb/jdbc/BQStatementRoot.java | 11 +-- .../clouddb/jdbc/BQSupportFuncts.java | 7 +- .../BQForwardOnlyResultSetFunctionTest.java | 68 +++++++++++++++---- .../BQScrollableResultSetFunctionTest.java | 33 ++++++++- .../starschema/clouddb/jdbc/JdbcUrlTest.java | 24 +++++++ .../clouddb/jdbc/StatelessQuery.java | 42 ++++++++++++ 11 files changed, 242 insertions(+), 28 deletions(-) create mode 100644 src/test/java/net/starschema/clouddb/jdbc/StatelessQuery.java diff --git a/src/main/java/net/starschema/clouddb/jdbc/BQConnection.java b/src/main/java/net/starschema/clouddb/jdbc/BQConnection.java index 48d0b8e..363b701 100644 --- a/src/main/java/net/starschema/clouddb/jdbc/BQConnection.java +++ b/src/main/java/net/starschema/clouddb/jdbc/BQConnection.java @@ -72,6 +72,32 @@ public class BQConnection implements Connection { /** Boolean to determine whether or not to use legacy sql (default: false) * */ private final boolean useLegacySql; + /** + * Enum that describes whether to create a job in projects that support stateless queries. Copied + * from BigQueryImpl + */ + public static enum JobCreationMode { + /** If unspecified JOB_CREATION_REQUIRED is the default. */ + JOB_CREATION_MODE_UNSPECIFIED, + /** Default. Job creation is always required. */ + JOB_CREATION_REQUIRED, + + /** + * Job creation is optional. Returning immediate results is prioritized. BigQuery will + * automatically determine if a Job needs to be created. The conditions under which BigQuery can + * decide to not create a Job are subject to change. If Job creation is required, + * JOB_CREATION_REQUIRED mode should be used, which is the default. + * + *

Note that no job ID will be created if the results were returned immediately. + */ + JOB_CREATION_OPTIONAL; + + private JobCreationMode() {} + } + + /** The job creation mode - */ + private JobCreationMode jobCreationMode = JobCreationMode.JOB_CREATION_MODE_UNSPECIFIED; + /** getter for useLegacySql */ public boolean getUseLegacySql() { return useLegacySql; @@ -210,6 +236,9 @@ public BQConnection(String url, Properties loginProp, HttpTransport httpTranspor this.useQueryCache = parseBooleanQueryParam(caseInsensitiveProps.getProperty("querycache"), true); + this.jobCreationMode = + parseJobCreationMode(caseInsensitiveProps.getProperty("jobcreationmode")); + // Create Connection to BigQuery if (serviceAccount) { try { @@ -322,6 +351,21 @@ private static List parseArrayQueryParam(@Nullable String string, Charac : Arrays.asList(string.split(delimiter + "\\s*")); } + /** + * Return a {@link JobCreationMode} or raise an exception if the string does not match a variant. + */ + private static JobCreationMode parseJobCreationMode(@Nullable String string) + throws BQSQLException { + if (string == null) { + return null; + } + try { + return JobCreationMode.valueOf(string); + } catch (IllegalArgumentException e) { + throw new BQSQLException("could not parse " + string + " as job creation mode", e); + } + } + /** * * @@ -1214,4 +1258,8 @@ public Long getMaxBillingBytes() { public Integer getTimeoutMs() { return timeoutMs; } + + public JobCreationMode getJobCreationMode() { + return jobCreationMode; + } } diff --git a/src/main/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSet.java b/src/main/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSet.java index 9000b05..5b0bbf1 100644 --- a/src/main/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSet.java +++ b/src/main/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSet.java @@ -105,6 +105,8 @@ public class BQForwardOnlyResultSet implements java.sql.ResultSet { private String projectId; /** Reference for the Job */ private @Nullable Job completedJob; + /** The BigQuery query ID; set if the query completed without a Job */ + private final @Nullable String queryId; /** The total number of bytes processed while creating this ResultSet */ private final @Nullable Long totalBytesProcessed; /** Whether the ResultSet came from BigQuery's cache */ @@ -127,12 +129,14 @@ public BQForwardOnlyResultSet( Bigquery bigquery, String projectId, @Nullable Job completedJob, + String queryId, BQStatementRoot bqStatementRoot) throws SQLException { this( bigquery, projectId, completedJob, + queryId, bqStatementRoot, null, false, @@ -160,6 +164,7 @@ public BQForwardOnlyResultSet( Bigquery bigquery, String projectId, @Nullable Job completedJob, + @Nullable String queryId, BQStatementRoot bqStatementRoot, List prefetchedRows, boolean prefetchedAllRows, @@ -172,6 +177,7 @@ public BQForwardOnlyResultSet( logger.debug("Created forward only resultset TYPE_FORWARD_ONLY"); this.Statementreference = (Statement) bqStatementRoot; this.completedJob = completedJob; + this.queryId = queryId; this.projectId = projectId; if (bigquery == null) { throw new BQSQLException("Failed to fetch results. Connection is closed."); @@ -2992,4 +2998,8 @@ public boolean wasNull() throws SQLException { return null; } } + + public @Nullable String getQueryId() { + return queryId; + } } diff --git a/src/main/java/net/starschema/clouddb/jdbc/BQPreparedStatement.java b/src/main/java/net/starschema/clouddb/jdbc/BQPreparedStatement.java index 476e2da..a191caa 100644 --- a/src/main/java/net/starschema/clouddb/jdbc/BQPreparedStatement.java +++ b/src/main/java/net/starschema/clouddb/jdbc/BQPreparedStatement.java @@ -254,7 +254,7 @@ public ResultSet executeQuery() throws SQLException { this); } else { return new BQForwardOnlyResultSet( - this.connection.getBigquery(), this.projectId, referencedJob, this); + this.connection.getBigquery(), this.projectId, referencedJob, null, this); } } // Pause execution for half second before polling job status diff --git a/src/main/java/net/starschema/clouddb/jdbc/BQScrollableResultSet.java b/src/main/java/net/starschema/clouddb/jdbc/BQScrollableResultSet.java index b938852..d51fef0 100644 --- a/src/main/java/net/starschema/clouddb/jdbc/BQScrollableResultSet.java +++ b/src/main/java/net/starschema/clouddb/jdbc/BQScrollableResultSet.java @@ -66,7 +66,10 @@ public class BQScrollableResultSet extends ScrollableResultset */ private final @Nullable List biEngineReasons; - private final JobReference jobReference; + private final @Nullable JobReference jobReference; + + /** The BigQuery query ID; set if the query completed without a Job */ + private final @Nullable String queryId; private TableSchema schema; @@ -86,7 +89,8 @@ public BQScrollableResultSet( bigQueryGetQueryResultResponse.getCacheHit(), null, null, - bigQueryGetQueryResultResponse.getJobReference()); + bigQueryGetQueryResultResponse.getJobReference(), + null); BigInteger maxrow; try { @@ -104,7 +108,8 @@ public BQScrollableResultSet( @Nullable Boolean cacheHit, @Nullable String biEngineMode, @Nullable List biEngineReasons, - JobReference jobReference) { + @Nullable JobReference jobReference, + @Nullable String queryId) { logger.debug("Created Scrollable resultset TYPE_SCROLL_INSENSITIVE"); try { maxFieldSize = bqStatementRoot.getMaxFieldSize(); @@ -126,6 +131,7 @@ public BQScrollableResultSet( this.biEngineMode = biEngineMode; this.biEngineReasons = biEngineReasons; this.jobReference = jobReference; + this.queryId = queryId; } /** {@inheritDoc} */ @@ -302,4 +308,8 @@ public String getString(int columnIndex) throws SQLException { return null; } } + + public @Nullable String getQueryId() { + return queryId; + } } diff --git a/src/main/java/net/starschema/clouddb/jdbc/BQStatement.java b/src/main/java/net/starschema/clouddb/jdbc/BQStatement.java index 2350a89..6310a99 100644 --- a/src/main/java/net/starschema/clouddb/jdbc/BQStatement.java +++ b/src/main/java/net/starschema/clouddb/jdbc/BQStatement.java @@ -214,6 +214,7 @@ private ResultSet executeQueryHelper(String querySql, boolean unlimitedBillingBy this.connection.getBigquery(), projectId, referencedJob, + qr.getQueryId(), this, rows, fetchedAll, @@ -234,7 +235,8 @@ private ResultSet executeQueryHelper(String querySql, boolean unlimitedBillingBy qr.getCacheHit(), biEngineMode, biEngineReasons, - qr.getJobReference()); + qr.getJobReference(), + qr.getQueryId()); } jobAlreadyCompleted = true; } @@ -285,7 +287,7 @@ private ResultSet executeQueryHelper(String querySql, boolean unlimitedBillingBy this); } else { return new BQForwardOnlyResultSet( - this.connection.getBigquery(), projectId, referencedJob, this); + this.connection.getBigquery(), projectId, referencedJob, null, this); } } // Pause execution for half second before polling job status @@ -345,7 +347,8 @@ protected QueryResponse runSyncQuery(String querySql, boolean unlimitedBillingBy // socket timeouts (long) getMaxRows(), this.getAllLabels(), - this.connection.getUseQueryCache()); + this.connection.getUseQueryCache(), + this.connection.getJobCreationMode()); syncResponseFromCurrentQuery.set(resp); this.mostRecentJobReference.set(resp.getJobReference()); } catch (Exception e) { diff --git a/src/main/java/net/starschema/clouddb/jdbc/BQStatementRoot.java b/src/main/java/net/starschema/clouddb/jdbc/BQStatementRoot.java index 627dbf1..bf8d08d 100644 --- a/src/main/java/net/starschema/clouddb/jdbc/BQStatementRoot.java +++ b/src/main/java/net/starschema/clouddb/jdbc/BQStatementRoot.java @@ -265,7 +265,8 @@ private int executeDML(String sql) throws SQLException { (long) querytimeout * 1000, (long) getMaxRows(), this.getAllLabels(), - this.connection.getUseQueryCache()); + this.connection.getUseQueryCache(), + this.connection.getJobCreationMode()); this.mostRecentJobReference.set(qr.getJobReference()); if (defaultValueIfNull(qr.getJobComplete(), false)) { @@ -327,7 +328,8 @@ public ResultSet executeQuery(String querySql, boolean unlimitedBillingBytes) (long) querytimeout * 1000, (long) getMaxRows(), this.getAllLabels(), - this.connection.getUseQueryCache()); + this.connection.getUseQueryCache(), + this.connection.getJobCreationMode()); this.mostRecentJobReference.set(qr.getJobReference()); referencedJob = @@ -362,7 +364,8 @@ public ResultSet executeQuery(String querySql, boolean unlimitedBillingBytes) qr.getCacheHit(), biEngineMode, biEngineReasons, - referencedJob.getJobReference()); + referencedJob.getJobReference(), + qr.getQueryId()); } jobAlreadyCompleted = true; } @@ -384,7 +387,7 @@ public ResultSet executeQuery(String querySql, boolean unlimitedBillingBytes) this); } else { return new BQForwardOnlyResultSet( - this.connection.getBigquery(), projectId, referencedJob, this); + this.connection.getBigquery(), projectId, referencedJob, null, this); } } // Pause execution for half second before polling job status diff --git a/src/main/java/net/starschema/clouddb/jdbc/BQSupportFuncts.java b/src/main/java/net/starschema/clouddb/jdbc/BQSupportFuncts.java index 4f39d0a..bc86999 100644 --- a/src/main/java/net/starschema/clouddb/jdbc/BQSupportFuncts.java +++ b/src/main/java/net/starschema/clouddb/jdbc/BQSupportFuncts.java @@ -44,6 +44,7 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Stream; +import net.starschema.clouddb.jdbc.BQConnection.JobCreationMode; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -644,7 +645,8 @@ static QueryResponse runSyncQuery( Long queryTimeoutMs, Long maxResults, Map labels, - boolean useQueryCache) + boolean useQueryCache, + JobCreationMode jobCreationMode) throws IOException { QueryRequest qr = new QueryRequest() @@ -654,6 +656,9 @@ static QueryResponse runSyncQuery( .setQuery(querySql) .setUseLegacySql(useLegacySql) .setMaximumBytesBilled(maxBillingBytes); + if (jobCreationMode != null) { + qr = qr.setJobCreationMode(jobCreationMode.name()); + } if (dataSet != null) { qr.setDefaultDataset(new DatasetReference().setDatasetId(dataSet).setProjectId(projectId)); } diff --git a/src/test/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSetFunctionTest.java b/src/test/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSetFunctionTest.java index 41195c8..71626be 100644 --- a/src/test/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSetFunctionTest.java +++ b/src/test/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSetFunctionTest.java @@ -28,7 +28,12 @@ import com.google.gson.Gson; import java.io.IOException; import java.math.BigDecimal; -import java.sql.*; +import java.sql.DriverManager; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Statement; +import java.sql.Time; +import java.sql.Timestamp; import java.text.DateFormat; import java.text.ParseException; import java.text.SimpleDateFormat; @@ -40,6 +45,8 @@ import java.util.Properties; import java.util.TimeZone; import junit.framework.Assert; +import org.assertj.core.api.Assertions; +import org.junit.After; import org.junit.Before; import org.junit.Test; import org.slf4j.Logger; @@ -72,6 +79,14 @@ public void setup() throws SQLException, IOException { this.defaultConn = new BQConnection(url, new Properties()); } + @After + public void teardown() throws SQLException { + if (defaultConn != null) { + defaultConn.close(); + defaultConn = null; + } + } + private BQConnection conn() throws SQLException, IOException { return this.defaultConn; } @@ -202,21 +217,26 @@ public void isClosedValidtest() { */ @Before public void NewConnection() { - NewConnection(true); + NewConnection("&useLegacySql=true"); } - void NewConnection(boolean useLegacySql) { - + void NewConnection(String extraUrl) { this.logger.info("Testing the JDBC driver"); try { Class.forName("net.starschema.clouddb.jdbc.BQDriver"); Properties props = BQSupportFuncts.readFromPropFile( getClass().getResource("/installedaccount1.properties").getFile()); - props.setProperty("useLegacySql", String.valueOf(useLegacySql)); + String jdcbUrl = BQSupportFuncts.constructUrlFromPropertiesFile(props); + if (extraUrl != null) { + jdcbUrl += extraUrl; + } + if (BQForwardOnlyResultSetFunctionTest.con != null) { + BQForwardOnlyResultSetFunctionTest.con.close(); + } BQForwardOnlyResultSetFunctionTest.con = DriverManager.getConnection( - BQSupportFuncts.constructUrlFromPropertiesFile(props), + jdcbUrl, BQSupportFuncts.readFromPropFile( getClass().getResource("/installedaccount1.properties").getFile())); } catch (Exception e) { @@ -438,7 +458,7 @@ public void testResultSetTypesInGetString() throws SQLException { + "STRUCT(1 as a, ['an', 'array'] as b)," + "TIMESTAMP('2012-01-01 00:00:03.0000') as t"; - this.NewConnection(false); + this.NewConnection("&useLegacySql=false"); java.sql.ResultSet result = null; try { Statement stmt = @@ -500,7 +520,7 @@ public void testResultSetDateTimeType() throws SQLException, ParseException { final DateFormat utcDateFormatter = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSS"); utcDateFormatter.setTimeZone(TimeZone.getTimeZone("UTC")); - this.NewConnection(false); + this.NewConnection("&useLegacySql=false"); Statement stmt = BQForwardOnlyResultSetFunctionTest.con.createStatement( ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY); @@ -534,7 +554,7 @@ public void testResultSetDateTimeType() throws SQLException, ParseException { public void testResultSetTimestampType() throws SQLException, ParseException { final String sql = "SELECT TIMESTAMP('2012-01-01 01:02:03.04567')"; - this.NewConnection(false); + this.NewConnection("&useLegacySql=false"); Statement stmt = BQForwardOnlyResultSetFunctionTest.con.createStatement( ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY); @@ -567,7 +587,7 @@ public void testResultSetTypesInGetObject() throws SQLException, ParseException + "CAST('2011-04-03' AS DATE), " + "CAST('nan' AS FLOAT)"; - this.NewConnection(true); + this.NewConnection("&useLegacySql=true"); java.sql.ResultSet result = null; try { Statement stmt = @@ -595,7 +615,7 @@ public void testResultSetTypesInGetObject() throws SQLException, ParseException public void testResultSetArraysInGetObject() throws SQLException, ParseException { final String sql = "SELECT [1, 2, 3], [TIMESTAMP(\"2010-09-07 15:30:00 America/Los_Angeles\")]"; - this.NewConnection(false); + this.NewConnection("&useLegacySql=false"); java.sql.ResultSet result = null; try { Statement stmt = @@ -629,7 +649,8 @@ public void testResultSetArraysInGetObject() throws SQLException, ParseException @Test public void testResultSetTimeType() throws SQLException, ParseException { final String sql = "select current_time(), CAST('00:00:02.12345' AS TIME)"; - this.NewConnection(false); + this.NewConnection("&useLegacySql=false"); + java.sql.ResultSet result = null; try { Statement stmt = @@ -686,7 +707,7 @@ public void testResultSetProcedures() throws SQLException, ParseException { final String sql = "CREATE PROCEDURE looker_test.procedure_test(target_id INT64)\n" + "BEGIN\n" + "END;"; - this.NewConnection(false); + this.NewConnection("&useLegacySql=false"); java.sql.ResultSet result = null; try { Statement stmt = @@ -713,7 +734,7 @@ public void testResultSetProcedures() throws SQLException, ParseException { public void testResultSetProceduresAsync() throws SQLException { final String sql = "CREATE PROCEDURE looker_test.long_procedure(target_id INT64)\n" + "BEGIN\n" + "END;"; - this.NewConnection(false); + this.NewConnection("&useLegacySql=false"); try { BQConnection bq = conn(); @@ -756,7 +777,7 @@ public void testBQForwardOnlyResultSetDoesntThrowNPE() throws Exception { // before the results have been fetched. This was throwing a NPE. bq.close(); try { - new BQForwardOnlyResultSet(bq.getBigquery(), defaultProjectId, ref, stmt); + new BQForwardOnlyResultSet(bq.getBigquery(), defaultProjectId, ref, null, stmt); Assert.fail("Initalizing BQForwardOnlyResultSet should throw something other than a NPE."); } catch (SQLException e) { Assert.assertEquals(e.getMessage(), "Failed to fetch results. Connection is closed."); @@ -807,4 +828,21 @@ private void mockResponse(String jsonResponse) throws Exception { results.getBiEngineMode(); results.getBiEngineReasons(); } + + @Test + public void testStatelessQuery() throws SQLException { + NewConnection("&useLegacySql=false&jobcreationmode=JOB_CREATION_OPTIONAL"); + StatelessQuery.assumeStatelessQueriesEnabled( + BQForwardOnlyResultSetFunctionTest.con.getCatalog()); + final Statement stmt = + BQForwardOnlyResultSetFunctionTest.con.createStatement( + ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY); + final ResultSet result = stmt.executeQuery(StatelessQuery.exampleQuery()); + final String[][] rows = BQSupportMethods.GetQueryResult(result); + Assertions.assertThat(rows).isEqualTo(StatelessQuery.exampleValues()); + + final BQForwardOnlyResultSet bqResultSet = (BQForwardOnlyResultSet) result; + Assertions.assertThat(bqResultSet.getJobId()).isNull(); + Assertions.assertThat(bqResultSet.getQueryId()).contains("!"); + } } diff --git a/src/test/java/net/starschema/clouddb/jdbc/BQScrollableResultSetFunctionTest.java b/src/test/java/net/starschema/clouddb/jdbc/BQScrollableResultSetFunctionTest.java index 2f40868..c7e8190 100644 --- a/src/test/java/net/starschema/clouddb/jdbc/BQScrollableResultSetFunctionTest.java +++ b/src/test/java/net/starschema/clouddb/jdbc/BQScrollableResultSetFunctionTest.java @@ -28,6 +28,8 @@ import java.sql.Statement; import java.util.Properties; import junit.framework.Assert; +import org.assertj.core.api.Assertions; +import org.junit.After; import org.junit.Before; import org.junit.Test; import org.slf4j.Logger; @@ -242,7 +244,16 @@ public void isClosedValidtest() { */ @Before public void NewConnection() { + NewConnection("&useLegacySql=true"); + } + + @After + public void closeConnection() throws SQLException { + BQScrollableResultSetFunctionTest.con.close(); + BQScrollableResultSetFunctionTest.con = null; + } + public void NewConnection(String extraUrl) { try { if (BQScrollableResultSetFunctionTest.con == null || !BQScrollableResultSetFunctionTest.con.isValid(0)) { @@ -253,7 +264,9 @@ public void NewConnection() { BQSupportFuncts.constructUrlFromPropertiesFile( BQSupportFuncts.readFromPropFile( getClass().getResource("/installedaccount1.properties").getFile())); - jdbcUrl += "&useLegacySql=true"; + if (jdbcUrl != null) { + jdbcUrl += extraUrl; + } BQScrollableResultSetFunctionTest.con = DriverManager.getConnection( jdbcUrl, @@ -714,4 +727,22 @@ private void mockResponse(String jsonResponse) throws Exception { results.getBiEngineMode(); results.getBiEngineReasons(); } + + @Test + public void testStatelessQuery() throws SQLException { + closeConnection(); + NewConnection("&useLegacySql=true&jobcreationmode=JOB_CREATION_OPTIONAL"); + StatelessQuery.assumeStatelessQueriesEnabled( + BQScrollableResultSetFunctionTest.con.getCatalog()); + final Statement stmt = + BQScrollableResultSetFunctionTest.con.createStatement( + ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY); + final ResultSet result = stmt.executeQuery(StatelessQuery.exampleQuery()); + final String[][] rows = BQSupportMethods.GetQueryResult(result); + Assertions.assertThat(rows).isEqualTo(StatelessQuery.exampleValues()); + + final BQScrollableResultSet bqResultSet = (BQScrollableResultSet) result; + Assertions.assertThat(bqResultSet.getJobId()).isNull(); + Assertions.assertThat(bqResultSet.getQueryId()).contains("!"); + } } diff --git a/src/test/java/net/starschema/clouddb/jdbc/JdbcUrlTest.java b/src/test/java/net/starschema/clouddb/jdbc/JdbcUrlTest.java index 3732de3..6b32a9d 100644 --- a/src/test/java/net/starschema/clouddb/jdbc/JdbcUrlTest.java +++ b/src/test/java/net/starschema/clouddb/jdbc/JdbcUrlTest.java @@ -13,6 +13,7 @@ import java.util.Map; import java.util.Properties; import junit.framework.Assert; +import net.starschema.clouddb.jdbc.BQConnection.JobCreationMode; import org.assertj.core.api.Assertions; import org.junit.Before; import org.junit.Rule; @@ -508,4 +509,27 @@ private Properties getProperties(String pathToProp) throws IOException { private String getUrl(String pathToProp, String dataset) throws IOException { return BQSupportFuncts.constructUrlFromPropertiesFile(getProperties(pathToProp), true, dataset); } + + @Test + public void missingJobCreationModeDefaultsToNull() throws Exception { + final String url = getUrl("/protectedaccount.properties", null); + Assertions.assertThat(url).doesNotContain("jobcreationmode"); + bq = new BQConnection(url, new Properties()); + final JobCreationMode mode = bq.getJobCreationMode(); + Assertions.assertThat(mode).isNull(); + } + + @Test + public void jobCreationModeTest() throws Exception { + final String url = getUrl("/protectedaccount.properties", null); + Assertions.assertThat(url).doesNotContain("jobcreationmode"); + final JobCreationMode[] modes = JobCreationMode.values(); + for (JobCreationMode mode : modes) { + final String fullURL = String.format("%s&jobcreationmode=%s", url, mode.name()); + try (BQConnection bq = new BQConnection(fullURL, new Properties())) { + final JobCreationMode parsedMode = bq.getJobCreationMode(); + Assertions.assertThat(parsedMode).isEqualTo(mode); + } + } + } } diff --git a/src/test/java/net/starschema/clouddb/jdbc/StatelessQuery.java b/src/test/java/net/starschema/clouddb/jdbc/StatelessQuery.java new file mode 100644 index 0000000..f86f2a6 --- /dev/null +++ b/src/test/java/net/starschema/clouddb/jdbc/StatelessQuery.java @@ -0,0 +1,42 @@ +package net.starschema.clouddb.jdbc; + +import com.google.common.collect.ImmutableSet; +import java.util.Set; +import org.junit.Assume; + +/** Helpers for tests that require projects with stateless queries enabled */ +public final class StatelessQuery { + + private StatelessQuery() {} + + private static final Set ENABLED_PROJECTS = + ImmutableSet.of("disco-parsec-659", "looker-db-test"); + + /** + * Raise an {@link org.junit.AssumptionViolatedException} if the provided project isn't one that's + * known to have stateless queries enabled + * + * @param project the project to check - get it from {@link BQConnection.getCatalog() } + */ + public static void assumeStatelessQueriesEnabled(String project) { + Assume.assumeTrue(ENABLED_PROJECTS.contains(project)); + } + + /** + * A small query that should run statelessly (that is, without a job). + * + * @return the query + */ + public static String exampleQuery() { + return "SELECT 9876"; + } + + /** + * The values returned by {@link StatelessQuery} + * + * @return An array of strings representing the returned values + */ + public static String[][] exampleValues() { + return new String[][] {new String[] {"9876"}}; + } +} From 71bc3a9d978b80d3a3de2f8c54a19ff96c86b295 Mon Sep 17 00:00:00 2001 From: Mark Williams Date: Wed, 13 Dec 2023 23:55:18 +0000 Subject: [PATCH 05/19] Add microbenchmark for stateless queries. This adds a Java Microbenchmark Harness (JMH) benchmark that compares a small query with and without job creation. Documentation on running the microbenchmark can be found in its class' Javadoc. A small query with job creation takes almost twice as long as one with optional job creation. --- pom.xml | 37 +++++++- .../clouddb/jdbc/ConnectionFromResources.java | 44 ++++++++++ .../jdbc/StatelessSmallQueryBenchmark.java | 84 +++++++++++++++++++ 3 files changed, 163 insertions(+), 2 deletions(-) create mode 100644 src/test/java/net/starschema/clouddb/jdbc/ConnectionFromResources.java create mode 100644 src/test/java/net/starschema/clouddb/jdbc/StatelessSmallQueryBenchmark.java diff --git a/pom.xml b/pom.xml index 5e248e8..dc202f9 100644 --- a/pom.xml +++ b/pom.xml @@ -142,6 +142,18 @@ 1.19.0 test + + org.openjdk.jmh + jmh-core + 1.37 + test + + + org.openjdk.jmh + jmh-generator-annprocess + 1.37 + test + @@ -221,8 +233,29 @@ + + org.codehaus.mojo + exec-maven-plugin + 3.1.0 + + + benchmark-stateless-small + exec + + test + java + + -classpath + + org.openjdk.jmh.Main + + net.starschema.clouddb.jdbc.StatelessSmallQueryBenchmark + + + + + + - - diff --git a/src/test/java/net/starschema/clouddb/jdbc/ConnectionFromResources.java b/src/test/java/net/starschema/clouddb/jdbc/ConnectionFromResources.java new file mode 100644 index 0000000..ad6120d --- /dev/null +++ b/src/test/java/net/starschema/clouddb/jdbc/ConnectionFromResources.java @@ -0,0 +1,44 @@ +package net.starschema.clouddb.jdbc; + +import java.io.IOException; +import java.io.InputStream; +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.SQLException; +import java.util.Properties; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** Utility class to enable BigQuery connections from properties resources. */ +public class ConnectionFromResources { + private static final Logger logger = LoggerFactory.getLogger(ConnectionFromResources.class); + + /** + * Connect to a BigQuery project as described in a properties file + * + * @param propertiesFilePath the path to the properties in file in src/test/resources + * @param extraUrl extra URL arguments to add; must start with & + * @return A {@link BQConnection} connected to the given database + * @throws IOException if the properties file cannot be read + * @throws SQLException if the configuration can't connect to BigQuery + */ + public static BQConnection connect(String propertiesFilePath, String extraUrl) + throws IOException, SQLException { + final Properties properties = new Properties(); + final ClassLoader loader = ConnectionFromResources.class.getClassLoader(); + try (InputStream stream = loader.getResourceAsStream(propertiesFilePath)) { + properties.load(stream); + } + final StringBuilder jdcbUrlBuilder = + new StringBuilder(BQSupportFuncts.constructUrlFromPropertiesFile(properties)); + if (extraUrl != null) { + jdcbUrlBuilder.append(extraUrl); + } + final String jdbcUrl = jdcbUrlBuilder.toString(); + + final Connection connection = DriverManager.getConnection(jdbcUrl, properties); + final BQConnection bqConnection = (BQConnection) connection; + logger.info("Created connection from {} to {}", propertiesFilePath, bqConnection.getURLPART()); + return bqConnection; + } +} diff --git a/src/test/java/net/starschema/clouddb/jdbc/StatelessSmallQueryBenchmark.java b/src/test/java/net/starschema/clouddb/jdbc/StatelessSmallQueryBenchmark.java new file mode 100644 index 0000000..d07c07f --- /dev/null +++ b/src/test/java/net/starschema/clouddb/jdbc/StatelessSmallQueryBenchmark.java @@ -0,0 +1,84 @@ +package net.starschema.clouddb.jdbc; + +import java.io.IOException; +import java.sql.Connection; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Statement; +import org.assertj.core.api.Assertions; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Level; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.TearDown; +import org.openjdk.jmh.annotations.Threads; + +/** + * Performance microbenchmark for stateless queries. This uses the example query from {@link + * StatelessQuery}, same as the tests. + * + *

Run with mvn exec:exec@benchmark-stateless - note that mvn install must have been run at least + * once before. + */ +@Fork(1) +@Threads(10) +@BenchmarkMode(Mode.Throughput) +public class StatelessSmallQueryBenchmark { + private static final String CONNECTION_PROPERTIES = "installedaccount1.properties"; + + @State(Scope.Thread) + public static class RequiredJob { + private BQConnection connection; + + @Setup(Level.Trial) + public void connect() throws SQLException, IOException { + connection = ConnectionFromResources.connect(CONNECTION_PROPERTIES, null); + } + + @TearDown + public void disconnect() throws SQLException { + connection.close(); + } + } + + @State(Scope.Thread) + public static class OptionalJob { + private BQConnection connection; + + @Setup(Level.Trial) + public void connect() throws SQLException, IOException { + connection = + ConnectionFromResources.connect( + CONNECTION_PROPERTIES, "&jobcreationmode=JOB_CREATION_OPTIONAL"); + } + + @TearDown + public void disconnect() throws SQLException { + connection.close(); + } + } + + private String[][] benchmarkSmallQuery(final Connection connection) throws SQLException { + final Statement statement = connection.createStatement(); + final ResultSet results = statement.executeQuery(StatelessQuery.exampleQuery()); + final String[][] rows = BQSupportMethods.GetQueryResult(results); + Assertions.assertThat(rows).isEqualTo(StatelessQuery.exampleValues()); + return rows; + } + + @Benchmark + public String[][] benchmarkSmallQueryRequiredJob(final RequiredJob requiredJob) + throws SQLException { + return benchmarkSmallQuery(requiredJob.connection); + } + + @Benchmark + public String[][] benchmarkSmallQueryOptionalJob(final OptionalJob optionalJob) + throws SQLException { + return benchmarkSmallQuery(optionalJob.connection); + } +} From 40e7b132af617a9b1adc56aa26d8c5941eeb3403 Mon Sep 17 00:00:00 2001 From: Mark Williams Date: Thu, 14 Dec 2023 00:12:18 +0000 Subject: [PATCH 06/19] Address review comment about docs; fix Javadoc. Method references need # not . --- src/main/java/net/starschema/clouddb/jdbc/BQConnection.java | 4 +++- src/test/java/net/starschema/clouddb/jdbc/StatelessQuery.java | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/main/java/net/starschema/clouddb/jdbc/BQConnection.java b/src/main/java/net/starschema/clouddb/jdbc/BQConnection.java index 363b701..db624fd 100644 --- a/src/main/java/net/starschema/clouddb/jdbc/BQConnection.java +++ b/src/main/java/net/starschema/clouddb/jdbc/BQConnection.java @@ -74,7 +74,9 @@ public class BQConnection implements Connection { /** * Enum that describes whether to create a job in projects that support stateless queries. Copied - * from BigQueryImpl + * from google-cloud-bigquery + * 2.34.0 */ public static enum JobCreationMode { /** If unspecified JOB_CREATION_REQUIRED is the default. */ diff --git a/src/test/java/net/starschema/clouddb/jdbc/StatelessQuery.java b/src/test/java/net/starschema/clouddb/jdbc/StatelessQuery.java index f86f2a6..f36dfb3 100644 --- a/src/test/java/net/starschema/clouddb/jdbc/StatelessQuery.java +++ b/src/test/java/net/starschema/clouddb/jdbc/StatelessQuery.java @@ -16,7 +16,7 @@ private StatelessQuery() {} * Raise an {@link org.junit.AssumptionViolatedException} if the provided project isn't one that's * known to have stateless queries enabled * - * @param project the project to check - get it from {@link BQConnection.getCatalog() } + * @param project the project to check - get it from {@link BQConnection#getCatalog() } */ public static void assumeStatelessQueriesEnabled(String project) { Assume.assumeTrue(ENABLED_PROJECTS.contains(project)); @@ -32,7 +32,7 @@ public static String exampleQuery() { } /** - * The values returned by {@link StatelessQuery} + * The values returned by {@link StatelessQuery#exampleQuery()} * * @return An array of strings representing the returned values */ From 3636f183fd65aaac202f0195ad8f8876677ef34a Mon Sep 17 00:00:00 2001 From: Mark Williams Date: Thu, 14 Dec 2023 00:37:23 +0000 Subject: [PATCH 07/19] Include benchmark in Javadoc --- .../clouddb/jdbc/StatelessSmallQueryBenchmark.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/test/java/net/starschema/clouddb/jdbc/StatelessSmallQueryBenchmark.java b/src/test/java/net/starschema/clouddb/jdbc/StatelessSmallQueryBenchmark.java index d07c07f..4ba0ac0 100644 --- a/src/test/java/net/starschema/clouddb/jdbc/StatelessSmallQueryBenchmark.java +++ b/src/test/java/net/starschema/clouddb/jdbc/StatelessSmallQueryBenchmark.java @@ -23,6 +23,12 @@ * *

Run with mvn exec:exec@benchmark-stateless - note that mvn install must have been run at least * once before. + * + *

Representative summary as of 2023-12-14: + * Benchmark Mode Cnt Score Error Units + * StatelessSmallQueryBenchmark.benchmarkSmallQueryOptionalJob thrpt 5 67.994 ± 10.326 ops/s + * StatelessSmallQueryBenchmark.benchmarkSmallQueryRequiredJob thrpt 5 37.171 ± 3.041 ops/s + * */ @Fork(1) @Threads(10) From b861a6c39828603ef402e45b6c9ea658e6a2bd58 Mon Sep 17 00:00:00 2001 From: Mark Williams Date: Fri, 15 Dec 2023 00:12:53 +0000 Subject: [PATCH 08/19] Add query-ID-less constructor for backwards compat. --- .../clouddb/jdbc/BQForwardOnlyResultSet.java | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/src/main/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSet.java b/src/main/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSet.java index 5b0bbf1..fe69724 100644 --- a/src/main/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSet.java +++ b/src/main/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSet.java @@ -125,11 +125,41 @@ public class BQForwardOnlyResultSet implements java.sql.ResultSet { */ private int Cursor = -1; + /** + * Constructor without query ID for backwards compatibility. + * + * @param bigquery Bigquery driver instance for which this is a result + * @param projectId the project from which these results were queried + * @param completedJob the query's job, if any + * @param bqStatementRoot the statement for which this is a result + * @throws SQLException thrown if the results can't be retrieved + */ public BQForwardOnlyResultSet( Bigquery bigquery, String projectId, @Nullable Job completedJob, - String queryId, + BQStatementRoot bqStatementRoot) + throws SQLException { + this( + bigquery, + projectId, + completedJob, + null, + bqStatementRoot, + null, + false, + null, + 0L, + false, + null, + null); + } + + public BQForwardOnlyResultSet( + Bigquery bigquery, + String projectId, + @Nullable Job completedJob, + @Nullable String queryId, BQStatementRoot bqStatementRoot) throws SQLException { this( From 091b88f98b696963fa36793e3a1f2fc232d5d441 Mon Sep 17 00:00:00 2001 From: Mark Williams Date: Mon, 18 Dec 2023 19:04:22 +0000 Subject: [PATCH 09/19] Remove job creation mode parsing method. Addresses review comment --- .../starschema/clouddb/jdbc/BQConnection.java | 28 ++++++++----------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/src/main/java/net/starschema/clouddb/jdbc/BQConnection.java b/src/main/java/net/starschema/clouddb/jdbc/BQConnection.java index db624fd..783609c 100644 --- a/src/main/java/net/starschema/clouddb/jdbc/BQConnection.java +++ b/src/main/java/net/starschema/clouddb/jdbc/BQConnection.java @@ -238,8 +238,17 @@ public BQConnection(String url, Properties loginProp, HttpTransport httpTranspor this.useQueryCache = parseBooleanQueryParam(caseInsensitiveProps.getProperty("querycache"), true); - this.jobCreationMode = - parseJobCreationMode(caseInsensitiveProps.getProperty("jobcreationmode")); + final String jobCreationModeString = caseInsensitiveProps.getProperty("jobcreationmode"); + if (jobCreationModeString == null) { + jobCreationMode = null; + } else { + try { + jobCreationMode = JobCreationMode.valueOf(jobCreationModeString); + } catch (IllegalArgumentException e) { + throw new BQSQLException( + "could not parse " + jobCreationModeString + " as job creation mode", e); + } + } // Create Connection to BigQuery if (serviceAccount) { @@ -353,21 +362,6 @@ private static List parseArrayQueryParam(@Nullable String string, Charac : Arrays.asList(string.split(delimiter + "\\s*")); } - /** - * Return a {@link JobCreationMode} or raise an exception if the string does not match a variant. - */ - private static JobCreationMode parseJobCreationMode(@Nullable String string) - throws BQSQLException { - if (string == null) { - return null; - } - try { - return JobCreationMode.valueOf(string); - } catch (IllegalArgumentException e) { - throw new BQSQLException("could not parse " + string + " as job creation mode", e); - } - } - /** * * From cbee7244240b83ab4da0cbb7aa4896904817268e Mon Sep 17 00:00:00 2001 From: Mark Williams Date: Tue, 19 Dec 2023 19:54:40 +0000 Subject: [PATCH 10/19] Fix `findColumn` and `get*(String label)` methods. The ResultSet `findColumn` methods (`BQForwardOnlyResultSet.findColumn`, `BQScrollableResultSet.findColumn`, and `BQResultSet.findColumn`) called `BQResultSetMetadata.getCatalogName` to determine the name of each column instead of `BQResultSetMetadata.getColumnLabel`. This was wrong because `getCatalogName` always returns the BigQuery project ID, not the column name. This bug in turn broke the get methods that are implemented in terms of `findColumn`, e.g. `getString(String)`. This change fixes `findColumn` and adds tests for methods that use it with some caveats: - `BQResultSet` has its findColumn fixed but it's not tested. That's because it's unused, even by its tests, which request a `TYPE_SCROLL_INSENSITIVE` result set and thus get a `BQScrollableResultSet`. - `BQScrollableResultSet` only accepts dates and times that can be parsed as longs, which disagrees with the more widely used `BQForwardOnlyResultSet`. Its date and time accessors are uncovered. - `getBigDecimal` is excluded because it calls `BigDecimal.setScale` without a rounding mode, which no longer works in modern Java. A separate change can fix this. --- .../clouddb/jdbc/BQForwardOnlyResultSet.java | 16 +- .../starschema/clouddb/jdbc/BQResultSet.java | 7 +- .../clouddb/jdbc/BQScrollableResultSet.java | 7 +- .../BQForwardOnlyResultSetFunctionTest.java | 9 +- .../BQScrollableResultSetFunctionTest.java | 31 +- .../jdbc/CommonTestsForResultSets.java | 274 ++++++++++++++++++ 6 files changed, 331 insertions(+), 13 deletions(-) create mode 100644 src/test/java/net/starschema/clouddb/jdbc/CommonTestsForResultSets.java diff --git a/src/main/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSet.java b/src/main/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSet.java index fe69724..2b7d602 100644 --- a/src/main/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSet.java +++ b/src/main/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSet.java @@ -61,7 +61,12 @@ import java.sql.Statement; import java.sql.Time; import java.sql.Timestamp; -import java.util.*; +import java.util.ArrayList; +import java.util.Calendar; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; import javax.annotation.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -593,12 +598,13 @@ public void deleteRow() throws SQLException { */ @Override public int findColumn(String columnLabel) throws SQLException { - if (this.isClosed()) { + if (isClosed()) { throw new BQSQLException("This Resultset is Closed"); } - int columncount = this.getMetaData().getColumnCount(); - for (int i = 1; i <= columncount; i++) { - if (this.getMetaData().getCatalogName(i).equals(columnLabel)) { + final ResultSetMetaData metadata = this.getMetaData(); + int columns = metadata.getColumnCount(); + for (int i = 1; i <= columns; i++) { + if (metadata.getColumnLabel(i).equals(columnLabel)) { return i; } } diff --git a/src/main/java/net/starschema/clouddb/jdbc/BQResultSet.java b/src/main/java/net/starschema/clouddb/jdbc/BQResultSet.java index ac36169..852c1fb 100644 --- a/src/main/java/net/starschema/clouddb/jdbc/BQResultSet.java +++ b/src/main/java/net/starschema/clouddb/jdbc/BQResultSet.java @@ -112,9 +112,10 @@ public int findColumn(String columnLabel) throws SQLException { if (this.isClosed()) { throw new BQSQLException("This Resultset is Closed"); } - int columncount = this.getMetaData().getColumnCount(); - for (int i = 1; i <= columncount; i++) { - if (this.getMetaData().getCatalogName(i).equals(columnLabel)) { + final ResultSetMetaData metadata = this.getMetaData(); + int columns = metadata.getColumnCount(); + for (int i = 1; i <= columns; i++) { + if (metadata.getColumnLabel(i).equals(columnLabel)) { return i; } } diff --git a/src/main/java/net/starschema/clouddb/jdbc/BQScrollableResultSet.java b/src/main/java/net/starschema/clouddb/jdbc/BQScrollableResultSet.java index d51fef0..d917344 100644 --- a/src/main/java/net/starschema/clouddb/jdbc/BQScrollableResultSet.java +++ b/src/main/java/net/starschema/clouddb/jdbc/BQScrollableResultSet.java @@ -140,9 +140,10 @@ public int findColumn(String columnLabel) throws SQLException { if (this.isClosed()) { throw new BQSQLException("This Resultset is Closed"); } - int columncount = this.getMetaData().getColumnCount(); - for (int i = 1; i <= columncount; i++) { - if (this.getMetaData().getCatalogName(i).equals(columnLabel)) { + final ResultSetMetaData metadata = this.getMetaData(); + int columns = metadata.getColumnCount(); + for (int i = 1; i <= columns; i++) { + if (metadata.getColumnLabel(i).equals(columnLabel)) { return i; } } diff --git a/src/test/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSetFunctionTest.java b/src/test/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSetFunctionTest.java index 71626be..7c1b4a3 100644 --- a/src/test/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSetFunctionTest.java +++ b/src/test/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSetFunctionTest.java @@ -28,6 +28,7 @@ import com.google.gson.Gson; import java.io.IOException; import java.math.BigDecimal; +import java.sql.Connection; import java.sql.DriverManager; import java.sql.ResultSet; import java.sql.SQLException; @@ -58,7 +59,7 @@ * @author Horváth Attila * @author Gunics Balazs */ -public class BQForwardOnlyResultSetFunctionTest { +public class BQForwardOnlyResultSetFunctionTest extends CommonTestsForResultSets { private static java.sql.Connection con = null; private java.sql.ResultSet resultForTest = null; @@ -845,4 +846,10 @@ public void testStatelessQuery() throws SQLException { Assertions.assertThat(bqResultSet.getJobId()).isNull(); Assertions.assertThat(bqResultSet.getQueryId()).contains("!"); } + + @Override + protected Statement createStatementForCommonTests(final Connection connection) + throws SQLException { + return connection.createStatement(ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY); + } } diff --git a/src/test/java/net/starschema/clouddb/jdbc/BQScrollableResultSetFunctionTest.java b/src/test/java/net/starschema/clouddb/jdbc/BQScrollableResultSetFunctionTest.java index c7e8190..d0dfa48 100644 --- a/src/test/java/net/starschema/clouddb/jdbc/BQScrollableResultSetFunctionTest.java +++ b/src/test/java/net/starschema/clouddb/jdbc/BQScrollableResultSetFunctionTest.java @@ -22,6 +22,7 @@ import com.google.api.client.testing.http.MockHttpTransport; import com.google.api.client.testing.http.MockLowLevelHttpResponse; +import java.sql.Connection; import java.sql.DriverManager; import java.sql.ResultSet; import java.sql.SQLException; @@ -31,6 +32,7 @@ import org.assertj.core.api.Assertions; import org.junit.After; import org.junit.Before; +import org.junit.Ignore; import org.junit.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -41,7 +43,7 @@ * @author Horváth Attila * @author Gunics Balazs */ -public class BQScrollableResultSetFunctionTest { +public class BQScrollableResultSetFunctionTest extends CommonTestsForResultSets { private static java.sql.Connection con = null; private static java.sql.ResultSet Result = null; @@ -745,4 +747,31 @@ public void testStatelessQuery() throws SQLException { Assertions.assertThat(bqResultSet.getJobId()).isNull(); Assertions.assertThat(bqResultSet.getQueryId()).contains("!"); } + + @Override + protected Statement createStatementForCommonTests(Connection connection) throws SQLException { + return connection.createStatement( + ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY); + } + + @Override + @Test + @Ignore( + "Contradiction between BQSCrollableResultSet and BQForwardOnlyResultSet " + + "dates and times: b/317107706") + public void testGetTimeByLabel() throws SQLException {} + + @Override + @Test + @Ignore( + "Contradiction between BQSCrollableResultSet and BQForwardOnlyResultSet " + + "dates and times: b/317107706") + public void testGetDateByLabel() throws SQLException {} + + @Override + @Test + @Ignore( + "Contradiction between BQSCrollableResultSet and BQForwardOnlyResultSet " + + "dates and times: b/317107706") + public void testGetTimestampByLabel() throws SQLException {} } diff --git a/src/test/java/net/starschema/clouddb/jdbc/CommonTestsForResultSets.java b/src/test/java/net/starschema/clouddb/jdbc/CommonTestsForResultSets.java new file mode 100644 index 0000000..f1368ca --- /dev/null +++ b/src/test/java/net/starschema/clouddb/jdbc/CommonTestsForResultSets.java @@ -0,0 +1,274 @@ +package net.starschema.clouddb.jdbc; + +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStreamReader; +import java.io.Reader; +import java.net.MalformedURLException; +import java.net.URL; +import java.nio.charset.StandardCharsets; +import java.sql.Connection; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.SQLXML; +import java.sql.Statement; +import java.sql.Time; +import java.sql.Timestamp; +import java.util.Calendar; +import java.util.Date; +import java.util.stream.Collectors; +import org.assertj.core.api.Assertions; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +public abstract class CommonTestsForResultSets { + + private Connection connection; + + @Before + public void connectForCommonTests() throws SQLException, IOException { + connection = + ConnectionFromResources.connect("installedaccount1.properties", "&useLegacySql=false"); + } + + @After + public void closeConnectionForCommonTests() throws SQLException { + connection.close(); + } + + /** + * A {@link java.util.function.Consumer} that can throw a {@link SQLException} + * + * @param The return value's type + */ + private interface ConsumerWithSQLException { + + void accept(T arg) throws SQLException; + } + + /** + * Create a {@link Statement} that returns the appropriate result set. + * + * @param connection the connection from which to create the statement + * @return a statement used by common tests + * @throws SQLException if the {@link Connection} raises one + */ + protected abstract Statement createStatementForCommonTests(final Connection connection) + throws SQLException; + + /** + * Assert something about the single row and column returned by a query. + * + * @param query must result in a single row and column + * @param check a Consumer that can throw a {@link SQLException} - put your assertions about the + * result set in this + * @throws SQLException whenever check throws it + */ + private void assertSingleRowAndColumn( + final String query, final ConsumerWithSQLException check) throws SQLException { + try (Statement stmt = createStatementForCommonTests(connection)) { + final ResultSet result = stmt.executeQuery(query); + Assertions.assertThat(result.next()).isTrue(); + check.accept(result); + Assertions.assertThat(result.next()).isFalse(); + } + } + + @Test + public void testFindColumn() throws SQLException { + assertSingleRowAndColumn( + "SELECT 2 AS two", rs -> Assertions.assertThat(rs.findColumn("two")).isEqualTo(1)); + } + + /** + * A {@link java.util.function.Function} that can throw a {@link SQLException} + * + * @param the argument's type + * @param the returned value's type + */ + private interface FunctionWithSQLException { + R apply(T arg) throws SQLException; + } + + private void assertReaderAsString( + final String query, + final FunctionWithSQLException getter, + final String value) + throws SQLException { + assertSingleRowAndColumn( + query, + rs -> { + final Reader reader = getter.apply(rs); + final String contents = + new BufferedReader(reader).lines().collect(Collectors.joining("")); + Assertions.assertThat(contents).isEqualTo(value); + }); + } + + @Test + public void testGetAsciiStreamByLabel() throws SQLException { + assertReaderAsString( + "SELECT 'an ASCII string' AS stream", + rs -> new InputStreamReader(rs.getAsciiStream("stream"), StandardCharsets.US_ASCII), + "an ASCII string"); + } + + @Test + public void testGetBinaryStreamByLabel() throws SQLException { + assertReaderAsString( + "SELECT 'a binary string' AS stream", + rs -> new InputStreamReader(rs.getBinaryStream("stream"), StandardCharsets.US_ASCII), + "a binary string"); + } + + @Test + public void testGetBooleanByLabel() throws SQLException { + assertSingleRowAndColumn( + "SELECT false AS bool", rs -> Assertions.assertThat(rs.getBoolean("bool")).isFalse()); + } + + @Test + public void testGetByteByLabel() throws SQLException { + assertSingleRowAndColumn( + "SELECT 98 AS byte", rs -> Assertions.assertThat(rs.getByte("byte")).isEqualTo((byte) 98)); + } + + @Test + public void testGetBytesByLabel() throws SQLException { + assertSingleRowAndColumn( + "SELECT 'abc' AS bytes", + rs -> Assertions.assertThat(rs.getBytes("bytes")).isEqualTo(new byte[] {'a', 'b', 'c'})); + } + + @Test + public void testGetCharacterStreamByLabel() throws SQLException { + assertReaderAsString( + "SELECT 'some characters' AS chars", + rs -> rs.getCharacterStream("chars"), + "some characters"); + } + + @Test + public void testGetDateByLabel() throws SQLException { + final Calendar calendar = Calendar.getInstance(); + calendar.clear(); + calendar.set(2023, Calendar.DECEMBER, 19); + final Date expected = calendar.getTime(); + + assertSingleRowAndColumn( + "SELECT '2023-12-19' AS date", + rs -> { + final Date value = rs.getDate("date"); + Assertions.assertThat(value).isEqualTo(expected); + }); + + assertSingleRowAndColumn( + "SELECT '2023-12-19' AS date", + rs -> { + final Date value = rs.getDate("date", calendar); + Assertions.assertThat(value).isEqualTo(expected); + }); + } + + @Test + public void testGetDoubleByLabel() throws SQLException { + assertSingleRowAndColumn( + "SELECT 0.1 AS double", + rs -> + Assertions.assertThat(rs.getDouble("double")) + .isEqualTo(0.1, Assertions.withPrecision(1d))); + } + + @Test + public void testGetFloatByLabel() throws SQLException { + assertSingleRowAndColumn( + "SELECT 0.1 AS float", + rs -> + Assertions.assertThat(rs.getDouble("float")) + .isEqualTo(0.1, Assertions.withPrecision(1d))); + } + + @Test + public void testGetIntByLabel() throws SQLException { + assertSingleRowAndColumn( + "SELECT 6 AS integer", rs -> Assertions.assertThat(rs.getInt("integer")).isEqualTo(6)); + } + + @Test + public void testGetLongByLabel() throws SQLException { + assertSingleRowAndColumn( + "SELECT 6 AS long", rs -> Assertions.assertThat(rs.getInt("long")).isEqualTo(6L)); + } + + @Test + public void testGetObjectByLabel() throws SQLException { + assertSingleRowAndColumn( + "SELECT 'an object' AS obj", + rs -> Assertions.assertThat(rs.getObject("obj")).isEqualTo("an object")); + } + + @Test + public void testGetShortByLabel() throws SQLException { + assertSingleRowAndColumn( + "SELECT 32767 AS short", + rs -> Assertions.assertThat(rs.getShort("short")).isEqualTo((short) 32767)); + } + + @Test + public void testGetSQLXMLByLabel() throws SQLException { + assertSingleRowAndColumn( + "SELECT 'xml' AS xml", + rs -> { + final SQLXML xml = rs.getSQLXML("xml"); + final String actual = xml.getString().trim(); + final String expected = "xml"; + Assertions.assertThat(actual).isEqualTo(expected); + }); + } + + @Test + public void testGetStringByLabel() throws SQLException { + assertSingleRowAndColumn( + "SELECT 'a string' AS string", + rs -> Assertions.assertThat(rs.getString("string")).isEqualTo("a string")); + } + + @Test + public void testGetTimeByLabel() throws SQLException { + final Time expected = Time.valueOf("01:02:03"); + assertSingleRowAndColumn( + "SELECT '01:02:03' AS time", + rs -> Assertions.assertThat(rs.getTime("time")).isEqualTo(expected)); + + final Calendar calendar = Calendar.getInstance(); + calendar.clear(); + calendar.set(2023, 12, 19); + assertSingleRowAndColumn( + "SELECT '01:02:03' AS time", + rs -> Assertions.assertThat(rs.getTime("time", calendar)).isEqualTo(expected)); + } + + @Test + public void testGetTimestampByLabel() throws SQLException { + final Timestamp expected = Timestamp.valueOf("2023-12-19 01:02:03"); + assertSingleRowAndColumn( + "SELECT '2023-12-19 01:02:03' AS timestamp", + rs -> Assertions.assertThat(rs.getTimestamp("timestamp")).isEqualTo(expected)); + + final Calendar calendar = Calendar.getInstance(); + calendar.clear(); + calendar.set(2023, 12, 19); + assertSingleRowAndColumn( + "SELECT '2023-12-19 01:02:03' AS timestamp", + rs -> Assertions.assertThat(rs.getTimestamp("timestamp", calendar)).isEqualTo(expected)); + } + + @Test + public void testGetURLByLabel() throws SQLException, MalformedURLException { + final URL expected = new URL("https://127.0.0.1/p?q=1"); + assertSingleRowAndColumn( + "SELECT 'https://127.0.0.1/p?q=1' AS url", + rs -> Assertions.assertThat(rs.getURL("url")).isEqualTo(expected)); + } +} From 30350ea94a329cae68d44a7e93b498d0185caca1 Mon Sep 17 00:00:00 2001 From: Mark Williams Date: Tue, 19 Dec 2023 23:45:03 +0000 Subject: [PATCH 11/19] Add Javadoc, use common findColumn implementation. Addresses code review comments. --- .../clouddb/jdbc/BQForwardOnlyResultSet.java | 23 ++-------------- .../starschema/clouddb/jdbc/BQResultSet.java | 13 +-------- .../clouddb/jdbc/BQScrollableResultSet.java | 12 +-------- .../clouddb/jdbc/CommonResultSet.java | 27 +++++++++++++++++++ .../jdbc/CommonTestsForResultSets.java | 7 +++++ 5 files changed, 38 insertions(+), 44 deletions(-) create mode 100644 src/main/java/net/starschema/clouddb/jdbc/CommonResultSet.java diff --git a/src/main/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSet.java b/src/main/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSet.java index 2b7d602..0880824 100644 --- a/src/main/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSet.java +++ b/src/main/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSet.java @@ -586,29 +586,10 @@ public void deleteRow() throws SQLException { throw new BQSQLFeatureNotSupportedException("deleteRow()"); } - /** - * - * - *

Implementation Details:

- * - *
- * Not implemented yet. - * - * @throws BQSQLException - */ + /** {@inheritDoc} */ @Override public int findColumn(String columnLabel) throws SQLException { - if (isClosed()) { - throw new BQSQLException("This Resultset is Closed"); - } - final ResultSetMetaData metadata = this.getMetaData(); - int columns = metadata.getColumnCount(); - for (int i = 1; i <= columns; i++) { - if (metadata.getColumnLabel(i).equals(columnLabel)) { - return i; - } - } - throw new BQSQLException("No Such column labeled: " + columnLabel); + return CommonResultSet.findColumn(columnLabel, getMetaData()); } /** diff --git a/src/main/java/net/starschema/clouddb/jdbc/BQResultSet.java b/src/main/java/net/starschema/clouddb/jdbc/BQResultSet.java index 852c1fb..0bfebfe 100644 --- a/src/main/java/net/starschema/clouddb/jdbc/BQResultSet.java +++ b/src/main/java/net/starschema/clouddb/jdbc/BQResultSet.java @@ -109,18 +109,7 @@ public BQResultSet( /** {@inheritDoc} */ @Override public int findColumn(String columnLabel) throws SQLException { - if (this.isClosed()) { - throw new BQSQLException("This Resultset is Closed"); - } - final ResultSetMetaData metadata = this.getMetaData(); - int columns = metadata.getColumnCount(); - for (int i = 1; i <= columns; i++) { - if (metadata.getColumnLabel(i).equals(columnLabel)) { - return i; - } - } - SQLException e = new BQSQLException("No Such column labeled: " + columnLabel); - throw e; + return CommonResultSet.findColumn(columnLabel, getMetaData()); } /** {@inheritDoc} */ diff --git a/src/main/java/net/starschema/clouddb/jdbc/BQScrollableResultSet.java b/src/main/java/net/starschema/clouddb/jdbc/BQScrollableResultSet.java index d917344..3cb0300 100644 --- a/src/main/java/net/starschema/clouddb/jdbc/BQScrollableResultSet.java +++ b/src/main/java/net/starschema/clouddb/jdbc/BQScrollableResultSet.java @@ -137,17 +137,7 @@ public BQScrollableResultSet( /** {@inheritDoc} */ @Override public int findColumn(String columnLabel) throws SQLException { - if (this.isClosed()) { - throw new BQSQLException("This Resultset is Closed"); - } - final ResultSetMetaData metadata = this.getMetaData(); - int columns = metadata.getColumnCount(); - for (int i = 1; i <= columns; i++) { - if (metadata.getColumnLabel(i).equals(columnLabel)) { - return i; - } - } - throw new BQSQLException("No Such column labeled: " + columnLabel); + return CommonResultSet.findColumn(columnLabel, getMetaData()); } /** {@inheritDoc} */ diff --git a/src/main/java/net/starschema/clouddb/jdbc/CommonResultSet.java b/src/main/java/net/starschema/clouddb/jdbc/CommonResultSet.java new file mode 100644 index 0000000..be7a16d --- /dev/null +++ b/src/main/java/net/starschema/clouddb/jdbc/CommonResultSet.java @@ -0,0 +1,27 @@ +package net.starschema.clouddb.jdbc; + +import com.google.cloud.bigquery.BigQuerySQLException; +import java.sql.ResultSetMetaData; +import java.sql.SQLException; + +/** A static utility class of methods common to all {@link java.sql.ResultSet} implementations. */ +class CommonResultSet { + + /** + * A common implementation of {@link java.sql.ResultSet#findColumn(String)} + * + * @param label the column-to-find's label + * @param metadata the metadata from the {@link java.sql.ResultSet} + * @return the integer index of the labeled column, if it's found + * @throws SQLException if no column with the given label is found + */ + static int findColumn(final String label, final ResultSetMetaData metadata) throws SQLException { + int columnCount = metadata.getColumnCount(); + for (int column = 1; column <= columnCount; column++) { + if (metadata.getColumnLabel(column).equals(label)) { + return column; + } + } + throw new BigQuerySQLException("No such Column labeled: " + label); + } +} diff --git a/src/test/java/net/starschema/clouddb/jdbc/CommonTestsForResultSets.java b/src/test/java/net/starschema/clouddb/jdbc/CommonTestsForResultSets.java index f1368ca..a3b80a9 100644 --- a/src/test/java/net/starschema/clouddb/jdbc/CommonTestsForResultSets.java +++ b/src/test/java/net/starschema/clouddb/jdbc/CommonTestsForResultSets.java @@ -22,6 +22,13 @@ import org.junit.Before; import org.junit.Test; +/** + * An abstract class that contains tests common to {@link ResultSet} implementations. Its name + * doesn't end with Test so that JUnit doesn't try to run it. + * + *

Subclass this in @{link ResultSet} test classes and implement {@link + * CommonTestsForResultSets#createStatementForCommonTests(Connection)}. + */ public abstract class CommonTestsForResultSets { private Connection connection; From 134cd89b906018004f312ee60785ae39d302987a Mon Sep 17 00:00:00 2001 From: Will Noble <78444363+wnob@users.noreply.github.com> Date: Thu, 21 Dec 2023 14:06:02 -0800 Subject: [PATCH 12/19] Add fix for null-valued TIMESTAMP (#188) --- pom.xml | 1 + .../clouddb/jdbc/DateTimeUtils.java | 6 ++ .../BQForwardOnlyResultSetFunctionTest.java | 57 +++++++++++++++++++ 3 files changed, 64 insertions(+) diff --git a/pom.xml b/pom.xml index dc202f9..f61e876 100644 --- a/pom.xml +++ b/pom.xml @@ -230,6 +230,7 @@ 1.7 + diff --git a/src/main/java/net/starschema/clouddb/jdbc/DateTimeUtils.java b/src/main/java/net/starschema/clouddb/jdbc/DateTimeUtils.java index e54a65d..be123d1 100644 --- a/src/main/java/net/starschema/clouddb/jdbc/DateTimeUtils.java +++ b/src/main/java/net/starschema/clouddb/jdbc/DateTimeUtils.java @@ -99,6 +99,9 @@ static Timestamp parseDateTime(String value, Calendar cal) throws SQLException { /** Parse a BigQuery TIMESTAMP literal, represented as the number of seconds since epoch. */ static Timestamp parseTimestamp(String value) throws SQLException { + if (value == null) { + return null; + } try { // BigQuery TIMESTAMP has a string representation that looks like e.g. "1.288061375E9" // for 2010-10-26 02:49:35 UTC. @@ -119,6 +122,9 @@ static Timestamp parseTimestamp(String value) throws SQLException { } static String formatTimestamp(String rawString) throws SQLException { + if (rawString == null) { + return null; + } Timestamp timestamp = parseTimestamp(rawString); return DATETIME_FORMATTER.format(OffsetDateTime.ofInstant(timestamp.toInstant(), UTC_ZONE)) + " UTC"; diff --git a/src/test/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSetFunctionTest.java b/src/test/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSetFunctionTest.java index 7c1b4a3..a1e5a42 100644 --- a/src/test/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSetFunctionTest.java +++ b/src/test/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSetFunctionTest.java @@ -38,6 +38,7 @@ import java.text.DateFormat; import java.text.ParseException; import java.text.SimpleDateFormat; +import java.time.Instant; import java.util.Calendar; import java.util.Date; import java.util.GregorianCalendar; @@ -796,6 +797,62 @@ public void testHandlesAllNullResponseFields() throws Exception { throw new AssertionError("Expected graceful failure due to lack of job reference"); } + @Test + public void testHandlesNullTimeDateObjects() throws Exception { + this.NewConnection("&useLegacySql=false"); + Statement stmt = + BQForwardOnlyResultSetFunctionTest.con.createStatement( + ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY); + + final String date = "2011-11-11"; + final String time = "12:12:12"; + final String dateTime = date + " " + time; + final String dateTimeWithT = date + "T" + time; + // The number of milliseconds between epoch and 2011-11-11 12:12:12 UTC+0. + final long millis = 1321013532000L; + + // spotless:off + String sql = "SELECT " + + "TIMESTAMP('" + dateTime + "') AS ts, " + + "DATETIME('" + dateTime + "') AS dt, " + + "DATE('" + date + "') AS d, " + + "TIME(12, 12, 12) AS t\n" + + "UNION ALL SELECT " + + "CASE WHEN 1 = 0 THEN TIMESTAMP('" + dateTime + "') ELSE NULL END, " + + "CASE WHEN 1 = 0 THEN DATETIME('" + dateTime + "') ELSE NULL END, " + + "CASE WHEN 1 = 0 THEN DATE('" + date + "') ELSE NULL END, " + + "CASE WHEN 1 = 0 THEN TIME(12, 12, 12) ELSE NULL END"; + // spotless:on + + ResultSet results = stmt.executeQuery(sql); + + // First row has all non-null objects. + Assertions.assertThat(results.next()).isTrue(); + Assertions.assertThat(results.getObject("ts")) + .isEqualTo(Timestamp.from(Instant.ofEpochMilli(millis))); + Assertions.assertThat(results.getString("ts")).isEqualTo(dateTime + " UTC"); + Assertions.assertThat(results.getObject("dt")).isEqualTo(Timestamp.valueOf(dateTime)); + Assertions.assertThat(results.getString("dt")).isEqualTo(dateTimeWithT); + Assertions.assertThat(results.getObject("d")).isEqualTo(java.sql.Date.valueOf(date)); + Assertions.assertThat(results.getString("d")).isEqualTo(date); + Assertions.assertThat(results.getObject("t")).isEqualTo(java.sql.Time.valueOf(time)); + Assertions.assertThat(results.getString("t")).isEqualTo(time); + + // Second row is all null. + Assertions.assertThat(results.next()).isTrue(); + Assertions.assertThat(results.getObject("ts")).isNull(); + Assertions.assertThat(results.getString("ts")).isNull(); + Assertions.assertThat(results.getObject("dt")).isNull(); + Assertions.assertThat(results.getString("dt")).isNull(); + Assertions.assertThat(results.getObject("d")).isNull(); + Assertions.assertThat(results.getString("d")).isNull(); + Assertions.assertThat(results.getObject("t")).isNull(); + Assertions.assertThat(results.getString("t")).isNull(); + + // Only two rows. + Assertions.assertThat(results.next()).isFalse(); + } + @Test public void testHandlesSomeNullResponseFields() throws Exception { // Make sure we don't get any NPE's due to null values; From d293aa62a69f2fecb5cba2ac5ac8ec09a9a29dcd Mon Sep 17 00:00:00 2001 From: Will Noble <78444363+wnob@users.noreply.github.com> Date: Thu, 21 Dec 2023 14:42:13 -0800 Subject: [PATCH 13/19] Prepare for v3.1.0 release (#189) --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index f61e876..fb00d4e 100644 --- a/pom.xml +++ b/pom.xml @@ -4,7 +4,7 @@ to a groupId that we have ownership over --> com.google.looker-open-source bqjdbc - 3.0.0 + 3.1.0 Big Query over JDBC A simple JDBC driver, to reach Google's BigQuery From 28012fa34a2f7a0649f1a52cb730abaaa46822c4 Mon Sep 17 00:00:00 2001 From: Mark Williams Date: Wed, 20 Dec 2023 19:50:18 +0000 Subject: [PATCH 14/19] Refactor BQScrollableResultSetFunctionTest connection management. Use common `ConnectionFromResources.connect` method instead of a bespoke `NewConnection` method. Also, use instance variable/test case scoped connections and result sets instead of static/suite scoped ones. This should help enable test parallelization later. Local testing shows no reliable increase in `BQScrollableResultSetFunctionTest` total runtime (~30 seconds both before and after change). --- .../BQScrollableResultSetFunctionTest.java | 398 ++++++++---------- 1 file changed, 183 insertions(+), 215 deletions(-) diff --git a/src/test/java/net/starschema/clouddb/jdbc/BQScrollableResultSetFunctionTest.java b/src/test/java/net/starschema/clouddb/jdbc/BQScrollableResultSetFunctionTest.java index d0dfa48..81b7762 100644 --- a/src/test/java/net/starschema/clouddb/jdbc/BQScrollableResultSetFunctionTest.java +++ b/src/test/java/net/starschema/clouddb/jdbc/BQScrollableResultSetFunctionTest.java @@ -22,8 +22,8 @@ import com.google.api.client.testing.http.MockHttpTransport; import com.google.api.client.testing.http.MockLowLevelHttpResponse; +import java.io.IOException; import java.sql.Connection; -import java.sql.DriverManager; import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; @@ -45,40 +45,39 @@ */ public class BQScrollableResultSetFunctionTest extends CommonTestsForResultSets { - private static java.sql.Connection con = null; - private static java.sql.ResultSet Result = null; - + private java.sql.Connection connection; + private java.sql.ResultSet result; Logger logger = LoggerFactory.getLogger(BQScrollableResultSetFunctionTest.class); @Test public void ChainedCursorFunctionTest() { this.logger.info("ChainedFunctionTest"); try { - BQScrollableResultSetFunctionTest.Result.beforeFirst(); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.next()); - Assert.assertEquals("you", BQScrollableResultSetFunctionTest.Result.getString(1)); + result.beforeFirst(); + Assert.assertTrue(result.next()); + Assert.assertEquals("you", result.getString(1)); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); } try { - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.absolute(10)); - Assert.assertEquals("whom", BQScrollableResultSetFunctionTest.Result.getString(1)); + Assert.assertTrue(result.absolute(10)); + Assert.assertEquals("whom", result.getString(1)); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); } try { - Assert.assertFalse(BQScrollableResultSetFunctionTest.Result.next()); + Assert.assertFalse(result.next()); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); } try { - Assert.assertEquals("", BQScrollableResultSetFunctionTest.Result.getString(1)); + Assert.assertEquals("", result.getString(1)); } catch (SQLException e) { boolean ct = e.toString().contains("Cursor is not in a valid Position"); if (ct == true) { @@ -90,42 +89,42 @@ public void ChainedCursorFunctionTest() { } try { - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.first()); - Assert.assertEquals("you", BQScrollableResultSetFunctionTest.Result.getString(1)); + Assert.assertTrue(result.first()); + Assert.assertEquals("you", result.getString(1)); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); } try { - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.isFirst()); - Assert.assertFalse(BQScrollableResultSetFunctionTest.Result.previous()); - BQScrollableResultSetFunctionTest.Result.afterLast(); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.isAfterLast()); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.absolute(-1)); - Assert.assertEquals("whom", BQScrollableResultSetFunctionTest.Result.getString(1)); + Assert.assertTrue(result.isFirst()); + Assert.assertFalse(result.previous()); + result.afterLast(); + Assert.assertTrue(result.isAfterLast()); + Assert.assertTrue(result.absolute(-1)); + Assert.assertEquals("whom", result.getString(1)); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); } try { - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.relative(-5)); - Assert.assertEquals("without", BQScrollableResultSetFunctionTest.Result.getString(1)); + Assert.assertTrue(result.relative(-5)); + Assert.assertEquals("without", result.getString(1)); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); } try { - Assert.assertFalse(BQScrollableResultSetFunctionTest.Result.relative(6)); + Assert.assertFalse(result.relative(6)); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); } try { - Assert.assertEquals("without", BQScrollableResultSetFunctionTest.Result.getString(1)); + Assert.assertEquals("without", result.getString(1)); } catch (SQLException e) { boolean ct = e.toString().contains("Cursor is not in a valid Position"); if (ct == true) { @@ -149,7 +148,10 @@ public void databaseMetaDataGetTables() { // tableNamePattern:OUTLET_LOOKUP, columnNamePattern: null // result = con.getMetaData().getTables("OUTLET_LOOKUP", null, "starschema_net__clouddb", null // ); - result = con.getMetaData().getColumns(null, "starschema_net__clouddb", "OUTLET_LOOKUP", null); + result = + connection + .getMetaData() + .getColumns(null, "starschema_net__clouddb", "OUTLET_LOOKUP", null); // Function call getTables(catalog: ARTICLE_COLOR_LOOKUP, schemaPattern: null, // tableNamePattern: starschema_net__clouddb, types: TABLE , VIEW , SYSTEM TABLE , SYNONYM , // ALIAS , ) @@ -203,88 +205,56 @@ private boolean comparer(String[][] expected, String[][] reality) { @Test public void isClosedValidtest() { try { - Assert.assertEquals(true, BQScrollableResultSetFunctionTest.con.isValid(0)); + Assert.assertEquals(true, connection.isValid(0)); } catch (SQLException e) { Assert.fail("Got an exception" + e.toString()); e.printStackTrace(); } try { - Assert.assertEquals(true, BQScrollableResultSetFunctionTest.con.isValid(10)); + Assert.assertEquals(true, connection.isValid(10)); } catch (SQLException e) { Assert.fail("Got an exception" + e.toString()); e.printStackTrace(); } try { - BQScrollableResultSetFunctionTest.con.isValid(-10); + connection.isValid(-10); } catch (SQLException e) { Assert.assertTrue(true); // e.printStackTrace(); } try { - BQScrollableResultSetFunctionTest.con.close(); + connection.close(); } catch (SQLException e) { e.printStackTrace(); } try { - Assert.assertTrue(BQScrollableResultSetFunctionTest.con.isClosed()); + Assert.assertTrue(connection.isClosed()); } catch (SQLException e1) { e1.printStackTrace(); } try { - BQScrollableResultSetFunctionTest.con.isValid(0); + connection.isValid(0); } catch (SQLException e) { Assert.assertTrue(true); e.printStackTrace(); } } - /** - * Makes a new Bigquery Connection to URL in file and gives back the Connection to static con - * member. - */ + private Connection connect(final String extraUrl) throws SQLException, IOException { + return ConnectionFromResources.connect("installedaccount1.properties", extraUrl); + } + @Before - public void NewConnection() { - NewConnection("&useLegacySql=true"); + public void setConnection() throws SQLException, IOException { + connection = connect("&useLegacySql=true"); + QueryLoad(); } @After public void closeConnection() throws SQLException { - BQScrollableResultSetFunctionTest.con.close(); - BQScrollableResultSetFunctionTest.con = null; - } - - public void NewConnection(String extraUrl) { - try { - if (BQScrollableResultSetFunctionTest.con == null - || !BQScrollableResultSetFunctionTest.con.isValid(0)) { - this.logger.info("Testing the JDBC driver"); - try { - Class.forName("net.starschema.clouddb.jdbc.BQDriver"); - String jdbcUrl = - BQSupportFuncts.constructUrlFromPropertiesFile( - BQSupportFuncts.readFromPropFile( - getClass().getResource("/installedaccount1.properties").getFile())); - if (jdbcUrl != null) { - jdbcUrl += extraUrl; - } - BQScrollableResultSetFunctionTest.con = - DriverManager.getConnection( - jdbcUrl, - BQSupportFuncts.readFromPropFile( - getClass().getResource("/installedaccount1.properties").getFile())); - } catch (Exception e) { - e.printStackTrace(); - this.logger.error("Error in connection" + e.toString()); - Assert.fail("General Exception:" + e.toString()); - } - this.logger.info(((BQConnection) BQScrollableResultSetFunctionTest.con).getURLPART()); - } - } catch (SQLException e) { - logger.debug("Oops something went wrong", e); - } - this.QueryLoad(); + connection.close(); } // Comprehensive Tests: @@ -303,15 +273,14 @@ public void QueryLoad() { try { Statement stmt = - BQScrollableResultSetFunctionTest.con.createStatement( - ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY); + connection.createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY); stmt.setQueryTimeout(500); - BQScrollableResultSetFunctionTest.Result = stmt.executeQuery(sql); + result = stmt.executeQuery(sql); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); } - Assert.assertNotNull(BQScrollableResultSetFunctionTest.Result); + Assert.assertNotNull(result); this.logger.debug(description); HelperFunctions.printer(expectation); @@ -319,9 +288,7 @@ public void QueryLoad() { try { Assert.assertTrue( "Comparing failed in the String[][] array", - this.comparer( - expectation, - BQSupportMethods.GetQueryResult(BQScrollableResultSetFunctionTest.Result))); + this.comparer(expectation, BQSupportMethods.GetQueryResult(result))); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail(e.toString()); @@ -331,8 +298,8 @@ public void QueryLoad() { @Test public void ResultSetMetadata() { try { - this.logger.debug(BQScrollableResultSetFunctionTest.Result.getMetaData().getSchemaName(1)); - this.logger.debug("{}", BQScrollableResultSetFunctionTest.Result.getMetaData().getScale(1)); + this.logger.debug(result.getMetaData().getSchemaName(1)); + this.logger.debug("{}", result.getMetaData().getScale(1)); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); } @@ -342,7 +309,7 @@ public void ResultSetMetadata() { @Test public void TestResultIndexOutofBound() { try { - this.logger.debug("{}", BQScrollableResultSetFunctionTest.Result.getBoolean(99)); + this.logger.debug("{}", result.getBoolean(99)); } catch (SQLException e) { Assert.assertTrue(true); this.logger.error("SQLexception" + e.toString()); @@ -352,34 +319,34 @@ public void TestResultIndexOutofBound() { @Test public void TestResultSetAbsolute() { try { - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.absolute(1)); - Assert.assertEquals("you", BQScrollableResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.absolute(2)); - Assert.assertEquals("yet", BQScrollableResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.absolute(3)); - Assert.assertEquals("would", BQScrollableResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.absolute(4)); - Assert.assertEquals("world", BQScrollableResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.absolute(5)); - Assert.assertEquals("without", BQScrollableResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.absolute(6)); - Assert.assertEquals("with", BQScrollableResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.absolute(7)); - Assert.assertEquals("will", BQScrollableResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.absolute(8)); - Assert.assertEquals("why", BQScrollableResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.absolute(9)); - Assert.assertEquals("whose", BQScrollableResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.absolute(10)); - Assert.assertEquals("whom", BQScrollableResultSetFunctionTest.Result.getString(1)); + Assert.assertTrue(result.absolute(1)); + Assert.assertEquals("you", result.getString(1)); + Assert.assertTrue(result.absolute(2)); + Assert.assertEquals("yet", result.getString(1)); + Assert.assertTrue(result.absolute(3)); + Assert.assertEquals("would", result.getString(1)); + Assert.assertTrue(result.absolute(4)); + Assert.assertEquals("world", result.getString(1)); + Assert.assertTrue(result.absolute(5)); + Assert.assertEquals("without", result.getString(1)); + Assert.assertTrue(result.absolute(6)); + Assert.assertEquals("with", result.getString(1)); + Assert.assertTrue(result.absolute(7)); + Assert.assertEquals("will", result.getString(1)); + Assert.assertTrue(result.absolute(8)); + Assert.assertEquals("why", result.getString(1)); + Assert.assertTrue(result.absolute(9)); + Assert.assertEquals("whose", result.getString(1)); + Assert.assertTrue(result.absolute(10)); + Assert.assertEquals("whom", result.getString(1)); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); } try { - Assert.assertFalse(BQScrollableResultSetFunctionTest.Result.absolute(0)); - Assert.assertEquals("", BQScrollableResultSetFunctionTest.Result.getString(1)); + Assert.assertFalse(result.absolute(0)); + Assert.assertEquals("", result.getString(1)); } catch (SQLException e) { boolean ct = e.toString().contains("Cursor is not in a valid Position"); if (ct == true) { @@ -391,8 +358,8 @@ public void TestResultSetAbsolute() { } try { - Assert.assertFalse(BQScrollableResultSetFunctionTest.Result.absolute(11)); - Assert.assertEquals("", BQScrollableResultSetFunctionTest.Result.getString(1)); + Assert.assertFalse(result.absolute(11)); + Assert.assertEquals("", result.getString(1)); } catch (SQLException e) { boolean ct = e.toString().contains("Cursor is not in a valid Position"); if (ct == true) { @@ -407,17 +374,17 @@ public void TestResultSetAbsolute() { @Test public void TestResultSetAfterlast() { try { - BQScrollableResultSetFunctionTest.Result.afterLast(); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.previous()); - Assert.assertEquals("whom", BQScrollableResultSetFunctionTest.Result.getString(1)); + result.afterLast(); + Assert.assertTrue(result.previous()); + Assert.assertEquals("whom", result.getString(1)); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); } try { - BQScrollableResultSetFunctionTest.Result.afterLast(); - Assert.assertEquals("", BQScrollableResultSetFunctionTest.Result.getString(1)); + result.afterLast(); + Assert.assertEquals("", result.getString(1)); } catch (SQLException e) { boolean ct = e.toString().contains("Cursor is not in a valid Position"); if (ct == true) { @@ -432,17 +399,17 @@ public void TestResultSetAfterlast() { @Test public void TestResultSetBeforeFirst() { try { - BQScrollableResultSetFunctionTest.Result.beforeFirst(); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.next()); - Assert.assertEquals("you", BQScrollableResultSetFunctionTest.Result.getString(1)); + result.beforeFirst(); + Assert.assertTrue(result.next()); + Assert.assertEquals("you", result.getString(1)); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); } try { - BQScrollableResultSetFunctionTest.Result.beforeFirst(); - Assert.assertEquals("", BQScrollableResultSetFunctionTest.Result.getString(1)); + result.beforeFirst(); + Assert.assertEquals("", result.getString(1)); } catch (SQLException e) { boolean ct = e.toString().contains("Cursor is not in a valid Position"); if (ct == true) { @@ -457,8 +424,8 @@ public void TestResultSetBeforeFirst() { @Test public void TestResultSetFirst() { try { - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.first()); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.isFirst()); + Assert.assertTrue(result.first()); + Assert.assertTrue(result.isFirst()); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); @@ -468,9 +435,8 @@ public void TestResultSetFirst() { @Test public void TestResultSetgetBoolean() { try { - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.absolute(1)); - Assert.assertEquals( - Boolean.parseBoolean("42"), BQScrollableResultSetFunctionTest.Result.getBoolean(2)); + Assert.assertTrue(result.absolute(1)); + Assert.assertEquals(Boolean.parseBoolean("42"), result.getBoolean(2)); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); @@ -480,8 +446,8 @@ public void TestResultSetgetBoolean() { @Test public void TestResultSetgetFloat() { try { - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.absolute(1)); - Assert.assertEquals(new Float(42), BQScrollableResultSetFunctionTest.Result.getFloat(2)); + Assert.assertTrue(result.absolute(1)); + Assert.assertEquals(new Float(42), result.getFloat(2)); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); @@ -491,8 +457,8 @@ public void TestResultSetgetFloat() { @Test public void TestResultSetgetInteger() { try { - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.absolute(1)); - Assert.assertEquals(42, BQScrollableResultSetFunctionTest.Result.getInt(2)); + Assert.assertTrue(result.absolute(1)); + Assert.assertEquals(42, result.getInt(2)); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); @@ -503,19 +469,19 @@ public void TestResultSetgetInteger() { public void TestResultSetgetRow() { try { - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.absolute(1)); - Assert.assertEquals(1, BQScrollableResultSetFunctionTest.Result.getRow()); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.absolute(10)); - Assert.assertEquals(10, BQScrollableResultSetFunctionTest.Result.getRow()); + Assert.assertTrue(result.absolute(1)); + Assert.assertEquals(1, result.getRow()); + Assert.assertTrue(result.absolute(10)); + Assert.assertEquals(10, result.getRow()); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); } try { - BQScrollableResultSetFunctionTest.Result.beforeFirst(); - Assert.assertEquals(0, BQScrollableResultSetFunctionTest.Result.getRow()); - BQScrollableResultSetFunctionTest.Result.afterLast(); - Assert.assertEquals(0, BQScrollableResultSetFunctionTest.Result.getRow()); + result.beforeFirst(); + Assert.assertEquals(0, result.getRow()); + result.afterLast(); + Assert.assertEquals(0, result.getRow()); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); @@ -525,10 +491,10 @@ public void TestResultSetgetRow() { @Test public void TestResultSetgetString() { try { - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.first()); - Assert.assertEquals("you", BQScrollableResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.last()); - Assert.assertEquals("whom", BQScrollableResultSetFunctionTest.Result.getString(1)); + Assert.assertTrue(result.first()); + Assert.assertEquals("you", result.getString(1)); + Assert.assertTrue(result.last()); + Assert.assertEquals("whom", result.getString(1)); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); @@ -538,8 +504,8 @@ public void TestResultSetgetString() { @Test public void TestResultSetLast() { try { - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.last()); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.isLast()); + Assert.assertTrue(result.last()); + Assert.assertTrue(result.isLast()); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); @@ -549,33 +515,33 @@ public void TestResultSetLast() { @Test public void TestResultSetNext() { try { - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.first()); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.next()); - Assert.assertEquals("yet", BQScrollableResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.next()); - Assert.assertEquals("would", BQScrollableResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.next()); - Assert.assertEquals("world", BQScrollableResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.next()); - Assert.assertEquals("without", BQScrollableResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.next()); - Assert.assertEquals("with", BQScrollableResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.next()); - Assert.assertEquals("will", BQScrollableResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.next()); - Assert.assertEquals("why", BQScrollableResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.next()); - Assert.assertEquals("whose", BQScrollableResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.next()); - Assert.assertEquals("whom", BQScrollableResultSetFunctionTest.Result.getString(1)); - Assert.assertFalse(BQScrollableResultSetFunctionTest.Result.next()); + Assert.assertTrue(result.first()); + Assert.assertTrue(result.next()); + Assert.assertEquals("yet", result.getString(1)); + Assert.assertTrue(result.next()); + Assert.assertEquals("would", result.getString(1)); + Assert.assertTrue(result.next()); + Assert.assertEquals("world", result.getString(1)); + Assert.assertTrue(result.next()); + Assert.assertEquals("without", result.getString(1)); + Assert.assertTrue(result.next()); + Assert.assertEquals("with", result.getString(1)); + Assert.assertTrue(result.next()); + Assert.assertEquals("will", result.getString(1)); + Assert.assertTrue(result.next()); + Assert.assertEquals("why", result.getString(1)); + Assert.assertTrue(result.next()); + Assert.assertEquals("whose", result.getString(1)); + Assert.assertTrue(result.next()); + Assert.assertEquals("whom", result.getString(1)); + Assert.assertFalse(result.next()); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); } try { - Assert.assertEquals("", BQScrollableResultSetFunctionTest.Result.getString(1)); + Assert.assertEquals("", result.getString(1)); } catch (SQLException e) { boolean ct = e.toString().contains("Cursor is not in a valid Position"); if (ct == true) { @@ -590,32 +556,32 @@ public void TestResultSetNext() { @Test public void TestResultSetPrevious() { try { - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.last()); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.previous()); - Assert.assertEquals("whose", BQScrollableResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.previous()); - Assert.assertEquals("why", BQScrollableResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.previous()); - Assert.assertEquals("will", BQScrollableResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.previous()); - Assert.assertEquals("with", BQScrollableResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.previous()); - Assert.assertEquals("without", BQScrollableResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.previous()); - Assert.assertEquals("world", BQScrollableResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.previous()); - Assert.assertEquals("would", BQScrollableResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.previous()); - Assert.assertEquals("yet", BQScrollableResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.previous()); - Assert.assertEquals("you", BQScrollableResultSetFunctionTest.Result.getString(1)); - Assert.assertFalse(BQScrollableResultSetFunctionTest.Result.previous()); + Assert.assertTrue(result.last()); + Assert.assertTrue(result.previous()); + Assert.assertEquals("whose", result.getString(1)); + Assert.assertTrue(result.previous()); + Assert.assertEquals("why", result.getString(1)); + Assert.assertTrue(result.previous()); + Assert.assertEquals("will", result.getString(1)); + Assert.assertTrue(result.previous()); + Assert.assertEquals("with", result.getString(1)); + Assert.assertTrue(result.previous()); + Assert.assertEquals("without", result.getString(1)); + Assert.assertTrue(result.previous()); + Assert.assertEquals("world", result.getString(1)); + Assert.assertTrue(result.previous()); + Assert.assertEquals("would", result.getString(1)); + Assert.assertTrue(result.previous()); + Assert.assertEquals("yet", result.getString(1)); + Assert.assertTrue(result.previous()); + Assert.assertEquals("you", result.getString(1)); + Assert.assertFalse(result.previous()); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); } try { - Assert.assertEquals("", BQScrollableResultSetFunctionTest.Result.getString(1)); + Assert.assertEquals("", result.getString(1)); } catch (SQLException e) { boolean ct = e.toString().contains("Cursor is not in a valid Position"); if (ct == true) { @@ -630,28 +596,28 @@ public void TestResultSetPrevious() { @Test public void TestResultSetRelative() { try { - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.absolute(1)); - Assert.assertEquals("you", BQScrollableResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.relative(1)); - Assert.assertEquals("yet", BQScrollableResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.relative(2)); - Assert.assertEquals("world", BQScrollableResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.relative(5)); - Assert.assertEquals("whose", BQScrollableResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.relative(-5)); - Assert.assertEquals("world", BQScrollableResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.relative(-2)); - Assert.assertEquals("yet", BQScrollableResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.relative(-1)); - Assert.assertEquals("you", BQScrollableResultSetFunctionTest.Result.getString(1)); + Assert.assertTrue(result.absolute(1)); + Assert.assertEquals("you", result.getString(1)); + Assert.assertTrue(result.relative(1)); + Assert.assertEquals("yet", result.getString(1)); + Assert.assertTrue(result.relative(2)); + Assert.assertEquals("world", result.getString(1)); + Assert.assertTrue(result.relative(5)); + Assert.assertEquals("whose", result.getString(1)); + Assert.assertTrue(result.relative(-5)); + Assert.assertEquals("world", result.getString(1)); + Assert.assertTrue(result.relative(-2)); + Assert.assertEquals("yet", result.getString(1)); + Assert.assertTrue(result.relative(-1)); + Assert.assertEquals("you", result.getString(1)); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); } try { - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.first()); - Assert.assertFalse(BQScrollableResultSetFunctionTest.Result.relative(-1)); - Assert.assertEquals("", BQScrollableResultSetFunctionTest.Result.getString(1)); + Assert.assertTrue(result.first()); + Assert.assertFalse(result.relative(-1)); + Assert.assertEquals("", result.getString(1)); } catch (SQLException e) { boolean ct = e.toString().contains("Cursor is not in a valid Position"); if (ct == true) { @@ -663,9 +629,9 @@ public void TestResultSetRelative() { } try { - Assert.assertTrue(BQScrollableResultSetFunctionTest.Result.last()); - Assert.assertFalse(BQScrollableResultSetFunctionTest.Result.relative(1)); - Assert.assertEquals("", BQScrollableResultSetFunctionTest.Result.getString(1)); + Assert.assertTrue(result.last()); + Assert.assertFalse(result.relative(1)); + Assert.assertEquals("", result.getString(1)); } catch (SQLException e) { boolean ct = e.toString().contains("Cursor is not in a valid Position"); if (ct == true) { @@ -679,8 +645,8 @@ public void TestResultSetRelative() { @Test public void TestResultSetTotalBytesProcessedCacheHit() { - Assert.assertTrue(Result instanceof BQScrollableResultSet); - BQScrollableResultSet results = (BQScrollableResultSet) Result; + Assert.assertTrue(result instanceof BQScrollableResultSet); + BQScrollableResultSet results = (BQScrollableResultSet) result; final Boolean processedNoBytes = new Long(0L).equals(results.getTotalBytesProcessed()); Assert.assertEquals(processedNoBytes, results.getCacheHit()); } @@ -731,21 +697,23 @@ private void mockResponse(String jsonResponse) throws Exception { } @Test - public void testStatelessQuery() throws SQLException { + public void testStatelessQuery() throws SQLException, IOException { closeConnection(); - NewConnection("&useLegacySql=true&jobcreationmode=JOB_CREATION_OPTIONAL"); - StatelessQuery.assumeStatelessQueriesEnabled( - BQScrollableResultSetFunctionTest.con.getCatalog()); - final Statement stmt = - BQScrollableResultSetFunctionTest.con.createStatement( - ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY); - final ResultSet result = stmt.executeQuery(StatelessQuery.exampleQuery()); - final String[][] rows = BQSupportMethods.GetQueryResult(result); - Assertions.assertThat(rows).isEqualTo(StatelessQuery.exampleValues()); - - final BQScrollableResultSet bqResultSet = (BQScrollableResultSet) result; - Assertions.assertThat(bqResultSet.getJobId()).isNull(); - Assertions.assertThat(bqResultSet.getQueryId()).contains("!"); + try (Connection statelessConnection = + connect("&useLegacySql=true&jobcreationmode=JOB_CREATION_OPTIONAL")) { + StatelessQuery.assumeStatelessQueriesEnabled(statelessConnection.getCatalog()); + try (Statement stmt = + statelessConnection.createStatement( + ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY)) { + final ResultSet result = stmt.executeQuery(StatelessQuery.exampleQuery()); + final String[][] rows = BQSupportMethods.GetQueryResult(result); + Assertions.assertThat(rows).isEqualTo(StatelessQuery.exampleValues()); + + final BQScrollableResultSet bqResultSet = (BQScrollableResultSet) result; + Assertions.assertThat(bqResultSet.getJobId()).isNull(); + Assertions.assertThat(bqResultSet.getQueryId()).contains("!"); + } + } } @Override From 582b761f5e621c0dc52b1ea7006bfb1ba770cf8f Mon Sep 17 00:00:00 2001 From: Mark Williams Date: Wed, 20 Dec 2023 20:27:54 +0000 Subject: [PATCH 15/19] Refactor BQForwardOnlyResultSetFunctionTest connection management. Use common `ConnectionFromResources.connect` method instead of a bespoke `NewConnection` method. Also, use instance variable/test case scoped connections and result sets instead of static/suite scoped ones. This should help enable test parallelization later. Local testing shows no reliable increase in runtime (~30 seconds). This was eased by the introduction of a second BQ connection with legacy SQL disabled (`standardSqlConnection`). Not every test uses both connections, so this is a little wasteful and risks introducing some unreliability, but it isolates test better than continually closing and reopening a static connection. --- .../BQForwardOnlyResultSetFunctionTest.java | 167 +++++++----------- 1 file changed, 68 insertions(+), 99 deletions(-) diff --git a/src/test/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSetFunctionTest.java b/src/test/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSetFunctionTest.java index a1e5a42..0f6835d 100644 --- a/src/test/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSetFunctionTest.java +++ b/src/test/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSetFunctionTest.java @@ -29,7 +29,6 @@ import java.io.IOException; import java.math.BigDecimal; import java.sql.Connection; -import java.sql.DriverManager; import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; @@ -62,35 +61,46 @@ */ public class BQForwardOnlyResultSetFunctionTest extends CommonTestsForResultSets { - private static java.sql.Connection con = null; + private java.sql.Connection connection; + private java.sql.Connection standardSqlConnection; private java.sql.ResultSet resultForTest = null; Logger logger = LoggerFactory.getLogger(BQForwardOnlyResultSetFunctionTest.class); private Integer maxRows = null; private String defaultProjectId = null; - private BQConnection defaultConn = null; + + private Connection connect(final String extraUrl) throws SQLException, IOException { + return ConnectionFromResources.connect("installedaccount1.properties", extraUrl); + } @Before - public void setup() throws SQLException, IOException { - Properties props = - BQSupportFuncts.readFromPropFile( - getClass().getResource("/installedaccount.properties").getFile()); - String url = BQSupportFuncts.constructUrlFromPropertiesFile(props, true, null); - url += "&useLegacySql=false"; - this.defaultProjectId = props.getProperty("projectid"); - this.defaultConn = new BQConnection(url, new Properties()); + public void setConnection() throws SQLException, IOException { + connection = connect("&useLegacySql=true"); + final BQConnection bqConnection = (BQConnection) connection; + this.defaultProjectId = bqConnection.getProjectId(); + } + + @Before + public void setStandardSqlConnection() throws SQLException, IOException { + standardSqlConnection = connect("&useLegacySql=false"); } @After - public void teardown() throws SQLException { - if (defaultConn != null) { - defaultConn.close(); - defaultConn = null; - } + public void closeConnection() throws SQLException { + connection.close(); + } + + @After + public void closeStandardSqlConnection() throws SQLException { + standardSqlConnection.close(); + } + + private BQConnection bqConnection() { + return (BQConnection) connection; } - private BQConnection conn() throws SQLException, IOException { - return this.defaultConn; + private BQConnection bqStandardSqlConnection() { + return (BQConnection) standardSqlConnection; } @Test @@ -143,7 +153,10 @@ public void databaseMetaDataGetTables() { this.QueryLoad(); ResultSet result = null; try { - result = con.getMetaData().getColumns(null, "starschema_net__clouddb", "OUTLET_LOOKUP", null); + result = + connection + .getMetaData() + .getColumns(null, "starschema_net__clouddb", "OUTLET_LOOKUP", null); } catch (SQLException e) { e.printStackTrace(); Assert.fail(); @@ -176,79 +189,43 @@ public void databaseMetaDataGetTables() { public void isClosedValidtest() { this.QueryLoad(); try { - Assert.assertEquals(true, BQForwardOnlyResultSetFunctionTest.con.isValid(0)); + Assert.assertEquals(true, connection.isValid(0)); } catch (SQLException e) { Assert.fail("Got an exception" + e.toString()); e.printStackTrace(); } try { - Assert.assertEquals(true, BQForwardOnlyResultSetFunctionTest.con.isValid(10)); + Assert.assertEquals(true, connection.isValid(10)); } catch (SQLException e) { Assert.fail("Got an exception" + e.toString()); e.printStackTrace(); } try { - BQForwardOnlyResultSetFunctionTest.con.isValid(-10); + connection.isValid(-10); } catch (SQLException e) { Assert.assertTrue(true); // e.printStackTrace(); } try { - BQForwardOnlyResultSetFunctionTest.con.close(); + connection.close(); } catch (SQLException e) { e.printStackTrace(); } try { - Assert.assertTrue(BQForwardOnlyResultSetFunctionTest.con.isClosed()); + Assert.assertTrue(connection.isClosed()); } catch (SQLException e1) { e1.printStackTrace(); } try { - BQForwardOnlyResultSetFunctionTest.con.isValid(0); + connection.isValid(0); } catch (SQLException e) { Assert.assertTrue(true); e.printStackTrace(); } } - /** - * Makes a new Bigquery Connection to URL in file and gives back the Connection to static con - * member. - */ - @Before - public void NewConnection() { - NewConnection("&useLegacySql=true"); - } - - void NewConnection(String extraUrl) { - this.logger.info("Testing the JDBC driver"); - try { - Class.forName("net.starschema.clouddb.jdbc.BQDriver"); - Properties props = - BQSupportFuncts.readFromPropFile( - getClass().getResource("/installedaccount1.properties").getFile()); - String jdcbUrl = BQSupportFuncts.constructUrlFromPropertiesFile(props); - if (extraUrl != null) { - jdcbUrl += extraUrl; - } - if (BQForwardOnlyResultSetFunctionTest.con != null) { - BQForwardOnlyResultSetFunctionTest.con.close(); - } - BQForwardOnlyResultSetFunctionTest.con = - DriverManager.getConnection( - jdcbUrl, - BQSupportFuncts.readFromPropFile( - getClass().getResource("/installedaccount1.properties").getFile())); - } catch (Exception e) { - e.printStackTrace(); - this.logger.error("Error in connection" + e.toString()); - Assert.fail("General Exception:" + e.toString()); - } - this.logger.info(((BQConnection) BQForwardOnlyResultSetFunctionTest.con).getURLPART()); - } - // Comprehensive Tests: public void QueryLoad() { @@ -260,8 +237,7 @@ public void QueryLoad() { try { // Statement stmt = BQResultSetFunctionTest.con.createStatement(); Statement stmt = - BQForwardOnlyResultSetFunctionTest.con.createStatement( - ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY); + connection.createStatement(ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY); stmt.setQueryTimeout(500); if (this.maxRows != null) { stmt.setMaxRows(this.maxRows); @@ -460,11 +436,10 @@ public void testResultSetTypesInGetString() throws SQLException { + "STRUCT(1 as a, ['an', 'array'] as b)," + "TIMESTAMP('2012-01-01 00:00:03.0000') as t"; - this.NewConnection("&useLegacySql=false"); java.sql.ResultSet result = null; try { Statement stmt = - BQForwardOnlyResultSetFunctionTest.con.createStatement( + standardSqlConnection.createStatement( ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY); stmt.setQueryTimeout(500); result = stmt.executeQuery(sql); @@ -522,9 +497,8 @@ public void testResultSetDateTimeType() throws SQLException, ParseException { final DateFormat utcDateFormatter = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSS"); utcDateFormatter.setTimeZone(TimeZone.getTimeZone("UTC")); - this.NewConnection("&useLegacySql=false"); Statement stmt = - BQForwardOnlyResultSetFunctionTest.con.createStatement( + standardSqlConnection.createStatement( ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY); stmt.setQueryTimeout(500); ResultSet result = stmt.executeQuery(sql); @@ -556,9 +530,8 @@ public void testResultSetDateTimeType() throws SQLException, ParseException { public void testResultSetTimestampType() throws SQLException, ParseException { final String sql = "SELECT TIMESTAMP('2012-01-01 01:02:03.04567')"; - this.NewConnection("&useLegacySql=false"); Statement stmt = - BQForwardOnlyResultSetFunctionTest.con.createStatement( + standardSqlConnection.createStatement( ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY); stmt.setQueryTimeout(500); ResultSet result = stmt.executeQuery(sql); @@ -589,12 +562,10 @@ public void testResultSetTypesInGetObject() throws SQLException, ParseException + "CAST('2011-04-03' AS DATE), " + "CAST('nan' AS FLOAT)"; - this.NewConnection("&useLegacySql=true"); java.sql.ResultSet result = null; try { Statement stmt = - BQForwardOnlyResultSetFunctionTest.con.createStatement( - ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY); + connection.createStatement(ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY); stmt.setQueryTimeout(500); result = stmt.executeQuery(sql); } catch (SQLException e) { @@ -617,11 +588,10 @@ public void testResultSetTypesInGetObject() throws SQLException, ParseException public void testResultSetArraysInGetObject() throws SQLException, ParseException { final String sql = "SELECT [1, 2, 3], [TIMESTAMP(\"2010-09-07 15:30:00 America/Los_Angeles\")]"; - this.NewConnection("&useLegacySql=false"); java.sql.ResultSet result = null; try { Statement stmt = - BQForwardOnlyResultSetFunctionTest.con.createStatement( + standardSqlConnection.createStatement( ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY); stmt.setQueryTimeout(500); result = stmt.executeQuery(sql); @@ -651,12 +621,11 @@ public void testResultSetArraysInGetObject() throws SQLException, ParseException @Test public void testResultSetTimeType() throws SQLException, ParseException { final String sql = "select current_time(), CAST('00:00:02.12345' AS TIME)"; - this.NewConnection("&useLegacySql=false"); java.sql.ResultSet result = null; try { Statement stmt = - BQForwardOnlyResultSetFunctionTest.con.createStatement( + standardSqlConnection.createStatement( ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY); stmt.setQueryTimeout(500); result = stmt.executeQuery(sql); @@ -709,11 +678,10 @@ public void testResultSetProcedures() throws SQLException, ParseException { final String sql = "CREATE PROCEDURE looker_test.procedure_test(target_id INT64)\n" + "BEGIN\n" + "END;"; - this.NewConnection("&useLegacySql=false"); java.sql.ResultSet result = null; try { Statement stmt = - BQForwardOnlyResultSetFunctionTest.con.createStatement( + standardSqlConnection.createStatement( ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY); stmt.setQueryTimeout(500); result = stmt.executeQuery(sql); @@ -723,23 +691,22 @@ public void testResultSetProcedures() throws SQLException, ParseException { } finally { String cleanupSql = "DROP PROCEDURE looker_test.procedure_test;\n"; Statement stmt = - BQForwardOnlyResultSetFunctionTest.con.createStatement( + standardSqlConnection.createStatement( ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY); stmt.setQueryTimeout(500); stmt.executeQuery(cleanupSql); } - System.out.println(result.toString()); + // System.out.println(result.toString()); } @Test public void testResultSetProceduresAsync() throws SQLException { final String sql = "CREATE PROCEDURE looker_test.long_procedure(target_id INT64)\n" + "BEGIN\n" + "END;"; - this.NewConnection("&useLegacySql=false"); try { - BQConnection bq = conn(); + BQConnection bq = bqStandardSqlConnection(); BQStatement stmt = new BQStatement( bq.getProjectId(), bq, ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY) { @@ -751,13 +718,13 @@ protected long getSyncTimeoutMillis() { stmt.setQueryTimeout(500); stmt.executeQuery(sql); - } catch (SQLException | IOException e) { + } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); } finally { String cleanupSql = "DROP PROCEDURE looker_test.long_procedure;\n"; Statement stmt = - BQForwardOnlyResultSetFunctionTest.con.createStatement( + standardSqlConnection.createStatement( ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY); stmt.setQueryTimeout(500); stmt.executeQuery(cleanupSql); @@ -766,7 +733,7 @@ protected long getSyncTimeoutMillis() { @Test public void testBQForwardOnlyResultSetDoesntThrowNPE() throws Exception { - BQConnection bq = conn(); + BQConnection bq = bqConnection(); BQStatement stmt = (BQStatement) bq.createStatement(); QueryResponse qr = stmt.runSyncQuery("SELECT 1", false); Job ref = @@ -888,20 +855,22 @@ private void mockResponse(String jsonResponse) throws Exception { } @Test - public void testStatelessQuery() throws SQLException { - NewConnection("&useLegacySql=false&jobcreationmode=JOB_CREATION_OPTIONAL"); - StatelessQuery.assumeStatelessQueriesEnabled( - BQForwardOnlyResultSetFunctionTest.con.getCatalog()); - final Statement stmt = - BQForwardOnlyResultSetFunctionTest.con.createStatement( - ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY); - final ResultSet result = stmt.executeQuery(StatelessQuery.exampleQuery()); - final String[][] rows = BQSupportMethods.GetQueryResult(result); - Assertions.assertThat(rows).isEqualTo(StatelessQuery.exampleValues()); - - final BQForwardOnlyResultSet bqResultSet = (BQForwardOnlyResultSet) result; - Assertions.assertThat(bqResultSet.getJobId()).isNull(); - Assertions.assertThat(bqResultSet.getQueryId()).contains("!"); + public void testStatelessQuery() throws SQLException, IOException { + try (Connection statelessConnection = + connect("&useLegacySql=false&jobcreationmode=JOB_CREATION_OPTIONAL")) { + StatelessQuery.assumeStatelessQueriesEnabled(statelessConnection.getCatalog()); + try (Statement stmt = + statelessConnection.createStatement( + ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY)) { + final ResultSet result = stmt.executeQuery(StatelessQuery.exampleQuery()); + final String[][] rows = BQSupportMethods.GetQueryResult(result); + Assertions.assertThat(rows).isEqualTo(StatelessQuery.exampleValues()); + + final BQForwardOnlyResultSet bqResultSet = (BQForwardOnlyResultSet) result; + Assertions.assertThat(bqResultSet.getJobId()).isNull(); + Assertions.assertThat(bqResultSet.getQueryId()).contains("!"); + } + } } @Override From 3479fdceb72273c6f00dbac62de1a2c098e88283 Mon Sep 17 00:00:00 2001 From: Mark Williams Date: Wed, 20 Dec 2023 22:10:09 +0000 Subject: [PATCH 16/19] Refactor BQResultSetFunctionTest connection management. Use common `ConnectionFromResources.connect` method instead of a bespoke `NewConnection` method. Also, use instance variable/test case scoped connections and result sets instead of static/suite scoped ones. This should help enable test parallelization later. Local testing shows no reliable increase in runtime (~30 seconds). --- .../clouddb/jdbc/BQResultSetFunctionTest.java | 470 +++++++++--------- 1 file changed, 225 insertions(+), 245 deletions(-) diff --git a/src/test/java/net/starschema/clouddb/jdbc/BQResultSetFunctionTest.java b/src/test/java/net/starschema/clouddb/jdbc/BQResultSetFunctionTest.java index ece2c36..4af5204 100644 --- a/src/test/java/net/starschema/clouddb/jdbc/BQResultSetFunctionTest.java +++ b/src/test/java/net/starschema/clouddb/jdbc/BQResultSetFunctionTest.java @@ -20,11 +20,12 @@ */ package net.starschema.clouddb.jdbc; -import java.sql.DriverManager; +import java.io.IOException; import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; import junit.framework.Assert; +import org.junit.After; import org.junit.Before; import org.junit.Test; import org.slf4j.Logger; @@ -38,40 +39,111 @@ */ public class BQResultSetFunctionTest { - private static java.sql.Connection con = null; - private static java.sql.ResultSet Result = null; + private java.sql.Connection connection; + + private java.sql.ResultSet result; Logger logger = LoggerFactory.getLogger(BQResultSetFunctionTest.class); + @Before + public void setConnection() throws SQLException, IOException { + connection = + ConnectionFromResources.connect("installedaccount1.properties", "&useLegacySql=true"); + QueryLoad(); + } + + @After + public void closeConnection() throws SQLException { + connection.close(); + } + + /** + * Prints a String[][] QueryResult to Log + * + * @param input + */ + private void printer(String[][] input) { + for (int s = 0; s < input[0].length; s++) { + String Output = ""; + for (int i = 0; i < input.length; i++) { + if (i == input.length - 1) { + Output += input[i][s]; + } else { + Output += input[i][s] + "\t"; + } + } + this.logger.debug(Output); + } + } + + public void QueryLoad() { + final String sql = + "SELECT TOP(word,10) AS word, COUNT(*) as count FROM publicdata:samples.shakespeare"; + final String description = "The top 10 word from shakespeare #TOP #COUNT"; + String[][] expectation = + new String[][] { + {"you", "yet", "would", "world", "without", "with", "will", "why", "whose", "whom"}, + {"42", "42", "42", "42", "42", "42", "42", "42", "42", "42"} + }; + + this.logger.info("Test number: 01"); + this.logger.info("Running query:" + sql); + + try { + Statement stmt = + connection.createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY); + stmt.setQueryTimeout(500); + result = stmt.executeQuery(sql); + } catch (SQLException e) { + this.logger.error("SQLexception" + e.toString()); + Assert.fail("SQLException" + e.toString()); + } + Assert.assertNotNull(result); + + this.logger.debug(description); + this.printer(expectation); + + try { + Assert.assertTrue( + "Comparing failed in the String[][] array", + this.comparer(expectation, BQSupportMethods.GetQueryResult(result))); + } catch (SQLException e) { + this.logger.error("SQLexception" + e.toString()); + Assert.fail(e.toString()); + } + } + + // Comprehensive Tests: + @Test public void ChainedCursorFunctionTest() { this.logger.info("ChainedFunctionTest"); try { - BQResultSetFunctionTest.Result.beforeFirst(); - Assert.assertTrue(BQResultSetFunctionTest.Result.next()); - Assert.assertEquals("you", BQResultSetFunctionTest.Result.getString(1)); + result.beforeFirst(); + Assert.assertTrue(result.next()); + Assert.assertEquals("you", result.getString(1)); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); } try { - Assert.assertTrue(BQResultSetFunctionTest.Result.absolute(10)); - Assert.assertEquals("whom", BQResultSetFunctionTest.Result.getString(1)); + Assert.assertTrue(result.absolute(10)); + Assert.assertEquals("whom", result.getString(1)); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); } try { - Assert.assertFalse(BQResultSetFunctionTest.Result.next()); + Assert.assertFalse(result.next()); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); } try { - Assert.assertEquals("", BQResultSetFunctionTest.Result.getString(1)); + Assert.assertEquals("", result.getString(1)); } catch (SQLException e) { boolean ct = e.toString().contains("Cursor is not in a valid Position"); if (ct == true) { @@ -83,42 +155,42 @@ public void ChainedCursorFunctionTest() { } try { - Assert.assertTrue(BQResultSetFunctionTest.Result.first()); - Assert.assertEquals("you", BQResultSetFunctionTest.Result.getString(1)); + Assert.assertTrue(result.first()); + Assert.assertEquals("you", result.getString(1)); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); } try { - Assert.assertTrue(BQResultSetFunctionTest.Result.isFirst()); - Assert.assertFalse(BQResultSetFunctionTest.Result.previous()); - BQResultSetFunctionTest.Result.afterLast(); - Assert.assertTrue(BQResultSetFunctionTest.Result.isAfterLast()); - Assert.assertTrue(BQResultSetFunctionTest.Result.absolute(-1)); - Assert.assertEquals("whom", BQResultSetFunctionTest.Result.getString(1)); + Assert.assertTrue(result.isFirst()); + Assert.assertFalse(result.previous()); + result.afterLast(); + Assert.assertTrue(result.isAfterLast()); + Assert.assertTrue(result.absolute(-1)); + Assert.assertEquals("whom", result.getString(1)); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); } try { - Assert.assertTrue(BQResultSetFunctionTest.Result.relative(-5)); - Assert.assertEquals("without", BQResultSetFunctionTest.Result.getString(1)); + Assert.assertTrue(result.relative(-5)); + Assert.assertEquals("without", result.getString(1)); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); } try { - Assert.assertFalse(BQResultSetFunctionTest.Result.relative(6)); + Assert.assertFalse(result.relative(6)); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); } try { - Assert.assertEquals("without", BQResultSetFunctionTest.Result.getString(1)); + Assert.assertEquals("without", result.getString(1)); } catch (SQLException e) { boolean ct = e.toString().contains("Cursor is not in a valid Position"); if (ct == true) { @@ -142,7 +214,10 @@ public void databaseMetaDataGetTables() { // tableNamePattern:OUTLET_LOOKUP, columnNamePattern: null // result = con.getMetaData().getTables("OUTLET_LOOKUP", null, "starschema_net__clouddb", null // ); - result = con.getMetaData().getColumns(null, "starschema_net__clouddb", "OUTLET_LOOKUP", null); + result = + connection + .getMetaData() + .getColumns(null, "starschema_net__clouddb", "OUTLET_LOOKUP", null); // Function call getTables(catalog: ARTICLE_COLOR_LOOKUP, schemaPattern: null, // tableNamePattern: starschema_net__clouddb, types: TABLE , VIEW , SYSTEM TABLE , SYNONYM , // ALIAS , ) @@ -196,143 +271,48 @@ private boolean comparer(String[][] expected, String[][] reality) { @Test public void isClosedValidtest() { try { - Assert.assertEquals(true, BQResultSetFunctionTest.con.isValid(0)); + Assert.assertEquals(true, connection.isValid(0)); } catch (SQLException e) { Assert.fail("Got an exception" + e.toString()); e.printStackTrace(); } try { - Assert.assertEquals(true, BQResultSetFunctionTest.con.isValid(10)); + Assert.assertEquals(true, connection.isValid(10)); } catch (SQLException e) { Assert.fail("Got an exception" + e.toString()); e.printStackTrace(); } try { - BQResultSetFunctionTest.con.isValid(-10); + connection.isValid(-10); } catch (SQLException e) { Assert.assertTrue(true); // e.printStackTrace(); } try { - BQResultSetFunctionTest.con.close(); + connection.close(); } catch (SQLException e) { e.printStackTrace(); } try { - Assert.assertTrue(BQResultSetFunctionTest.con.isClosed()); + Assert.assertTrue(connection.isClosed()); } catch (SQLException e1) { e1.printStackTrace(); } try { - BQResultSetFunctionTest.con.isValid(0); + connection.isValid(0); } catch (SQLException e) { Assert.assertTrue(true); e.printStackTrace(); } } - /** - * Makes a new Bigquery Connection to URL in file and gives back the Connection to static con - * member. - */ - @Before - public void NewConnection() { - - try { - if (BQResultSetFunctionTest.con == null || !BQResultSetFunctionTest.con.isValid(0)) { - this.logger.info("Testing the JDBC driver"); - try { - Class.forName("net.starschema.clouddb.jdbc.BQDriver"); - String jdbcUrl = - BQSupportFuncts.constructUrlFromPropertiesFile( - BQSupportFuncts.readFromPropFile( - getClass().getResource("/installedaccount1.properties").getFile())); - jdbcUrl += "&useLegacySql=true"; - BQResultSetFunctionTest.con = - DriverManager.getConnection( - jdbcUrl, - BQSupportFuncts.readFromPropFile( - getClass().getResource("/installedaccount1.properties").getFile())); - } catch (Exception e) { - e.printStackTrace(); - this.logger.error("Error in connection" + e.toString()); - Assert.fail("General Exception:" + e.toString()); - } - this.logger.info(((BQConnection) BQResultSetFunctionTest.con).getURLPART()); - } - } catch (SQLException e) { - logger.debug("Oops something went wrong", e); - } - this.QueryLoad(); - } - - // Comprehensive Tests: - - /** - * Prints a String[][] QueryResult to Log - * - * @param input - */ - private void printer(String[][] input) { - for (int s = 0; s < input[0].length; s++) { - String Output = ""; - for (int i = 0; i < input.length; i++) { - if (i == input.length - 1) { - Output += input[i][s]; - } else { - Output += input[i][s] + "\t"; - } - } - this.logger.debug(Output); - } - } - - public void QueryLoad() { - final String sql = - "SELECT TOP(word,10) AS word, COUNT(*) as count FROM publicdata:samples.shakespeare"; - final String description = "The top 10 word from shakespeare #TOP #COUNT"; - String[][] expectation = - new String[][] { - {"you", "yet", "would", "world", "without", "with", "will", "why", "whose", "whom"}, - {"42", "42", "42", "42", "42", "42", "42", "42", "42", "42"} - }; - - this.logger.info("Test number: 01"); - this.logger.info("Running query:" + sql); - - try { - Statement stmt = - BQResultSetFunctionTest.con.createStatement( - ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY); - stmt.setQueryTimeout(500); - BQResultSetFunctionTest.Result = stmt.executeQuery(sql); - } catch (SQLException e) { - this.logger.error("SQLexception" + e.toString()); - Assert.fail("SQLException" + e.toString()); - } - Assert.assertNotNull(BQResultSetFunctionTest.Result); - - this.logger.debug(description); - this.printer(expectation); - - try { - Assert.assertTrue( - "Comparing failed in the String[][] array", - this.comparer( - expectation, BQSupportMethods.GetQueryResult(BQResultSetFunctionTest.Result))); - } catch (SQLException e) { - this.logger.error("SQLexception" + e.toString()); - Assert.fail(e.toString()); - } - } - @Test public void ResultSetMetadata() { try { - this.logger.debug(BQResultSetFunctionTest.Result.getMetaData().getSchemaName(1)); - this.logger.debug("{}", BQResultSetFunctionTest.Result.getMetaData().getScale(1)); + this.logger.debug(result.getMetaData().getSchemaName(1)); + this.logger.debug("{}", result.getMetaData().getScale(1)); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); } @@ -342,7 +322,7 @@ public void ResultSetMetadata() { @Test public void TestResultIndexOutofBound() { try { - this.logger.debug("{}", BQResultSetFunctionTest.Result.getBoolean(99)); + this.logger.debug("{}", result.getBoolean(99)); } catch (SQLException e) { Assert.assertTrue(true); this.logger.error("SQLexception" + e.toString()); @@ -352,34 +332,34 @@ public void TestResultIndexOutofBound() { @Test public void TestResultSetAbsolute() { try { - Assert.assertTrue(BQResultSetFunctionTest.Result.absolute(1)); - Assert.assertEquals("you", BQResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQResultSetFunctionTest.Result.absolute(2)); - Assert.assertEquals("yet", BQResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQResultSetFunctionTest.Result.absolute(3)); - Assert.assertEquals("would", BQResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQResultSetFunctionTest.Result.absolute(4)); - Assert.assertEquals("world", BQResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQResultSetFunctionTest.Result.absolute(5)); - Assert.assertEquals("without", BQResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQResultSetFunctionTest.Result.absolute(6)); - Assert.assertEquals("with", BQResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQResultSetFunctionTest.Result.absolute(7)); - Assert.assertEquals("will", BQResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQResultSetFunctionTest.Result.absolute(8)); - Assert.assertEquals("why", BQResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQResultSetFunctionTest.Result.absolute(9)); - Assert.assertEquals("whose", BQResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQResultSetFunctionTest.Result.absolute(10)); - Assert.assertEquals("whom", BQResultSetFunctionTest.Result.getString(1)); + Assert.assertTrue(result.absolute(1)); + Assert.assertEquals("you", result.getString(1)); + Assert.assertTrue(result.absolute(2)); + Assert.assertEquals("yet", result.getString(1)); + Assert.assertTrue(result.absolute(3)); + Assert.assertEquals("would", result.getString(1)); + Assert.assertTrue(result.absolute(4)); + Assert.assertEquals("world", result.getString(1)); + Assert.assertTrue(result.absolute(5)); + Assert.assertEquals("without", result.getString(1)); + Assert.assertTrue(result.absolute(6)); + Assert.assertEquals("with", result.getString(1)); + Assert.assertTrue(result.absolute(7)); + Assert.assertEquals("will", result.getString(1)); + Assert.assertTrue(result.absolute(8)); + Assert.assertEquals("why", result.getString(1)); + Assert.assertTrue(result.absolute(9)); + Assert.assertEquals("whose", result.getString(1)); + Assert.assertTrue(result.absolute(10)); + Assert.assertEquals("whom", result.getString(1)); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); } try { - Assert.assertFalse(BQResultSetFunctionTest.Result.absolute(0)); - Assert.assertEquals("", BQResultSetFunctionTest.Result.getString(1)); + Assert.assertFalse(result.absolute(0)); + Assert.assertEquals("", result.getString(1)); } catch (SQLException e) { boolean ct = e.toString().contains("Cursor is not in a valid Position"); if (ct == true) { @@ -391,8 +371,8 @@ public void TestResultSetAbsolute() { } try { - Assert.assertFalse(BQResultSetFunctionTest.Result.absolute(11)); - Assert.assertEquals("", BQResultSetFunctionTest.Result.getString(1)); + Assert.assertFalse(result.absolute(11)); + Assert.assertEquals("", result.getString(1)); } catch (SQLException e) { boolean ct = e.toString().contains("Cursor is not in a valid Position"); if (ct == true) { @@ -407,17 +387,17 @@ public void TestResultSetAbsolute() { @Test public void TestResultSetAfterlast() { try { - BQResultSetFunctionTest.Result.afterLast(); - Assert.assertTrue(BQResultSetFunctionTest.Result.previous()); - Assert.assertEquals("whom", BQResultSetFunctionTest.Result.getString(1)); + result.afterLast(); + Assert.assertTrue(result.previous()); + Assert.assertEquals("whom", result.getString(1)); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); } try { - BQResultSetFunctionTest.Result.afterLast(); - Assert.assertEquals("", BQResultSetFunctionTest.Result.getString(1)); + result.afterLast(); + Assert.assertEquals("", result.getString(1)); } catch (SQLException e) { boolean ct = e.toString().contains("Cursor is not in a valid Position"); if (ct == true) { @@ -432,17 +412,17 @@ public void TestResultSetAfterlast() { @Test public void TestResultSetBeforeFirst() { try { - BQResultSetFunctionTest.Result.beforeFirst(); - Assert.assertTrue(BQResultSetFunctionTest.Result.next()); - Assert.assertEquals("you", BQResultSetFunctionTest.Result.getString(1)); + result.beforeFirst(); + Assert.assertTrue(result.next()); + Assert.assertEquals("you", result.getString(1)); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); } try { - BQResultSetFunctionTest.Result.beforeFirst(); - Assert.assertEquals("", BQResultSetFunctionTest.Result.getString(1)); + result.beforeFirst(); + Assert.assertEquals("", result.getString(1)); } catch (SQLException e) { boolean ct = e.toString().contains("Cursor is not in a valid Position"); if (ct == true) { @@ -457,8 +437,8 @@ public void TestResultSetBeforeFirst() { @Test public void TestResultSetFirst() { try { - Assert.assertTrue(BQResultSetFunctionTest.Result.first()); - Assert.assertTrue(BQResultSetFunctionTest.Result.isFirst()); + Assert.assertTrue(result.first()); + Assert.assertTrue(result.isFirst()); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); @@ -468,8 +448,8 @@ public void TestResultSetFirst() { @Test public void TestResultSetgetBoolean() { try { - Assert.assertTrue(BQResultSetFunctionTest.Result.absolute(1)); - Assert.assertEquals(Boolean.parseBoolean("42"), BQResultSetFunctionTest.Result.getBoolean(2)); + Assert.assertTrue(result.absolute(1)); + Assert.assertEquals(Boolean.parseBoolean("42"), result.getBoolean(2)); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); @@ -479,8 +459,8 @@ public void TestResultSetgetBoolean() { @Test public void TestResultSetgetFloat() { try { - Assert.assertTrue(BQResultSetFunctionTest.Result.absolute(1)); - Assert.assertEquals(new Float(42), BQResultSetFunctionTest.Result.getFloat(2)); + Assert.assertTrue(result.absolute(1)); + Assert.assertEquals(new Float(42), result.getFloat(2)); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); @@ -490,8 +470,8 @@ public void TestResultSetgetFloat() { @Test public void TestResultSetgetInteger() { try { - Assert.assertTrue(BQResultSetFunctionTest.Result.absolute(1)); - Assert.assertEquals(42, BQResultSetFunctionTest.Result.getInt(2)); + Assert.assertTrue(result.absolute(1)); + Assert.assertEquals(42, result.getInt(2)); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); @@ -502,19 +482,19 @@ public void TestResultSetgetInteger() { public void TestResultSetgetRow() { try { - Assert.assertTrue(BQResultSetFunctionTest.Result.absolute(1)); - Assert.assertEquals(1, BQResultSetFunctionTest.Result.getRow()); - Assert.assertTrue(BQResultSetFunctionTest.Result.absolute(10)); - Assert.assertEquals(10, BQResultSetFunctionTest.Result.getRow()); + Assert.assertTrue(result.absolute(1)); + Assert.assertEquals(1, result.getRow()); + Assert.assertTrue(result.absolute(10)); + Assert.assertEquals(10, result.getRow()); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); } try { - BQResultSetFunctionTest.Result.beforeFirst(); - Assert.assertEquals(0, BQResultSetFunctionTest.Result.getRow()); - BQResultSetFunctionTest.Result.afterLast(); - Assert.assertEquals(0, BQResultSetFunctionTest.Result.getRow()); + result.beforeFirst(); + Assert.assertEquals(0, result.getRow()); + result.afterLast(); + Assert.assertEquals(0, result.getRow()); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); @@ -524,10 +504,10 @@ public void TestResultSetgetRow() { @Test public void TestResultSetgetString() { try { - Assert.assertTrue(BQResultSetFunctionTest.Result.first()); - Assert.assertEquals("you", BQResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQResultSetFunctionTest.Result.last()); - Assert.assertEquals("whom", BQResultSetFunctionTest.Result.getString(1)); + Assert.assertTrue(result.first()); + Assert.assertEquals("you", result.getString(1)); + Assert.assertTrue(result.last()); + Assert.assertEquals("whom", result.getString(1)); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); @@ -537,8 +517,8 @@ public void TestResultSetgetString() { @Test public void TestResultSetLast() { try { - Assert.assertTrue(BQResultSetFunctionTest.Result.last()); - Assert.assertTrue(BQResultSetFunctionTest.Result.isLast()); + Assert.assertTrue(result.last()); + Assert.assertTrue(result.isLast()); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); @@ -548,33 +528,33 @@ public void TestResultSetLast() { @Test public void TestResultSetNext() { try { - Assert.assertTrue(BQResultSetFunctionTest.Result.first()); - Assert.assertTrue(BQResultSetFunctionTest.Result.next()); - Assert.assertEquals("yet", BQResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQResultSetFunctionTest.Result.next()); - Assert.assertEquals("would", BQResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQResultSetFunctionTest.Result.next()); - Assert.assertEquals("world", BQResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQResultSetFunctionTest.Result.next()); - Assert.assertEquals("without", BQResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQResultSetFunctionTest.Result.next()); - Assert.assertEquals("with", BQResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQResultSetFunctionTest.Result.next()); - Assert.assertEquals("will", BQResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQResultSetFunctionTest.Result.next()); - Assert.assertEquals("why", BQResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQResultSetFunctionTest.Result.next()); - Assert.assertEquals("whose", BQResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQResultSetFunctionTest.Result.next()); - Assert.assertEquals("whom", BQResultSetFunctionTest.Result.getString(1)); - Assert.assertFalse(BQResultSetFunctionTest.Result.next()); + Assert.assertTrue(result.first()); + Assert.assertTrue(result.next()); + Assert.assertEquals("yet", result.getString(1)); + Assert.assertTrue(result.next()); + Assert.assertEquals("would", result.getString(1)); + Assert.assertTrue(result.next()); + Assert.assertEquals("world", result.getString(1)); + Assert.assertTrue(result.next()); + Assert.assertEquals("without", result.getString(1)); + Assert.assertTrue(result.next()); + Assert.assertEquals("with", result.getString(1)); + Assert.assertTrue(result.next()); + Assert.assertEquals("will", result.getString(1)); + Assert.assertTrue(result.next()); + Assert.assertEquals("why", result.getString(1)); + Assert.assertTrue(result.next()); + Assert.assertEquals("whose", result.getString(1)); + Assert.assertTrue(result.next()); + Assert.assertEquals("whom", result.getString(1)); + Assert.assertFalse(result.next()); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); } try { - Assert.assertEquals("", BQResultSetFunctionTest.Result.getString(1)); + Assert.assertEquals("", result.getString(1)); } catch (SQLException e) { boolean ct = e.toString().contains("Cursor is not in a valid Position"); if (ct == true) { @@ -589,32 +569,32 @@ public void TestResultSetNext() { @Test public void TestResultSetPrevious() { try { - Assert.assertTrue(BQResultSetFunctionTest.Result.last()); - Assert.assertTrue(BQResultSetFunctionTest.Result.previous()); - Assert.assertEquals("whose", BQResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQResultSetFunctionTest.Result.previous()); - Assert.assertEquals("why", BQResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQResultSetFunctionTest.Result.previous()); - Assert.assertEquals("will", BQResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQResultSetFunctionTest.Result.previous()); - Assert.assertEquals("with", BQResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQResultSetFunctionTest.Result.previous()); - Assert.assertEquals("without", BQResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQResultSetFunctionTest.Result.previous()); - Assert.assertEquals("world", BQResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQResultSetFunctionTest.Result.previous()); - Assert.assertEquals("would", BQResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQResultSetFunctionTest.Result.previous()); - Assert.assertEquals("yet", BQResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQResultSetFunctionTest.Result.previous()); - Assert.assertEquals("you", BQResultSetFunctionTest.Result.getString(1)); - Assert.assertFalse(BQResultSetFunctionTest.Result.previous()); + Assert.assertTrue(result.last()); + Assert.assertTrue(result.previous()); + Assert.assertEquals("whose", result.getString(1)); + Assert.assertTrue(result.previous()); + Assert.assertEquals("why", result.getString(1)); + Assert.assertTrue(result.previous()); + Assert.assertEquals("will", result.getString(1)); + Assert.assertTrue(result.previous()); + Assert.assertEquals("with", result.getString(1)); + Assert.assertTrue(result.previous()); + Assert.assertEquals("without", result.getString(1)); + Assert.assertTrue(result.previous()); + Assert.assertEquals("world", result.getString(1)); + Assert.assertTrue(result.previous()); + Assert.assertEquals("would", result.getString(1)); + Assert.assertTrue(result.previous()); + Assert.assertEquals("yet", result.getString(1)); + Assert.assertTrue(result.previous()); + Assert.assertEquals("you", result.getString(1)); + Assert.assertFalse(result.previous()); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); } try { - Assert.assertEquals("", BQResultSetFunctionTest.Result.getString(1)); + Assert.assertEquals("", result.getString(1)); } catch (SQLException e) { boolean ct = e.toString().contains("Cursor is not in a valid Position"); if (ct == true) { @@ -629,28 +609,28 @@ public void TestResultSetPrevious() { @Test public void TestResultSetRelative() { try { - Assert.assertTrue(BQResultSetFunctionTest.Result.absolute(1)); - Assert.assertEquals("you", BQResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQResultSetFunctionTest.Result.relative(1)); - Assert.assertEquals("yet", BQResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQResultSetFunctionTest.Result.relative(2)); - Assert.assertEquals("world", BQResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQResultSetFunctionTest.Result.relative(5)); - Assert.assertEquals("whose", BQResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQResultSetFunctionTest.Result.relative(-5)); - Assert.assertEquals("world", BQResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQResultSetFunctionTest.Result.relative(-2)); - Assert.assertEquals("yet", BQResultSetFunctionTest.Result.getString(1)); - Assert.assertTrue(BQResultSetFunctionTest.Result.relative(-1)); - Assert.assertEquals("you", BQResultSetFunctionTest.Result.getString(1)); + Assert.assertTrue(result.absolute(1)); + Assert.assertEquals("you", result.getString(1)); + Assert.assertTrue(result.relative(1)); + Assert.assertEquals("yet", result.getString(1)); + Assert.assertTrue(result.relative(2)); + Assert.assertEquals("world", result.getString(1)); + Assert.assertTrue(result.relative(5)); + Assert.assertEquals("whose", result.getString(1)); + Assert.assertTrue(result.relative(-5)); + Assert.assertEquals("world", result.getString(1)); + Assert.assertTrue(result.relative(-2)); + Assert.assertEquals("yet", result.getString(1)); + Assert.assertTrue(result.relative(-1)); + Assert.assertEquals("you", result.getString(1)); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); } try { - Assert.assertTrue(BQResultSetFunctionTest.Result.first()); - Assert.assertFalse(BQResultSetFunctionTest.Result.relative(-1)); - Assert.assertEquals("", BQResultSetFunctionTest.Result.getString(1)); + Assert.assertTrue(result.first()); + Assert.assertFalse(result.relative(-1)); + Assert.assertEquals("", result.getString(1)); } catch (SQLException e) { boolean ct = e.toString().contains("Cursor is not in a valid Position"); if (ct == true) { @@ -662,9 +642,9 @@ public void TestResultSetRelative() { } try { - Assert.assertTrue(BQResultSetFunctionTest.Result.last()); - Assert.assertFalse(BQResultSetFunctionTest.Result.relative(1)); - Assert.assertEquals("", BQResultSetFunctionTest.Result.getString(1)); + Assert.assertTrue(result.last()); + Assert.assertFalse(result.relative(1)); + Assert.assertEquals("", result.getString(1)); } catch (SQLException e) { boolean ct = e.toString().contains("Cursor is not in a valid Position"); if (ct == true) { From 62f1f3818bcc4a22520af8c04d27a3b76855fa8f Mon Sep 17 00:00:00 2001 From: Mark Williams Date: Wed, 20 Dec 2023 22:26:40 +0000 Subject: [PATCH 17/19] Refactor QueryResultTest connection management. Use common `ConnectionFromResources.connect` method instead of a bespoke `NewConnection` method. Also, use instance variable/test case scoped connections and result sets instead of static/suite scoped ones. This should help enable test parallelization later. Local testing shows no reliable increase in runtime (~12 seconds). This was eased by the introduction of a second BQ connection with legacy SQL disabled (`standardSqlConnection`). Not every test uses both connections, so this is a little wasteful and risks introducing some unreliability, but it isolates test better than continually closing and reopening a static connection. --- .../clouddb/jdbc/QueryResultTest.java | 106 +++++++----------- 1 file changed, 41 insertions(+), 65 deletions(-) diff --git a/src/test/java/net/starschema/clouddb/jdbc/QueryResultTest.java b/src/test/java/net/starschema/clouddb/jdbc/QueryResultTest.java index 7431dea..8e23ffd 100644 --- a/src/test/java/net/starschema/clouddb/jdbc/QueryResultTest.java +++ b/src/test/java/net/starschema/clouddb/jdbc/QueryResultTest.java @@ -20,7 +20,8 @@ */ package net.starschema.clouddb.jdbc; -import java.sql.DriverManager; +import java.io.IOException; +import java.sql.Connection; import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; @@ -40,7 +41,9 @@ */ public class QueryResultTest { - private static java.sql.Connection con = null; + private java.sql.Connection connection; + private java.sql.Connection standardSqlConnection; + // Logger logger = new Logger(QueryResultTest.class.getName()); Logger logger = LoggerFactory.getLogger(QueryResultTest.class.getName()); @@ -64,61 +67,37 @@ private boolean comparer(String[][] expected, String[][] reality) { return true; } - /** - * Makes a new Bigquery Connection to Hardcoded URL and gives back the Connection to static con - * member. - */ - public void NewConnection(String extraUrl) { - try { - if (QueryResultTest.con == null || !QueryResultTest.con.isValid(0) || extraUrl != null) { - - this.logger.info("Testing the JDBC driver"); - try { - Class.forName("net.starschema.clouddb.jdbc.BQDriver"); - String jdbcUrl = - BQSupportFuncts.constructUrlFromPropertiesFile( - BQSupportFuncts.readFromPropFile( - getClass().getResource("/serviceaccount.properties").getFile())); - if (extraUrl != null) { - jdbcUrl += extraUrl; - } - QueryResultTest.con = - DriverManager.getConnection( - jdbcUrl, - BQSupportFuncts.readFromPropFile( - getClass().getResource("/serviceaccount.properties").getFile())); - } catch (Exception e) { - this.logger.error("Error in connection" + e.toString()); - Assert.fail("General Exception:" + e.toString()); - } - this.logger.info(((BQConnection) QueryResultTest.con).getURLPART()); - } - } catch (SQLException e) { - // TODO Auto-generated catch block - e.printStackTrace(); - } + private Connection connect(final String extraUrl) throws SQLException, IOException { + return ConnectionFromResources.connect("installedaccount1.properties", extraUrl); + } + + @Before + public void setConnection() throws SQLException, IOException { + connection = connect("&useLegacySql=true"); } @Before - public void NewConnection() { - NewConnection("&useLegacySql=true"); + public void setStandardSqlConnection() throws SQLException, IOException { + standardSqlConnection = connect("&useLegacySql=false"); } @After - public void TeardownConnection() throws SQLException { - QueryResultTest.con.close(); - QueryResultTest.con = null; + public void closeConnection() throws SQLException { + connection.close(); + } + + @After + public void closeStandardSqlConnection() throws SQLException { + standardSqlConnection.close(); } @Test public void QueryResultTestWithDataset() throws SQLException { - NewConnection("&useLegacySql=false"); - - QueryResultTest.con.setSchema("foobar"); - Assert.assertEquals("foobar", QueryResultTest.con.getSchema()); + standardSqlConnection.setSchema("foobar"); + Assert.assertEquals("foobar", standardSqlConnection.getSchema()); - QueryResultTest.con.setSchema("tokyo_star"); - Assert.assertEquals("tokyo_star", QueryResultTest.con.getSchema()); + standardSqlConnection.setSchema("tokyo_star"); + Assert.assertEquals("tokyo_star", standardSqlConnection.getSchema()); final String sql = "SELECT meaning FROM meaning_of_life GROUP BY ROLLUP(meaning);"; String[][] expectation = new String[][] {{null, "42"}}; @@ -128,7 +107,7 @@ public void QueryResultTestWithDataset() throws SQLException { java.sql.ResultSet Result = null; try { - Statement s = QueryResultTest.con.createStatement(); + Statement s = standardSqlConnection.createStatement(); s.setMaxRows(1); Result = s.executeQuery(sql); } catch (SQLException e) { @@ -165,7 +144,7 @@ public void QueryResultTest01() { java.sql.ResultSet Result = null; try { - Result = QueryResultTest.con.createStatement().executeQuery(sql); + Result = connection.createStatement().executeQuery(sql); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); @@ -215,7 +194,7 @@ public void QueryResultTest02() { java.sql.ResultSet Result = null; try { - Result = QueryResultTest.con.createStatement().executeQuery(sql); + Result = connection.createStatement().executeQuery(sql); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); @@ -251,7 +230,7 @@ public void QueryResultTest03() { this.logger.debug(description); java.sql.ResultSet result = null; try { - Statement stmt = con.createStatement(); + Statement stmt = connection.createStatement(); // stmt.setQueryTimeout(60); result = stmt.executeQuery(sql); } catch (SQLException e) { @@ -277,7 +256,7 @@ public void QueryResultTest04() { java.sql.ResultSet Result = null; try { - Result = QueryResultTest.con.createStatement().executeQuery(sql); + Result = connection.createStatement().executeQuery(sql); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); @@ -308,7 +287,7 @@ public void QueryResultTest05() { java.sql.ResultSet Result = null; try { - Result = QueryResultTest.con.createStatement().executeQuery(sql); + Result = connection.createStatement().executeQuery(sql); this.logger.debug("{}", Result.getMetaData().getColumnCount()); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); @@ -344,7 +323,7 @@ public void QueryResultTest06() { java.sql.ResultSet Result = null; try { - Result = QueryResultTest.con.createStatement().executeQuery(sql); + Result = connection.createStatement().executeQuery(sql); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); @@ -382,7 +361,7 @@ public void QueryResultTest07() { java.sql.ResultSet Result = null; try { - Result = QueryResultTest.con.createStatement().executeQuery(sql); + Result = connection.createStatement().executeQuery(sql); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); @@ -423,7 +402,7 @@ public void QueryResultTest08() { java.sql.ResultSet Result = null; try { - Result = QueryResultTest.con.createStatement().executeQuery(sql); + Result = connection.createStatement().executeQuery(sql); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); @@ -461,7 +440,7 @@ public void QueryResultTest09() { java.sql.ResultSet Result = null; try { - Result = QueryResultTest.con.createStatement().executeQuery(sql); + Result = connection.createStatement().executeQuery(sql); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); @@ -500,7 +479,7 @@ public void QueryResultTest10() { this.logger.info("Running query:" + sql); try { - Statement stmt = QueryResultTest.con.createStatement(); + Statement stmt = connection.createStatement(); stmt.setQueryTimeout(1); stmt.executeQuery(sql); } catch (SQLException e) { @@ -516,7 +495,7 @@ public void QueryResultTest11() { this.logger.info("Testing databesmetadata ... getSchemas() "); try { - QueryResultTest.con.getMetaData().getSchemas(); + connection.getMetaData().getSchemas(); } catch (SQLException e) { this.logger.warn("SQLexception" + e.toString()); Assert.fail("schema problem"); @@ -534,7 +513,7 @@ public void QueryResultTest12() { java.sql.ResultSet Result = null; try { Statement stm = - con.createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY); + connection.createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY); stm.setFetchSize(1000); Result = stm.executeQuery(sql); } catch (SQLException e) { @@ -561,8 +540,6 @@ public void QueryResultTest12() { @Test public void QueryResultTestSyncQuery() { - // sync is the default, but let's test it explicitly declared anyway - NewConnection(); final String sql = "SELECT STRING(ROUND(weight_pounds)) FROM publicdata:samples.natality GROUP BY 1 ORDER BY" + " 1 DESC LIMIT 10;"; @@ -586,7 +563,7 @@ public void QueryResultTestSyncQuery() { java.sql.ResultSet Result = null; try { - Result = QueryResultTest.con.createStatement().executeQuery(sql); + Result = connection.createStatement().executeQuery(sql); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); @@ -615,7 +592,7 @@ public void QueryResultTestTokyo01() { java.sql.ResultSet Result = null; try { - Result = QueryResultTest.con.createStatement().executeQuery(sql); + Result = connection.createStatement().executeQuery(sql); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); @@ -636,7 +613,6 @@ public void QueryResultTestTokyo01() { @Test public void QueryResultTestTokyoForceFetchMoreRowsPath() { - NewConnection("&useLegacySql=false"); final String sql = "SELECT meaning FROM tokyo_star.meaning_of_life GROUP BY ROLLUP(meaning);"; String[][] expectation = new String[][] {{null, "42"}}; @@ -645,7 +621,7 @@ public void QueryResultTestTokyoForceFetchMoreRowsPath() { java.sql.ResultSet Result = null; try { - Statement s = QueryResultTest.con.createStatement(); + Statement s = standardSqlConnection.createStatement(); s.setMaxRows(1); Result = s.executeQuery(sql); } catch (SQLException e) { From 68f17f56e98063bfda947a4ad25448a0abb2a1e7 Mon Sep 17 00:00:00 2001 From: Mark Williams Date: Wed, 20 Dec 2023 22:32:31 +0000 Subject: [PATCH 18/19] Refactor QueryResultTest connection management. Use common `ConnectionFromResources.connect` method instead of a bespoke `NewConnection` method. Also, use instance variable/test case scoped connections and result sets instead of static/suite scoped ones. This should help enable test parallelization later. Local testing shows no reliable increase in total runtime (~6 seconds both before and after change). --- .../starschema/clouddb/jdbc/TimeoutTest.java | 63 +++++++------------ 1 file changed, 24 insertions(+), 39 deletions(-) diff --git a/src/test/java/net/starschema/clouddb/jdbc/TimeoutTest.java b/src/test/java/net/starschema/clouddb/jdbc/TimeoutTest.java index 89d1e4f..8b5c476 100644 --- a/src/test/java/net/starschema/clouddb/jdbc/TimeoutTest.java +++ b/src/test/java/net/starschema/clouddb/jdbc/TimeoutTest.java @@ -20,10 +20,11 @@ */ package net.starschema.clouddb.jdbc; -import java.sql.DriverManager; +import java.io.IOException; import java.sql.ResultSet; import java.sql.SQLException; import junit.framework.Assert; +import org.junit.After; import org.junit.Before; import org.junit.Test; import org.slf4j.Logger; @@ -31,7 +32,7 @@ public class TimeoutTest { - private static java.sql.Connection con = null; + private java.sql.Connection connection; Logger logger = LoggerFactory.getLogger(TimeoutTest.class); /** @@ -53,42 +54,26 @@ private boolean comparer(String[][] expected, String[][] reality) { return true; } + @Before + public void setConnection() throws SQLException, IOException { + connection = + ConnectionFromResources.connect("installedaccount1.properties", "&useLegacySql=true"); + } + + @After + public void closeConnection() throws SQLException { + connection.close(); + } + @Test public void isvalidtest() { try { - Assert.assertTrue(TimeoutTest.con.isValid(0)); + Assert.assertTrue(connection.isValid(0)); } catch (SQLException e) { } } - /** - * Makes a new Bigquery Connection to Hardcoded URL and gives back the Connection to static con - * member. - */ - @Before - public void NewConnection() throws Exception { - if (TimeoutTest.con == null || !TimeoutTest.con.isValid(0)) { - this.logger.info("Testing the JDBC driver"); - try { - - Class.forName("net.starschema.clouddb.jdbc.BQDriver"); - TimeoutTest.con = - DriverManager.getConnection( - BQSupportFuncts.constructUrlFromPropertiesFile( - BQSupportFuncts.readFromPropFile( - getClass().getResource("/serviceaccount.properties").getFile())) - + "&useLegacySql=true", - BQSupportFuncts.readFromPropFile( - getClass().getResource("/serviceaccount.properties").getFile())); - } catch (Exception e) { - this.logger.error("Error in connection" + e.toString()); - Assert.fail("General Exception:" + e.toString()); - } - this.logger.info(((BQConnection) TimeoutTest.con).getURLPART()); - } - } - @Test public void QueryResultTest01() { final String sql = "SELECT TOP(word, 10), COUNT(*) FROM publicdata:samples.shakespeare"; @@ -104,7 +89,7 @@ public void QueryResultTest01() { java.sql.ResultSet Result = null; try { - Result = TimeoutTest.con.createStatement().executeQuery(sql); + Result = connection.createStatement().executeQuery(sql); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); @@ -139,7 +124,7 @@ public void QueryResultTest02() { java.sql.ResultSet Result = null; try { - Result = TimeoutTest.con.createStatement().executeQuery(sql); + Result = connection.createStatement().executeQuery(sql); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); @@ -176,7 +161,7 @@ public void QueryResultTest03() { this.logger.info("Running query:" + sql); this.logger.debug(description); try { - TimeoutTest.con.createStatement().executeQuery(sql); + connection.createStatement().executeQuery(sql); } catch (SQLException e) { this.logger.debug("SQLexception" + e.toString()); // fail("SQLException" + e.toString()); @@ -201,7 +186,7 @@ public void QueryResultTest04() { java.sql.ResultSet Result = null; try { - Result = TimeoutTest.con.createStatement().executeQuery(sql); + Result = connection.createStatement().executeQuery(sql); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); @@ -234,7 +219,7 @@ public void QueryResultTest05() { java.sql.ResultSet Result = null; try { - Result = TimeoutTest.con.createStatement().executeQuery(sql); + Result = connection.createStatement().executeQuery(sql); this.logger.debug("{}", Result.getMetaData().getColumnCount()); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); @@ -270,7 +255,7 @@ public void QueryResultTest06() { java.sql.ResultSet Result = null; try { - Result = TimeoutTest.con.createStatement().executeQuery(sql); + Result = connection.createStatement().executeQuery(sql); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); @@ -310,7 +295,7 @@ public void QueryResultTest07() { java.sql.ResultSet Result = null; try { - Result = TimeoutTest.con.createStatement().executeQuery(sql); + Result = connection.createStatement().executeQuery(sql); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); @@ -351,7 +336,7 @@ public void QueryResultTest08() { java.sql.ResultSet Result = null; try { - Result = TimeoutTest.con.createStatement().executeQuery(sql); + Result = connection.createStatement().executeQuery(sql); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); @@ -389,7 +374,7 @@ public void QueryResultTest09() { java.sql.ResultSet Result = null; try { - Result = TimeoutTest.con.createStatement().executeQuery(sql); + Result = connection.createStatement().executeQuery(sql); } catch (SQLException e) { this.logger.error("SQLexception" + e.toString()); Assert.fail("SQLException" + e.toString()); From e61041d5ebcbe675d1b9b07e0db799153c94fd3d Mon Sep 17 00:00:00 2001 From: Mark Williams Date: Thu, 28 Dec 2023 21:10:13 +0000 Subject: [PATCH 19/19] Remove extra instance variable fixtures. Now each @Before method sets a single instance variable. This is to address code review comments. --- .../jdbc/BQForwardOnlyResultSetFunctionTest.java | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/test/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSetFunctionTest.java b/src/test/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSetFunctionTest.java index 0f6835d..5ffaa53 100644 --- a/src/test/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSetFunctionTest.java +++ b/src/test/java/net/starschema/clouddb/jdbc/BQForwardOnlyResultSetFunctionTest.java @@ -67,7 +67,6 @@ public class BQForwardOnlyResultSetFunctionTest extends CommonTestsForResultSets Logger logger = LoggerFactory.getLogger(BQForwardOnlyResultSetFunctionTest.class); private Integer maxRows = null; - private String defaultProjectId = null; private Connection connect(final String extraUrl) throws SQLException, IOException { return ConnectionFromResources.connect("installedaccount1.properties", extraUrl); @@ -76,8 +75,6 @@ private Connection connect(final String extraUrl) throws SQLException, IOExcepti @Before public void setConnection() throws SQLException, IOException { connection = connect("&useLegacySql=true"); - final BQConnection bqConnection = (BQConnection) connection; - this.defaultProjectId = bqConnection.getProjectId(); } @Before @@ -739,14 +736,14 @@ public void testBQForwardOnlyResultSetDoesntThrowNPE() throws Exception { Job ref = bq.getBigquery() .jobs() - .get(defaultProjectId, qr.getJobReference().getJobId()) + .get(bq.getProjectId(), qr.getJobReference().getJobId()) .setLocation(qr.getJobReference().getLocation()) .execute(); // Under certain race conditions we could close the connection after the job is complete but // before the results have been fetched. This was throwing a NPE. bq.close(); try { - new BQForwardOnlyResultSet(bq.getBigquery(), defaultProjectId, ref, null, stmt); + new BQForwardOnlyResultSet(bq.getBigquery(), bq.getProjectId(), ref, null, stmt); Assert.fail("Initalizing BQForwardOnlyResultSet should throw something other than a NPE."); } catch (SQLException e) { Assert.assertEquals(e.getMessage(), "Failed to fetch results. Connection is closed."); @@ -766,9 +763,8 @@ public void testHandlesAllNullResponseFields() throws Exception { @Test public void testHandlesNullTimeDateObjects() throws Exception { - this.NewConnection("&useLegacySql=false"); Statement stmt = - BQForwardOnlyResultSetFunctionTest.con.createStatement( + standardSqlConnection.createStatement( ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY); final String date = "2011-11-11";