Skip to content

clickhouse-library-bridge for library dictionary source#21509

Merged
alesapin merged 61 commits intoClickHouse:masterfrom
kssenii:library-bridge
Apr 6, 2021
Merged

clickhouse-library-bridge for library dictionary source#21509
alesapin merged 61 commits intoClickHouse:masterfrom
kssenii:library-bridge

Conversation

@kssenii
Copy link
Copy Markdown
Member

@kssenii kssenii commented Mar 7, 2021

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

Changelog category (leave one):

  • Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Add clickhouse-library-bridge for library dictionary source. Closes #9502.

@robot-clickhouse robot-clickhouse added doc-alert pr-feature Pull request with new product feature labels Mar 7, 2021
@alesapin alesapin self-assigned this Mar 7, 2021
@robot-clickhouse robot-clickhouse added pr-improvement Pull request with some product improvements and removed pr-feature Pull request with new product feature labels Mar 7, 2021
@kssenii kssenii removed the doc-alert label Mar 7, 2021
@kssenii kssenii marked this pull request as ready for review March 17, 2021 08:48
Copy link
Copy Markdown
Member

@alesapin alesapin left a comment

Choose a reason for hiding this comment

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

Two main problems:

  1. We should pass IDs through the POST body.
  2. Seems like the attributes list is redundant and we don't need to serialize it.

Copy link
Copy Markdown
Member

@alesapin alesapin left a comment

Choose a reason for hiding this comment

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

Almost OK.

void handleRequest(HTTPServerRequest & request, HTTPServerResponse & response) override;

private:
size_t keep_alive_timeout;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

const?

@alesapin
Copy link
Copy Markdown
Member

alesapin commented Apr 6, 2021

All failures not related to changes.

@alesapin alesapin merged commit 86a843b into ClickHouse:master Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-improvement Pull request with some product improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Library dictionary source is harmful

4 participants