Jelajahi Sumber

SQL: Remove JDBC dependency on ES lib geo (#82166)

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 #80277
Costin Leau 3 tahun lalu
induk
melakukan
ec57fbec6f

+ 1 - 0
docs/reference/migration/migrate_8_0.asciidoc

@@ -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]]

+ 27 - 0
docs/reference/migration/migrate_8_0/sql-jdbc-changes.asciidoc

@@ -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[]

+ 0 - 4
x-pack/plugin/sql/jdbc/build.gradle

@@ -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")

+ 1 - 9
x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/TypeConverter.java

@@ -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;
@@ -253,11 +249,7 @@ final class TypeConverter {
             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:

+ 0 - 10
x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/geo/GeoSqlSpecTestCase.java

@@ -9,7 +9,6 @@ package org.elasticsearch.xpack.sql.qa.geo;
 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;
@@ -22,7 +21,6 @@ import java.text.NumberFormat;
 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
@@ -90,12 +88,4 @@ public abstract class GeoSqlSpecTestCase extends SpecBaseIntegrationTestCase {
             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;
-    }
 }

+ 22 - 14
x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcAssert.java

@@ -9,7 +9,6 @@ package org.elasticsearch.xpack.sql.qa.jdbc;
 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;
@@ -309,20 +308,19 @@ public class JdbcAssert {
                     } 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);
                         }
@@ -350,6 +348,16 @@ public class JdbcAssert {
         }
     }
 
+    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.
      */