Dictionaries ddl loader by alesapin · Pull Request #7360 · ClickHouse/ClickHouse

alesapin · 2019-10-16T18:00:47Z

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

For changelog. Remove if this is non-significant change.

Category (leave one):

New Feature

Short description (up to few sentences):
Add the ability to create dictionaries with DDL queries.

alesapin · 2019-10-16T18:03:45Z

TODO:

Engine = Dictionary() table for each dictionary inside database.
system.dictionaries
More tests (also integeration for MySQL)
Bug fixes

dbms/src/Storages/System/StorageSystemDictionaries.cpp

alex-krash · 2019-10-16T21:19:59Z

dbms/tests/queries/0_stateless/01018_ddl_dictionaries_create.sql

+  second_column UInt8 DEFAULT 1 EXPRESSION rand() % 222,
+  third_column String DEFAULT 'qqq'
+)
+PRIMARY KEY key_column, second_column


This one looks pretty cool, but inconsistent with current implementation for MergeTree ordering/pkey qualifier, which requires (key_column, second_column) format.
Just wondering - would this approach become available for ordinal table creation as well (in future)?

I don't see anything against your proposal. Maybe it would be better to make round brackets optional for dictionaries.

alex-krash · 2019-10-16T21:28:04Z

dbms/tests/queries/0_stateless/01018_ddl_dictionaries_create.sql

+)
+PRIMARY KEY key_column
+SOURCE(CLICKHOUSE(HOST 'localhost' PORT 9000 USER 'default' TABLE 'table_for_dict' PASSWORD '' DB 'database_for_dict'))
+LIFETIME(MIN 1 MAX 10)


As I can see - we have here two mandatory options (min, max). Maybe, it is better to omit them, allowing to specify in a form like LIFETIME(1,10)?
Pros:

No extra syntax constructions needed

Less symbols to type :)

Looks more native for SQL users (like function call)

Cons:

If lifetime will have more complicated configuration, it will get stuck. E.g.: LIFETIME(MAX 10 IF_ONCE_QUERIED_REMEMBER_FOREVER 1) (here MIN 1 is omitted, but another option is introduced)

Current LIFETIME definition is consistent with other parts RANGE, SOURCE , etc.

filimonov · 2019-10-23T23:28:54Z

dbms/tests/queries/0_stateless/01018_ddl_dictionaries_special.sql

+
+SELECT dictGetString('database_for_dict.dictionary_with_hierarchy', 'RegionName', toUInt64(2));
+SELECT dictGetHierarchy('database_for_dict.dictionary_with_hierarchy', toUInt64(3));
+SELECT dictIsIn('database_for_dict.dictionary_with_hierarchy', toUInt64(3), toUInt64(2));


will it be posiible to have dict with same name in 2 different databases?

Cool! So i think it will also resolve #4201

filimonov · 2019-10-23T23:31:10Z

dbms/tests/queries/0_stateless/01018_dictionaries_from_dictionaries.sql

+LIFETIME(MIN 1 MAX 10)
+LAYOUT(FLAT());
+
+SELECT count(*) from database_for_dict.dict1;


does it mean that it will also allow to access dictionaries data with simple select (as with Engine=Dictionary)?

filimonov · 2019-10-23T23:32:37Z

dbms/tests/queries/0_stateless/01018_ddl_dictionaries_select.sql

+SELECT second_column FROM database_for_dict.dict1 WHERE key_column = 11;
+SELECT dictGetString('database_for_dict.dict1', 'third_column', toUInt64(12));
+SELECT third_column FROM database_for_dict.dict1 WHERE key_column = 12;
+SELECT dictGetFloat64('database_for_dict.dict1', 'fourth_column', toUInt64(14));


is database part in dictionary name mandatory? Or it will use current database / global scope if not provided?

As tables, it will use the current database.

how that will work with existing dicts (defined in "global"context)

For existing dictionaries, you have to used the full name in dictGet* functions. For normal selects, you have to create database with an engine Dictionary().

filimonov · 2019-10-23T23:34:24Z

dbms/tests/queries/0_stateless/01018_ddl_dictionaries_create.sql

+
+SHOW DICTIONARIES FROM ordinary_db LIKE 'dict1';
+
+EXISTS DICTIONARY ordinary_db.dict1;


and what will happen if same dict is described both in xml and ddl?

I hope we will not be able to load dictionary and got an exception on creation, but I need to add test for this!

