github-actions[bot] commented on code in PR #61245:
URL: https://github.com/apache/doris/pull/61245#discussion_r2937636397
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/maxcompute/MaxComputeExternalCatalog.java:
##########
@@ -191,6 +193,10 @@ protected void initLocalObjectsImpl() {
props.getOrDefault(MCProperties.READ_TIMEOUT,
MCProperties.DEFAULT_READ_TIMEOUT));
retryTimes = Integer.parseInt(
props.getOrDefault(MCProperties.RETRY_COUNT,
MCProperties.DEFAULT_RETRY_COUNT));
+ maxFieldSize = Long.parseLong(
+ props.getOrDefault(MCProperties.MAX_FIELD_SIZE,
MCProperties.DEFAULT_MAX_FIELD_SIZE));
+ maxWriteBatchRows = Integer.parseInt(
+ props.getOrDefault(MCProperties.MAX_WRITE_BATCH_ROWS,
MCProperties.DEFAULT_MAX_WRITE_BATCH_ROWS));
Review Comment:
**[Suggestion]** Neither `maxWriteBatchRows` nor `maxFieldSize` is validated
in `checkProperties()`. This is inconsistent with how `connectTimeout`,
`readTimeout`, and `retryTimes` are validated there with `> 0` checks.
More critically, if a user sets `mc.max_write_batch_rows` to `0` or a
negative value, `MaxComputeJniWriter.writeInternal()` will enter an **infinite
loop** because `Math.min(0, numRows - rowOffset)` returns `0`, making
`rowOffset` never advance:
```java
while (rowOffset < numRows) {
int batchRows = Math.min(maxWriteBatchRows, numRows - rowOffset); // 0
if maxWriteBatchRows <= 0
// ... rowOffset += batchRows; // never advances
}
```
Recommendation: Add validation in `checkProperties()` for both new
properties, e.g.:
```java
maxWriteBatchRows = Integer.parseInt(props.getOrDefault(...));
if (maxWriteBatchRows <= 0) {
throw new DdlException(MCProperties.MAX_WRITE_BATCH_ROWS + " must be
greater than 0");
}
maxFieldSize = Long.parseLong(props.getOrDefault(...));
if (maxFieldSize <= 0) {
throw new DdlException(MCProperties.MAX_FIELD_SIZE + " must be greater
than 0");
}
```
Alternatively (or additionally), add a guard in `MaxComputeJniWriter`
constructor:
```java
if (this.maxWriteBatchRows <= 0) {
this.maxWriteBatchRows =
Integer.parseInt(MCProperties.DEFAULT_MAX_WRITE_BATCH_ROWS);
}
```
--
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]
