From ab0c5fad51d655cc3befd6ba5faf5d6d42ff7801 Mon Sep 17 00:00:00 2001
From: "Mateusz \"Serafin\" Gajewski" <mateusz.gajewski@gmail.com>
Date: Tue, 16 Apr 2024 13:26:09 +0200
Subject: [PATCH] Update to Phoenix 5.2.0

---
 plugin/trino-phoenix5/pom.xml                 | 94 +++++++------------
 .../trino/plugin/phoenix5/PhoenixClient.java  |  2 +-
 .../plugin/phoenix5/PhoenixClientModule.java  |  5 +-
 .../plugin/phoenix5/PhoenixSplitManager.java  |  2 +-
 pom.xml                                       | 11 +--
 5 files changed, 39 insertions(+), 75 deletions(-)

diff --git a/plugin/trino-phoenix5/pom.xml b/plugin/trino-phoenix5/pom.xml
index 18aa2d7ef050..655b45dddf96 100644
--- a/plugin/trino-phoenix5/pom.xml
+++ b/plugin/trino-phoenix5/pom.xml
@@ -15,7 +15,8 @@
 
     <properties>
         <air.main.basedir>${project.parent.basedir}</air.main.basedir>
-        <dep.hbase.version>2.2.7</dep.hbase.version>
+        <dep.hbase.version>2.5.8</dep.hbase.version>
+        <dep.phoenix5.version>5.2.0</dep.phoenix5.version>
 
         <!-- This is required for JDK 17 to start HBase server due to illegal reflective access -->
         <air.test.jvm.additional-arguments>${air.test.jvm.additional-arguments.default} --add-opens=java.base/sun.nio.ch=ALL-UNNAMED --add-opens=java.base/java.nio=ALL-UNNAMED</air.test.jvm.additional-arguments>
@@ -57,23 +58,17 @@
             <artifactId>trino-base-jdbc</artifactId>
         </dependency>
 
