Address the review comments

bakwc · Sep 12, 2023 · dddea92 · dddea92
1 parent 611a75a
commit dddea92
Show file tree

Hide file tree

Showing 16 changed files with 459 additions and 460 deletions.
diff --git a/docker/test/integration/mysql_java_client/Dockerfile b/docker/test/integration/mysql_java_client/Dockerfile
@@ -1,16 +1,9 @@
 # docker build -t clickhouse/mysql-java-client .
 # MySQL Java client docker container
 
-FROM ubuntu:22.04
+FROM openjdk:8-jdk-alpine
 
-RUN apt-get update && \
-    apt-get install -y software-properties-common build-essential openjdk-8-jdk curl
-
-RUN rm -rf \
-        /var/lib/apt/lists/* \
-        /var/cache/debconf \
-        /tmp/* \
-RUN apt-get clean
+RUN apk --no-cache add curl
 
 ARG ver=8.1.0
 RUN curl -L -o /mysql-connector-j-${ver}.jar https://repo1.maven.org/maven2/com/mysql/mysql-connector-j/${ver}/mysql-connector-j-${ver}.jar

diff --git a/docker/test/integration/mysql_java_client/PreparedStatementsTest.java b/docker/test/integration/mysql_java_client/PreparedStatementsTest.java
@@ -33,8 +33,8 @@ public static void main(String[] args) {
             }
         }
 
-        // useServerPrepStmts allows us to send COM_STMT_PREPARE and COM_STMT_EXECUTE
-        // to test the binary protocol implementation
+        // useServerPrepStmts uses COM_STMT_PREPARE and COM_STMT_EXECUTE
+        // instead of COM_QUERY which allows us to test the binary protocol
         String jdbcUrl = String.format("jdbc:mysql://%s:%s/%s?useSSL=false&useServerPrepStmts=true",
                 host, port, database);
 
@@ -174,4 +174,5 @@ private static String getMysqlType(ResultSet rs, String columnLabel) throws SQLE
         return String.format("%s type is %s", columnLabel,
                 MysqlType.getByJdbcType(meta.getColumnType(rs.findColumn(columnLabel))));
     }
+
 }
diff --git a/docker/test/integration/runner/compose/docker_compose_mysql_java_client.yml b/docker/test/integration/runner/compose/docker_compose_mysql_java_client.yml
@@ -3,4 +3,4 @@ services:
   java1:
     image: clickhouse/mysql-java-client:${DOCKER_MYSQL_JAVA_CLIENT_TAG:-latest}
     # to keep container running
-    command: sleep infinity
+    command: sleep 1d
diff --git a/docs/en/sql-reference/statements/show.md b/docs/en/sql-reference/statements/show.md
@@ -208,7 +208,7 @@ The optional keyword `FULL` causes the output to include the collation, comment
 The statement produces a result table with the following structure:
 - field - The name of the column (String)
 - type - The column data type (String)
-- null - If the column data type is Nullable (UInt8)
+- null - If the column data type is Nullable (String)
 - key - `PRI` if the column is part of the primary key, `SOR` if the column is part of the sorting key, empty otherwise (String)
 - default - Default expression of the column if it is of type `ALIAS`, `DEFAULT`, or `MATERIALIZED`, otherwise `NULL`. (Nullable(String))
 - extra - Additional information, currently unused (String)

diff --git a/src/Core/MySQL/PacketsPreparedStatements.cpp b/src/Core/MySQL/PacketsPreparedStatements.cpp
@@ -2,33 +2,41 @@
 #include <IO/WriteHelpers.h>
 #include <Common/logger_useful.h>
 
+
 namespace DB
 {
 namespace MySQLProtocol
 {
-    namespace PreparedStatements
-    {
-        size_t PrepareStatementResponseOK::getPayloadSize() const
-        {
-            return 13;
-        }
+namespace PreparedStatements
+{
+size_t PreparedStatementResponseOK::getPayloadSize() const
+{
+    // total = 13
+    return 1 // status
+        + 4 // statement_id
+        + 2 // num_columns
+        + 2 // num_params
+        + 1 // reserved_1 (filler)
+        + 2 // warnings_count
+        + 1; // metadata_follows
+}
 
-        void PrepareStatementResponseOK::writePayloadImpl(WriteBuffer & buffer) const
-        {
-            buffer.write(reinterpret_cast<const char *>(&status), 1);
-            buffer.write(reinterpret_cast<const char *>(&statement_id), 4);
-            buffer.write(reinterpret_cast<const char *>(&num_columns), 2);
-            buffer.write(reinterpret_cast<const char *>(&num_params), 2);
-            buffer.write(reinterpret_cast<const char *>(&reserved_1), 1);
-            buffer.write(reinterpret_cast<const char *>(&warnings_count), 2);
-            buffer.write(0x0); // RESULTSET_METADATA_NONE
-        }
+void PreparedStatementResponseOK::writePayloadImpl(WriteBuffer & buffer) const
+{
+    buffer.write(reinterpret_cast<const char *>(&status), 1);
+    buffer.write(reinterpret_cast<const char *>(&statement_id), 4);
+    buffer.write(reinterpret_cast<const char *>(&num_columns), 2);
+    buffer.write(reinterpret_cast<const char *>(&num_params), 2);
+    buffer.write(reinterpret_cast<const char *>(&reserved_1), 1);
+    buffer.write(reinterpret_cast<const char *>(&warnings_count), 2);
+    buffer.write(0x0); // RESULTSET_METADATA_NONE
+}
 
-        PrepareStatementResponseOK::PrepareStatementResponseOK(
-            uint32_t statement_id_, uint16_t num_columns_, uint16_t num_params_, uint16_t warnings_count_)
-            : statement_id(statement_id_), num_columns(num_columns_), num_params(num_params_), warnings_count(warnings_count_)
-        {
-        }
-    }
+PreparedStatementResponseOK::PreparedStatementResponseOK(
+    uint32_t statement_id_, uint16_t num_columns_, uint16_t num_params_, uint16_t warnings_count_)
+    : statement_id(statement_id_), num_columns(num_columns_), num_params(num_params_), warnings_count(warnings_count_)
+{
+}
+}
 }
 }
diff --git a/src/Core/MySQL/PacketsPreparedStatements.h b/src/Core/MySQL/PacketsPreparedStatements.h
@@ -6,27 +6,27 @@ namespace DB
 {
 namespace MySQLProtocol
 {
-    namespace PreparedStatements
-    {
-        // https://dev.mysql.com/doc/dev/mysql-server/latest/page_protocol_com_stmt_prepare.html#sect_protocol_com_stmt_prepare_response_ok
-        class PrepareStatementResponseOK : public IMySQLWritePacket
-        {
-        public:
-            uint8_t status = 0x00;
-            uint32_t statement_id;
-            uint16_t num_columns;
-            uint16_t num_params;
-            uint8_t reserved_1 = 0;
-            uint16_t warnings_count;
+namespace PreparedStatements
+{
+// https://dev.mysql.com/doc/dev/mysql-server/latest/page_protocol_com_stmt_prepare.html#sect_protocol_com_stmt_prepare_response_ok
+class PreparedStatementResponseOK : public IMySQLWritePacket
+{
+public:
+    const uint8_t status = 0x00;
+    uint32_t statement_id;
+    uint16_t num_columns;
+    uint16_t num_params;
+    const uint8_t reserved_1 = 0;
+    uint16_t warnings_count;
 
-        protected:
-            size_t getPayloadSize() const override;
+protected:
+    size_t getPayloadSize() const override;
 
-            void writePayloadImpl(WriteBuffer & buffer) const override;
+    void writePayloadImpl(WriteBuffer & buffer) const override;
 
-        public:
-            PrepareStatementResponseOK(uint32_t statement_id_, uint16_t num_columns_, uint16_t num_params_, uint16_t warnings_count_);
-        };
-    }
+public:
+    PreparedStatementResponseOK(uint32_t statement_id_, uint16_t num_columns_, uint16_t num_params_, uint16_t warnings_count_);
+};
+}
 }
 }