You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/12/07 00:10:56 UTC

[GitHub] [pulsar] tisonkun opened a new pull request, #18774: [feat][io] Upgarde ClickHouse driver to support loadbalance policy

tisonkun opened a new pull request, #18774:
URL: https://github.com/apache/pulsar/pull/18774

   This closes #12998.
   
   ### Documentation
   
   May or may not we add some docs on this feature. I'm not sure.
   
   <!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
   
   - [ ] `doc` <!-- Your PR contains doc changes. Please attach the local preview screenshots (run `sh start.sh` at `pulsar/site2/website`) to your PR description, or else your PR might not get merged. -->
   - [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
   - [x] `doc-not-needed` <!-- Your PR changes do not impact docs -->
   - [ ] `doc-complete` <!-- Docs have been already added -->
   
   ### Matching PR in forked repository
   
   PR in forked repository: <!-- ENTER URL HERE -->
   
   <!--
   After opening this PR, the build in apache/pulsar will fail and instructions will
   be provided for opening a PR in the PR author's forked repository.
   
   apache/pulsar pull requests should be first tested in your own fork since the 
   apache/pulsar CI based on GitHub Actions has constrained resources and quota.
   GitHub Actions provides separate quota for pull requests that are executed in 
   a forked repository.
   
   The tests will be run in the forked repository until all PR review comments have
   been handled, the tests pass and the PR is approved by a reviewer.
   -->
   


-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] zhicwu commented on a diff in pull request #18774: [feat][io] Upgarde ClickHouse driver to support loadbalance policy

Posted by GitBox <gi...@apache.org>.
zhicwu commented on code in PR #18774:
URL: https://github.com/apache/pulsar/pull/18774#discussion_r1045323865


##########
pulsar-io/jdbc/clickhouse/pom.xml:
##########
@@ -38,7 +38,7 @@
       <version>${project.version}</version>
     </dependency>
     <dependency>
-      <groupId>ru.yandex.clickhouse</groupId>
+      <groupId>com.clickhouse</groupId>

Review Comment:
   > What's the most common dependency conflict that can be avoided by using the shaded jar?
   
   LZ4 and Gson. Put native library aside, if there's custom classloader for isolation, it should not be a problem.
   
   > I can see using the basic JAR, the final connector NAR is in size 9.7M, and after switching to the shaded jar, it grows to 46M.
   
   Did you exclude all dependencies when specifying classifier? I'm not familiar with the Nifi maven plugin, but the JDBC driver with only http implementation is ~1MB. `-all` contains more like gRPC etc., so it's 20+ MB.



-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] tisonkun commented on a diff in pull request #18774: [feat][io] Upgarde ClickHouse driver to support loadbalance policy

Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #18774:
URL: https://github.com/apache/pulsar/pull/18774#discussion_r1045338915


##########
pulsar-io/jdbc/clickhouse/pom.xml:
##########
@@ -38,7 +38,7 @@
       <version>${project.version}</version>
     </dependency>
     <dependency>
-      <groupId>ru.yandex.clickhouse</groupId>
+      <groupId>com.clickhouse</groupId>

Review Comment:
   > Did you exclude all dependencies when specifying classifier?
   
   No. I'm unsure what dependencies should be excluded XD.



-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] tisonkun commented on a diff in pull request #18774: [feat][io] Upgarde ClickHouse driver to support loadbalance policy

Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #18774:
URL: https://github.com/apache/pulsar/pull/18774#discussion_r1045358603


##########
pulsar-io/jdbc/clickhouse/pom.xml:
##########
@@ -38,7 +38,7 @@
       <version>${project.version}</version>
     </dependency>
     <dependency>
-      <groupId>ru.yandex.clickhouse</groupId>
+      <groupId>com.clickhouse</groupId>

Review Comment:
   Now it reduces to 25M



-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] zhicwu commented on pull request #18774: [feat][io] Upgarde ClickHouse driver to support loadbalance policy

