You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by "linghengqian (via GitHub)" <gi...@apache.org> on 2025/12/18 06:23:27 UTC

[PR] Fix integration issues with clickhouse-server `25.10` [shardingsphere]

linghengqian opened a new pull request, #37421:
URL: https://github.com/apache/shardingsphere/pull/37421

   For #37406 .
   
   Changes proposed in this pull request:
     - Bump Antlr4 to `4.13.2` to fix integration issues with clickhouse-server `25.10`. This is consistent with ClickHouse JDBC Driver V2 `com.clickhouse:clickhouse-jdbc:0.9.4-all`.
     - Bump ClickHouse JDBC Driver to `0.9.4-all`.
     - Bump `testcontainers-java` to `2.0.1`. This is mainly to prevent JUnit from continuing to throw warnings, see https://github.com/testcontainers/testcontainers-java/pull/10529.
     - Update doc.
        - Because ClickHouse JDBC Driver V2 removed the `Response Column Renaming` feature, ShardingSphere's distributed sequence feature is now unavailable for ClickHouse integration. This is related to https://github.com/ClickHouse/ClickHouse/issues/21697 and https://clickhouse.com/docs/integrations/language-clients/java/jdbc#changes-from-07x .
       - Reveals deficiencies in the parsing of `INNER JOIN` syntax in HiveServer2 and ClickHouse. 
       - The `http` classifier in the ClickHouse JDBC Driver has been removed. See https://github.com/ClickHouse/clickhouse-java/tree/v0.9.4?tab=readme-ov-file#artifact-changes-in-090-june .
     - Add GraalVM Reachability Metadata of `com.clickhouse:clickhouse-jdbc:0.9.4-all`, `com.github.docker-java:docker-java-api:3.6.0`, `org.testcontainers:testcontainers-junit-jupiter:2.0.1`.
   
   ---
   
   Before committing this PR, I'm sure that I have checked the following options:
   - [x] My code follows the [code of conduct](https://shardingsphere.apache.org/community/en/involved/conduct/code/) of this project.
   - [x] I have self-reviewed the commit code.
   - [x] I have (or in comment I request) added corresponding labels for the pull request.
   - [x] I have passed maven check locally : `./mvnw clean install -B -T1C -Dmaven.javadoc.skip -Dmaven.jacoco.skip -e`.
   - [x] I have made corresponding changes to the documentation.
   - [x] I have added corresponding unit tests for my changes.
   - [ ] I have updated the Release Notes of the current development version. For more details, see [Update Release Note](https://shardingsphere.apache.org/community/en/involved/contribute/contributor/)
   


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix integration issues with clickhouse-server `25.10` [shardingsphere]

Posted by "linghengqian (via GitHub)" <gi...@apache.org>.
linghengqian commented on code in PR #37421:
URL: https://github.com/apache/shardingsphere/pull/37421#discussion_r2629933823


##########
docs/document/content/user-manual/shardingsphere-jdbc/optional-plugins/hiveserver2/_index.cn.md:
##########
@@ -366,9 +366,9 @@ ShardingSphere 仅针对 HiveServer2 `4.0.1` 进行集成测试。
 
 ### SQL 限制
 
-HiveServer2 并不能保证每一条 `insert` 相关的 DML SQL 都能成功执行,尽管可能没有任何异常被抛出。
-
 ShardingSphere JDBC DataSource 尚不支持执行 HiveServer2 的 `set` 语句。
+当前 ShardingSphere 对 HiveServer2 的 `INNER JOIN` 语法解析存在不足,
+对 `SELECT i.* FROM t_order o, t_order_item i WHERE o.order_id = i.order_id` 这类 SQL 可能返回错误的查询结果。

Review Comment:
   Fixed.



-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix integration issues with clickhouse-server `25.10` [shardingsphere]

Posted by "linghengqian (via GitHub)" <gi...@apache.org>.
linghengqian commented on code in PR #37421:
URL: https://github.com/apache/shardingsphere/pull/37421#discussion_r2629934579


##########
test/native/src/test/java/org/apache/shardingsphere/test/natived/commons/TestShardingService.java:
##########
@@ -68,63 +68,36 @@ public void processSuccess() throws SQLException {
     }
     
     /**
-     * Process success in ClickHouse. ClickHouse JDBC Driver does not support the use of transactions.
+     * Process success in ClickHouse.
+     * ClickHouse JDBC Driver does not support the use of transactions.
+     * Databases like ClickHouse do not support returning auto generated keys after executing SQL,
+     * see <a href="https://github.com/ClickHouse/ClickHouse/issues/56228">ClickHouse/ClickHouse#56228</a> .
+     * TODO The current ShardingSphere parsing of ClickHouse's `INNER JOIN` syntax has shortcomings,
+     *  and it return incorrect query results for SQL statements such as `SELECT i.* FROM t_order o, t_order_item i WHERE o.order_id = i.order_id`.
      *
      * @throws SQLException An exception that provides information on a database access error or other errors
      */
     public void processSuccessInClickHouse() throws SQLException {
-        Collection<Long> orderIds = insertData(Statement.NO_GENERATED_KEYS);
-        assertQueryInClickHouse();
+        Collection<Long> orderIds = insertDataWithoutGeneratedKeys();
+        assertQueryLoose();
         deleteDataInClickHouse(orderIds);
         assertTrue(orderRepository.selectAll().isEmpty());
         assertTrue(orderItemRepository.selectAll().isEmpty());
         assertTrue(addressRepository.selectAll().isEmpty());
     }
     
-    private void assertQueryInClickHouse() throws SQLException {
-        Collection<Order> orders = orderRepository.selectAll();
-        assertThat(orders.stream().map(Order::getOrderId).collect(Collectors.toList()), not(empty()));
-        assertThat(orders.stream().map(Order::getOrderType).collect(Collectors.toList()),
-                containsInAnyOrder(0, 1, 0, 1, 0, 1, 0, 1, 0, 1));
-        assertThat(orders.stream().map(Order::getUserId).collect(Collectors.toList()),
-                containsInAnyOrder(1, 2, 3, 4, 5, 6, 7, 8, 9, 10));
-        assertThat(orders.stream().map(Order::getAddressId).collect(Collectors.toList()),
-                containsInAnyOrder(1L, 2L, 3L, 4L, 5L, 6L, 7L, 8L, 9L, 10L));
-        assertThat(orders.stream().map(Order::getStatus).collect(Collectors.toList()),
-                is(IntStream.range(1, 11).mapToObj(i -> "INSERT_TEST").collect(Collectors.toList())));
-        Collection<OrderItem> orderItems = orderItemRepository.selectAll();
-        assertThat(orderItems.stream().map(OrderItem::getOrderItemId).collect(Collectors.toList()), not(empty()));
-        assertThat(orderItems.stream().map(OrderItem::getOrderId).collect(Collectors.toList()), not(empty()));
-        assertThat(orderItems.stream().map(OrderItem::getUserId).collect(Collectors.toList()),
-                containsInAnyOrder(1, 2, 3, 4, 5, 6, 7, 8, 9, 10));
-        assertThat(orderItems.stream().map(OrderItem::getPhone).collect(Collectors.toList()),
-                is(IntStream.range(1, 11).mapToObj(i -> "13800000001").collect(Collectors.toList())));
-        assertThat(orderItems.stream().map(OrderItem::getStatus).collect(Collectors.toList()),
-                is(IntStream.range(1, 11).mapToObj(i -> "INSERT_TEST").collect(Collectors.toList())));
-        assertThat(new HashSet<>(addressRepository.selectAll()),
-                is(LongStream.range(1L, 11L).mapToObj(each -> new Address(each, "address_test_" + each)).collect(Collectors.toSet())));
-    }
-    
-    private void deleteDataInClickHouse(final Collection<Long> orderIds) throws SQLException {
-        long count = 1L;
-        for (Long each : orderIds) {
-            orderRepository.deleteInClickHouse(each);
-            orderItemRepository.deleteInClickHouse(each);
-            addressRepository.deleteInClickHouse(count++);
-        }
-    }
-    
     /**
      * Process success in Hive.
      * Hive has not fully supported BEGIN, COMMIT, and ROLLBACK. Refer to <a href="https://cwiki.apache.org/confluence/display/Hive/Hive+Transactions">Hive Transactions</a>.
      * So ShardingSphere should not use {@link OrderItemRepository#assertRollbackWithTransactions()}
-     * TODO It looks like HiveServer2 insert statements are inserted out of order. Waiting for further investigation.
-     *  The result of the insert is not currently asserted.
+     * TODO The current ShardingSphere parsing of HiveServer2's `INNER JOIN` syntax has shortcomings,
+     *  and it return incorrect query results for SQL statements such as `SELECT i.* FROM t_order o, t_order_item i WHERE o.order_id = i.order_id`.

Review Comment:
   I don't think we need to worry about the text syntax issues in the comments; they're temporary.



##########
test/native/src/test/java/org/apache/shardingsphere/test/natived/commons/TestShardingService.java:
##########
@@ -68,63 +68,36 @@ public void processSuccess() throws SQLException {
     }
     
     /**
-     * Process success in ClickHouse. ClickHouse JDBC Driver does not support the use of transactions.
+     * Process success in ClickHouse.
+     * ClickHouse JDBC Driver does not support the use of transactions.
+     * Databases like ClickHouse do not support returning auto generated keys after executing SQL,
+     * see <a href="https://github.com/ClickHouse/ClickHouse/issues/56228">ClickHouse/ClickHouse#56228</a> .
+     * TODO The current ShardingSphere parsing of ClickHouse's `INNER JOIN` syntax has shortcomings,
+     *  and it return incorrect query results for SQL statements such as `SELECT i.* FROM t_order o, t_order_item i WHERE o.order_id = i.order_id`.

Review Comment:
   I don't think we need to worry about the text syntax issues in the comments; they're temporary.



-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix integration issues with clickhouse-server `25.10` [shardingsphere]

Posted by "strongduanmu (via GitHub)" <gi...@apache.org>.
strongduanmu merged PR #37421:
URL: https://github.com/apache/shardingsphere/pull/37421


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix integration issues with clickhouse-server `25.10` [shardingsphere]

Posted by "linghengqian (via GitHub)" <gi...@apache.org>.
linghengqian commented on code in PR #37421:
URL: https://github.com/apache/shardingsphere/pull/37421#discussion_r2629934072


##########
docs/document/content/user-manual/shardingsphere-jdbc/optional-plugins/clickhouse/_index.en.md:
##########
@@ -170,27 +170,15 @@ public class ExampleUtils {
 
 ShardingSphere JDBC DataSource does not yet support executing ClickHouse's `create table`, `truncate table`,
 and `drop table` statements.
-Users should consider submitting a PR containing unit tests for ShardingSphere.
+The current ShardingSphere parsing of ClickHouse's `INNER JOIN` syntax has shortcomings, 
+and may return incorrect query results for SQL statements such as `SELECT i.* FROM t_order o, t_order_item i WHERE o.order_id = i.order_id`.

Review Comment:
   Fixed.



-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix integration issues with clickhouse-server `25.10` [shardingsphere]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #37421:
URL: https://github.com/apache/shardingsphere/pull/37421#discussion_r2629928714


##########
docs/document/content/user-manual/shardingsphere-jdbc/optional-plugins/hiveserver2/_index.en.md:
##########
@@ -372,9 +372,9 @@ Reference https://issues.apache.org/jira/browse/HIVE-28418 .
 
 ### SQL Limitations
 
-HiveServer2 does not guarantee that every `insert` related DML SQL can be executed successfully, although no exception may be thrown.
-
 ShardingSphere JDBC DataSource does not yet support executing the `set` statement of HiveServer2.
+The current ShardingSphere parsing of HiveServer2's `INNER JOIN` syntax has shortcomings,
+and may return incorrect query results for SQL statements such as `SELECT i.* FROM t_order o, t_order_item i WHERE o.order_id = i.order_id`.

Review Comment:
   Grammar issue: "and may return incorrect query results" should be "and it may return incorrect query results" or simply keep "and may return incorrect query results". The current wording lacks proper subject-verb construction for clarity, though it's technically acceptable. Consider adding "it" for better readability.
   ```suggestion
   and it may return incorrect query results for SQL statements such as `SELECT i.* FROM t_order o, t_order_item i WHERE o.order_id = i.order_id`.
   ```



##########
docs/document/content/user-manual/shardingsphere-jdbc/optional-plugins/clickhouse/_index.en.md:
##########
@@ -170,27 +170,15 @@ public class ExampleUtils {
 
 ShardingSphere JDBC DataSource does not yet support executing ClickHouse's `create table`, `truncate table`,
 and `drop table` statements.
-Users should consider submitting a PR containing unit tests for ShardingSphere.
+The current ShardingSphere parsing of ClickHouse's `INNER JOIN` syntax has shortcomings, 
+and may return incorrect query results for SQL statements such as `SELECT i.* FROM t_order o, t_order_item i WHERE o.order_id = i.order_id`.

Review Comment:
   Grammar issue: "and may return incorrect query results" should be "and it may return incorrect query results" for better subject-verb construction and clarity.
   ```suggestion
   and it may return incorrect query results for SQL statements such as `SELECT i.* FROM t_order o, t_order_item i WHERE o.order_id = i.order_id`.
   ```



##########
test/native/src/test/java/org/apache/shardingsphere/test/natived/commons/TestShardingService.java:
##########
@@ -68,63 +68,36 @@ public void processSuccess() throws SQLException {
     }
     
     /**
-     * Process success in ClickHouse. ClickHouse JDBC Driver does not support the use of transactions.
+     * Process success in ClickHouse.
+     * ClickHouse JDBC Driver does not support the use of transactions.
+     * Databases like ClickHouse do not support returning auto generated keys after executing SQL,
+     * see <a href="https://github.com/ClickHouse/ClickHouse/issues/56228">ClickHouse/ClickHouse#56228</a> .
+     * TODO The current ShardingSphere parsing of ClickHouse's `INNER JOIN` syntax has shortcomings,
+     *  and it return incorrect query results for SQL statements such as `SELECT i.* FROM t_order o, t_order_item i WHERE o.order_id = i.order_id`.
      *
      * @throws SQLException An exception that provides information on a database access error or other errors
      */
     public void processSuccessInClickHouse() throws SQLException {
-        Collection<Long> orderIds = insertData(Statement.NO_GENERATED_KEYS);
-        assertQueryInClickHouse();
+        Collection<Long> orderIds = insertDataWithoutGeneratedKeys();
+        assertQueryLoose();
         deleteDataInClickHouse(orderIds);
         assertTrue(orderRepository.selectAll().isEmpty());
         assertTrue(orderItemRepository.selectAll().isEmpty());
         assertTrue(addressRepository.selectAll().isEmpty());
     }
     
-    private void assertQueryInClickHouse() throws SQLException {
-        Collection<Order> orders = orderRepository.selectAll();
-        assertThat(orders.stream().map(Order::getOrderId).collect(Collectors.toList()), not(empty()));
-        assertThat(orders.stream().map(Order::getOrderType).collect(Collectors.toList()),
-                containsInAnyOrder(0, 1, 0, 1, 0, 1, 0, 1, 0, 1));
-        assertThat(orders.stream().map(Order::getUserId).collect(Collectors.toList()),
-                containsInAnyOrder(1, 2, 3, 4, 5, 6, 7, 8, 9, 10));
-        assertThat(orders.stream().map(Order::getAddressId).collect(Collectors.toList()),
-                containsInAnyOrder(1L, 2L, 3L, 4L, 5L, 6L, 7L, 8L, 9L, 10L));
-        assertThat(orders.stream().map(Order::getStatus).collect(Collectors.toList()),
-                is(IntStream.range(1, 11).mapToObj(i -> "INSERT_TEST").collect(Collectors.toList())));
-        Collection<OrderItem> orderItems = orderItemRepository.selectAll();
-        assertThat(orderItems.stream().map(OrderItem::getOrderItemId).collect(Collectors.toList()), not(empty()));
-        assertThat(orderItems.stream().map(OrderItem::getOrderId).collect(Collectors.toList()), not(empty()));
-        assertThat(orderItems.stream().map(OrderItem::getUserId).collect(Collectors.toList()),
-                containsInAnyOrder(1, 2, 3, 4, 5, 6, 7, 8, 9, 10));
-        assertThat(orderItems.stream().map(OrderItem::getPhone).collect(Collectors.toList()),
-                is(IntStream.range(1, 11).mapToObj(i -> "13800000001").collect(Collectors.toList())));
-        assertThat(orderItems.stream().map(OrderItem::getStatus).collect(Collectors.toList()),
-                is(IntStream.range(1, 11).mapToObj(i -> "INSERT_TEST").collect(Collectors.toList())));
-        assertThat(new HashSet<>(addressRepository.selectAll()),
-                is(LongStream.range(1L, 11L).mapToObj(each -> new Address(each, "address_test_" + each)).collect(Collectors.toSet())));
-    }
-    
-    private void deleteDataInClickHouse(final Collection<Long> orderIds) throws SQLException {
-        long count = 1L;
-        for (Long each : orderIds) {
-            orderRepository.deleteInClickHouse(each);
-            orderItemRepository.deleteInClickHouse(each);
-            addressRepository.deleteInClickHouse(count++);
-        }
-    }
-    
     /**
      * Process success in Hive.
      * Hive has not fully supported BEGIN, COMMIT, and ROLLBACK. Refer to <a href="https://cwiki.apache.org/confluence/display/Hive/Hive+Transactions">Hive Transactions</a>.
      * So ShardingSphere should not use {@link OrderItemRepository#assertRollbackWithTransactions()}
-     * TODO It looks like HiveServer2 insert statements are inserted out of order. Waiting for further investigation.
-     *  The result of the insert is not currently asserted.
+     * TODO The current ShardingSphere parsing of HiveServer2's `INNER JOIN` syntax has shortcomings,
+     *  and it return incorrect query results for SQL statements such as `SELECT i.* FROM t_order o, t_order_item i WHERE o.order_id = i.order_id`.

Review Comment:
   Grammar issue: "it return incorrect" should be "it may return incorrect" or "it returns incorrect". The subject-verb agreement is incorrect as written.



##########
test/native/src/test/java/org/apache/shardingsphere/test/natived/commons/TestShardingService.java:
##########
@@ -68,63 +68,36 @@ public void processSuccess() throws SQLException {
     }
     
     /**
-     * Process success in ClickHouse. ClickHouse JDBC Driver does not support the use of transactions.
+     * Process success in ClickHouse.
+     * ClickHouse JDBC Driver does not support the use of transactions.
+     * Databases like ClickHouse do not support returning auto generated keys after executing SQL,
+     * see <a href="https://github.com/ClickHouse/ClickHouse/issues/56228">ClickHouse/ClickHouse#56228</a> .
+     * TODO The current ShardingSphere parsing of ClickHouse's `INNER JOIN` syntax has shortcomings,
+     *  and it return incorrect query results for SQL statements such as `SELECT i.* FROM t_order o, t_order_item i WHERE o.order_id = i.order_id`.

Review Comment:
   Grammar issue: "it return incorrect" should be "it may return incorrect" or "it returns incorrect". The subject-verb agreement is incorrect as written.



##########
docs/document/content/user-manual/shardingsphere-jdbc/optional-plugins/hiveserver2/_index.cn.md:
##########
@@ -366,9 +366,9 @@ ShardingSphere 仅针对 HiveServer2 `4.0.1` 进行集成测试。
 
 ### SQL 限制
 
-HiveServer2 并不能保证每一条 `insert` 相关的 DML SQL 都能成功执行,尽管可能没有任何异常被抛出。
-
 ShardingSphere JDBC DataSource 尚不支持执行 HiveServer2 的 `set` 语句。
+当前 ShardingSphere 对 HiveServer2 的 `INNER JOIN` 语法解析存在不足,
+对 `SELECT i.* FROM t_order o, t_order_item i WHERE o.order_id = i.order_id` 这类 SQL 可能返回错误的查询结果。

Review Comment:
   Grammar issue: "and may return incorrect query results" should be "and it may return incorrect query results" or simply keep "and may return incorrect query results". The current wording lacks proper subject-verb construction for clarity, though it's technically acceptable. Consider adding "it" for better readability to match the English documentation pattern.
   ```suggestion
   对 `SELECT i.* FROM t_order o, t_order_item i WHERE o.order_id = i.order_id` 这类 SQL,它可能返回错误的查询结果。
   ```



##########
docs/document/content/user-manual/shardingsphere-jdbc/optional-plugins/clickhouse/_index.cn.md:
##########
@@ -167,24 +167,14 @@ public class ExampleUtils {
 ### SQL 限制
 
 ShardingSphere JDBC DataSource 尚不支持执行 ClickHouse 的 `create table`,`truncate table` 和 `drop table` 语句。
-用户应考虑为 ShardingSphere 提交包含单元测试的 PR。
+当前 ShardingSphere 对 ClickHouse 的 `INNER JOIN` 语法解析存在不足,
+对 `SELECT i.* FROM t_order o, t_order_item i WHERE o.order_id = i.order_id` 这类 SQL 可能返回错误的查询结果。

Review Comment:
   Grammar issue: "对 `SELECT i.* FROM t_order o, t_order_item i WHERE o.order_id = i.order_id` 这类 SQL 可能返回错误的查询结果。" should be "对 `SELECT i.* FROM t_order o, t_order_item i WHERE o.order_id = i.order_id` 这类 SQL 它可能返回错误的查询结果。" for better clarity by adding the subject "它".
   ```suggestion
   对 `SELECT i.* FROM t_order o, t_order_item i WHERE o.order_id = i.order_id` 这类 SQL 它可能返回错误的查询结果。
   ```



-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix integration issues with clickhouse-server `25.10` [shardingsphere]

Posted by "linghengqian (via GitHub)" <gi...@apache.org>.
linghengqian commented on code in PR #37421:
URL: https://github.com/apache/shardingsphere/pull/37421#discussion_r2629933482


##########
docs/document/content/user-manual/shardingsphere-jdbc/optional-plugins/hiveserver2/_index.en.md:
##########
@@ -372,9 +372,9 @@ Reference https://issues.apache.org/jira/browse/HIVE-28418 .
 
 ### SQL Limitations
 
-HiveServer2 does not guarantee that every `insert` related DML SQL can be executed successfully, although no exception may be thrown.
-
 ShardingSphere JDBC DataSource does not yet support executing the `set` statement of HiveServer2.
+The current ShardingSphere parsing of HiveServer2's `INNER JOIN` syntax has shortcomings,
+and may return incorrect query results for SQL statements such as `SELECT i.* FROM t_order o, t_order_item i WHERE o.order_id = i.order_id`.

Review Comment:
   Fixed.



-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix integration issues with clickhouse-server `25.10` [shardingsphere]

Posted by "linghengqian (via GitHub)" <gi...@apache.org>.
linghengqian commented on code in PR #37421:
URL: https://github.com/apache/shardingsphere/pull/37421#discussion_r2629934891


##########
test/native/src/test/java/org/apache/shardingsphere/test/natived/commons/TestShardingService.java:
##########
@@ -68,63 +68,36 @@ public void processSuccess() throws SQLException {
     }
     
     /**
-     * Process success in ClickHouse. ClickHouse JDBC Driver does not support the use of transactions.
+     * Process success in ClickHouse.
+     * ClickHouse JDBC Driver does not support the use of transactions.
+     * Databases like ClickHouse do not support returning auto generated keys after executing SQL,
+     * see <a href="https://github.com/ClickHouse/ClickHouse/issues/56228">ClickHouse/ClickHouse#56228</a> .
+     * TODO The current ShardingSphere parsing of ClickHouse's `INNER JOIN` syntax has shortcomings,
+     *  and it return incorrect query results for SQL statements such as `SELECT i.* FROM t_order o, t_order_item i WHERE o.order_id = i.order_id`.

Review Comment:
   Fixed.



-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix integration issues with clickhouse-server `25.10` [shardingsphere]

Posted by "linghengqian (via GitHub)" <gi...@apache.org>.
linghengqian commented on code in PR #37421:
URL: https://github.com/apache/shardingsphere/pull/37421#discussion_r2629934303


##########
docs/document/content/user-manual/shardingsphere-jdbc/optional-plugins/clickhouse/_index.cn.md:
##########
@@ -167,24 +167,14 @@ public class ExampleUtils {
 ### SQL 限制
 
 ShardingSphere JDBC DataSource 尚不支持执行 ClickHouse 的 `create table`,`truncate table` 和 `drop table` 语句。
-用户应考虑为 ShardingSphere 提交包含单元测试的 PR。
+当前 ShardingSphere 对 ClickHouse 的 `INNER JOIN` 语法解析存在不足,
+对 `SELECT i.* FROM t_order o, t_order_item i WHERE o.order_id = i.order_id` 这类 SQL 可能返回错误的查询结果。

Review Comment:
   Fixed.



-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix integration issues with clickhouse-server `25.10` [shardingsphere]

Posted by "linghengqian (via GitHub)" <gi...@apache.org>.
linghengqian commented on code in PR #37421:
URL: https://github.com/apache/shardingsphere/pull/37421#discussion_r2629933482


##########
docs/document/content/user-manual/shardingsphere-jdbc/optional-plugins/hiveserver2/_index.en.md:
##########
@@ -372,9 +372,9 @@ Reference https://issues.apache.org/jira/browse/HIVE-28418 .
 
 ### SQL Limitations
 
-HiveServer2 does not guarantee that every `insert` related DML SQL can be executed successfully, although no exception may be thrown.
-
 ShardingSphere JDBC DataSource does not yet support executing the `set` statement of HiveServer2.
+The current ShardingSphere parsing of HiveServer2's `INNER JOIN` syntax has shortcomings,
+and may return incorrect query results for SQL statements such as `SELECT i.* FROM t_order o, t_order_item i WHERE o.order_id = i.order_id`.

Review Comment:
   I don't think we need to worry about the text syntax issues in the comments; they're temporary.



##########
docs/document/content/user-manual/shardingsphere-jdbc/optional-plugins/hiveserver2/_index.cn.md:
##########
@@ -366,9 +366,9 @@ ShardingSphere 仅针对 HiveServer2 `4.0.1` 进行集成测试。
 
 ### SQL 限制
 
-HiveServer2 并不能保证每一条 `insert` 相关的 DML SQL 都能成功执行,尽管可能没有任何异常被抛出。
-
 ShardingSphere JDBC DataSource 尚不支持执行 HiveServer2 的 `set` 语句。
+当前 ShardingSphere 对 HiveServer2 的 `INNER JOIN` 语法解析存在不足,
+对 `SELECT i.* FROM t_order o, t_order_item i WHERE o.order_id = i.order_id` 这类 SQL 可能返回错误的查询结果。

Review Comment:
   I don't think we need to worry about the text syntax issues in the comments; they're temporary.



-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix integration issues with clickhouse-server `25.10` [shardingsphere]

Posted by "linghengqian (via GitHub)" <gi...@apache.org>.
linghengqian commented on code in PR #37421:
URL: https://github.com/apache/shardingsphere/pull/37421#discussion_r2629934579


##########
test/native/src/test/java/org/apache/shardingsphere/test/natived/commons/TestShardingService.java:
##########
@@ -68,63 +68,36 @@ public void processSuccess() throws SQLException {
     }
     
     /**
-     * Process success in ClickHouse. ClickHouse JDBC Driver does not support the use of transactions.
+     * Process success in ClickHouse.
+     * ClickHouse JDBC Driver does not support the use of transactions.
+     * Databases like ClickHouse do not support returning auto generated keys after executing SQL,
+     * see <a href="https://github.com/ClickHouse/ClickHouse/issues/56228">ClickHouse/ClickHouse#56228</a> .
+     * TODO The current ShardingSphere parsing of ClickHouse's `INNER JOIN` syntax has shortcomings,
+     *  and it return incorrect query results for SQL statements such as `SELECT i.* FROM t_order o, t_order_item i WHERE o.order_id = i.order_id`.
      *
      * @throws SQLException An exception that provides information on a database access error or other errors
      */
     public void processSuccessInClickHouse() throws SQLException {
-        Collection<Long> orderIds = insertData(Statement.NO_GENERATED_KEYS);
-        assertQueryInClickHouse();
+        Collection<Long> orderIds = insertDataWithoutGeneratedKeys();
+        assertQueryLoose();
         deleteDataInClickHouse(orderIds);
         assertTrue(orderRepository.selectAll().isEmpty());
         assertTrue(orderItemRepository.selectAll().isEmpty());
         assertTrue(addressRepository.selectAll().isEmpty());
     }
     
-    private void assertQueryInClickHouse() throws SQLException {
-        Collection<Order> orders = orderRepository.selectAll();
-        assertThat(orders.stream().map(Order::getOrderId).collect(Collectors.toList()), not(empty()));
-        assertThat(orders.stream().map(Order::getOrderType).collect(Collectors.toList()),
-                containsInAnyOrder(0, 1, 0, 1, 0, 1, 0, 1, 0, 1));
-        assertThat(orders.stream().map(Order::getUserId).collect(Collectors.toList()),
-                containsInAnyOrder(1, 2, 3, 4, 5, 6, 7, 8, 9, 10));
-        assertThat(orders.stream().map(Order::getAddressId).collect(Collectors.toList()),
-                containsInAnyOrder(1L, 2L, 3L, 4L, 5L, 6L, 7L, 8L, 9L, 10L));
-        assertThat(orders.stream().map(Order::getStatus).collect(Collectors.toList()),
-                is(IntStream.range(1, 11).mapToObj(i -> "INSERT_TEST").collect(Collectors.toList())));
-        Collection<OrderItem> orderItems = orderItemRepository.selectAll();
-        assertThat(orderItems.stream().map(OrderItem::getOrderItemId).collect(Collectors.toList()), not(empty()));
-        assertThat(orderItems.stream().map(OrderItem::getOrderId).collect(Collectors.toList()), not(empty()));
-        assertThat(orderItems.stream().map(OrderItem::getUserId).collect(Collectors.toList()),
-                containsInAnyOrder(1, 2, 3, 4, 5, 6, 7, 8, 9, 10));
-        assertThat(orderItems.stream().map(OrderItem::getPhone).collect(Collectors.toList()),
-                is(IntStream.range(1, 11).mapToObj(i -> "13800000001").collect(Collectors.toList())));
-        assertThat(orderItems.stream().map(OrderItem::getStatus).collect(Collectors.toList()),
-                is(IntStream.range(1, 11).mapToObj(i -> "INSERT_TEST").collect(Collectors.toList())));
-        assertThat(new HashSet<>(addressRepository.selectAll()),
-                is(LongStream.range(1L, 11L).mapToObj(each -> new Address(each, "address_test_" + each)).collect(Collectors.toSet())));
-    }
-    
-    private void deleteDataInClickHouse(final Collection<Long> orderIds) throws SQLException {
-        long count = 1L;
-        for (Long each : orderIds) {
-            orderRepository.deleteInClickHouse(each);
-            orderItemRepository.deleteInClickHouse(each);
-            addressRepository.deleteInClickHouse(count++);
-        }
-    }
-    
     /**
      * Process success in Hive.
      * Hive has not fully supported BEGIN, COMMIT, and ROLLBACK. Refer to <a href="https://cwiki.apache.org/confluence/display/Hive/Hive+Transactions">Hive Transactions</a>.
      * So ShardingSphere should not use {@link OrderItemRepository#assertRollbackWithTransactions()}
-     * TODO It looks like HiveServer2 insert statements are inserted out of order. Waiting for further investigation.
-     *  The result of the insert is not currently asserted.
+     * TODO The current ShardingSphere parsing of HiveServer2's `INNER JOIN` syntax has shortcomings,
+     *  and it return incorrect query results for SQL statements such as `SELECT i.* FROM t_order o, t_order_item i WHERE o.order_id = i.order_id`.

Review Comment:
   Fixed.



-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix integration issues with clickhouse-server `25.10` [shardingsphere]

Posted by "linghengqian (via GitHub)" <gi...@apache.org>.
linghengqian commented on code in PR #37421:
URL: https://github.com/apache/shardingsphere/pull/37421#discussion_r2629934072


##########
docs/document/content/user-manual/shardingsphere-jdbc/optional-plugins/clickhouse/_index.en.md:
##########
@@ -170,27 +170,15 @@ public class ExampleUtils {
 
 ShardingSphere JDBC DataSource does not yet support executing ClickHouse's `create table`, `truncate table`,
 and `drop table` statements.
-Users should consider submitting a PR containing unit tests for ShardingSphere.
+The current ShardingSphere parsing of ClickHouse's `INNER JOIN` syntax has shortcomings, 
+and may return incorrect query results for SQL statements such as `SELECT i.* FROM t_order o, t_order_item i WHERE o.order_id = i.order_id`.

Review Comment:
   I don't think we need to worry about the text syntax issues in the comments; they're temporary.



##########
docs/document/content/user-manual/shardingsphere-jdbc/optional-plugins/clickhouse/_index.cn.md:
##########
@@ -167,24 +167,14 @@ public class ExampleUtils {
 ### SQL 限制
 
 ShardingSphere JDBC DataSource 尚不支持执行 ClickHouse 的 `create table`,`truncate table` 和 `drop table` 语句。
-用户应考虑为 ShardingSphere 提交包含单元测试的 PR。
+当前 ShardingSphere 对 ClickHouse 的 `INNER JOIN` 语法解析存在不足,
+对 `SELECT i.* FROM t_order o, t_order_item i WHERE o.order_id = i.order_id` 这类 SQL 可能返回错误的查询结果。

Review Comment:
   I don't think we need to worry about the text syntax issues in the comments; they're temporary.



-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org