Ecosyste.ms: Timeline
Browse the timeline of events for every public repo on GitHub. Data updated hourly from GH Archive.
pamarcos closed a pull request on ClickHouse/ClickHouse
CollapsingMergeTree: Do not include rows with invalid sign for FINAL
In https://github.com/ClickHouse/ClickHouse/pull/73864 (which is unreleased), I implemented merging rows with invalid signs in CollapsingMergeTree. However, I missed an (undocumented) implementatio...
pamarcos created a comment on an issue on ClickHouse/ClickHouse
> To collect the feedback, the PR [#75087](https://github.com/ClickHouse/ClickHouse/pull/75087) needed two rebase/merge-master today to solve the issue that normally wouldn't be there Please elabo...
pamarcos created a review on a pull request on ClickHouse/ClickHouse
This means we won't be able to select to use the merge commit for specific PRs if required, right? That's a pity How come we could use it before without inconsistencies? Or we had them, but did ne...
pamarcos created a review on a pull request on ClickHouse/ClickHouse
This means we won't be able to select to use the merge commit for specific PRs if required, right? That's a pity How come we could use it before without inconsistencies? Or we had them, but did ne...
pamarcos created a comment on an issue on ClickHouse/ClickHouse
I've still seen some instances where this test fails. Very rare, but still. From the log: ``` zstdgrep -ina "QueryMetricLog" clickhouse-server5.log.zst | grep -i "test_lpzezq5e_1000" 8096028:2025....
pamarcos created a review comment on a pull request on ClickHouse/ClickHouse
If you want al sockets to be closed, you should move this within the for-loop. Before L1035: ```cpp SCOPE_EXIT({ if (sock) close(sock); }); ```
pamarcos created a review comment on a pull request on ClickHouse/ClickHouse
You're right. I can't see any way a `Poco::MD5Engine` takes a `Poco::FIleInputStream`. I don't know if I misread when I wrote the comment or I assumed it would exist :roll_eyes:
pamarcos created a review comment on a pull request on ClickHouse/ClickHouse
You don't use `getTypeClass` because you really need to point to a valid object of the type, right?
pamarcos created a review comment on a pull request on ClickHouse/ClickHouse
This should probably be a `LOGICAL_ERROR`, but if you really want to use `assert` here and in other places, you'd rather use `chassert` which reports much nicer
pamarcos created a review comment on a pull request on ClickHouse/ClickHouse
Not a big issue, but I've seen `std::vector<String>` quite a few times already. There's a `Strings` defined in `src/Core/Types.h`
pamarcos created a review comment on a pull request on ClickHouse/ClickHouse
This code is exactly the same as in L367-369. Not a big issue, but maybe could be refactored so the same code returns these three things?
pamarcos created a review comment on a pull request on ClickHouse/ClickHouse
Comments use 3 slashes and need a whitespace afterwards: ``` /// Like this ```