Posted by GitBox <gi...@apache.org>.
zhicwu commented on PR #18774:
URL: https://github.com/apache/pulsar/pull/18774#issuecomment-1354234777

   > What's the behavior if we pass null to a non-nullable column previously?
   
   Before v0.3.2, it depends on table structure(whether the column has default value) and server setting(whether `input_format_null_as_default` is `1`, which is the default). Starting from v0.3.2, you'll end up with a SQLException complaining you should not insert null into non-nullable column, unless client option `nullAsDefault` is set to `2`.
   
   > Is there a CHANGELOG for the details how the mapping and handling change?
   
   Unfortunately the answer is no. We use to have a CHANGELOG but it did not cover details like that. As I mentioned above, the impact to JDBC driver should be mainly about timestamp with timezone.
   
   @tisonkun & @nlu90, with no doubt that adding tests is needed. However, I'd suggest to start with acceptance tests from pulsar's point of view, instead of being distracted by compatibility issue of specific driver ;)


-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] tisonkun commented on pull request #18774: [feat][io] Upgarde ClickHouse driver to support loadbalance policy

Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #18774:
URL: https://github.com/apache/pulsar/pull/18774#issuecomment-1342342969

   @zhicwu Thanks for your explanation! I think most of the usages of ClickHouse JDBC Driver are managed within the Pulsar IO framework, so the impact should also be managed.
   
   Still, it doesn't tell why we should use a shaded dependency here - what's the benefit? The NAR file size is a downsize, BTW.


-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] tisonkun merged pull request #18774: [feat][io] Upgarde ClickHouse driver to support loadbalance policy

Posted by GitBox <gi...@apache.org>.
tisonkun merged PR #18774:
URL: https://github.com/apache/pulsar/pull/18774