-        <!-- TODO(https://github.com/trinodb/trino/issues/13051): Use org.apache.phoenix:phoenix-client-embedded-hbase-2.4:5.2.0 instead when Phoenix 5.2 is released -->
         <dependency>
             <groupId>io.trino</groupId>
-            <artifactId>trino-phoenix5-patched</artifactId>
+            <artifactId>trino-plugin-toolkit</artifactId>
             <exclusions>
                 <exclusion>
-                    <groupId>org.apache.phoenix</groupId>
-                    <artifactId>*</artifactId>
+                    <groupId>org.slf4j</groupId>
+                    <artifactId>slf4j-api</artifactId>
                 </exclusion>
             </exclusions>
         </dependency>
 
-        <dependency>
-            <groupId>io.trino</groupId>
-            <artifactId>trino-plugin-toolkit</artifactId>
-        </dependency>
-
         <dependency>
             <groupId>jakarta.annotation</groupId>
             <artifactId>jakarta.annotation-api</artifactId>
@@ -89,6 +84,12 @@
             <artifactId>joda-time</artifactId>
         </dependency>
 
+        <dependency>
+            <groupId>org.apache.phoenix</groupId>
+            <artifactId>phoenix-client-embedded-hbase-2.5.0</artifactId>
+            <version>${dep.phoenix5.version}</version>
+        </dependency>
+
         <dependency>
             <groupId>org.weakref</groupId>
             <artifactId>jmxutils</artifactId>
@@ -130,16 +131,6 @@
             <scope>provided</scope>
         </dependency>
 
-        <!-- TODO: This explicit dependency can be removed once we update to Phoenix 5.1.4 which includes
-        https://github.com/apache/phoenix/commit/cd35c5239bd7e3f263d3f2a99ce1b996205bde4b -->
-        <!-- Phoenix depends on log4j api and since it's banned, let's use hardened API-compatible fork -->
-        <dependency>
-            <groupId>ch.qos.reload4j</groupId>
-            <artifactId>reload4j</artifactId>
-            <version>1.2.25</version>
-            <scope>runtime</scope>
-        </dependency>
-
         <dependency>
             <groupId>com.fasterxml.jackson.core</groupId>
             <artifactId>jackson-databind</artifactId>
@@ -162,10 +153,6 @@
                     <groupId>org.slf4j</groupId>
                     <artifactId>jcl-over-slf4j</artifactId>
                 </exclusion>
-                <exclusion>
-                    <groupId>org.slf4j</groupId>
-                    <artifactId>log4j-over-slf4j</artifactId>
-                </exclusion>
                 <exclusion>
                     <groupId>org.slf4j</groupId>
                     <artifactId>slf4j-api</artifactId>
@@ -251,6 +238,13 @@
             <scope>test</scope>
         </dependency>
 
+        <dependency>
+            <groupId>javax.servlet</groupId>
+            <artifactId>javax.servlet-api</artifactId>
+            <version>4.0.1</version>
+            <scope>test</scope>
+        </dependency>
+
         <dependency>
             <groupId>org.apache.hadoop</groupId>
             <artifactId>hadoop-hdfs</artifactId>
@@ -386,47 +380,25 @@
                 <groupId>org.basepom.maven</groupId>
                 <artifactId>duplicate-finder-maven-plugin</artifactId>
                 <configuration>
-                    <ignoredResourcePatterns>
+                    <ignoredResourcePatterns combine.children="append">
                         <ignoredResourcePattern>mrapp-generated-classpath</ignoredResourcePattern>
-                        <!-- org.apache.commons:commons-math3 french localization file duplicate-->
-                        <ignoredResourcePattern>assets/org/apache/commons/math3/exception/util/LocalizedFormats_fr.properties</ignoredResourcePattern>
-                        <!-- io.airlift:joni and phoenix-client's org.jruby.joni:joni resource duplicates-->
-                        <ignoredResourcePattern>tables/.*\.bin</ignoredResourcePattern>
-                        <ignoredResourcePattern>jetty-dir.css</ignoredResourcePattern>
                     </ignoredResourcePatterns>
-                    <ignoredDependencies>
-                        <dependency>
-                            <groupId>com.clearspring.analytics</groupId>
-                            <artifactId>stream</artifactId>
-                        </dependency>
-                    </ignoredDependencies>
+                </configuration>
+            </plugin>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-enforcer-plugin</artifactId>
+                <configuration>
+                    <rules>
+                        <bannedDependencies>
+                            <includes combine.children="append">
+                                <!-- needed by embedded hbase -->
+                                <include>javax.servlet:javax.servlet-api</include>
+                            </includes>
+                        </bannedDependencies>
+                    </rules>
                 </configuration>
             </plugin>
         </plugins>
     </build>
-
-    <!--
-      TODO(https://github.com/trinodb/trino/issues/13051): convert this back into normal dependency
-      The Phoenix code actually comes from trino-phoenix5-patched dependency, but IntelliJ does not support multi-module
-      projects with module using shading (as trino-phoenix5-patched does). See e.g. https://youtrack.jetbrains.com/issue/IDEA-266746
-      This dependency is here only to fool IntelliJ into compiling the project. -->
-    <profiles>
-        <profile>
-            <id>lib/tools.jar</id>
-            <activation>
-                <property>
-                    <name>idea.maven.embedder.version</name>
-                </property>
-            </activation>
-            <dependencies>
-                <dependency>
-                    <groupId>org.apache.phoenix</groupId>
-                    <artifactId>phoenix-client-embedded-hbase-2.2</artifactId>
-                    <version>5.1.3</version>
-                    <scope>provided</scope>
-                    <optional>true</optional>
-                </dependency>
-            </dependencies>
-        </profile>
-    </profiles>
 </project>
diff --git a/plugin/trino-phoenix5/src/main/java/io/trino/plugin/phoenix5/PhoenixClient.java b/plugin/trino-phoenix5/src/main/java/io/trino/plugin/phoenix5/PhoenixClient.java
index 0f7b55cffc22..068447e6e0a1 100644
--- a/plugin/trino-phoenix5/src/main/java/io/trino/plugin/phoenix5/PhoenixClient.java
+++ b/plugin/trino-phoenix5/src/main/java/io/trino/plugin/phoenix5/PhoenixClient.java
@@ -209,7 +209,7 @@
 import static java.util.stream.Collectors.joining;
 import static java.util.stream.Collectors.toSet;
 import static org.apache.hadoop.hbase.HConstants.FOREVER;
-import static org.apache.phoenix.coprocessor.BaseScannerRegionObserver.SKIP_REGION_BOUNDARY_CHECK;
+import static org.apache.phoenix.coprocessorclient.BaseScannerRegionObserverConstants.SKIP_REGION_BOUNDARY_CHECK;
 import static org.apache.phoenix.util.PhoenixRuntime.getTable;
 import static org.apache.phoenix.util.SchemaUtil.ESCAPE_CHARACTER;
 import static org.apache.phoenix.util.SchemaUtil.getEscapedArgument;
diff --git a/plugin/trino-phoenix5/src/main/java/io/trino/plugin/phoenix5/PhoenixClientModule.java b/plugin/trino-phoenix5/src/main/java/io/trino/plugin/phoenix5/PhoenixClientModule.java
index 2164885a16af..6639b6f87899 100644
--- a/plugin/trino-phoenix5/src/main/java/io/trino/plugin/phoenix5/PhoenixClientModule.java
+++ b/plugin/trino-phoenix5/src/main/java/io/trino/plugin/phoenix5/PhoenixClientModule.java
@@ -62,8 +62,8 @@
 import io.trino.spi.connector.ConnectorSplitManager;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.Path;
+import org.apache.phoenix.jdbc.ConnectionInfo;
 import org.apache.phoenix.jdbc.PhoenixDriver;
-import org.apache.phoenix.jdbc.PhoenixEmbeddedDriver;
 
 import java.sql.SQLException;
 import java.util.Map;
@@ -82,6 +82,7 @@
 import static io.trino.plugin.phoenix5.PhoenixClient.DEFAULT_DOMAIN_COMPACTION_THRESHOLD;
 import static io.trino.plugin.phoenix5.PhoenixErrorCode.PHOENIX_CONFIG_ERROR;
 import static java.util.Objects.requireNonNull;
+import static org.apache.phoenix.util.ReadOnlyProps.EMPTY_PROPS;
 import static org.weakref.jmx.guice.ExportBinder.newExporter;
 
 public class PhoenixClientModule
@@ -195,7 +196,7 @@ public static Properties getConnectionProperties(PhoenixConfig config)
             connectionProperties.setProperty(entry.getKey(), entry.getValue());
         }
 
-        PhoenixEmbeddedDriver.ConnectionInfo connectionInfo = PhoenixEmbeddedDriver.ConnectionInfo.create(config.getConnectionUrl());
+        ConnectionInfo connectionInfo = ConnectionInfo.create(config.getConnectionUrl(), EMPTY_PROPS, new Properties());
         connectionInfo.asProps().asMap().forEach(connectionProperties::setProperty);
         return connectionProperties;
     }
