Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix decimal aggregates test for s390x #49382

Merged
merged 3 commits into from
May 2, 2023

Conversation

HarryLeeIBM
Copy link
Contributor

@HarryLeeIBM HarryLeeIBM commented May 2, 2023

Functional test 00700_decimal_aggregates fails on s390x because the following SQL returns different results on LE and BE machines:

SELECT (uniqHLL12(a), uniqHLL12(b), uniqHLL12(c))
FROM (SELECT * FROM decimal ORDER BY a);

It returns (102,100,101) on LE machines and (99,101,102) on BE machines.

Actually both results are correct since HyperLogLog algorithm allows some small errors and it hashes LE numbers on LE machines and BE numbers on BE machines which causes different results(See the following code in src/AggregateFunctions/UniqVariadicHash.h).

struct UniqVariadicHash<false, false>
{
    static inline UInt64 apply(size_t num_args, const IColumn ** columns, size_t row_num)
    {
        UInt64 hash;

        const IColumn ** column = columns;
        const IColumn ** columns_end = column + num_args;

        {
            StringRef value = (*column)->getDataAt(row_num);
            hash = CityHash_v1_0_2::CityHash64(value.data, value.size);
            ++column;
        }

So the fix is to modify the SQL so that it works correct on both LE and BE machines.

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

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

Fixed decimal aggregates functional test for x390x.

Information about CI checks: https://clickhouse.com/docs/en/development/continuous-integration/

@robot-clickhouse-ci-2
Copy link
Contributor

robot-clickhouse-ci-2 commented May 2, 2023

This is an automated comment for commit 3ab9475 with description of existing statuses. It's updated for the latest CI running
The full report is available here
The overall status of the commit is 🟡 pending

Check nameDescriptionStatus
CI runningA 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
Mergeable CheckChecks if all other necessary checks are successful🟢 success
Push to DockerhubThe check for building and pushing the CI related docker images to docker hub🟢 success

@hanfei1991 hanfei1991 self-assigned this May 2, 2023
@hanfei1991 hanfei1991 added can be tested Allows running workflows for external contributors pr-not-for-changelog This PR should not be mentioned in the changelog labels May 2, 2023
@hanfei1991
Copy link
Member

unrelated flaky test #49408

@hanfei1991 hanfei1991 merged commit b5eddf3 into ClickHouse:master May 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
can be tested Allows running workflows for external contributors pr-not-for-changelog This PR should not be mentioned in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants