[CALCITE-6239] Add a postgis dialect that supports ST functions #3668

bchapuis · 2024-02-02T22:58:50Z

In JDBC adapter, add a PostGIS dialect so that ST functions can be pushed down

core/src/main/java/org/apache/calcite/sql/dialect/PostgisSqlDialect.java

core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java

core/src/main/java/org/apache/calcite/sql/SqlDialectFactoryImpl.java

bchapuis · 2024-02-03T10:57:13Z

@YiwenWu @JiajunBernoulli What would be the best way to test these changes against a postgis database? Ideally, I'd like to add an integration test that executes queries against postgis (e.g. with testcontainers). But I havn't been able to find such tests in calcite.

macroguo-ghy · 2024-02-04T05:29:24Z

@YiwenWu @JiajunBernoulli What would be the best way to test these changes against a postgis database? Ideally, I'd like to add an integration test that executes queries against postgis (e.g. with testcontainers). But I havn't been able to find such tests in calcite.

I remember that testContainer has been used in redis apapter test.

JiajunBernoulli · 2024-02-05T01:50:35Z

@YiwenWu @JiajunBernoulli What would be the best way to test these changes against a postgis database? Ideally, I'd like to add an integration test that executes queries against postgis (e.g. with testcontainers). But I havn't been able to find such tests in calcite.

There are some integration tests for Druid: https://github.com/zabetak/calcite-druid-dataset
Here is CI config in calcite:

calcite/.github/workflows/main.yml

Line 440 in 2aabf21

repository: zabetak/calcite-druid-dataset

bchapuis · 2024-02-05T10:02:12Z

Thanks a lot for the pointer, I will adopt the same approach and implement some integration tests in a third-party repository for now.

julianhyde

Can you change the summary to "In JDBC adapter, add a PostGIS dialect so that ST_ functions can be pushed down". It will make the release notes easier to understand.

core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcSchema.java

core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcToEnumerableConverter.java

core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java

julianhyde · 2024-02-12T23:33:17Z

core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactoryImpl.java

@@ -39,9 +39,11 @@
 import com.google.common.collect.Interners;

 import org.checkerframework.checker.nullness.qual.Nullable;
+import org.locationtech.jts.geom.Geometry;


Do we really want this dependency here?

I'm trying to wrap my head around this. From what I understand, calling a method of RelDataTypeFactoryImpl without JTS would trigger a class not found exception. As the use of JTS is currently circumscribed to a few spatial type classes, calcite works just fine without adding a dependency to JTS. Is that correct?

Given the popularity of JTS in the geospatial world, I wouldn't try to abstract it completely in JTS. An option could be to use reflection to dynamically load the Geometry class and provide some sort of fallback if the class is not in the class path.

public static class GeometryFallback { } static Class<?> loadGeometryClass() { try { return Class.forName("org.locationtech.jts.geom.Geometry"); } catch (ClassNotFoundException e) { return GeometryFallback.class; } } private static final Map<Class, RelDataTypeFamily> CLASS_FAMILIES = ImmutableMap.<Class, RelDataTypeFamily>builder() .put(String.class, SqlTypeFamily.CHARACTER) // ... .put(loadGeometryClass(), SqlTypeFamily.GEO) .build();

What do you think?

Another option could be to add this type only when JTS is in the classpath.

