Remove garbage. #56350

alexey-milovidov · 2023-11-06T00:13:41Z

Changelog category (leave one):

Backward Incompatible Change

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

You can no longer use the aggregate function avgWeighted with arguments of type Decimal. Workaround: convert arguments to Float64. This closes #43928. This closes #31768. This closes #56435. If you have used this function inside materialized views or projections with Decimal arguments, contact [email protected]. Fixed error in aggregate function sumMap and made it slower around 1.5..2 times. It does not matter because the function is garbage anyway. This closes #54955. This closes #53134. This closes #55148. Fix a bug in function groupArraySample - it used the same random seed in case more than one aggregate state is generated in a query.

robot-ch-test-poll2 · 2023-11-06T00:18:01Z

This is an automated comment for commit c4de404 with description of existing statuses. It's updated for the latest CI running

❌ Click here to open a full report in a separate page

Successful checks

Check name	Description	Status
AST fuzzer	Runs randomly generated queries to catch program errors. The build type is optionally given in parenthesis. If it fails, ask a maintainer for help	✅ success
ClickHouse build check	Builds ClickHouse in various configurations for use in further steps. You have to fix the builds that fail. Build logs often has enough information to fix the error, but you might have to reproduce the failure locally. The cmake options can be found in the build log, grepping for cmake. Use these options and follow the general build process	✅ success
Compatibility check	Checks that clickhouse binary runs on distributions with old libc versions. If it fails, ask a maintainer for help	✅ success
Docker image for servers	The check to build and optionally push the mentioned image to docker hub	✅ success
Fast test	Normally this is the first check that is ran for a PR. It builds ClickHouse and runs most of stateless functional tests, omitting some. If it fails, further checks are not started until it is fixed. Look at the report to see which tests fail, then reproduce the failure locally as described here	✅ success
Flaky tests	Checks if new added or modified tests are flaky by running them repeatedly, in parallel, with more randomization. Functional tests are run 100 times with address sanitizer, and additional randomization of thread scheduling. Integrational tests are run up to 10 times. If at least once a new test has failed, or was too long, this check will be red. We don't allow flaky tests, read the doc	✅ success
Install packages	Checks that the built packages are installable in a clear environment	✅ success
Mergeable Check	Checks if all other necessary checks are successful	✅ success
Push to Dockerhub	The check for building and pushing the CI related docker images to docker hub	✅ success
SQLTest	There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS	✅ success
SQLancer	Fuzzing tests that detect logical bugs with SQLancer tool	✅ success
Sqllogic	Run clickhouse on the sqllogic test set against sqlite and checks that all statements are passed	✅ success
Stateful tests	Runs stateful functional tests for ClickHouse binaries built in various configurations -- release, debug, with sanitizers, etc	✅ success
Stress test	Runs stateless functional tests concurrently from several clients to detect concurrency-related errors	✅ success
Style Check	Runs a set of checks to keep the code style clean. If some of tests failed, see the related log from the report	✅ success
Unit tests	Runs the unit tests for different release types	✅ success
Upgrade check	Runs stress tests on server version from last release and then tries to upgrade it to the version from the PR. It checks if the new server can successfully startup without any errors, crashes or sanitizer asserts	✅ success

Check name	Description	Status
CI running	A meta-check that indicates the running CI. Normally, it's in success or pending state. The failed status indicates some problems with the PR	⏳ pending
Integration tests	The integration tests report. In parenthesis the package type is given, and in square brackets are the optional part/total tests	❌ failure
Performance Comparison	Measure changes in query performance. The performance test report is described in detail here. In square brackets are the optional part/total tests	❌ failure
Stateless tests	Runs stateless functional tests for ClickHouse binaries built in various configurations -- release, debug, with sanitizers, etc	❌ failure

alexey-milovidov · 2023-11-12T04:53:09Z

sumMap is utter garbage. It uses promoted Decimal type, e.g. Decimal128, for state serialization while using non-promoted Decimal type for the result.

alexey-milovidov · 2023-11-12T05:27:29Z

@Algunenano, WTF? https://pastila.nl/?0003a09b/4d7bd2d352a396961eaf91dcdd69c3f3#NshzW/pAMeXr/GF9QxXfMA==

alexey-milovidov · 2023-11-12T06:47:40Z

