This is an automated email from the ASF dual-hosted git repository.
morningman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new d39e5a0df3e [fix](jdbc) Map VARBINARY of a remote Doris to VARBINARY
(#68968)
d39e5a0df3e is described below
commit d39e5a0df3e234dcf8cd0e23cfa0954c5fee27a4
Author: Mingyu Chen (Rayner) <[email protected]>
AuthorDate: Tue Oct 13 15:57:01 2026 +0800
[fix](jdbc) Map VARBINARY of a remote Doris to VARBINARY (#68968)
### What problem does this PR solve?
Issue Number: None
Related PR: #62183, #55651, #68786, #56181
Problem Summary:
**In short.** A JDBC catalog that points at another Doris shows the
remote VARBINARY columns as STRING (`text`). This PR maps them to
VARBINARY again, as the fe-core JDBC client and branch-4.x do; a
VARBINARY element of an array stays STRING, because the scanner reads
array elements only as text. Reading a VARBINARY column through a Java
scanner, which the new mapping now does for a remote Doris, has leaked a
native buffer on every batch since VARBINARY was added to the JNI
vectors, so this PR frees it too. It is not a port of a branch-4.x
change; it came up while comparing the binary type mappings of the two
branches.
**Background.** When the JDBC connector became a plugin (#62183), the
Doris dialect of `JdbcMySQLConnectorClient` mapped a remote `VARBINARY`
to `STRING`. The fe-core `JdbcMySQLClient` it replaced, and branch-4.x,
map it to `VARBINARY` with the column size the remote reports (#55651).
For every other JDBC source, binary types already map to `VARBINARY`
regardless of the deprecated `enable.mapping.*` flags (#68786, pinned by
`JdbcLogicalTypeMappingTest`). A remote Doris exposes VARBINARY through
views and external tables.
A remote Doris returns an `ARRAY` column as text, which spells a
VARBINARY element as `0x` hex (`[0x616263, null, 0x]`), and
`MySQLTypeHandler` decodes the elements from that text: into strings,
numbers and dates, not into bytes.
On the BE side, a Java scanner hands each batch over in a `VectorTable`.
A VARBINARY column of that table keeps a 16-byte `[length, address]`
record per row in an off-heap buffer of its own, and each record points
at a separate block with the row's bytes.
**The problem, and what it cost.**
| A JDBC catalog connected to the local Doris | master | with this PR |
|---|---|---|
| `DESC` of a view with `cast('abc' as varbinary(16))` | `b text` | `b
varbinary(65535)` (the length the remote reports through the JDBC
metadata) |
| `SELECT b` | the value as a string | `0x616263`, the remote bytes;
`hex(b)` = `616263` |
| a view with `array(cast('abc' as varbinary(16)), cast(null as
varbinary(16)), cast('' as varbinary(16)), cast(unhex('00ff') as
varbinary(16)))` | `array<text>`, `["0x616263", null, "0x", "0x00FF"]` |
the same |
Mapping the array element to VARBINARY as well, as branch-4.x does,
makes the column `array<varbinary(2147483647)>`, and reading it fails
with `Unsupported array child type: VARBINARY`.
`VectorColumn.close()` freed the row blocks of a VARBINARY column but
never its record buffer (16 bytes per row of batch capacity, about 64 KB
for a batch of 4064 rows). BE closes the Java table after every batch
(the V1 reader through `releaseTable`, the V2 reader through
`releaseColumn` per column), and a Java UDF closes its output table
after every call, so every JNI read of a binary column (the binary types
of every JDBC source, Paimon, Hudi, MaxCompute) and every Java UDF that
returns VARBINARY lost one such buffer per VARBINARY column per batch.
The allocator behind it is plain malloc / free, so nothing reclaims the
buffer.
**How this PR fixes it.**
- The Doris dialect maps `VARBINARY` to `VARBINARY`, taking the length
from the type name when it carries one and otherwise from the column
size, like the dialect's `VARCHAR` branch and the fe-core client.
- A VARBINARY element of a remote Doris array, nested or not, keeps
mapping to `STRING`.
- `VectorColumn.close()` frees the record buffer of a VARBINARY column
after the row blocks it points to.
**Results.**
- FE UT:
`JdbcLogicalTypeMappingTest#remoteDorisVarbinaryKeepsBytesAndDeclaredLength`
fails without the mapping (STRING) and passes with it;
`#remoteDorisVarbinaryArrayElementsStayString` (`ARRAY<VARBINARY>`,
`ARRAY<VARBINARY(16)>`, `ARRAY<ARRAY<VARBINARY(16)>>`) fails when the
array element follows the scalar mapping. All fe-connector-jdbc tests
(251) pass.
- jni-spi UT:
`VectorTableVarbinaryCloseTest#closeFreesTheRowRecordsOfAVarbinaryColumn`
closes a table with a VARBINARY column, as a scanner does after each
batch, and checks that the record buffer is freed; it fails without the
fix. All jni-spi tests (24) pass. jni-spi gains `mockito-inline` in test
scope to observe the static `OffHeap` calls.
- Regression: `test_doris_jdbc_catalog_varbinary` connects a JDBC
catalog to the local Doris and reads a view with a VARBINARY column and
a view with a VARBINARY array. With the plugin from master the scalar
column is described as `text` and the suite fails; with the scalar
mapping alone the array column is described as
`array<varbinary(2147483647)>` and the suite fails; with this PR it
passes.
---
fe/be-java-extensions/jni-spi/pom.xml | 6 ++
.../org/apache/doris/jni/spi/vec/VectorColumn.java | 4 +-
.../jni/spi/vec/VectorTableVarbinaryCloseTest.java | 51 ++++++++++++++++
.../jdbc/client/JdbcMySQLConnectorClient.java | 18 +++++-
.../jdbc/client/JdbcLogicalTypeMappingTest.java | 40 +++++++++++++
.../jdbc/test_doris_jdbc_catalog_varbinary.out | 21 +++++++
.../jdbc/test_doris_jdbc_catalog_varbinary.groovy | 68 ++++++++++++++++++++++
7 files changed, 204 insertions(+), 4 deletions(-)
diff --git a/fe/be-java-extensions/jni-spi/pom.xml
b/fe/be-java-extensions/jni-spi/pom.xml
index 083a1503995..fb206934328 100644
--- a/fe/be-java-extensions/jni-spi/pom.xml
+++ b/fe/be-java-extensions/jni-spi/pom.xml
@@ -81,6 +81,12 @@ under the License.
<artifactId>log4j-1.2-api</artifactId>
<scope>test</scope>
</dependency>
+ <!-- Lets a test observe the static OffHeap calls, such as which
blocks a close frees. -->
+ <dependency>
+ <groupId>org.mockito</groupId>
+ <artifactId>mockito-inline</artifactId>
+ <scope>test</scope>
+ </dependency>
</dependencies>
<build>
diff --git
a/fe/be-java-extensions/jni-spi/src/main/java/org/apache/doris/jni/spi/vec/VectorColumn.java
b/fe/be-java-extensions/jni-spi/src/main/java/org/apache/doris/jni/spi/vec/VectorColumn.java
index db0916218f4..f644faab1a8 100644
---
a/fe/be-java-extensions/jni-spi/src/main/java/org/apache/doris/jni/spi/vec/VectorColumn.java
+++
b/fe/be-java-extensions/jni-spi/src/main/java/org/apache/doris/jni/spi/vec/VectorColumn.java
@@ -283,10 +283,10 @@ public class VectorColumn {
}
if (data != 0) {
if (columnType.isVarbinaryType()) {
+ // data holds a [length, address] record per row; free the row
bytes the records point to first.
freeVarbinaryData();
- } else {
- OffHeap.freeMemory(data);
}
+ OffHeap.freeMemory(data);
}
if (offsets != 0) {
diff --git
a/fe/be-java-extensions/jni-spi/src/test/java/org/apache/doris/jni/spi/vec/VectorTableVarbinaryCloseTest.java
b/fe/be-java-extensions/jni-spi/src/test/java/org/apache/doris/jni/spi/vec/VectorTableVarbinaryCloseTest.java
new file mode 100644
index 00000000000..4d34a1d40eb
--- /dev/null
+++
b/fe/be-java-extensions/jni-spi/src/test/java/org/apache/doris/jni/spi/vec/VectorTableVarbinaryCloseTest.java
@@ -0,0 +1,51 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.jni.spi.vec;
+
+import org.apache.doris.jni.spi.utils.OffHeap;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.mockito.MockedStatic;
+import org.mockito.Mockito;
+
+import java.nio.charset.StandardCharsets;
+
+/**
+ * A VARBINARY column keeps a [length, address] record per row in an off-heap
buffer of its own, apart from the row
+ * bytes the records point to. A scanner closes its table after every batch,
so a close that leaves the buffer behind
+ * loses it once per batch and column.
+ */
+class VectorTableVarbinaryCloseTest {
+
+ @Test
+ void closeFreesTheRowRecordsOfAVarbinaryColumn() {
+ OffHeap.setTesting();
+ VectorTable table = VectorTable.createWritableTable(
+ new ColumnType[] {ColumnType.parseType("b", "varbinary(16)")},
new String[] {"b"}, 4);
+ table.getColumn(0).appendVarbinary(
+ new byte[][] {"abc".getBytes(StandardCharsets.UTF_8), null,
new byte[0]}, true);
+ long records = table.getColumn(0).dataAddress();
+ Assertions.assertNotEquals(0L, records);
+
+ try (MockedStatic<OffHeap> offHeap = Mockito.mockStatic(OffHeap.class,
Mockito.CALLS_REAL_METHODS)) {
+ table.close();
+ offHeap.verify(() -> OffHeap.freeMemory(records));
+ }
+ }
+}
diff --git
a/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/client/JdbcMySQLConnectorClient.java
b/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/client/JdbcMySQLConnectorClient.java
index 992bf5b2b44..5e8e6ae6594 100644
---
a/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/client/JdbcMySQLConnectorClient.java
+++
b/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/client/JdbcMySQLConnectorClient.java
@@ -332,6 +332,11 @@ public class JdbcMySQLConnectorClient extends
JdbcConnectorClient {
if (upperType.startsWith("ARRAY")) {
// ARRAY<INNER_TYPE> -> extract inner type and recursively convert
String innerType = upperType.substring(6, upperType.length() -
1).trim();
+ if (innerType.startsWith("VARBINARY")) {
+ // The remote returns an array as text that spells each
VARBINARY element as 0x hex, and the scanner
+ // decodes array elements only into strings, so such an
element stays STRING.
+ return ConnectorType.arrayOf(ConnectorType.of("STRING"));
+ }
return ConnectorType.arrayOf(dorisTypeToConnectorType(innerType,
null));
}
@@ -420,8 +425,17 @@ public class JdbcMySQLConnectorClient extends
JdbcConnectorClient {
return ConnectorType.of("BITMAP");
case "QUANTILE_STATE":
return ConnectorType.of("QUANTILE_STATE");
- case "VARBINARY":
- return ConnectorType.of("STRING");
+ case "VARBINARY": {
+ // Keep the remote Doris bytes and length, like the binary
types of every other source.
+ if (openParen == -1) {
+ if (fieldInfo != null) {
+ return ConnectorType.of("VARBINARY",
fieldInfo.requiredColumnSize(), -1);
+ }
+ return ConnectorType.of("VARBINARY");
+ }
+ int len = Integer.parseInt(upperType.substring(openParen + 1,
upperType.length() - 1));
+ return ConnectorType.of("VARBINARY", len, -1);
+ }
default:
return ConnectorType.of("UNSUPPORTED");
}
diff --git
a/fe/fe-connector/fe-connector-jdbc/src/test/java/org/apache/doris/connector/jdbc/client/JdbcLogicalTypeMappingTest.java
b/fe/fe-connector/fe-connector-jdbc/src/test/java/org/apache/doris/connector/jdbc/client/JdbcLogicalTypeMappingTest.java
index a156d2f0002..bb5971b4770 100644
---
a/fe/fe-connector/fe-connector-jdbc/src/test/java/org/apache/doris/connector/jdbc/client/JdbcLogicalTypeMappingTest.java
+++
b/fe/fe-connector/fe-connector-jdbc/src/test/java/org/apache/doris/connector/jdbc/client/JdbcLogicalTypeMappingTest.java
@@ -23,6 +23,7 @@ import org.apache.doris.connector.spi.ConnectorType;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
+import java.lang.reflect.Field;
import java.sql.Types;
import java.util.Collections;
import java.util.Optional;
@@ -48,6 +49,30 @@ class JdbcLogicalTypeMappingTest {
}
}
+ @Test
+ void remoteDorisVarbinaryKeepsBytesAndDeclaredLength() throws
ReflectiveOperationException {
+ for (boolean flag : new boolean[] {false, true}) {
+ assertVarbinary(remoteDoris(flag), "VARBINARY");
+ }
+ }
+
+ @Test
+ void remoteDorisVarbinaryArrayElementsStayString() throws
ReflectiveOperationException {
+ // The scanner decodes the elements of a remote Doris array only into
strings, so a VARBINARY element,
+ // nested or not, keeps the STRING it had before the top-level
VARBINARY mapping.
+ for (boolean flag : new boolean[] {false, true}) {
+ JdbcConnectorClient doris = remoteDoris(flag);
+ for (String typeName : new String[] {"ARRAY<VARBINARY>",
"ARRAY<VARBINARY(16)>"}) {
+ ConnectorType type =
doris.jdbcTypeToConnectorType(arrayField(typeName));
+ Assertions.assertEquals("ARRAY", type.getTypeName(), typeName);
+ Assertions.assertEquals("STRING",
type.getChildren().get(0).getTypeName(), typeName);
+ }
+ ConnectorType nested =
doris.jdbcTypeToConnectorType(arrayField("ARRAY<ARRAY<VARBINARY(16)>>"));
+ Assertions.assertEquals("ARRAY",
nested.getChildren().get(0).getTypeName());
+ Assertions.assertEquals("STRING",
nested.getChildren().get(0).getChildren().get(0).getTypeName());
+ }
+ }
+
@Test
void instantTimestampsMapToTimestamptzWhileLocalTimestampsStayDatetime() {
for (boolean flag : new boolean[] {false, true}) {
@@ -88,6 +113,21 @@ class JdbcLogicalTypeMappingTest {
Assertions.assertFalse(local.isAllowNull());
}
+ private static JdbcConnectorClient remoteDoris(boolean flag) throws
ReflectiveOperationException {
+ JdbcMySQLConnectorClient doris = new JdbcMySQLConnectorClient("test",
JdbcDbType.MYSQL,
+ "jdbc:mysql://localhost/test", false, Collections.emptyMap(),
Collections.emptyMap(), flag, flag);
+ // A remote Doris is detected through the connection, which a unit
test does not have.
+ Field isDoris =
JdbcMySQLConnectorClient.class.getDeclaredField("isDoris");
+ isDoris.setAccessible(true);
+ isDoris.setBoolean(doris, true);
+ return doris;
+ }
+
+ private static JdbcFieldInfo arrayField(String typeName) {
+ return new JdbcFieldInfo("a", Optional.of(typeName), Types.OTHER,
+ Optional.of(65535), Optional.of(0), Optional.empty());
+ }
+
private static JdbcConnectorClient postgres(boolean flag) {
return new JdbcPostgreSQLConnectorClient("test",
JdbcDbType.POSTGRESQL, "jdbc:postgresql://localhost/test",
false, Collections.emptyMap(), Collections.emptyMap(), flag,
flag);
diff --git
a/regression-test/data/external_table_p0/jdbc/test_doris_jdbc_catalog_varbinary.out
b/regression-test/data/external_table_p0/jdbc/test_doris_jdbc_catalog_varbinary.out
new file mode 100644
index 00000000000..7439a7ba9e2
--- /dev/null
+++
b/regression-test/data/external_table_p0/jdbc/test_doris_jdbc_catalog_varbinary.out
@@ -0,0 +1,21 @@
+-- This file is automatically generated. You should know what you did if you
want to edit this
+-- !internal --
+1 0x616263
+2 \N
+
+-- !desc --
+b varbinary(65535) Yes true \N
+id tinyint No true \N
+
+-- !select --
+1 0x616263 616263
+2 \N \N
+
+-- !array_desc --
+a array<text> Yes true \N
+id tinyint No true \N
+
+-- !array_select --
+1 ["0x616263", null, "0x"] 0x616263
+2 \N \N
+
diff --git
a/regression-test/suites/external_table_p0/jdbc/test_doris_jdbc_catalog_varbinary.groovy
b/regression-test/suites/external_table_p0/jdbc/test_doris_jdbc_catalog_varbinary.groovy
new file mode 100644
index 00000000000..2a32e06dad6
--- /dev/null
+++
b/regression-test/suites/external_table_p0/jdbc/test_doris_jdbc_catalog_varbinary.groovy
@@ -0,0 +1,68 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+// A JDBC catalog that points at another Doris keeps the remote VARBINARY
columns as VARBINARY with their
+// declared length, like the binary types of every other JDBC source; the
plugin connector used to map them to
+// STRING. A remote Doris exposes VARBINARY through views and external tables,
so the remote side is a view.
+suite("test_doris_jdbc_catalog_varbinary", "p0,external") {
+ String jdbcUrl = context.config.jdbcUrl
+ String jdbcUser = context.config.jdbcUser
+ String jdbcPassword = context.config.jdbcPassword
+ String s3_endpoint = getS3Endpoint()
+ String bucket = getS3BucketName()
+ String driver_url =
"https://${bucket}.${s3_endpoint}/regression/jdbc_driver/mysql-connector-j-8.4.0.jar";
+ String catalog_name = "test_doris_jdbc_catalog_varbinary"
+ String internal_db_name = "regression_test_jdbc_catalog_varbinary"
+
+ sql """create database if not exists ${internal_db_name}"""
+ sql """drop view if exists ${internal_db_name}.varbinary_view"""
+ sql """
+ create view ${internal_db_name}.varbinary_view as
+ select 1 as id, cast('abc' as varbinary(16)) as b
+ union all
+ select 2, cast(null as varbinary(16))
+ """
+ order_qt_internal """select id, b from
internal.${internal_db_name}.varbinary_view order by id"""
+ sql """drop view if exists ${internal_db_name}.varbinary_array_view"""
+ sql """
+ create view ${internal_db_name}.varbinary_array_view as
+ select 1 as id, array(cast('abc' as varbinary(16)), cast(null as
varbinary(16)), cast('' as varbinary(16))) as a
+ union all
+ select 2, cast(null as array<varbinary(16)>)
+ """
+
+ sql """drop catalog if exists ${catalog_name}"""
+ sql """CREATE CATALOG `${catalog_name}` PROPERTIES (
+ "user" = "${jdbcUser}",
+ "type" = "jdbc",
+ "password" = "${jdbcPassword}",
+ "jdbc_url" = "${jdbcUrl}",
+ "driver_url" = "${driver_url}",
+ "driver_class" = "com.mysql.cj.jdbc.Driver"
+ )"""
+
+ // The remote VARBINARY column stays VARBINARY instead of becoming STRING;
its length is the one the remote
+ // reports through the JDBC metadata.
+ order_qt_desc """desc ${catalog_name}.${internal_db_name}.varbinary_view"""
+ // The bytes read through the catalog are the remote ones.
+ order_qt_select """select id, b, hex(b) from
${catalog_name}.${internal_db_name}.varbinary_view order by id"""
+
+ // The remote returns an array as text that spells each VARBINARY element
as 0x hex, and the scanner decodes
+ // array elements only into strings, so a VARBINARY element stays STRING
and reads as that text.
+ order_qt_array_desc """desc
${catalog_name}.${internal_db_name}.varbinary_array_view"""
+ order_qt_array_select """select id, a, a[1] from
${catalog_name}.${internal_db_name}.varbinary_array_view order by id"""
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
