Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 33 commits into
base: main
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
edd6adb
[CALCITE-6239] Add a postgis dialect that supports ST functions
bchapuis Jan 17, 2024
7f3a1a2
Fix the registration of the geometry schema
bchapuis Feb 3, 2024
84abbce
Place postgis before postgres whenever it makes sense
bchapuis Feb 3, 2024
c515d58
Add support for implicit type conversion from binary to geometry
bchapuis Feb 5, 2024
d37cbb6
Add a geometry parser to bind jdbc geometries to calcite geometries
bchapuis Feb 6, 2024
1a297e7
Improve naming and fix minor issues
bchapuis Feb 7, 2024
3b37b54
Use lenient conformance
bchapuis Feb 8, 2024
2607023
Remove blank line
bchapuis Feb 13, 2024
ef5a48e
Ignore casing when checking the type name
bchapuis Feb 13, 2024
a9caca8
Add a built in method to decode postgis geometries
bchapuis Feb 13, 2024
65dab8a
Remove unnecessary comment
bchapuis Feb 13, 2024
a855375
Use reflection to register the geometry type family
bchapuis Feb 13, 2024
671d316
Consistently use PostGIS instead of Posgis
bchapuis Feb 13, 2024
93568cd
Remove unnecessary functions from SqlKind
bchapuis Feb 13, 2024
fc31956
Document the extension of the PostgreSQL dialect by the PostGIS dialect
bchapuis Feb 13, 2024
151bbe5
Use SqlKind instead of Strings in the PostGIS dialect
bchapuis Feb 13, 2024
67b97f3
Fix the error message of the unit test for the OTHER type
bchapuis Feb 13, 2024
3b05b97
Format the code with autostyle
bchapuis Feb 13, 2024
90fe0b6
Fix style and coding issues
bchapuis Feb 13, 2024
e826ab5
Fix the javadoc
bchapuis Feb 13, 2024
5be15b3
Move the PostgisGeometryDecoder into the util package
bchapuis Feb 13, 2024
94d49fe
Add test cases for the postgis geometry decoder
bchapuis Feb 13, 2024
5fb31b8
Remove reference to removed ST functions
bchapuis Feb 13, 2024
76ae1e6
Format the code
bchapuis Feb 13, 2024
5096e84
Revert changes in the sql dialect
bchapuis Feb 13, 2024
488df2e
Check null value in the postgis geometry decoder
bchapuis Feb 14, 2024
5432471
Add a lexical policy for PostgreSQL
bchapuis Feb 14, 2024
40d4418
Add nullable annotation to geometry decoder
bchapuis Feb 14, 2024
75893e7
Improve casting of decoded geometries
bchapuis Feb 14, 2024
cf66678
Add annotation Parameter.sqlType
julianhyde Mar 5, 2024
3634120
More
julianhyde Mar 7, 2024
ef38d08
Adapt more functions to the geometry type parameter
bchapuis Mar 7, 2024
05985b4
Remove the JTS type
bchapuis Mar 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,9 @@ private Expression getConvertExpression(
case CHAR:
case VARCHAR:
return Expressions.call(BuiltInMethod.ST_GEOM_FROM_EWKT.method, operand);

case BINARY:
case VARBINARY:
return Expressions.call(BuiltInMethod.ST_GEOM_FROM_EWKB.method, operand);
bchapuis marked this conversation as resolved.
Show resolved Hide resolved
default:
return defaultExpression.get();
}
Expand Down
6 changes: 6 additions & 0 deletions core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcSchema.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,12 @@ private static RelDataType sqlType(RelDataTypeFactory typeFactory, int dataType,
typeFactory.createSqlType(SqlTypeName.ANY), true);
}
return typeFactory.createArrayType(component, -1);
case OTHER:
bchapuis marked this conversation as resolved.
Show resolved Hide resolved
if (typeString != null && typeString.startsWith("geometry")) {
return typeFactory.createTypeWithNullability(
typeFactory.createSqlType(SqlTypeName.GEOMETRY), true);
}
break;
default:
break;
}
Expand Down
10 changes: 10 additions & 0 deletions core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcToEnumerableConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import org.apache.calcite.runtime.SqlFunctions;
import org.apache.calcite.schema.Schemas;
import org.apache.calcite.sql.SqlDialect;
import org.apache.calcite.sql.dialect.PostgisGeometryDecoder;
import org.apache.calcite.sql.type.SqlTypeName;
import org.apache.calcite.sql.util.SqlString;
import org.apache.calcite.util.BuiltInMethod;
Expand Down Expand Up @@ -283,6 +284,15 @@ private static void generateGet(EnumerableRelImplementor implementor,
java.sql.Array.class);
source = Expressions.call(BuiltInMethod.JDBC_ARRAY_TO_LIST.method, x);
break;
case GEOMETRY:
source =
Expressions.call(
bchapuis marked this conversation as resolved.
Show resolved Hide resolved
PostgisGeometryDecoder.class,
"decode",
Expressions.call(
resultSet_, "getString",
Expressions.constant(i + 1)));
break;
case NULL:
source = RexImpTable.NULL_EXPR;
break;
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/org/apache/calcite/prepare/CalcitePrepareImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -821,7 +821,7 @@ private static ColumnMetaData.AvaticaType avaticaType(JavaTypeFactory typeFactor
}
return ColumnMetaData.struct(columns);
case ExtraSqlTypes.GEOMETRY:
typeOrdinal = Types.VARCHAR;
typeOrdinal = Types.OTHER; // same as PostGIS
// fall through
default:
final Type clazz =
Expand Down
18 changes: 18 additions & 0 deletions core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
import org.apache.calcite.rex.RexUtil;
import org.apache.calcite.rex.RexWindow;
import org.apache.calcite.rex.RexWindowBound;
import org.apache.calcite.runtime.SpatialTypeUtils;
import org.apache.calcite.sql.JoinType;
import org.apache.calcite.sql.SqlAggFunction;
import org.apache.calcite.sql.SqlBasicCall;
Expand Down Expand Up @@ -112,6 +113,7 @@

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

import java.math.BigDecimal;
import java.util.AbstractList;
Expand Down Expand Up @@ -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) {
Copy link
Contributor

@YiwenWu YiwenWu Feb 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

case CHAR:
case VARCHAR:
return SqlLiteral.createCharString(
SpatialTypeUtils.asEwkt(geom), POS);
case BINARY:
case VARBINARY:
return SqlLiteral.createBinaryString(
SpatialTypeUtils.asWkbArray(geom), POS);
default:
return SqlLiteral.createNull(POS);
}
}
case NUMERIC:
case EXACT_NUMERIC: {
if (SqlTypeName.APPROX_TYPES.contains(typeName)) {
Expand Down Expand Up @@ -1488,6 +1505,7 @@ public static SqlNode toSql(RexLiteral literal) {
default:
break;
}

bchapuis marked this conversation as resolved.
Show resolved Hide resolved
// fall through
default:
throw new AssertionError(literal + ": " + typeName);
Expand Down
5 changes: 4 additions & 1 deletion core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactoryImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,11 @@
import com.google.common.collect.Interners;

import org.checkerframework.checker.nullness.qual.Nullable;
import org.locationtech.jts.geom.Geometry;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really want this dependency here?

Copy link
Member Author

@bchapuis bchapuis Feb 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
  }
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


import java.lang.reflect.Field;
import java.nio.charset.Charset;
import java.sql.Date;
import java.sql.Time;
import java.sql.Timestamp;
import java.util.AbstractList;
Expand Down Expand Up @@ -106,9 +108,10 @@ private static RelDataType keyToType(Key key) {
.put(Float.class, SqlTypeFamily.APPROXIMATE_NUMERIC)
.put(double.class, SqlTypeFamily.APPROXIMATE_NUMERIC)
.put(Double.class, SqlTypeFamily.APPROXIMATE_NUMERIC)
.put(java.sql.Date.class, SqlTypeFamily.DATE)
.put(Date.class, SqlTypeFamily.DATE)
.put(Time.class, SqlTypeFamily.TIME)
.put(Timestamp.class, SqlTypeFamily.TIMESTAMP)
.put(Geometry.class, SqlTypeFamily.GEO)
.build();

protected final RelDataTypeSystem typeSystem;
Expand Down
24 changes: 22 additions & 2 deletions core/src/main/java/org/apache/calcite/runtime/SpatialTypeUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,19 @@ public static Geometry fromGml(String gml) {
* @return a geometry
*/
public static Geometry fromWkb(ByteString wkb) {
return fromWkbArray(wkb.getBytes());
}

/**
* Constructs a geometry from a Well-Known binary (WKB) representation.
*
* @param wkb a WKB
* @return a geometry
*/
public static Geometry fromWkbArray(byte[] wkb) {
try {
WKBReader reader = new WKBReader();
return reader.read(wkb.getBytes());
return reader.read(wkb);
} catch (ParseException e) {
throw new RuntimeException("Unable to parse WKB");
}
Expand Down Expand Up @@ -231,9 +241,19 @@ public static String asGml(Geometry geometry) {
* @return an WKB
*/
public static ByteString asWkb(Geometry geometry) {
return new ByteString(asWkbArray(geometry));
}

/**
* Returns the Extended Well-Known binary (WKB) representation of the geometry.
*
* @param geometry a geometry
* @return an WKB
*/
public static byte[] asWkbArray(Geometry geometry) {
int outputDimension = dimension(geometry);
WKBWriter wkbWriter = new WKBWriter(outputDimension);
return new ByteString(wkbWriter.write(geometry));
return wkbWriter.write(geometry);
}

/**
Expand Down
3 changes: 3 additions & 0 deletions core/src/main/java/org/apache/calcite/sql/SqlDialect.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,8 @@ public static DatabaseProduct getProduct(
return DatabaseProduct.NEOVIEW;
} else if (upperProductName.contains("POSTGRE")) {
return DatabaseProduct.POSTGRESQL;
} else if (upperProductName.contains("POSTGIS")) {
return DatabaseProduct.POSTGIS;
} else if (upperProductName.contains("SQL SERVER")) {
return DatabaseProduct.MSSQL;
} else if (upperProductName.contains("SYBASE")) {
Expand Down Expand Up @@ -1382,6 +1384,7 @@ public enum DatabaseProduct {
LUCIDDB("LucidDB", "\"", NullCollation.HIGH),
INTERBASE("Interbase", null, NullCollation.HIGH),
PHOENIX("Phoenix", "\"", NullCollation.HIGH),
POSTGIS("Postgis", "\"", NullCollation.HIGH),
bchapuis marked this conversation as resolved.
Show resolved Hide resolved
POSTGRESQL("PostgreSQL", "\"", NullCollation.HIGH),
PRESTO("Presto", "\"", NullCollation.LAST),
NETEZZA("Netezza", "\"", NullCollation.HIGH),
Expand Down
3 changes: 3 additions & 0 deletions core/src/main/java/org/apache/calcite/sql/SqlDialectFactoryImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import org.apache.calcite.sql.dialect.OracleSqlDialect;
import org.apache.calcite.sql.dialect.ParaccelSqlDialect;
import org.apache.calcite.sql.dialect.PhoenixSqlDialect;
import org.apache.calcite.sql.dialect.PostgisSqlDialect;
import org.apache.calcite.sql.dialect.PostgresqlSqlDialect;
import org.apache.calcite.sql.dialect.PrestoSqlDialect;
import org.apache.calcite.sql.dialect.RedshiftSqlDialect;
Expand Down Expand Up @@ -220,6 +221,8 @@ public class SqlDialectFactoryImpl implements SqlDialectFactory {
return ParaccelSqlDialect.DEFAULT;
case PHOENIX:
return PhoenixSqlDialect.DEFAULT;
case POSTGIS:
bchapuis marked this conversation as resolved.
Show resolved Hide resolved
return PostgisSqlDialect.DEFAULT;
case POSTGRESQL:
return PostgresqlSqlDialect.DEFAULT;
case PRESTO:
Expand Down
Loading
Loading