-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] codecov-commenter commented on pull request #18774: [feat][io] Upgarde ClickHouse driver to support loadbalance policy

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #18774:
URL: https://github.com/apache/pulsar/pull/18774#issuecomment-1340220741

   # [Codecov](https://codecov.io/gh/apache/pulsar/pull/18774?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#18774](https://codecov.io/gh/apache/pulsar/pull/18774?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (cf7bb1a) into [master](https://codecov.io/gh/apache/pulsar/commit/b36e012a36bba82a7303490e6b8d8f7b50929ce0?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b36e012) will **decrease** coverage by `16.56%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/pulsar/pull/18774/graphs/tree.svg?width=650&height=150&src=pr&token=acYqCpsK9J&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/pulsar/pull/18774?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@              Coverage Diff              @@
   ##             master   #18774       +/-   ##
   =============================================
   - Coverage     47.74%   31.17%   -16.57%     
   + Complexity    10623     6928     -3695     
   =============================================
     Files           703      737       +34     
     Lines         68828    71340     +2512     
     Branches       7381     7669      +288     
   =============================================
   - Hits          32861    22242    -10619     
   - Misses        32307    46067    +13760     
   + Partials       3660     3031      -629     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | unittests | `31.17% <ø> (-16.57%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/pulsar/pull/18774?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ava/org/apache/pulsar/broker/admin/v1/Brokers.java](https://codecov.io/gh/apache/pulsar/pull/18774/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9hZG1pbi92MS9Ccm9rZXJzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...va/org/apache/pulsar/broker/admin/v1/Clusters.java](https://codecov.io/gh/apache/pulsar/pull/18774/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9hZG1pbi92MS9DbHVzdGVycy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../org/apache/pulsar/broker/admin/v1/Properties.java](https://codecov.io/gh/apache/pulsar/pull/18774/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9hZG1pbi92MS9Qcm9wZXJ0aWVzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../apache/pulsar/broker/admin/v2/ResourceGroups.java](https://codecov.io/gh/apache/pulsar/pull/18774/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9hZG1pbi92Mi9SZXNvdXJjZUdyb3Vwcy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...sar/broker/stats/metrics/ManagedLedgerMetrics.java](https://codecov.io/gh/apache/pulsar/pull/18774/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9zdGF0cy9tZXRyaWNzL01hbmFnZWRMZWRnZXJNZXRyaWNzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...ar/common/naming/PartitionedManagedLedgerInfo.java](https://codecov.io/gh/apache/pulsar/pull/18774/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NvbW1vbi9uYW1pbmcvUGFydGl0aW9uZWRNYW5hZ2VkTGVkZ2VySW5mby5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...ker/service/schema/exceptions/SchemaException.java](https://codecov.io/gh/apache/pulsar/pull/18774/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9zZXJ2aWNlL3NjaGVtYS9leGNlcHRpb25zL1NjaGVtYUV4Y2VwdGlvbi5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...naming/RangeEquallyDivideBundleSplitAlgorithm.java](https://codecov.io/gh/apache/pulsar/pull/18774/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NvbW1vbi9uYW1pbmcvUmFuZ2VFcXVhbGx5RGl2aWRlQnVuZGxlU3BsaXRBbGdvcml0aG0uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...e/pulsar/broker/admin/impl/ResourceQuotasBase.java](https://codecov.io/gh/apache/pulsar/pull/18774/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9hZG1pbi9pbXBsL1Jlc291cmNlUXVvdGFzQmFzZS5qYXZh) | `0.00% <0.00%> (-96.43%)` | :arrow_down: |
   | [...he/pulsar/broker/service/AnalyzeBacklogResult.java](https://codecov.io/gh/apache/pulsar/pull/18774/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9zZXJ2aWNlL0FuYWx5emVCYWNrbG9nUmVzdWx0LmphdmE=) | `0.00% <0.00%> (-92.31%)` | :arrow_down: |
   | ... and [256 more](https://codecov.io/gh/apache/pulsar/pull/18774/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   


-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] zhicwu commented on a diff in pull request #18774: [feat][io] Upgarde ClickHouse driver to support loadbalance policy

Posted by GitBox <gi...@apache.org>.
zhicwu commented on code in PR #18774:
URL: https://github.com/apache/pulsar/pull/18774#discussion_r1045352319


##########
pulsar-io/jdbc/clickhouse/pom.xml:
##########
@@ -38,7 +38,7 @@
       <version>${project.version}</version>
     </dependency>
     <dependency>
-      <groupId>ru.yandex.clickhouse</groupId>
+      <groupId>com.clickhouse</groupId>

Review Comment:
   All of them because you only need the shaded jar :)
   
   ```xml
   <dependency>
       <groupId>com.clickhouse</groupId>
       <artifactId>clickhouse-jdbc</artifactId>
       <version>0.3.2-patch11</version>
       <classifier>all</classifier>
       <exclusions>
           <exclusion>
               <groupId>*</groupId>
               <artifactId>*</artifactId>
           </exclusion>
       </exclusions>
   </dependency>
   ```



-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] tisonkun commented on pull request #18774: [feat][io] Upgarde ClickHouse driver to support loadbalance policy

Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #18774:
URL: https://github.com/apache/pulsar/pull/18774#issuecomment-1352354251

   @nlu90 Agree. Test coverage is lack for this connector. I'm trying to add some.


-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] zhicwu commented on pull request #18774: [feat][io] Upgarde ClickHouse driver to support loadbalance policy

Posted by GitBox <gi...@apache.org>.
zhicwu commented on PR #18774:
URL: https://github.com/apache/pulsar/pull/18774#issuecomment-1341982189

   Hi @tisonkun, `com.clickhouse/clickhouse-jdbc:0.3.2-patch11` is a complete rewrite, so not surprisingly it's not fully backward-compatible. It seems `ClickHouseJdbcAutoSchemaSink` is the only usage without any test. As a result, we cannot tell if anything broken from CI. So I'm listing key changes here to help you understand and estimate the impact of upgrading the driver:
   
   1. Apache Http Client 4.x was replaced by Http(s)URLConnection, meaning http [properties](https://docs.oracle.com/javase/8/docs/api/java/net/doc-files/net-properties.html#MiscHTTP) and [proxies](https://docs.oracle.com/javase/8/docs/api/java/net/doc-files/net-properties.html#Proxies) are now considered which may cause problem within a complex runtime
   2. Data format between client and server was changed from TabSeparated to RowBinary, meaning we no longer can pass `null` to a non-nullable column
   3. Type mapping and timezone handling were changed as well, meaning same query may give us different results but I guess it should be mainly about timestamp with timezone
   4. Introduced non-standard table types in meta data: `DICTIONARY`, `LOG TABLE`, `MEMORY TABLE`, `REMOTE TABLE`, `SYSTEM TABLE`, `TEMPORARY TABLE`, so you may find some tables disappeared
   
   Having said that, since v0.3.2 is packed with both legacy and new JDBC drivers(`ru.yandex.clickhouse.ClickHouseDriver` vs. `com.clickhouse.jdbc.ClickHouseDriver`), you may add an option to let user to choose during the transition period. Starting from v0.3.3, there'll be no legacy driver anymore.
   
   Lastly, some more comments for you to consider regarding ClickHouse sink:
   * support nested data types like `Array`, `Tuple`, `Map`, `JSON`
   * ClickHouse supports [multiple data formats](https://clickhouse.com/docs/en/interfaces/formats/) and `Avro` is one of them, so maybe we don't have to convert Avro data type to JDBC type and then ClickHouse data type
   


-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] zhicwu commented on a diff in pull request #18774: [feat][io] Upgarde ClickHouse driver to support loadbalance policy

Posted by GitBox <gi...@apache.org>.
zhicwu commented on code in PR #18774:
URL: https://github.com/apache/pulsar/pull/18774#discussion_r1042894216


##########
pulsar-io/jdbc/clickhouse/pom.xml:
##########
@@ -38,7 +38,7 @@
       <version>${project.version}</version>
     </dependency>
     <dependency>
-      <groupId>ru.yandex.clickhouse</groupId>
+      <groupId>com.clickhouse</groupId>

Review Comment:
   Perhaps it's better to use shaded jar?
   
   <dependency>
       <groupId>com.clickhouse</groupId>
       <artifactId>clickhouse-jdbc</artifactId>
       <version>0.3.2-patch11</version>
       <classifier>all</classifier>
       <exclusions>
           <exclusion>
               <groupId>*</groupId>
               <artifactId>*</artifactId>
           </exclusion>
       </exclusions>
   </dependency>



-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] tisonkun commented on a diff in pull request #18774: [feat][io] Upgarde ClickHouse driver to support loadbalance policy

Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #18774:
URL: https://github.com/apache/pulsar/pull/18774#discussion_r1042902671


##########
pulsar-io/jdbc/clickhouse/pom.xml:
##########
@@ -38,7 +38,7 @@
       <version>${project.version}</version>
     </dependency>
     <dependency>
-      <groupId>ru.yandex.clickhouse</groupId>
+      <groupId>com.clickhouse</groupId>

Review Comment:
   Thanks for your review @zhicwu!
   
   What's the most common dependency conflict that can be avoided by using the shaded jar?
   
   I can see using the basic JAR, the final connector NAR is in size 9.7M, and after switching to the shaded jar, it grows to 46M.
   
   If no user reports the current solution's issue, I tend to keep it as is.



-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] tisonkun commented on pull request #18774: [feat][io] Upgarde ClickHouse driver to support loadbalance policy

Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #18774:
URL: https://github.com/apache/pulsar/pull/18774#issuecomment-1360832869

   @zhicwu Thanks for your explanation! I think it's good to merge now.
   
   Merging...
   
   @codelipenghui @eolivelli @Technoboy- FYI release note required on the next release.


-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] tisonkun commented on pull request #18774: [feat][io] Upgarde ClickHouse driver to support loadbalance policy

Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #18774:
URL: https://github.com/apache/pulsar/pull/18774#issuecomment-1352373155

   Hi @zhicwu! Let me ask some questions in details:
   
   > Data format between client and server was changed from TabSeparated to RowBinary, meaning we no longer can pass null to a non-nullable column
   
   What's the behavior if we pass null to a non-nullable column previously?
   
   > Type mapping and timezone handling were changed as well, meaning same query may give us different results but I guess it should be mainly about timestamp with timezone
   
   Is there a CHANGELOG for the details how the mapping and handling change?


-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] tisonkun commented on a diff in pull request #18774: [feat][io] Upgarde ClickHouse driver to support loadbalance policy

Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #18774:
URL: https://github.com/apache/pulsar/pull/18774#discussion_r1045353703


##########
pulsar-io/jdbc/clickhouse/pom.xml:
##########
@@ -38,7 +38,7 @@
       <version>${project.version}</version>
     </dependency>
     <dependency>
-      <groupId>ru.yandex.clickhouse</groupId>
+      <groupId>com.clickhouse</groupId>

Review Comment:
   @zhicwu That's it!



-- 
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: commits-unsubscribe@pulsar.apache.org

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