diff --git a/plugin/trino-phoenix5/src/main/java/io/trino/plugin/phoenix5/PhoenixSplitManager.java b/plugin/trino-phoenix5/src/main/java/io/trino/plugin/phoenix5/PhoenixSplitManager.java
index c4049636b315..a4f45eea650b 100644
--- a/plugin/trino-phoenix5/src/main/java/io/trino/plugin/phoenix5/PhoenixSplitManager.java
+++ b/plugin/trino-phoenix5/src/main/java/io/trino/plugin/phoenix5/PhoenixSplitManager.java
@@ -53,7 +53,7 @@
 import static io.trino.plugin.phoenix5.PhoenixErrorCode.PHOENIX_SPLIT_ERROR;
 import static java.util.Objects.requireNonNull;
 import static java.util.stream.Collectors.toList;
-import static org.apache.phoenix.coprocessor.BaseScannerRegionObserver.EXPECTED_UPPER_REGION_KEY;
+import static org.apache.phoenix.coprocessorclient.BaseScannerRegionObserverConstants.EXPECTED_UPPER_REGION_KEY;
 
 public class PhoenixSplitManager
         implements ConnectorSplitManager
diff --git a/pom.xml b/pom.xml
index 2b2b488f3c33..53760bc1bc3e 100644
--- a/pom.xml
+++ b/pom.xml
@@ -54,8 +54,6 @@
         <module>lib/trino-memory-context</module>
         <module>lib/trino-orc</module>
         <module>lib/trino-parquet</module>
-        <!-- TODO(https://github.com/trinodb/trino/issues/13051): Remove whole module when Phoenix5 is released -->
-        <module>lib/trino-phoenix5-patched</module>
         <module>lib/trino-plugin-toolkit</module>
         <module>lib/trino-record-decoder</module>
         <module>plugin/trino-accumulo</module>
@@ -188,7 +186,7 @@
         <dep.aws-sdk.version>1.12.701</dep.aws-sdk.version>
         <dep.cassandra.version>4.17.0</dep.cassandra.version>
         <dep.confluent.version>7.5.1</dep.confluent.version>
-        <dep.docker.images.version>93</dep.docker.images.version>
+        <dep.docker.images.version>94</dep.docker.images.version>
         <dep.drift.version>1.21</dep.drift.version>
         <dep.errorprone.version>2.26.1</dep.errorprone.version>
         <dep.flyway.version>10.11.0</dep.flyway.version>
@@ -1284,13 +1282,6 @@
                 <version>${project.version}</version>
             </dependency>
 
-            <!-- TODO(https://github.com/trinodb/trino/issues/13051): Remove when Phoenix5 is released -->
-            <dependency>
-                <groupId>io.trino</groupId>
-                <artifactId>trino-phoenix5-patched</artifactId>
-                <version>${project.version}</version>
-            </dependency>
-
             <dependency>
                 <groupId>io.trino</groupId>
                 <artifactId>trino-pinot</artifactId>