Add ClickHouse support #498
Conversation
tolkonepiu
force-pushed
the
clickhouse
branch
from
e372567 to
fa3b12e
Compare
| @@ -78,9 +81,9 @@ public void test() throws SQLException { | |||
| } | |||
| } | |||
|
|
|||
| private void performSimpleTest(String jdbcUrl) throws SQLException { | |||
| private void performSimpleTest(String jdbcUrl, boolean pmdKnownBroken) throws SQLException { | |||
There was a problem hiding this comment.
I'm not so happy with adding an additional param to the method if it's only needed by on tested class.
The tests will fail if not set?
There was a problem hiding this comment.
Method getParameterMetaData not implemented on clickhouse jdbc driver.
|
|
||
| @Override | ||
| public String getDriverClassName() { | ||
| return "ru.yandex.clickhouse.ClickHouseDriver"; |
There was a problem hiding this comment.
Please put the String into a constant.
| private String password = ""; | ||
|
|
||
| public ClickHouseContainer() { | ||
| super(IMAGE + ":1.1.54310"); |
|
|
||
| @Override | ||
| public String getJdbcUrl() { | ||
| return "jdbc:clickhouse://" + getContainerIpAddress() + ":" + getMappedPort(HTTP_PORT) + "/" + databaseName; |
|
|
||
| @Override | ||
| public String getTestQueryString() { | ||
| return "SELECT 1"; |
There was a problem hiding this comment.
In order to be consistent, the query String could go into a constant as well 🙂
|
We're shortly going to be merging #574, which changes our build system to Gradle. This is in part intended to make contributions of modules easier (per #564), but unfortunately means that for a short while your PR is going to show merge conflicts with the master branch. I just want to let you know we don't want to create new work for you: we'll take care of the merge conflicts shortly. Please don't worry - we're grateful for your PR and want to help integrate it soon. Thank you. |
|
@tolkonepiu Don't you mind if I'd fix code review problems and open new pull request base on your changes? |
|
@bsideup @VladRassokhin @tolkonepiu Hi guys. |
|
I'll rebase code onto master, test it and submit separate PR by the end of this week. |
|
superseded by #846 |
Add support for ClickHouse