@Algunenano I've added a test, and it produces MSan error on master:
https://pastila.nl/?000b9ea7/0b76fa5d47173281f3f5160e7b4e9836#C3+cFjMNJwkTHcg28gAlEQ==

It means we have to merge this ASAP.

alexey-milovidov · 2023-11-12T07:12:59Z

This PR will fix #54955 and #53134.

alexey-milovidov · 2023-11-12T07:14:49Z

I regret that we let #7898 and #1250 into our codebase.

…-symbols

alexey-milovidov · 2023-11-13T03:09:41Z

#56650

Algunenano · 2023-11-13T11:58:04Z

src/AggregateFunctions/AggregateFunctionUniqCombined.h

+};
+
+template <UInt8 K, typename HashValueType>
+AggregateFunctionPtr createAggregateFunctionWithK(const DataTypes & argument_types, const Array & params)


I wonder why K needs to be part of the template and force compiling the same functions over and over. It seems HyperLogLogCounter could just get k as a parameter

tavplubix · 2023-11-15T15:38:52Z

@alexey-milovidov, you forgot to check the failed tests before merging:
Performance Comparison [3/4] — 2 errors - https://s3.amazonaws.com/clickhouse-test-reports/56350/c4de4045f15df2a835d7fb73793d75e3216e62bf/performance_comparison_[3_4]/report.html
it was related, this PR breaks Performance tests in master. Should we revert this PR?

tavplubix · 2023-11-15T15:42:52Z

Fix: #56797

