Data Skipping Indices by nikvas0 · Pull Request #4143 · ClickHouse/ClickHouse

nikvas0 · 2019-01-24T15:18:03Z

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):
Data Skipping Indices for ClickHouse.
These indices aggregate some info about big blocks of data during writing/merging. After that, they are used to avoid reading the data which do not satisfy the query.

Detailed description (optional):

Indices description in the CREATE TABLE query.
ALTER TABLE ... ADD/DROP INDEX
min-max index (stores extremes of the specified expression)
unique index (stores a set of unique values of the specified expression)

Nikvas0/index creation

Nikvas0/index stream

Nikvas0/unique index

…kvas0/index

nikvas0 · 2019-01-30T16:32:55Z

dbms/src/Storages/MergeTree/MergeTreeUniqueIndex.cpp

+        const auto one = std::make_shared<ASTLiteral>(Field(1));
+        const auto two = std::make_shared<ASTLiteral>(Field(2));
+
+        node = makeASTFunction(


Would special function for swapping last two bits be better in this case?

Yes, but this function would not be usable for anything else.

Let's make this function nevertheless.

alexey-milovidov · 2019-01-30T17:38:21Z

dbms/src/Storages/MergeTree/MergeTreeUniqueIndex.cpp

+    else if (select.prewhere_expression)
+        new_expression = select.prewhere_expression->clone();
+    else
+        /// 11_2 -- can be true and false at the same time


11_2 - I guess you mean "11 in binary".
If you write 0b11 - it will be more obvious.

alesapin

My current remarks mostly about style. Code is really cool. I'll try to look one more time later.

alesapin · 2019-02-01T12:58:38Z

dbms/src/Storages/MergeTree/MergeTreeUniqueIndex.cpp

+
+String MergeTreeUniqueGranule::toString() const
+{
+    String res = "";


some stream (like ostringstream) would be better

alesapin · 2019-02-01T13:01:37Z

dbms/src/Storages/MergeTree/MergeTreeIndices.h

+
+
+/// Structure for storing basic index info like columns, expression, arguments, ...
+class MergeTreeIndex


Better IMergeTreeIndex

alesapin · 2019-02-01T13:03:33Z

dbms/src/Storages/MergeTree/MergeTreeIndices.h

+using MutableMergeTreeIndexPtr = std::shared_ptr<MergeTreeIndex>;
+
+
+struct MergeTreeIndexGranule


IMergeTreeIndexGranule

Need comment

alesapin · 2019-02-01T13:07:10Z

dbms/src/Storages/MergeTree/MergeTreeMinMaxIndex.cpp

+IndexConditionPtr MergeTreeMinMaxIndex::createIndexCondition(
+    const SelectQueryInfo & query, const Context & context) const
+{
+return std::make_shared<MinMaxCondition>(query, context, *this);


alesapin · 2019-02-01T13:14:29Z

dbms/src/Storages/MergeTree/MergeTreeMinMaxIndex.h

+
+};
+
+std::unique_ptr<MergeTreeIndex> MergeTreeMinMaxIndexCreator(


Move definition of this function to .cpp and add declaration to factory .cpp file. Also function name starts from lowercase.

alesapin · 2019-02-01T13:25:59Z

dbms/src/Storages/MergeTree/checkDataPart.cpp

+            {
+                stream.assertMark();
+            }
+            catch (Exception &e)


alesapin · 2019-02-01T13:41:49Z

dbms/src/Storages/MergeTree/MergeTreeReaderStream.cpp

+        /// If there are no marks after the end of range, just use max_read_buffer_size
+        if (right >= marks_count
+            || (right + 1 == marks_count
+                && getMark(right).offset_in_compressed_file


Hard to read, maybe save into several tmp variables?

alesapin · 2019-02-01T13:46:14Z

dbms/src/Storages/MergeTree/MergeTreeIndices.h

+    virtual String toString() const = 0;
+    virtual bool empty() const = 0;
+
+    virtual void update(const Block & block, size_t * pos, size_t limit) = 0;


need comment

alesapin · 2019-02-01T13:53:10Z

dbms/src/Storages/MergeTree/MergeTreeMinMaxIndex.cpp

+
+void MergeTreeMinMaxGranule::update(const Block & block, size_t * pos, size_t limit)
+{
+    size_t rows_read = std::min(limit, block.rows() - *pos);


Maybe add check that *pos is less than number of rows? I know from outer code, that it's always true.

alesapin · 2019-02-01T13:56:33Z

dbms/src/Storages/MergeTree/MergeTreeUniqueIndex.cpp

+    auto granule = std::dynamic_pointer_cast<MergeTreeUniqueGranule>(idx_granule);
+    if (!granule)
+        throw Exception(
+                "Unique index condition got wrong granule", ErrorCodes::LOGICAL_ERROR);


granule with wrong type?

alesapin · 2019-02-05T13:18:23Z

dbms/tests/queries/0_stateless/00825_minmax_index.sql

+INSERT INTO test.minmax_idx VALUES (1, 5, 6.9, 1.57, 'bac', 'c', '2014-07-11');
+
+/* simple select */
+SELECT * FROM test.minmax_idx WHERE i32 = 5 AND i32 + f64 < 12 AND 3 < d AND d < 7 AND (s = 'bac' OR s = 'cba') ORDER BY dt;


maybe it would be better to append FORMAT JSON to end of query. After that we can see how many rows were read:

{ "meta": [ { "name": "u64", "type": "UInt64" }, { "name": "i32", "type": "Int32" }, { "name": "f64", "type": "Float64" }, { "name": "d", "type": "Decimal(10, 2)" }, { "name": "s", "type": "String" }, { "name": "e", "type": "Enum8('a' = 1, 'b' = 2, 'c' = 3)" }, { "name": "dt", "type": "Date" } ], "data": [ { "u64": "0", "i32": 5, "f64": 4.7, "d": 6.50, "s": "cba", "e": "b", "dt": "2014-01-04" } ], "rows": 1, "statistics": { "elapsed": 0.000601825, "rows_read": 1, "bytes_read": 43 } }

alesapin · 2019-02-05T13:41:27Z

docs/en/query_language/alter.md

+
+* `ALTER ADD INDEX name expression TYPE type GRANULARITY value AFTER name [AFTER name2]` - Adds index description to tables metadata.
+
+* `ALTER DROP INDEX name` - Removes index description from tables metadata and deletes index files from disk.


ALTER TABLE ... DROP INDEX?

alesapin · 2019-02-05T13:46:05Z

dbms/src/Storages/IndicesDescription.h

+namespace DB
+{
+
+using IndicesAsts = std::vector<std::shared_ptr<ASTIndexDeclaration>>;


IndiciesASTs

alesapin · 2019-02-05T13:50:55Z

dbms/src/Parsers/ASTIndexDeclaration.h

+    String name;
+    IAST * expr;
+    ASTFunction * type;
+    Field granularity;


Why we store granularity as Field if it's unsigned integer?

alesapin · 2019-02-05T14:23:21Z

dbms/src/Storages/StorageReplicatedMergeTree.cpp

    extern const int KEEPER_EXCEPTION;
    extern const int ALL_REPLICAS_LOST;
    extern const int REPLICA_STATUS_CHANGED;
+    extern const int INCORRECT_QUERY;


alesapin · 2019-02-05T14:26:00Z

dbms/src/Storages/MergeTree/MergeTreeReaderStream.h

+    MergeTreeReaderStream(
+            const String &path_prefix_, const String &extension_, size_t marks_count_,
+            const MarkRanges &all_mark_ranges,
+            MarkCache *mark_cache, bool save_marks_in_cache,


alexey-milovidov · 2019-02-05T14:57:03Z

docs/ru/operations/table_engines/mergetree.md

+* `minmax`
+Хранит минимум и максимум выражения (если выражение - `tuple`, то для каждого элемента `tuple`), используя их для пропуска блоков аналогично первичному ключу.
+
+* `unique(max_rows)`


It can be easily confused with unique constraint. Let's think about better name for it.

nikvas0 · 2019-02-08T09:57:35Z

Fixed in #4286

nikvas0 and others added 30 commits December 25, 2018 21:42

made index parser

6f98649

added index parsing

fcd49af

some fixes

36083e1

added index interface and factory

04a8ea8

fixed compilation

b62197b

ptrs

c89df91

Merge pull request #1 from nikvas0/nikvas0/index_creation

9818d27

Nikvas0/index creation

added indexParts

9bf5b6b

indextypes

06d8416

index condition

68c3879

IndexCondition

5079330

added indexes in selectexecutor

f90cdca

fix

33cf4c9

changed comment

ad2a453

fix

1b7c0ae

added granularity

f704a32

comments

b2da3a3

fix

69052b3

fix

35dbb94

added writing indexes

92a850c

removed indexpart class

f927502

fix

1c80628

added setSkipIndexes

82cc39d

add rw for MergeTreeIndexes

61b9c77

fixes

c3f1784

upd error

83368a4

fix

7e0e301

Merge pull request #2 from nikvas0/nikvas0/index_stream

f345941

Nikvas0/index stream

Merge branch 'master' into nikvas0/index

e95376e

fix

17f6618

nikvas0 and others added 14 commits January 29, 2019 20:28

spaces

9311c01

test

514987e

tests

d3b430d

fix

c4dad05

unique

4de473a

fix

371e165

Merge pull request #12 from nikvas0/nikvas0/unique_index

f6a6a44

Nikvas0/unique index

Merge remote-tracking branch 'upstream/master' into nikvas0/index

c12b03d

fix

6d7ccc6

Merge branch 'nikvas0/index' of github.com:nikvas0/ClickHouse into ni…

c0d7a8b

…kvas0/index

fixed bug with duplicate column

160c8c0

removed unused data

69daa33

fix

bcd07a4

fixes

0492ed7

nikvas0 changed the title ~~Data Skipping Indices [WIP]~~ Data Skipping Indices Jan 30, 2019

nikvas0 commented Jan 30, 2019

View reviewed changes

alexey-milovidov reviewed Jan 30, 2019

View reviewed changes

nikvas0 added 2 commits January 30, 2019 22:40

__bitSwapLastTwo

476f33f

fix

094ae0f

alesapin reviewed Feb 1, 2019

View reviewed changes

alesapin reviewed Feb 5, 2019

View reviewed changes

alexey-milovidov merged commit a1b0ded into ClickHouse:master Feb 5, 2019

alexey-milovidov reviewed Feb 5, 2019

View reviewed changes

alexey-milovidov added a commit that referenced this pull request Feb 5, 2019

Fixed warnings in clang 8 #4143

9dd2e75

nikvas0 mentioned this pull request Feb 6, 2019

Data Skipping Indices fix #4286

Merged

filimonov added the comp-skip-index Skipping indexes (minmax, set, bloom) and their evaluation. label May 11, 2019

4ertus2 mentioned this pull request Jun 10, 2019

T64 column codec #5557

Merged



		/// Structure for storing basic index info like columns, expression, arguments, ...
		class MergeTreeIndex

		using MutableMergeTreeIndexPtr = std::shared_ptr<MergeTreeIndex>;


		struct MergeTreeIndexGranule


		};

		std::unique_ptr<MergeTreeIndex> MergeTreeMinMaxIndexCreator(


		* `ALTER ADD INDEX name expression TYPE type GRANULARITY value AFTER name [AFTER name2]` - Adds index description to tables metadata.

		* `ALTER DROP INDEX name` - Removes index description from tables metadata and deletes index files from disk.

Conversation

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

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!

alesapin left a comment

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!

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!

nikvas0 commented Feb 8, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nikvas0 commented Jan 24, 2019 •

edited

Loading