Skip to content

Support optimize_syntax_fuse_functions for sum/count/avg via analyzer#42689

Merged
kitaisreal merged 10 commits intomasterfrom
vdimir/identifier-resolver-fuse-sum
Nov 10, 2022
Merged

Support optimize_syntax_fuse_functions for sum/count/avg via analyzer#42689
kitaisreal merged 10 commits intomasterfrom
vdimir/identifier-resolver-fuse-sum

Conversation

@vdimir
Copy link
Copy Markdown
Member

@vdimir vdimir commented Oct 26, 2022

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):

Support optimize_fuse_sum_count_avg, ref #42648

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

@robot-clickhouse robot-clickhouse added the pr-not-for-changelog This PR should not be mentioned in the changelog label Oct 26, 2022
@kitaisreal kitaisreal self-assigned this Oct 26, 2022
kitaisreal
kitaisreal suggested changes Oct 26, 2022
View reviewed changes
kitaisreal
kitaisreal reviewed Oct 26, 2022
View reviewed changes
@novikd novikd mentioned this pull request Oct 30, 2022
Closed
@vdimir vdimir force-pushed the vdimir/identifier-resolver-fuse-sum branch 2 times, most recently from 1eaf928 to 947fd73 Compare November 4, 2022 15:14
kitaisreal
kitaisreal reviewed Nov 7, 2022
View reviewed changes
kitaisreal
kitaisreal suggested changes Nov 8, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@kitaisreal kitaisreal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. There are a couple of small comments to fix.

@vdimir vdimir force-pushed the vdimir/identifier-resolver-fuse-sum branch 2 times, most recently from 1b5294f to ad12e0f Compare November 9, 2022 11:46
@vdimir vdimir force-pushed the vdimir/identifier-resolver-fuse-sum branch from ad12e0f to 5f5fbce Compare November 9, 2022 11:48
@kitaisreal
Copy link
Copy Markdown
Contributor

kitaisreal commented Nov 10, 2022

Fuzzer tests are related to analyzer, but unrelated to pull request changes, and already registered.

@kitaisreal kitaisreal merged commit 8c33381 into master Nov 10, 2022
@kitaisreal kitaisreal deleted the vdimir/identifier-resolver-fuse-sum branch November 10, 2022 10:05
@CurtizJ
Copy link
Copy Markdown
Member

CurtizJ commented Nov 10, 2022 •
edited
Loading

Some fuzzer errors related to this PR:

https://s3.amazonaws.com/clickhouse-test-reports/0/8c333817e8bbb21129fa318e3cfadbfdec33b8bc/fuzzer_astfuzzerubsan//report.html

CREATE TABLE fuse_tbl__fuzz_3 (`a` UInt16, `b` Array(Nullable(Int8))) ENGINE = Log

...

2022.11.10 15:00:46.268780 [ 147 ] {ea3a722f-e4a7-41d6-a744-def6e4553c5f} <Fatal> : Logical error: 'Unsupported function 'divide''.
2022.11.10 15:00:46.269100 [ 411 ] {} <Fatal> BaseDaemon: ########################################
2022.11.10 15:00:46.269137 [ 411 ] {} <Fatal> BaseDaemon: (version 22.11.1.1 (official build), build id: 2CFFDB268057C264C33588EF966096D58BD0BEBD) (from thread 147) (query_id: ea3a722f-e4a7-41d6-a744-def6e4553c5f) (query: SELECT quantile('10')(b - NULL, b) FROM (SELECT avg(b) * 1.1754943508222875e-38, x + -2 AS b FROM (SELECT quantile(0.5)(b) AS x, quantile(1.1920928955078125e-7)(b) FROM fuse_tbl__fuzz_3) GROUP BY x WITH CUBE)) Received signal Aborted (6)

https://s3.amazonaws.com/clickhouse-test-reports/0/8c333817e8bbb21129fa318e3cfadbfdec33b8bc/fuzzer_astfuzzermsan//report.html

CREATE TABLE fuse_tbl__fuzz_9 (`a` Array(UInt16), `b` Nullable(Int32)) ENGINE = Log

...

2022.11.10 15:17:30.647636 [ 423 ] {} <Fatal> BaseDaemon: (version 22.11.1.1 (official build), build id: D30ACE577B310F0B2AAA302C8B9953D53529C457) (from thread 163) (query_id: c49e983c-e134-4e85-b13f-6767fbbc5bdc) (query: SELECT sum(a + 1.1754943508222875e-38), sum(b), count(b), avg(a + 3.4028234663852886e38), sum(a + NULL), count(a) FROM fuse_tbl__fuzz_9 SETTINGS optimize_syntax_fuse_functions = 0) Received signal Unknown signal -3 (-3)
2022.11.10 15:17:30.647914 [ 423 ] {} <Fatal> BaseDaemon: Sanitizer trap.

@kitaisreal
Copy link
Copy Markdown
Contributor

kitaisreal commented Nov 10, 2022

@vdimir could you please take a look into them ?

@vdimir vdimir mentioned this pull request Nov 14, 2022
Merged
@vdimir vdimir mentioned this pull request Nov 30, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@kitaisreal kitaisreal kitaisreal approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@kitaisreal kitaisreal

Labels

pr-not-for-changelog This PR should not be mentioned in the changelog

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@vdimir @kitaisreal @CurtizJ @robot-clickhouse