gnodet-bot commented on code in PR #28922:
URL: https://github.com/apache/camel/pull/28922#discussion_r4445598355
##########
components/camel-clickhouse/src/main/java/org/apache/camel/component/clickhouse/ClickHouseProducer.java:
##########
@@ -122,46 +124,72 @@ private void doInsert(Exchange exchange) throws Exception
{
}
private long insertList(Client client, String target, List<?> data,
InsertSettings settings) throws Exception {
+ if (!data.isEmpty() && data.get(0) instanceof Map) {
+ throw new ClickHouseException(
+ "Cannot insert a List of Maps into " + target + ": a List
body must hold objects whose class is"
+ + " registered on the Client with
Client.register(Class, TableSchema)."
+ + " Marshal the rows to the format
option (such as JSONEachRow) instead.");
+ }
int batchSize = endpoint.getBatchSize();
if (batchSize <= 0 || data.size() <= batchSize) {
- try (InsertResponse response = client.insert(target, data,
settings).get()) {
- return response.getWrittenRows();
- }
+ return insertBatch(client, target, data, settings);
}
// split the list into batches of batchSize, inserting each batch
separately
long writtenRows = 0;
for (int start = 0; start < data.size(); start += batchSize) {
List<?> batch = data.subList(start, Math.min(start + batchSize,
data.size()));
- try (InsertResponse response = client.insert(target, batch,
settings).get()) {
- writtenRows += response.getWrittenRows();
- }
+ writtenRows += insertBatch(client, target, batch, settings);
}
return writtenRows;
}
+ private long insertBatch(Client client, String target, List<?> batch,
InsertSettings settings) throws Exception {
+ CompletableFuture<InsertResponse> future;
+ try {
+ future = client.insert(target, batch, settings);
+ } catch (NullPointerException e) {
+ // client-v2 throws a NullPointerException that does not say why
when no class is registered for the table
+ throw new ClickHouseException(
+ "Cannot insert a List of " +
batch.get(0).getClass().getName() + " into " + target
+ + ": no class is registered on the
Client for this table. Register the class"
+ + " with Client.register(Class,
TableSchema), or send the rows as text, bytes,"
+ + " a stream or a file in the format
option.",
+ e);
Review Comment:
⚠️ **Defect: `batch.get(0)` throws `IndexOutOfBoundsException` on an empty
batch, masking the original `NullPointerException`.**
An empty `List` body reaches `insertBatch` without being stopped: the
`!data.isEmpty()` guard in `insertList` lets it through, and the `data.size()
<= batchSize` condition in `insertList` routes it directly to `insertBatch`. If
`client.insert` then throws an NPE (e.g. for a ClickHouse client bug on empty
input), `batch.get(0)` blows up with an unintelligible
`IndexOutOfBoundsException`, discarding the original NPE as the cause.
```suggestion
} catch (NullPointerException e) {
// client-v2 throws a NullPointerException that does not say why
when no class is registered for the table
String className = batch.isEmpty() ? "?" :
batch.get(0).getClass().getName();
throw new ClickHouseException(
"Cannot insert a List of " + className + " into " +
target
+ ": no class is registered on the
Client for this table. Register the class"
+ " with Client.register(Class,
TableSchema), or send the rows as text, bytes,"
+ " a stream or a file in the
format option.",
e);
```
--
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]
