Skip to content

(feature) Use Map data type for system logs tables#18762

Closed
sundy-li wants to merge 20 commits intoClickHouse:masterfrom
sundy-li:system-querylog-map
Closed

(feature) Use Map data type for system logs tables#18762
sundy-li wants to merge 20 commits intoClickHouse:masterfrom
sundy-li:system-querylog-map

Conversation

@sundy-li
Copy link
Copy Markdown
Contributor

@sundy-li sundy-li commented Jan 5, 2021 •
edited
Loading

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

Changelog category (leave one):

  • Backward Incompatible Change

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

Backward Incompatible Change:

  • Use Map data type for system logs tables (system.query_log, system.query_thread_log, system.processes, system.OpenTelemetrySpanLog).
    These tables will be auto created with new datatypes, old queries maybe incompatible.

Detailed description / Documentation draft:

...

closes #18698

@robot-clickhouse robot-clickhouse added doc-alert pr-feature Pull request with new product feature labels Jan 5, 2021
@sundy-li
Copy link
Copy Markdown
Contributor Author

sundy-li commented Jan 5, 2021

Some tests use WHERE has(ProfileEvents.Names, 'OSCPUVirtualTimeMicroseconds').
we can change it to ProfileEvents['OSCPUVirtualTimeMicroseconds'] > 0.

But I think we should add mapContains(map, key) function first.

@alexey-milovidov
Copy link
Copy Markdown
Member

alexey-milovidov commented Jan 5, 2021

Ok, let's add this function.

@hexiaoting
Copy link
Copy Markdown
Contributor

hexiaoting commented Jan 6, 2021

Ok, let's add this function.

added. #18788

@alexey-milovidov
Copy link
Copy Markdown
Member

alexey-milovidov commented Jan 16, 2021

@sundy-li Let's change the tests and continue.

@alexey-milovidov alexey-milovidov self-assigned this Jan 16, 2021
@sundy-li
Copy link
Copy Markdown
Contributor Author

sundy-li commented Jan 17, 2021 •
edited
Loading

@sundy-li Let's change the tests and continue.

Ok，better to do it after #18788 is merged.

@qoega
Copy link
Copy Markdown
Member

qoega commented Jan 25, 2021

Looks great! But later we should not forget to add backward incompatible change to changelog if we change default behavior for this tables. Am I right that old behavior without enabled new flag will no longer br checked in tests after this PR?

@sundy-li
Copy link
Copy Markdown
Contributor Author

sundy-li commented Jan 25, 2021 •
edited
Loading

Looks great! But later we should not forget to add backward incompatible change to changelog if we change default behavior for this tables. Am I right that old behavior without enabled new flag will no longer br checked in tests after this PR?

No, old behavior will not be compatible after this PR, because we already change the datatype to Map. It's for system tables. It's global, but allow_experimental_map_type is for each separated query.

Currently there's one failed test in 01455_opentelemetry_distributed.sh, I'm working on solving it.

Size of filter doesn't match size of colum

@robot-clickhouse robot-clickhouse added pr-backward-incompatible Pull request with backwards incompatible changes and removed pr-feature Pull request with new product feature labels Jan 25, 2021
@sundy-li
Copy link
Copy Markdown
Contributor Author

sundy-li commented Jan 25, 2021

Failed tests are not related, ready to review. cc @alexey-milovidov @qoega

@alexey-milovidov
Copy link
Copy Markdown
Member

alexey-milovidov commented Feb 18, 2021

Test failures are related somehow: https://clickhouse-test-reports.s3.yandex.net/18762/0d2d2dc2ed06ea70274a384846e2d421ea0f0b9e/functional_stateless_tests_flaky_check_(address).html#fail1

@hexiaoting hexiaoting mentioned this pull request May 7, 2021
Merged
@sundy-li
Copy link
Copy Markdown
Contributor Author

sundy-li commented May 8, 2021

continue in #23934

@sundy-li sundy-li closed this May 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@alexey-milovidov alexey-milovidov

Labels

pr-backward-incompatible Pull request with backwards incompatible changes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Use Map data type for system logs tables.

5 participants

@sundy-li @alexey-milovidov @hexiaoting @qoega @robot-clickhouse