Skip to content

Conversation

@chernser
Copy link
Contributor

@chernser chernser commented May 8, 2025

Summary

Fixes #2355 by using default JVM pool in case executor is no specified by client nor client is configured for async operations

Checklist

Delete items not relevant to your PR:

@chernser chernser requested a review from mzitnik May 8, 2025 18:12
initTable(tableName, createSQL);

InsertSettings localSettings = new InsertSettings(settings.getAllSettings());
localSettings.setOption(ClientConfigProperties.ASYNC_OPERATIONS.getKey(), true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also have a test with async disabled

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@mshustov mshustov requested a review from Copilot May 9, 2025 08:36
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a Null Pointer Exception by ensuring that a default JVM pool is used when no executor is provided, and it adds integration tests for async insert operations.

  • Adds a new async insert test verifying the async operation behavior.
  • Modifies the async operation method in the client to handle cases where the shared executor is null.
  • Comments out legacy logging configuration.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
client-v2/src/test/java/com/clickhouse/client/insert/InsertTests.java Introduces a new async data insertion test and updates logging code.
client-v2/src/main/java/com/clickhouse/client/api/Client.java Refines asynchronous operation handling by checking for a null executor.

@sonarqubecloud
Copy link

sonarqubecloud bot commented May 9, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@chernser chernser requested a review from mzitnik May 9, 2025 18:18
@chernser chernser merged commit 175976b into main May 13, 2025
24 of 25 checks passed
@chernser chernser deleted the fix_npe_no_scheduler branch May 13, 2025 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[client-v2] NPE when async requested on operation level

3 participants