static { try { CLASS_FAMILIES.put(Class.forName("org.locationtech.jts.geom.Geometry"), SqlTypeFamily.GEO); } catch (ClassNotFoundException e) { // JTS is not available in the classpath } }

Can we discuss in jira? Increasing the coupling to JTS is an architectural change.

Yes, here is the jira issue I just created:
https://issues.apache.org/jira/browse/CALCITE-6263

core/src/main/java/org/apache/calcite/sql/SqlDialect.java

core/src/main/java/org/apache/calcite/sql/SqlKind.java

julianhyde · 2024-02-12T23:41:06Z

core/src/main/java/org/apache/calcite/sql/dialect/PostgisSqlDialect.java

+
+  @Override public void unparseCall(SqlWriter writer, SqlCall call, int leftPrec, int rightPrec) {
+    if (call.getOperator().getName().equals("ST_UNARYUNION")) {
+      RelToSqlConverterUtil.specialOperatorByName("ST_UNION")


use SqlKind rather than name?

core/src/main/java/org/apache/calcite/sql/dialect/PostgisSqlDialect.java

core/src/main/java/org/apache/calcite/sql/dialect/PostgisGeometryDecoder.java

core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java

This prevent reflection errors at runtime.

For now, let's assume that most databases behave as postgis.

sonarcloud · 2024-02-14T13:41:40Z

Quality Gate passed

Issues
12 New issues

Measures
0 Security Hotspots
84.4% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

YiwenWu · 2024-02-22T01:05:22Z

core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcSchema.java

@@ -433,6 +433,17 @@ private static RelDataType sqlType(RelDataTypeFactory typeFactory, int dataType,
                typeFactory.createSqlType(SqlTypeName.ANY), true);
      }
      return typeFactory.createArrayType(component, -1);
+    case GEOMETRY:


According to the elements in SqlTypeName#JDBC_TYPE_TO_NAME , whether there are any cases that can be matched GEOMETRY?

YiwenWu · 2024-02-22T01:20:55Z

core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java

@@ -1445,6 +1447,21 @@ public static SqlNode toSql(RexLiteral literal) {
      // Create a string without specifying a charset
      return SqlLiteral.createCharString((String) castNonNull(literal.getValue2()), POS);
    }
+    case GEO: {
+      Geometry geom = castNonNull(literal.getValueAs(Geometry.class));
+      switch (typeName) {


I am not sure that typeName CHAR or VARCHAR can be obtained directly under GEO family.

The annotation allows you to define the SQL type of a parameter, for cases where it cannot be deduced automatically from the Java type. This is especially useful for GEOMETRY values, for which Calcite has no built-in Java type. (JTS Geometry is widely used in Calcite, but it is an implementation, and we do not want it to become the specification.) This commit is incomplete, but illustrates the general idea. A related change is to add a 'family' parameter to the `TypeFactory.createJavaType` method. It is nullable, and if null, the type becomes its own family. In JavaType, we should add a `family` field. When this change is complete we should be able to remove JTS Geometry from RelDataTypeFactoryImpl.CLASS_FAMILIES.

mihaibudiu · 2024-04-08T22:25:28Z

Will this be ready for 1.37?

bchapuis · 2024-04-09T07:47:02Z

No, I hadn't enought time to work on this and the decoupling of the GEOMETRY type from the JTS Geometry class will require additional efforts.

https://issues.apache.org/jira/browse/CALCITE-6239

bchapuis force-pushed the 6239-postgis-dialect branch from 26be417 to 4869424 Compare February 2, 2024 23:49

YiwenWu reviewed Feb 3, 2024

View reviewed changes

core/src/main/java/org/apache/calcite/sql/dialect/PostgisSqlDialect.java Show resolved Hide resolved

core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java Show resolved Hide resolved

JiajunBernoulli reviewed Feb 3, 2024

View reviewed changes

core/src/main/java/org/apache/calcite/sql/SqlDialectFactoryImpl.java Show resolved Hide resolved

bchapuis force-pushed the 6239-postgis-dialect branch from 9b9f342 to 95c33c9 Compare February 7, 2024 11:10

bchapuis added 7 commits February 8, 2024 17:16

[CALCITE-6239] Add a postgis dialect that supports ST functions

edd6adb

Fix the registration of the geometry schema

7f3a1a2

Place postgis before postgres whenever it makes sense

84abbce

Add support for implicit type conversion from binary to geometry

c515d58

Add a geometry parser to bind jdbc geometries to calcite geometries

d37cbb6

Improve naming and fix minor issues

1a297e7

Use lenient conformance

3b37b54

bchapuis force-pushed the 6239-postgis-dialect branch from d4afb29 to 3b37b54 Compare February 8, 2024 16:37

julianhyde reviewed Feb 12, 2024

View reviewed changes

jnh5y reviewed Feb 13, 2024

View reviewed changes

core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java Show resolved Hide resolved

bchapuis added 12 commits February 13, 2024 11:19

Remove blank line

2607023

Ignore casing when checking the type name

ef5a48e

Add a built in method to decode postgis geometries

a9caca8

This prevent reflection errors at runtime.

Remove unnecessary comment

65dab8a

For now, let's assume that most databases behave as postgis.

Use reflection to register the geometry type family

a855375

Consistently use PostGIS instead of Posgis

671d316

Remove unnecessary functions from SqlKind

93568cd

Document the extension of the PostgreSQL dialect by the PostGIS dialect

fc31956

Use SqlKind instead of Strings in the PostGIS dialect

151bbe5

Fix the error message of the unit test for the OTHER type

67b97f3

Format the code with autostyle

3b05b97

Fix style and coding issues

90fe0b6

bchapuis added 4 commits February 13, 2024 22:14

Fix the javadoc

e826ab5

Move the PostgisGeometryDecoder into the util package

5be15b3

Add test cases for the postgis geometry decoder

94d49fe

Remove reference to removed ST functions

5fb31b8

bchapuis force-pushed the 6239-postgis-dialect branch from c44f5c6 to 5fb31b8 Compare February 13, 2024 21:55

bchapuis added 5 commits February 13, 2024 23:19

Format the code

76ae1e6

Revert changes in the sql dialect

5096e84

Check null value in the postgis geometry decoder

488df2e

Add a lexical policy for PostgreSQL

5432471

Add nullable annotation to geometry decoder

40d4418

bchapuis force-pushed the 6239-postgis-dialect branch from 5e0a0c7 to 40d4418 Compare February 14, 2024 12:17

Improve casting of decoded geometries

75893e7

bchapuis force-pushed the 6239-postgis-dialect branch from 9aed8e5 to 75893e7 Compare February 14, 2024 13:28

YiwenWu reviewed Feb 22, 2024

View reviewed changes

julianhyde and others added 4 commits March 7, 2024 23:13

More

3634120

Adapt more functions to the geometry type parameter

ef38d08

Remove the JTS type

05985b4

bchapuis mentioned this pull request Apr 10, 2024

Create tiles with data from multiple databases apache/incubator-baremaps#845

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CALCITE-6239] Add a postgis dialect that supports ST functions #3668

[CALCITE-6239] Add a postgis dialect that supports ST functions #3668

bchapuis commented Feb 2, 2024 •

edited

Loading

bchapuis commented Feb 3, 2024 •

edited

Loading

macroguo-ghy commented Feb 4, 2024

JiajunBernoulli commented Feb 5, 2024

bchapuis commented Feb 5, 2024

julianhyde left a comment

julianhyde Feb 12, 2024

bchapuis Feb 13, 2024 •

edited

Loading

bchapuis Feb 13, 2024

julianhyde Feb 13, 2024

bchapuis Feb 14, 2024

julianhyde Feb 12, 2024

sonarcloud bot commented Feb 14, 2024

YiwenWu Feb 22, 2024

YiwenWu Feb 22, 2024 •

edited

Loading

mihaibudiu commented Apr 8, 2024

bchapuis commented Apr 9, 2024

[CALCITE-6239] Add a postgis dialect that supports ST functions #3668

Are you sure you want to change the base?

[CALCITE-6239] Add a postgis dialect that supports ST functions #3668

Conversation

bchapuis commented Feb 2, 2024 • edited Loading

bchapuis commented Feb 3, 2024 • edited Loading

macroguo-ghy commented Feb 4, 2024

JiajunBernoulli commented Feb 5, 2024

bchapuis commented Feb 5, 2024

julianhyde left a comment

Choose a reason for hiding this comment

julianhyde Feb 12, 2024

Choose a reason for hiding this comment

bchapuis Feb 13, 2024 • edited Loading

Choose a reason for hiding this comment

bchapuis Feb 13, 2024

Choose a reason for hiding this comment

julianhyde Feb 13, 2024

Choose a reason for hiding this comment

bchapuis Feb 14, 2024

Choose a reason for hiding this comment

julianhyde Feb 12, 2024

Choose a reason for hiding this comment

sonarcloud bot commented Feb 14, 2024

Quality Gate passed

YiwenWu Feb 22, 2024

Choose a reason for hiding this comment

YiwenWu Feb 22, 2024 • edited Loading

Choose a reason for hiding this comment

mihaibudiu commented Apr 8, 2024

bchapuis commented Apr 9, 2024

bchapuis commented Feb 2, 2024 •

edited

Loading

bchapuis commented Feb 3, 2024 •

edited

Loading

bchapuis Feb 13, 2024 •

edited

Loading

YiwenWu Feb 22, 2024 •

edited

Loading