Copilot commented on code in PR #4303:
URL: https://github.com/apache/flink-cdc/pull/4303#discussion_r2893990746
##########
docs/content.zh/docs/connectors/pipeline-connectors/starrocks.md:
##########
@@ -337,6 +337,21 @@ pipeline:
<td>CDC 中长度表示字符数，而 StarRocks 中长度表示字节数。根据 UTF-8 编码，一个中文字符占用三个字节，因此 CDC
中的长度对应到 StarRocks
中为 n * 3。</td>
</tr>
+ <tr>
+ <td>BINARY(n)</td>
+ <td>VARBINARY(n)</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>VARBINARY(n)</td>
+ <td>VARBINARY(n)</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>BYTES</td>
+ <td>VARBINARY(1048576)</td>
+ <td>BYTES 映射为最大长度为 1048576 的 VARBINARY。</td>
+ </tr>
Review Comment:
The mapping rows for `BINARY(n)`/`VARBINARY(n)` state `VARBINARY(n)` without
qualification, but the code caps binary lengths at `MAX_VARBINARY_SIZE`
(1048576). Please update this table to describe the cap for `n > 1048576` so
users aren't misled about the resulting StarRocks column definition.
##########
flink-cdc-connect/flink-cdc-pipeline-connectors/flink-cdc-pipeline-connector-starrocks/src/main/java/org/apache/flink/cdc/connectors/starrocks/sink/StarRocksUtils.java:
##########
@@ -273,6 +279,7 @@ fieldPos, getPrecision(fieldType))
public static final String STRING = "STRING";
public static final String DATE = "DATE";
public static final String DATETIME = "DATETIME";
+ public static final String VARBINARY_TYPE = "VARBINARY";
Review Comment:
The new constant name `VARBINARY_TYPE` is inconsistent with the other
StarRocks type constants in this class (e.g., `VARCHAR`, `DATE`, `DATETIME`),
which are named directly after the StarRocks type string. Consider renaming it
to `VARBINARY` for consistency and to reduce cognitive overhead when scanning
the mappings.
```suggestion
public static final String VARBINARY = "VARBINARY";
```
##########
docs/content/docs/connectors/pipeline-connectors/starrocks.md:
##########
@@ -348,6 +348,21 @@ pipeline:
<td>CDC defines the length by characters, and StarRocks defines it by
bytes. According to UTF-8, one Chinese
character is equal to three bytes, so the length for StarRocks is n *
3.</td>
</tr>
+ <tr>
+ <td>BINARY(n)</td>
+ <td>VARBINARY(n)</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>VARBINARY(n)</td>
+ <td>VARBINARY(n)</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>BYTES</td>
+ <td>VARBINARY(1048576)</td>
+ <td>BYTES is mapped to VARBINARY with max length 1048576.</td>
+ </tr>
Review Comment:
The table says `BINARY(n)`/`VARBINARY(n)` map to `VARBINARY(n)`
unconditionally, but the implementation caps the length via `Math.min(...,
MAX_VARBINARY_SIZE)` (1048576). For `n > 1048576` the actual mapping becomes
`VARBINARY(1048576)`, so the documentation should reflect the cap (e.g.,
`VARBINARY(min(n,1048576))` or split into two rows like other connector docs).
--
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]
