From 8c65a3071cd1b0a91fa7e365059c758941027f16 Mon Sep 17 00:00:00 2001
From: Bertil Chapuis <bchapuis@gmail.com>
Date: Tue, 14 Jan 2025 09:10:57 +0100
Subject: [PATCH] Generate different queries depending on the postgresql
 version (#914)

- Generate different queries depending on the postgresql version
- Caching is not needed anymore as String concatenation is faster than query parsing.
---
 .../java/org/apache/baremaps/cli/map/Dev.java |   3 +-
 .../org/apache/baremaps/cli/map/Serve.java    |   3 +-
 .../baremaps/tasks/ExportVectorTiles.java     |  14 ++-
 .../tilestore/postgres/PostgresTileStore.java | 110 ++++++++++++------
 .../postgres/PostgresTileStoreTest.java       |  29 ++++-
 .../postgres/utils/PostgresUtils.java         |  13 +++
 6 files changed, 126 insertions(+), 46 deletions(-)

diff --git a/baremaps-cli/src/main/java/org/apache/baremaps/cli/map/Dev.java b/baremaps-cli/src/main/java/org/apache/baremaps/cli/map/Dev.java
index e44fcd434..e899dabee 100644
--- a/baremaps-cli/src/main/java/org/apache/baremaps/cli/map/Dev.java
+++ b/baremaps-cli/src/main/java/org/apache/baremaps/cli/map/Dev.java
@@ -87,6 +87,7 @@ public Integer call() throws Exception {
     var objectMapper = objectMapper();
     var tileset = objectMapper.readValue(configReader.read(this.tilesetPath), Tileset.class);
     var datasource = PostgresUtils.createDataSourceFromObject(tileset.getDatabase());
+    var postgresVersion = PostgresUtils.getPostgresVersion(datasource);
 
     var tilesetSupplier = (Supplier<Tileset>) () -> {
       try {
@@ -99,7 +100,7 @@ public Integer call() throws Exception {
 
     var tileStoreSupplier = (Supplier<TileStore<ByteBuffer>>) () -> {
       var tileJSON = tilesetSupplier.get();
-      return new PostgresTileStore(datasource, tileJSON);
+      return new PostgresTileStore(datasource, tileJSON, postgresVersion);
     };
 
     var styleSupplier = (Supplier<Style>) () -> {
diff --git a/baremaps-cli/src/main/java/org/apache/baremaps/cli/map/Serve.java b/baremaps-cli/src/main/java/org/apache/baremaps/cli/map/Serve.java
index cdaa79192..59afb21fc 100644
--- a/baremaps-cli/src/main/java/org/apache/baremaps/cli/map/Serve.java
+++ b/baremaps-cli/src/main/java/org/apache/baremaps/cli/map/Serve.java
@@ -93,9 +93,10 @@ public Integer call() throws Exception {
     var caffeineSpec = CaffeineSpec.parse(cache);
     var tileset = objectMapper.readValue(configReader.read(tilesetPath), Tileset.class);
     var datasource = PostgresUtils.createDataSourceFromObject(tileset.getDatabase());
+    var postgresVersion = PostgresUtils.getPostgresVersion(datasource);
 
     try (
-        var tileStore = new PostgresTileStore(datasource, tileset);
+        var tileStore = new PostgresTileStore(datasource, tileset, postgresVersion);
         var tileCache = new VectorTileCache(tileStore, caffeineSpec)) {
 
       var tileStoreSupplier = (Supplier<TileStore<ByteBuffer>>) () -> tileCache;
diff --git a/baremaps-core/src/main/java/org/apache/baremaps/tasks/ExportVectorTiles.java b/baremaps-core/src/main/java/org/apache/baremaps/tasks/ExportVectorTiles.java
index 97cbbb72b..95e8e8001 100644
--- a/baremaps-core/src/main/java/org/apache/baremaps/tasks/ExportVectorTiles.java
+++ b/baremaps-core/src/main/java/org/apache/baremaps/tasks/ExportVectorTiles.java
@@ -22,9 +22,11 @@
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import java.io.IOException;
+import java.nio.ByteBuffer;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.StandardOpenOption;
+import java.sql.SQLException;
 import java.util.*;
 import java.util.stream.Collectors;
 import javax.sql.DataSource;
@@ -34,6 +36,7 @@
 import org.apache.baremaps.maplibre.tileset.TilesetQuery;
 import org.apache.baremaps.openstreetmap.stream.ProgressLogger;
 import org.apache.baremaps.openstreetmap.stream.StreamUtils;
+import org.apache.baremaps.postgres.utils.PostgresUtils;
 import org.apache.baremaps.tilestore.*;
 import org.apache.baremaps.tilestore.file.FileTileStore;
 import org.apache.baremaps.tilestore.mbtiles.MBTilesStore;
@@ -146,7 +149,7 @@ public void execute(WorkflowContext context) throws Exception {
 
       var bufferedTileEntryStream = StreamUtils.bufferInCompletionOrder(tileCoordStream, tile -> {
         try {
-          return new TileEntry(tile, sourceTileStore.read(tile));
+          return new TileEntry<>(tile, sourceTileStore.read(tile));
         } catch (TileStoreException e) {
           throw new WorkflowException(e);
         }
@@ -166,11 +169,14 @@ public void execute(WorkflowContext context) throws Exception {
     }
   }
 
-  private TileStore sourceTileStore(Tileset tileset, DataSource datasource) {
-    return new PostgresTileStore(datasource, tileset);
+  private TileStore<ByteBuffer> sourceTileStore(Tileset tileset, DataSource datasource)
+      throws SQLException {
+    var postgresVersion = PostgresUtils.getPostgresVersion(datasource);
+    return new PostgresTileStore(datasource, tileset, postgresVersion);
   }
 
-  private TileStore targetTileStore(Tileset source) throws TileStoreException, IOException {
+  private TileStore<ByteBuffer> targetTileStore(Tileset source)
+      throws TileStoreException, IOException {
     switch (format) {
       case FILE:
         return new FileTileStore(repository.resolve("tiles"));
diff --git a/baremaps-core/src/main/java/org/apache/baremaps/tilestore/postgres/PostgresTileStore.java b/baremaps-core/src/main/java/org/apache/baremaps/tilestore/postgres/PostgresTileStore.java
index b43514593..8c9caf173 100644
--- a/baremaps-core/src/main/java/org/apache/baremaps/tilestore/postgres/PostgresTileStore.java
+++ b/baremaps-core/src/main/java/org/apache/baremaps/tilestore/postgres/PostgresTileStore.java
@@ -22,8 +22,6 @@
 import java.io.OutputStream;
 import java.nio.ByteBuffer;
 import java.sql.ResultSet;
-import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
 import java.util.zip.GZIPOutputStream;
 import javax.sql.DataSource;
 import org.apache.baremaps.maplibre.tileset.Tileset;
@@ -46,22 +44,20 @@ public class PostgresTileStore implements TileStore<ByteBuffer> {
 
   private final Tileset tileset;
 
+  private final int postgresVersion;
+
   /**
    * Constructs a {@code PostgresTileStore}.
    *
    * @param datasource the datasource
    * @param tileset the tileset
    */
-  public PostgresTileStore(DataSource datasource, Tileset tileset) {
+  public PostgresTileStore(DataSource datasource, Tileset tileset, int postgresVersion) {
     this.datasource = datasource;
     this.tileset = tileset;
+    this.postgresVersion = postgresVersion;
   }
 
-  /**
-   * A cache of queries.
-   */
-  private final Map<Integer, Query> cache = new ConcurrentHashMap<>();
-
   /**
    * A record that holds the sql of a prepared statement and the number of parameters.
    *
@@ -76,7 +72,7 @@ public ByteBuffer read(TileCoord tileCoord) throws TileStoreException {
     var start = System.currentTimeMillis();
 
     // Prepare and cache the query
-    var query = cache.computeIfAbsent(tileCoord.z(), z -> prepareQuery(tileset, z));
+    var query = prepareQuery(tileCoord);
 
     // Fetch and compress the tile data
     try (var connection = datasource.getConnection();
@@ -119,14 +115,13 @@ public ByteBuffer read(TileCoord tileCoord) throws TileStoreException {
   }
 
   /**
-   * Prepare the sql query for a given tileset and zoom level.
+   * Prepare the sql query for a given zoom level.
    *
-   * @param tileset the tileset
-   * @param zoom the zoom level
-   * @return
+   * @param tileCoord the tile coordinate
+   * @return the prepared query
    */
   @SuppressWarnings("squid:S3776")
-  protected static Query prepareQuery(Tileset tileset, int zoom) {
+  protected Query prepareQuery(TileCoord tileCoord) {
     // Initialize a builder for the tile sql
     var tileSql = new StringBuilder();
     tileSql.append("SELECT ");
@@ -150,7 +145,7 @@ protected static Query prepareQuery(Tileset tileset, int zoom) {
       for (var query : queries) {
 
         // Only include the sql if the zoom level is in the range
-        if (query.getMinzoom() <= zoom && zoom < query.getMaxzoom()) {
+        if (query.getMinzoom() <= tileCoord.z() && tileCoord.z() < query.getMaxzoom()) {
 
           // Add a union between queries
           if (queryCount > 0) {
@@ -162,28 +157,14 @@ protected static Query prepareQuery(Tileset tileset, int zoom) {
               .replaceAll("\\s+", " ")
               .replace(";", "")
               .replace("?", "??")
-              .replace("$zoom", String.valueOf(zoom));
+              .replace("$zoom", String.valueOf(tileCoord.z()))
+              .replace("$z", String.valueOf(tileCoord.z()))
+              .replace("$x", String.valueOf(tileCoord.x()))
+              .replace("$y", String.valueOf(tileCoord.y()));
 
-          // Append a new condition or a where clause
-          if (querySql.toLowerCase().contains("where")) {
-            querySql += " AND ";
-          } else {
-            querySql += " WHERE ";
-          }
+          var querySqlWithParams =
+              postgresVersion >= 16 ? prepareNewQuery(querySql) : prepareLegacyQuery(querySql);
 
-          // Append the condition to the query sql
-          querySql +=
-              "geom IS NOT NULL AND geom && ST_TileEnvelope(?, ?, ?, margin => (64.0/4096))";
-
-          var querySqlWithParams = String.format(
-              """
-                  SELECT
-                    tile.id AS id,
-                    tile.tags - 'id' AS tags,
-                    ST_AsMVTGeom(tile.geom, ST_TileEnvelope(?, ?, ?)) AS geom
-                  FROM (%s) as tile
-                  """,
-              querySql);
           layerSql.append(querySqlWithParams);
 
           // Increase the parameter count (e.g. ?) and sql count
@@ -222,6 +203,65 @@ protected static Query prepareQuery(Tileset tileset, int zoom) {
     return new Query(sql, paramCount);
   }
 
+  /**
+   * Prepare the sql query for the new versions of postgresql (>= 16).
+   * <p>
+   * Recent versions of the postgresql database better optimize subqueries. Using subqueries is more
+   * robust and allows for more complex queries.
+   *
+   * @param sql the sql query
+   * @return the prepared query
+   */
+  @SuppressWarnings("squid:S3776")
+  private String prepareNewQuery(final String sql) {
+    return String.format(
+        """
+            SELECT
+            mvtData.id AS id,
+            mvtData.tags - 'id' AS tags,
+            ST_AsMVTGeom(mvtData.geom, ST_TileEnvelope(?, ?, ?)) AS geom
+            FROM (%s) AS mvtData
+            WHERE mvtData.geom IS NOT NULL
+            AND mvtData.geom && ST_TileEnvelope(?, ?, ?, margin => (64.0/4096))
+            """,
+        sql);
+  }
+
+  /**
+   * Prepare the sql query for the legacy versions of postgresql (< 16).
+   * <p>
+   * Older versions of the postgresql database do not optimize subqueries. Therefore, the conditions
+   * are appended to the sql query, which is less robust and error-prone.
+   *
+   * @param sql the sql query
+   * @return the prepared query
+   */
+  @SuppressWarnings("squid:S3776")
+  private String prepareLegacyQuery(final String sql) {
+    String query = sql;
+
+    // Append a new condition or a where clause
+    if (sql.toLowerCase().contains("where")) {
+      query += " AND ";
+    } else {
+      query += " WHERE ";
+    }
+
+    // Append the condition to the query sql
+    query +=
+        "geom IS NOT NULL AND geom && ST_TileEnvelope(?, ?, ?, margin => (64.0/4096))";
+
+    return String.format(
+        """
+            SELECT
+              mvtData.id AS id,
+              mvtData.tags - 'id' AS tags,
+              ST_AsMVTGeom(mvtData.geom, ST_TileEnvelope(?, ?, ?)) AS geom
+            FROM (%s) as mvtData
+            """,
+        query);
+  }
+
   /**
    * This operation is not supported.
    */
diff --git a/baremaps-core/src/test/java/org/apache/baremaps/tilestore/postgres/PostgresTileStoreTest.java b/baremaps-core/src/test/java/org/apache/baremaps/tilestore/postgres/PostgresTileStoreTest.java
index bc593c4c8..becaece27 100644
--- a/baremaps-core/src/test/java/org/apache/baremaps/tilestore/postgres/PostgresTileStoreTest.java
+++ b/baremaps-core/src/test/java/org/apache/baremaps/tilestore/postgres/PostgresTileStoreTest.java
@@ -24,13 +24,17 @@
 import org.apache.baremaps.maplibre.tileset.Tileset;
 import org.apache.baremaps.maplibre.tileset.TilesetLayer;
 import org.apache.baremaps.maplibre.tileset.TilesetQuery;
+import org.apache.baremaps.tilestore.TileCoord;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 class PostgresTileStoreTest {
 
-  @Test
-  void prepareQuery() {
-    var tileset = new Tileset();
+  private Tileset tileset;
+
+  @BeforeEach
+  void prepare() {
+    tileset = new Tileset();
     tileset.setMinzoom(0);
     tileset.setMaxzoom(20);
     tileset.setVectorLayers(List.of(
@@ -38,9 +42,24 @@ void prepareQuery() {
             List.of(new TilesetQuery(0, 20, "SELECT id, tags, geom FROM table"))),
         new TilesetLayer("b", Map.of(), "", 0, 20,
             List.of(new TilesetQuery(0, 20, "SELECT id, tags, geom FROM table")))));
-    var query = PostgresTileStore.prepareQuery(tileset, 10);
+
+  }
+
+  @Test
+  void prepareNewQuery() {
+    var postgresTileStore = new PostgresTileStore(null, tileset, 16);
+    var query = postgresTileStore.prepareQuery(new TileCoord(1, 1, 10));
+    assertEquals(
+        "SELECT (SELECT ST_AsMVT(mvtGeom.*, 'a') FROM (SELECT mvtData.id AS id, mvtData.tags - 'id' AS tags, ST_AsMVTGeom(mvtData.geom, ST_TileEnvelope(?, ?, ?)) AS geom FROM (SELECT id, tags, geom FROM table) AS mvtData WHERE mvtData.geom IS NOT NULL AND mvtData.geom && ST_TileEnvelope(?, ?, ?, margin => (64.0/4096)) ) AS mvtGeom) || (SELECT ST_AsMVT(mvtGeom.*, 'b') FROM (SELECT mvtData.id AS id, mvtData.tags - 'id' AS tags, ST_AsMVTGeom(mvtData.geom, ST_TileEnvelope(?, ?, ?)) AS geom FROM (SELECT id, tags, geom FROM table) AS mvtData WHERE mvtData.geom IS NOT NULL AND mvtData.geom && ST_TileEnvelope(?, ?, ?, margin => (64.0/4096)) ) AS mvtGeom) AS mvtTile",
+        query.sql());
+  }
+
+  @Test
+  void prepareLegacyQuery() {
+    var postgresTileStore = new PostgresTileStore(null, tileset, 15);
+    var query = postgresTileStore.prepareQuery(new TileCoord(1, 1, 10));
     assertEquals(
-        "SELECT (SELECT ST_AsMVT(mvtGeom.*, 'a') FROM (SELECT tile.id AS id, tile.tags - 'id' AS tags, ST_AsMVTGeom(tile.geom, ST_TileEnvelope(?, ?, ?)) AS geom FROM (SELECT id, tags, geom FROM table WHERE geom IS NOT NULL AND geom && ST_TileEnvelope(?, ?, ?, margin => (64.0/4096))) as tile ) AS mvtGeom) || (SELECT ST_AsMVT(mvtGeom.*, 'b') FROM (SELECT tile.id AS id, tile.tags - 'id' AS tags, ST_AsMVTGeom(tile.geom, ST_TileEnvelope(?, ?, ?)) AS geom FROM (SELECT id, tags, geom FROM table WHERE geom IS NOT NULL AND geom && ST_TileEnvelope(?, ?, ?, margin => (64.0/4096))) as tile ) AS mvtGeom) AS mvtTile",
+        "SELECT (SELECT ST_AsMVT(mvtGeom.*, 'a') FROM (SELECT mvtData.id AS id, mvtData.tags - 'id' AS tags, ST_AsMVTGeom(mvtData.geom, ST_TileEnvelope(?, ?, ?)) AS geom FROM (SELECT id, tags, geom FROM table WHERE geom IS NOT NULL AND geom && ST_TileEnvelope(?, ?, ?, margin => (64.0/4096))) as mvtData ) AS mvtGeom) || (SELECT ST_AsMVT(mvtGeom.*, 'b') FROM (SELECT mvtData.id AS id, mvtData.tags - 'id' AS tags, ST_AsMVTGeom(mvtData.geom, ST_TileEnvelope(?, ?, ?)) AS geom FROM (SELECT id, tags, geom FROM table WHERE geom IS NOT NULL AND geom && ST_TileEnvelope(?, ?, ?, margin => (64.0/4096))) as mvtData ) AS mvtGeom) AS mvtTile",
         query.sql());
   }
 }
diff --git a/baremaps-postgres/src/main/java/org/apache/baremaps/postgres/utils/PostgresUtils.java b/baremaps-postgres/src/main/java/org/apache/baremaps/postgres/utils/PostgresUtils.java
index c70db2408..a65cdb13f 100644
--- a/baremaps-postgres/src/main/java/org/apache/baremaps/postgres/utils/PostgresUtils.java
+++ b/baremaps-postgres/src/main/java/org/apache/baremaps/postgres/utils/PostgresUtils.java
@@ -178,4 +178,17 @@ public static void executeResource(Connection connection, String resource)
       statement.execute(queries);
     }
   }
+
+  /**
+   * Gets the version of the Postgres database.
+   *
+   * @param datasource the data source
+   * @return the version of the Postgres database
+   * @throws SQLException if a database access error occurs
+   */
+  public static int getPostgresVersion(DataSource datasource) throws SQLException {
+    try (Connection connection = datasource.getConnection()) {
+      return connection.getMetaData().getDatabaseMajorVersion();
+    }
+  }
 }