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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