alesapin · 2019-10-24T18:09:33Z

my own tests failed :(

I have missed config file :(

alesapin · 2019-10-25T15:34:05Z

@alexey-milovidov recommend to merge this PR.

azat · 2019-11-08T04:35:04Z

dbms/src/Databases/DatabaseOrdinary.cpp

+    auto & external_loader = context.getExternalDictionariesLoader();
+    external_loader.addConfigRepository(getDatabaseName(), std::move(dictionaries_repository));
+    bool lazy_load = context.getConfigRef().getBool("dictionaries_lazy_load", true);
+    external_loader.reload(!lazy_load);


So after this change all dictionaries will be reloaded, regardless the lifetime?
And this can be not the desired behavior if the dictionary is big enough

@alesapin ?

This ignores any lifetime, while dictionaries can be quite big. Fixes: c7cd911 ("Merge pull request ClickHouse#7360") Refs: ClickHouse#7360 (comment)

alesapin added 22 commits October 10, 2019 20:33

Preparing databases for dictionaries

67fc334

Compilable code

2a5713e

Changes

77c6d4d

Remove accident change

7159cde

Merge branch 'master' into dictionaries_ddl_interpreter

3dfe961

Attach detach and introspection queries for dictionaries

ae42dc0

Compilable code

dca0798

Add simpliest tests

967fe64

Fix unit tests and segfault

653aa7c

Merge branch 'master' into dictionaries_ddl_interpreter

155731a

Fix segmentation fault in database factory

67ea666

More tests

ecde224

Appropriate error codes numbers

1ce0eb6

Convert AST to Abstract configuration

f850408

Add loading of external dictionaries

3e068b8

Half working code

e690a3c

Almost working

b4e0ded

Better load of dictionaries

01e23f0

Better tests

fa557ab

Better tests

51de7a1

Fix confused variables

a19a699

More tests

842a31b

alesapin mentioned this pull request Oct 16, 2019

Dictionaries ddl interpreter #7280

Closed

alesapin commented Oct 16, 2019

View reviewed changes

dbms/src/Storages/System/StorageSystemDictionaries.cpp Outdated Show resolved Hide resolved

alex-krash reviewed Oct 16, 2019

View reviewed changes

alesapin added 4 commits October 17, 2019 12:05

Fix build and style

3987a45

Better introspection

6dfe5c7

Fix deadlock, add test and refactoring

05392fd

Fix race condition, add test

c29b390

alesapin added 2 commits October 22, 2019 16:41

More readable code in external loader

dc6090a

Remove complex logic with lazy load

dfa9b0c

alesapin added pr-feature Pull request with new product feature and removed pr-build Pull request with build/testing/packaging improvement labels Oct 22, 2019

alesapin added 5 commits October 22, 2019 19:47

Fix shared build

c12014c

Fix ubsan error

fb34975

Remove strange logic

0abb2e5

Fix if

6a0246f

Better check of dictionary lifetime for updates

c3519ff

filimonov reviewed Oct 23, 2019

View reviewed changes

alesapin added 2 commits October 24, 2019 12:25

Add test for existing dictionary

7edd80c

Merge with master

a6199b7

alesapin added 2 commits October 24, 2019 23:08

Add missed config

8755f5f

Merge branch 'master' into dictionaries_ddl_loader

04ca163

alesapin merged commit c7cd911 into master Oct 25, 2019

alesapin mentioned this pull request Oct 28, 2019

[WIP] DDL Dictionaries #4178

Closed

15 tasks

azat reviewed Nov 8, 2019

View reviewed changes

azat mentioned this pull request Nov 25, 2019

Do not reload *all* dictionaries on CREATE DATABASE #7916

Merged

BayoNet mentioned this pull request Feb 16, 2020

DOCS-442: RU translation of the docs on DDL queries creating extenal dictionary #9141

Merged

filimonov mentioned this pull request Apr 21, 2020

Feature request: dictionaries scope #4201

Closed


		SHOW DICTIONARIES FROM ordinary_db LIKE 'dict1';

		EXISTS DICTIONARY ordinary_db.dict1;

Conversation

alesapin commented Oct 16, 2019

Uh oh!

alesapin commented Oct 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alesapin Oct 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alesapin commented Oct 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alesapin commented Oct 25, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

alesapin commented Oct 16, 2019 •

edited

Loading

alesapin Oct 24, 2019 •

edited

Loading

alesapin commented Oct 24, 2019 •

edited

Loading