* Add aggregation function for Largest-Triangle-Three-Buckets (#53145) * Added a simple lttb aggregate function * Added support for multiple datatypes * Added support for Date and Date32, updated LTTBData struct * Updated code to handle bucket size 0 and 1 * Added sort for LTTBData * Added tests and documentation * Added some code style fixes * Added function to new func ref file * Removed function from new func ref file * Apply suggestions from code review * Updated unit tests * updated LTTB data code * Minor style fixes * Updated code with std sort * updated tests * Renamed lttb to largestTriangleThreeBuckets * Added alias lttb --------- Co-authored-by: Alexey Milovidov <[email protected]> * ClickHouse/ClickHouse#56350 (partial) * fix: split lttb bucket strategy, first bucket and last bucket should only contain single point (#57003) * fix: split lttb bucket policy, first bucket and last bucket should only contain single point * add comments and modify the corresponding query test * style: update code format * style: remove useless comments * feat: add lttb bucket size test * fix: typo, duplicate sql * Merge pull request #62646 from Algunenano/i_like_triangles Fix crash in largestTriangleThreeBuckets * ClickHouse/ClickHouse#60469 (partial) * ClickHouse/ClickHouse#68135 (partial) * Merge pull request #73172 from ucasfl/fix-ubsan Fix UBSAN in largestTriangleThreeBuckets * fixes --------- Co-authored-by: Sinan <[email protected]> Co-authored-by: Alexey Milovidov <[email protected]> Co-authored-by: FFish <[email protected]> Co-authored-by: Raúl Marín <[email protected]> Co-authored-by: Yarik Briukhovetskyi <[email protected]>

* Add aggregation function for Largest-Triangle-Three-Buckets (#53145) * Added a simple lttb aggregate function * Added support for multiple datatypes * Added support for Date and Date32, updated LTTBData struct * Updated code to handle bucket size 0 and 1 * Added sort for LTTBData * Added tests and documentation * Added some code style fixes * Added function to new func ref file * Removed function from new func ref file * Apply suggestions from code review * Updated unit tests * updated LTTB data code * Minor style fixes * Updated code with std sort * updated tests * Renamed lttb to largestTriangleThreeBuckets * Added alias lttb --------- Co-authored-by: Alexey Milovidov <[email protected]> * ClickHouse/ClickHouse#56350 (partial) * fix: split lttb bucket strategy, first bucket and last bucket should only contain single point (#57003) * fix: split lttb bucket policy, first bucket and last bucket should only contain single point * add comments and modify the corresponding query test * style: update code format * style: remove useless comments * feat: add lttb bucket size test * fix: typo, duplicate sql * Merge pull request #62646 from Algunenano/i_like_triangles Fix crash in largestTriangleThreeBuckets * ClickHouse/ClickHouse#60469 (partial) * ClickHouse/ClickHouse#68135 (partial) * Merge pull request #73172 from ucasfl/fix-ubsan Fix UBSAN in largestTriangleThreeBuckets * fixes

alexey-milovidov added 4 commits November 6, 2023 01:11

Add table system.symbols

8182d51

Minor changes

99fe7a4

Remove fat symbols

995e8a9

Add a test

9976006

robot-ch-test-poll2 added the pr-feature Pull request with new product feature label Nov 6, 2023

alexey-milovidov added 24 commits November 6, 2023 01:34

Remove useless header

2d1351c

Attempt to remove garbage

99d90a1

Continue removal of garbage

f9689bd

Can I get away with removing so much garbage?

9fe9379

Add docs

0b5c77b

This is actually better

46af71a

This is actually better

9c9fbf2

This is actually better

e85c695

This is actually better

a710ac6

Remove useless header files

184e6f8

Remove useless header files

34e52b8

Remove useless header files

0f9e759

Remove useless header files

1c31889

Fix style

19f178f

Fix style

1f058d6

Remove trash (C++ templates)

4a10b3e

Fix test

9320f2c

Fix error

c0e0581

Fix test

23eca82

Fix test

a906f88

Fix tidy

145c37f

Fix FreeBSD

66b180d

Fix FreeBSD

9e6dee9

Fix tidy

d01108a

alexey-milovidov added 5 commits November 11, 2023 08:58

Maybe fix tidy

7138174

Merge branch 'master' into system-symbols

bd78cc3

Fix tidy

0de9f83

Parallelize compilation for RISC-V

84bd67d

Fix style

faee91a

alexey-milovidov added 2 commits November 12, 2023 06:39

Something

345f393

Fix idiotic behavior of sumMap aggregate function

e6149cb

This was referenced Nov 12, 2023

Invalid Field get from type Decimal64 to type Decimal128 #54955

Closed

AggregateFunctionSumMap fails with ASAN build with RelWithDebInfo on v23.3-lts #53134

Closed

Merge branch 'master' of github.com:ClickHouse/ClickHouse into system…

a044c61

…-symbols

alexey-milovidov added 2 commits November 13, 2023 06:25

Remove garbage from libssh

194c696

Fix error

c4de404

robot-ch-test-poll2 added the submodule changed At least one submodule changed in this PR. label Nov 13, 2023

alexey-milovidov merged commit 7a48905 into master Nov 13, 2023

alexey-milovidov deleted the system-symbols branch November 13, 2023 11:25

Algunenano reviewed Nov 13, 2023

View reviewed changes

tavplubix mentioned this pull request Nov 15, 2023

Update avg_weighted.xml #56797

Merged

alexey-milovidov mentioned this pull request Nov 19, 2023

MemorySanitizer: use-of-uninitialized-value ParallelFormattingOutputFormat #55148

Closed

Algunenano mentioned this pull request Jan 18, 2024

sumMapFiltered aggregate function wrong result starting from 23.11 #58741

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove garbage. #56350

Remove garbage. #56350

alexey-milovidov commented Nov 6, 2023 •

edited

Loading

robot-ch-test-poll2 commented Nov 6, 2023 •

edited

Loading

alexey-milovidov commented Nov 12, 2023

alexey-milovidov commented Nov 12, 2023

alexey-milovidov commented Nov 12, 2023

alexey-milovidov commented Nov 12, 2023

alexey-milovidov commented Nov 12, 2023

alexey-milovidov commented Nov 13, 2023

Algunenano Nov 13, 2023

tavplubix commented Nov 15, 2023

tavplubix commented Nov 15, 2023

Remove garbage. #56350

Remove garbage. #56350

Conversation

alexey-milovidov commented Nov 6, 2023 • edited Loading

Changelog category (leave one):

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

robot-ch-test-poll2 commented Nov 6, 2023 • edited Loading

alexey-milovidov commented Nov 12, 2023

alexey-milovidov commented Nov 12, 2023

alexey-milovidov commented Nov 12, 2023

alexey-milovidov commented Nov 12, 2023

alexey-milovidov commented Nov 12, 2023

alexey-milovidov commented Nov 13, 2023

Algunenano Nov 13, 2023

Choose a reason for hiding this comment

tavplubix commented Nov 15, 2023

tavplubix commented Nov 15, 2023

alexey-milovidov commented Nov 6, 2023 •

edited

Loading

robot-ch-test-poll2 commented Nov 6, 2023 •

edited

Loading