morningman commented on code in PR #68968:
URL: https://github.com/apache/doris/pull/68968#discussion_r4254757169
##########
fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/client/JdbcMySQLConnectorClient.java:
##########
@@ -420,8 +420,17 @@ private ConnectorType dorisTypeToConnectorType(String
rawType, JdbcFieldInfo fie
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);
Review Comment:
Fixed in a9f0018c405, by not publishing a type the scanner cannot read: a
VARBINARY element of a remote Doris array, nested or not, stays STRING, which
is what it was before this PR, and only a top-level VARBINARY maps to
VARBINARY. The remote spells each element as 0x hex in the array text
(`[0x616263, null, 0x]`), so the element reads as that text, as on master.
On a local cluster, through a JDBC catalog over the same Doris, a view with
`array(cast('abc' as varbinary(16)), cast(null as varbinary(16)), cast('' as
varbinary(16)), cast(unhex('00ff') as varbinary(16)))`:
| JDBC plugin | `DESC` | `SELECT a` |
|---|---|---|
| master | `array<text>` | `["0x616263", null, "0x", "0x00FF"]` |
| 9ab5ba2a1dc | `array<varbinary(2147483647)>` | `Unsupported array child
type: VARBINARY` |
| a9f0018c405 | `array<text>` | `["0x616263", null, "0x", "0x00FF"]` |
Decoding the elements into bytes would be new: branch-4.x maps such a column
to ARRAY<VARBINARY> and then fails the same way when it is read. Tests:
`JdbcLogicalTypeMappingTest#remoteDorisVarbinaryArrayElementsStayString` (also
`ARRAY<ARRAY<VARBINARY(16)>>`), and `test_doris_jdbc_catalog_varbinary` now
reads such a view.
##########
fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/client/JdbcMySQLConnectorClient.java:
##########
@@ -420,8 +420,17 @@ private ConnectorType dorisTypeToConnectorType(String
rawType, JdbcFieldInfo fie
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);
Review Comment:
Fixed in 370410c3731: `VectorColumn.close()` now frees the `[length,
address]` record buffer of a VARBINARY column after the row bytes the records
point to. The leak was not specific to remote Doris. Every writable VARBINARY
column has dropped that buffer since the type was added (#56181), so every JNI
scan of a binary column (the binary types of every JDBC source, and Paimon,
Hudi and MaxCompute binary columns) lost 16 bytes per row of batch capacity per
column on each batch, and so did a Java UDF that returns VARBINARY.
`VectorTableVarbinaryCloseTest` closes a table the way the scanner does after
each batch and checks that the buffer is freed; it fails without the fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
