Open OlegGalizin opened 2 months ago
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
What can I do to help you to merge the request?
Is I doing something wrong or nobody has time to merge the request?
@OlegGalizin Please sign sign our Contributor License Agreement And we will review your code.
Hi @OlegGalizin thanks for submitting a PR, please address mentioned issues.
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.
Please help me run the tests
Please fix the tests
@OlegGalizin please fix tests
@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.
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?
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)
please either get rid of
find_quoted_chars
or provide a benchmark
Please do it himself.