clickhouse-cpp icon indicating copy to clipboard operation
clickhouse-cpp copied to clipboard

Parameters in clickhouse-cpp

Open OlegGalizin opened this issue 1 year ago • 7 comments

  • params
  • Nullable params

OlegGalizin avatar Sep 20 '24 13:09 OlegGalizin

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

:white_check_mark: Enmk
:x: Oleg Galizin


Oleg Galizin seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

CLAassistant avatar Sep 20 '24 13:09 CLAassistant

What can I do to help you to merge the request?

OlegGalizin avatar Oct 01 '24 05:10 OlegGalizin

Is I doing something wrong or nobody has time to merge the request?

OlegGalizin avatar Oct 18 '24 07:10 OlegGalizin

@OlegGalizin Please sign sign our Contributor License Agreement And we will review your code.

1261385937 avatar Oct 19 '24 04:10 1261385937

Hi @OlegGalizin thanks for submitting a PR, please address mentioned issues.

Enmk avatar Oct 21 '24 15:10 Enmk

I've add ci test. I did'n found how can I change a PR description. The description is Support query with parameters in clickhouse-cpp

If You can please add the description in proper place please.

OlegGalizin avatar Oct 21 '24 18:10 OlegGalizin

Please help me run the tests

OlegGalizin avatar Oct 23 '24 08:10 OlegGalizin

Please fix the tests

Enmk avatar Nov 01 '24 23:11 Enmk

@OlegGalizin please fix tests

Enmk avatar Nov 08 '24 19:11 Enmk

@OlegGalizin ping, Client/ClientCase.QueryParameters is failing on almost every platform... BTW, to simplify build-test-review loop, you can fix test using your own fork of clickhouse-ccp. Runners are provided by github, so it would work on your repo.

Enmk avatar Nov 12 '24 15:11 Enmk

Client/ClientCase.QueryParameters is failing on almost every platform

@Enmk I see. Clickhouse server doesn't support parameters in the test system.

+-----------+
| version() |
+-----------+
|    String |
+-----------+
| 22.3.20.29 |
+-----------+

I checked ClickHouse 24.7.1.2195 (revision: 54488, git hash: 452d463d77eed30aab18c77933e68316f8c57295 To work with parameters revision of ClickHouse server must be >= 54459

Do you want to switch off the test? Will you update the test environment?

OlegGalizin avatar Nov 13 '24 08:11 OlegGalizin

Client/ClientCase.QueryParameters is failing on almost every platform

@Enmk I see. Clickhouse server doesn't support parameters in the test system.

+-----------+
| version() |
+-----------+
|    String |
+-----------+
| 22.3.20.29 |
+-----------+

I checked ClickHouse 24.7.1.2195 (revision: 54488, git hash: 452d463d77eed30aab18c77933e68316f8c57295 To work with parameters revision of ClickHouse server must be >= 54459

Do you want to switch off the test? Will you update the test environment?

Some unit tests are conditionally skipped based on clickhouse version, please check the code and modify your tests accordingly.

And please either get rid of find_quoted_chars or provide a benchmark that shows at least 10% smaller time against std::find_first_of. (e.g. using https://quick-bench.com)

Enmk avatar Nov 17 '24 21:11 Enmk

please either get rid of find_quoted_chars or provide a benchmark

Please do it himself.

OlegGalizin avatar Nov 26 '24 18:11 OlegGalizin

How about merge the PR to master?

OlegGalizin avatar Dec 19 '24 16:12 OlegGalizin