Skip to content

Commit

Permalink
SQL: Remove JDBC dependency on ES lib geo (elastic#82166)
Browse files Browse the repository at this point in the history
As part of the effort of making JDBC driver self sufficient, remove the
ES lib geo dependencies without any replacement.
Currently the JDBC driver takes the WKT text and instantiates a geo
object based on the ES lib geo.
Moving forward the driver will return the WKT string representation
without any conversion letting the user pick the geo library desired.
That can be ES lib geo, jts, spatial4j or others.

Note this is a breaking change.

Relates elastic#80277
  • Loading branch information
costin authored Jan 4, 2022
1 parent 704d8ca commit ec57fbe
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 37 deletions.
1 change: 1 addition & 0 deletions docs/reference/migration/migrate_8_0.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ include::migrate_8_0/painless-changes.asciidoc[]
include::migrate_8_0/rest-api-changes.asciidoc[]
include::migrate_8_0/system-req-changes.asciidoc[]
include::migrate_8_0/transform.asciidoc[]
include::migrate_8_0/sql-jdbc-changes.asciidoc[]

[discrete]
[[deprecated-8.0]]
Expand Down
27 changes: 27 additions & 0 deletions docs/reference/migration/migrate_8_0/sql-jdbc-changes.asciidoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
[discrete]
[[breaking_80_jdbc_changes]]
==== SQL JDBC changes

//NOTE: The notable-breaking-changes tagged regions are re-used in the
//Installation and Upgrade Guide

//tag::notable-breaking-changes[]
.JDBC driver returns geometry objects as well-known-text string instead of `org.elasticsearch.geo` objects.
[%collapsible]
====
*Details* +
To reduce the dependency of the JDBC driver onto Elasticsearch classes, the JDBC driver returns geometry data
as strings using the WKT (well-known text) format instead of classes from the `org.elasticsearch.geometry`.
Users can choose the geometry library desired to convert the string represantion into a full-blown objects
either such as the `elasticsearch-geo` library (which returned the object `org.elasticsearch.geo` as before),
jts or spatial4j.
*Impact* +
Before upgrading, replace any `org.elasticsearch.geo` classes on the `ResultSet#getObject` or `ResultSet#setObject`
Elasticsearch JDBC driver with their WKT representation by simply calling `toString` or
`org.elasticsearch.geometry.utils.WellKnownText#toWKT/fromWKT` methods.
This change does NOT impact users that do not use geometry classes.
====
// end::notable-breaking-changes[]
4 changes: 0 additions & 4 deletions x-pack/plugin/sql/jdbc/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@ dependencies {
}
// required by x-content
runtimeOnly project(':libs:elasticsearch-core')

api(project(':libs:elasticsearch-geo')) {
transitive = false
}
api "com.fasterxml.jackson.dataformat:jackson-dataformat-cbor:${versions.jackson}"
runtimeOnly "com.fasterxml.jackson.core:jackson-core:${versions.jackson}"
testImplementation project(":test:framework")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,14 @@
*/
package org.elasticsearch.xpack.sql.jdbc;

import org.elasticsearch.geometry.utils.StandardValidator;
import org.elasticsearch.geometry.utils.WellKnownText;
import org.elasticsearch.xpack.sql.proto.StringUtils;

import java.io.IOException;
import java.math.BigDecimal;
import java.sql.Date;
import java.sql.SQLException;
import java.sql.SQLFeatureNotSupportedException;
import java.sql.Time;
import java.sql.Timestamp;
import java.text.ParseException;
import java.time.Duration;
import java.time.LocalDate;
import java.time.LocalDateTime;
Expand Down Expand Up @@ -253,11 +249,7 @@ static Object convert(Object v, EsType columnType, String typeString) throws SQL
case GEO_POINT:
case GEO_SHAPE:
case SHAPE:
try {
return WellKnownText.fromWKT(StandardValidator.instance(true), true, v.toString());
} catch (IOException | ParseException ex) {
throw new SQLException("Cannot parse geo_shape", ex);
}
return v.toString();
case IP:
return v.toString();
default:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;

import org.elasticsearch.client.Request;
import org.elasticsearch.xpack.sql.jdbc.JdbcConfiguration;
import org.elasticsearch.xpack.sql.qa.jdbc.LocalH2;
import org.elasticsearch.xpack.sql.qa.jdbc.SpecBaseIntegrationTestCase;
import org.h2gis.functions.factory.H2GISFunctions;
Expand All @@ -22,7 +21,6 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
import java.util.Properties;

/**
* Tests comparing geo sql queries executed against our jdbc client
Expand Down Expand Up @@ -90,12 +88,4 @@ protected final void doTest() throws Throwable {
assertResults(expected, elasticResults);
}
}

// TODO: use UTC for now until deciding on a strategy for handling date extraction
@Override
protected Properties connectionProperties() {
Properties connectionProperties = new Properties();
connectionProperties.setProperty(JdbcConfiguration.TIME_ZONE, "UTC");
return connectionProperties;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import com.carrotsearch.hppc.IntObjectHashMap;

import org.apache.logging.log4j.Logger;
import org.elasticsearch.geometry.Geometry;
import org.elasticsearch.geometry.Point;
import org.elasticsearch.geometry.utils.StandardValidator;
import org.elasticsearch.geometry.utils.WellKnownText;
Expand Down Expand Up @@ -309,20 +308,19 @@ else if (type == Types.DOUBLE) {
} else if (type == Types.FLOAT) {
assertEquals(msg, (float) expectedObject, (float) actualObject, lenientFloatingNumbers ? 1f : 0.0f);
} else if (type == Types.OTHER) {
if (actualObject instanceof Geometry) {
// We need to convert the expected object to libs/geo Geometry for comparision
try {
expectedObject = WellKnownText.fromWKT(StandardValidator.instance(true), true, expectedObject.toString());
} catch (IOException | ParseException ex) {
fail(ex.getMessage());
// check geo case
if (actual.getMetaData().getColumnType(column) == EsType.GEO_SHAPE.getVendorTypeNumber()) {
// parse strings into actual objects
actualObject = fromWkt(actualObject.toString());
expectedObject = fromWkt(expectedObject.toString());

if (actualObject instanceof Point) {
// geo points are loaded form doc values where they are stored as long-encoded values leading
// to lose in precision
assertThat(expectedObject, instanceOf(Point.class));
assertEquals(((Point) expectedObject).getY(), ((Point) actualObject).getY(), 0.000001d);
assertEquals(((Point) expectedObject).getX(), ((Point) actualObject).getX(), 0.000001d);
}
}
if (actualObject instanceof Point) {
// geo points are loaded form doc values where they are stored as long-encoded values leading
// to lose in precision
assertThat(expectedObject, instanceOf(Point.class));
assertEquals(((Point) expectedObject).getY(), ((Point) actualObject).getY(), 0.000001d);
assertEquals(((Point) expectedObject).getX(), ((Point) actualObject).getX(), 0.000001d);
} else {
assertEquals(msg, expectedObject, actualObject);
}
Expand Down Expand Up @@ -350,6 +348,16 @@ else if (type == Types.VARCHAR && actualObject instanceof TemporalAmount) {
}
}

private static Object fromWkt(String source) {
try {
return WellKnownText.fromWKT(StandardValidator.instance(true), true, source);
} catch (IOException | ParseException ex) {
fail(ex.getMessage());
// won't execute
return null;
}
}

/**
* Returns the value of the given type either in a lenient fashion (widened) or strict.
*/
Expand Down

0 comments on commit ec57fbe

Please sign in to comment.