From 1d247d0e318ff44a356b85e000bd6e2daded2eee Mon Sep 17 00:00:00 2001 From: Vitaliy Lyudvichenko Date: Mon, 5 Mar 2018 17:45:19 +0300 Subject: [PATCH 1/8] Fixed segfault in case of WHERE . [#CLICKHOUSE-3616] Prohibited non-UInt8 constants in WHERE expressions. Fixed KILL QUERY syntax highlighting. --- dbms/src/DataStreams/FilterBlockInputStream.cpp | 11 +++++------ dbms/src/Parsers/ASTKillQueryQuery.cpp | 4 ++-- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/dbms/src/DataStreams/FilterBlockInputStream.cpp b/dbms/src/DataStreams/FilterBlockInputStream.cpp index 19905834468..ac117127b5c 100644 --- a/dbms/src/DataStreams/FilterBlockInputStream.cpp +++ b/dbms/src/DataStreams/FilterBlockInputStream.cpp @@ -33,19 +33,18 @@ FilterBlockInputStream::FilterBlockInputStream(const BlockInputStreamPtr & input expression->execute(header); filter_column = header.getPositionByName(filter_column_name); + auto & column_elem = header.safeGetByPosition(filter_column); /// Isn't the filter already constant? - ColumnPtr column = header.safeGetByPosition(filter_column).column; - - if (column) - constant_filter_description = ConstantFilterDescription(*column); + if (column_elem.column) + constant_filter_description = ConstantFilterDescription(*column_elem.column); if (!constant_filter_description.always_false && !constant_filter_description.always_true) { /// Replace the filter column to a constant with value 1. - auto & header_filter_elem = header.getByPosition(filter_column); - header_filter_elem.column = header_filter_elem.type->createColumnConst(header.rows(), UInt64(1)); + FilterDescription filter_description_check(*column_elem.column); + column_elem.column = column_elem.type->createColumnConst(header.rows(), UInt64(1)); } } diff --git a/dbms/src/Parsers/ASTKillQueryQuery.cpp b/dbms/src/Parsers/ASTKillQueryQuery.cpp index 77844be9833..8be944e8481 100644 --- a/dbms/src/Parsers/ASTKillQueryQuery.cpp +++ b/dbms/src/Parsers/ASTKillQueryQuery.cpp @@ -10,12 +10,12 @@ String ASTKillQueryQuery::getID() const void ASTKillQueryQuery::formatQueryImpl(const FormatSettings & settings, FormatState & state, FormatStateStacked frame) const { - settings.ostr << "KILL QUERY WHERE "; + settings.ostr << (settings.hilite ? hilite_keyword : "") << "KILL QUERY WHERE " << (settings.hilite ? hilite_none : ""); if (where_expression) where_expression->formatImpl(settings, state, frame); - settings.ostr << " " << (test ? "TEST" : (sync ? "SYNC" : "ASYNC")); + settings.ostr << " " << (settings.hilite ? hilite_keyword : "") << (test ? "TEST" : (sync ? "SYNC" : "ASYNC")) << (settings.hilite ? hilite_none : ""); } } From 753c3d098e6be827cb8d9e6d329c1ae16039ccdb Mon Sep 17 00:00:00 2001 From: robot-metrika-test Date: Tue, 6 Mar 2018 19:20:07 +0300 Subject: [PATCH 2/8] Auto version update to [54357] --- dbms/cmake/version.cmake | 4 ++-- debian/changelog | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dbms/cmake/version.cmake b/dbms/cmake/version.cmake index bd32fe433bd..7cf00d9353a 100644 --- a/dbms/cmake/version.cmake +++ b/dbms/cmake/version.cmake @@ -1,6 +1,6 @@ # This strings autochanged from release_lib.sh: -set(VERSION_DESCRIBE v1.1.54356-testing) -set(VERSION_REVISION 54356) +set(VERSION_DESCRIBE v1.1.54357-testing) +set(VERSION_REVISION 54357) # end of autochange set (VERSION_MAJOR 1) diff --git a/debian/changelog b/debian/changelog index 0bd9ab764b0..e5d888f4ad5 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,5 +1,5 @@ -clickhouse (1.1.54356) unstable; urgency=low +clickhouse (1.1.54357) unstable; urgency=low * Modified source code - -- Tue, 06 Mar 2018 00:07:31 +0300 + -- Tue, 06 Mar 2018 19:20:07 +0300 From ec91d29d6a13ae3fce31f0a546b91e02ec24798c Mon Sep 17 00:00:00 2001 From: Alexey Zatelepin Date: Tue, 6 Mar 2018 17:49:27 +0300 Subject: [PATCH 3/8] correct MergingParams columns check [#CLICKHOUSE-3599] --- dbms/src/Storages/MergeTree/MergeTreeData.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/dbms/src/Storages/MergeTree/MergeTreeData.cpp b/dbms/src/Storages/MergeTree/MergeTreeData.cpp index db830d77bf5..6a8b2f5deed 100644 --- a/dbms/src/Storages/MergeTree/MergeTreeData.cpp +++ b/dbms/src/Storages/MergeTree/MergeTreeData.cpp @@ -111,7 +111,8 @@ MergeTreeData::MergeTreeData( data_parts_by_info(data_parts_indexes.get()), data_parts_by_state_and_info(data_parts_indexes.get()) { - merging_params.check(columns); + /// NOTE: using the same columns list as is read when performing actual merges. + merging_params.check(getColumnsList()); if (!primary_expr_ast) throw Exception("Primary key cannot be empty", ErrorCodes::BAD_ARGUMENTS); @@ -355,10 +356,11 @@ void MergeTreeData::MergingParams::check(const NamesAndTypesList & columns) cons { if (column.name == version_column) { - if (!column.type->isUnsignedInteger() && !column.type->isDateOrDateTime()) - throw Exception("Version column (" + version_column + ")" - " for storage " + storage + " must have type of UInt family or Date or DateTime." - " Provided column of type " + column.type->getName() + ".", ErrorCodes::BAD_TYPE_OF_FIELD); + if (!column.type->canBeUsedAsVersion()) + throw Exception("The column " + version_column + + " cannot be used as a version column for storage " + storage + + " because it is of type " + column.type->getName() + + " (must use an integer or Date or DateTime)", ErrorCodes::BAD_TYPE_OF_FIELD); miss_column = false; break; } From 5d51c66bbb7241d0e9af8a929337f47117f56d98 Mon Sep 17 00:00:00 2001 From: Alexey Zatelepin Date: Tue, 6 Mar 2018 17:53:53 +0300 Subject: [PATCH 4/8] generic version comparison code for ReplacingMergeTree [#CLICKHOUSE-3599] --- .../DataStreams/MergingSortedBlockInputStream.h | 6 ++++++ .../ReplacingSortedBlockInputStream.cpp | 14 +++++++------- .../DataStreams/ReplacingSortedBlockInputStream.h | 2 -- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/dbms/src/DataStreams/MergingSortedBlockInputStream.h b/dbms/src/DataStreams/MergingSortedBlockInputStream.h index a8e0c4bf488..6391f52dcd5 100644 --- a/dbms/src/DataStreams/MergingSortedBlockInputStream.h +++ b/dbms/src/DataStreams/MergingSortedBlockInputStream.h @@ -105,6 +105,12 @@ protected: return !(*this == other); } + void reset() + { + RowRef empty; + swap(empty); + } + bool empty() const { return columns == nullptr; } size_t size() const { return empty() ? 0 : columns->size(); } }; diff --git a/dbms/src/DataStreams/ReplacingSortedBlockInputStream.cpp b/dbms/src/DataStreams/ReplacingSortedBlockInputStream.cpp index 96ac8a98355..197fac9c22d 100644 --- a/dbms/src/DataStreams/ReplacingSortedBlockInputStream.cpp +++ b/dbms/src/DataStreams/ReplacingSortedBlockInputStream.cpp @@ -73,10 +73,6 @@ void ReplacingSortedBlockInputStream::merge(MutableColumns & merged_columns, std if (current_key.empty()) setPrimaryKeyRef(current_key, current); - UInt64 version = version_column_number != -1 - ? current->all_columns[version_column_number]->get64(current->pos) - : 0; - setPrimaryKeyRef(next_key, current); bool key_differs = next_key != current_key; @@ -89,9 +85,9 @@ void ReplacingSortedBlockInputStream::merge(MutableColumns & merged_columns, std if (key_differs) { - max_version = 0; /// Write the data for the previous primary key. insertRow(merged_columns, merged_rows); + selected_row.reset(); current_key.swap(next_key); } @@ -101,9 +97,13 @@ void ReplacingSortedBlockInputStream::merge(MutableColumns & merged_columns, std current_row_sources.emplace_back(current.impl->order, true); /// A non-strict comparison, since we select the last row for the same version values. - if (version >= max_version) + if (version_column_number == -1 + || selected_row.empty() + || current->all_columns[version_column_number]->compareAt( + current->pos, selected_row.row_num, + *(*selected_row.columns)[version_column_number], + /* nan_direction_hint = */ 1) >= 0) { - max_version = version; max_pos = current_pos; setRowRef(selected_row, current); } diff --git a/dbms/src/DataStreams/ReplacingSortedBlockInputStream.h b/dbms/src/DataStreams/ReplacingSortedBlockInputStream.h index e64100b2207..b8592a0e5b6 100644 --- a/dbms/src/DataStreams/ReplacingSortedBlockInputStream.h +++ b/dbms/src/DataStreams/ReplacingSortedBlockInputStream.h @@ -43,8 +43,6 @@ private: RowRef next_key; /// Last row with maximum version for current primary key. RowRef selected_row; - /// Max version for current primary key. - UInt64 max_version = 0; /// The position (into current_row_sources) of the row with the highest version. size_t max_pos = 0; From f3812dddea13957f599740f66e8f7b4cc8f21b54 Mon Sep 17 00:00:00 2001 From: Alexey Zatelepin Date: Tue, 6 Mar 2018 19:02:09 +0300 Subject: [PATCH 5/8] generic comparison code in GraphiteRollupSortedBlockInputStream [#CLICKHOUSE-3599] --- .../GraphiteRollupSortedBlockInputStream.cpp | 11 +++++++---- .../GraphiteRollupSortedBlockInputStream.h | 1 - 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/dbms/src/DataStreams/GraphiteRollupSortedBlockInputStream.cpp b/dbms/src/DataStreams/GraphiteRollupSortedBlockInputStream.cpp index 2606658e828..5da53d8eea5 100644 --- a/dbms/src/DataStreams/GraphiteRollupSortedBlockInputStream.cpp +++ b/dbms/src/DataStreams/GraphiteRollupSortedBlockInputStream.cpp @@ -182,10 +182,12 @@ void GraphiteRollupSortedBlockInputStream::merge(MutableColumns & merged_columns /// Within all rows with same key, we should leave only one row with maximum version; /// and for rows with same maximum version - only last row. - UInt64 next_version = next_cursor->all_columns[version_column_num]->get64(next_cursor->pos); - if (is_new_key || next_version >= current_subgroup_max_version) + if (is_new_key + || next_cursor->all_columns[version_column_num]->compareAt( + next_cursor->pos, current_subgroup_newest_row.row_num, + *(*current_subgroup_newest_row.columns)[version_column_num], + /* nan_direction_hint = */ 1) >= 0) { - current_subgroup_max_version = next_version; setRowRef(current_subgroup_newest_row, next_cursor); /// Small hack: group and subgroups have the same path, so we can set current_group_path here instead of startNextGroup @@ -244,7 +246,8 @@ void GraphiteRollupSortedBlockInputStream::finishCurrentGroup(MutableColumns & m { /// Insert calculated values of the columns `time`, `value`, `version`. merged_columns[time_column_num]->insert(UInt64(current_time_rounded)); - merged_columns[version_column_num]->insert(current_subgroup_max_version); + merged_columns[version_column_num]->insertFrom( + *(*current_subgroup_newest_row.columns)[version_column_num], current_subgroup_newest_row.row_num); if (aggregate_state_created) { diff --git a/dbms/src/DataStreams/GraphiteRollupSortedBlockInputStream.h b/dbms/src/DataStreams/GraphiteRollupSortedBlockInputStream.h index 99d9466408d..c256d27064d 100644 --- a/dbms/src/DataStreams/GraphiteRollupSortedBlockInputStream.h +++ b/dbms/src/DataStreams/GraphiteRollupSortedBlockInputStream.h @@ -184,7 +184,6 @@ private: /// Last row with maximum version for current primary key (time bucket). RowRef current_subgroup_newest_row; - UInt64 current_subgroup_max_version = 0; /// Time of last read row time_t current_time = 0; From 90ea45a39704d290b909932feb9e4ee1727b56b6 Mon Sep 17 00:00:00 2001 From: Alex Zatelepin Date: Tue, 6 Mar 2018 22:01:45 +0300 Subject: [PATCH 6/8] better exception message [#CLICKHOUSE-3599] --- dbms/src/Storages/MergeTree/MergeTreeData.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/Storages/MergeTree/MergeTreeData.cpp b/dbms/src/Storages/MergeTree/MergeTreeData.cpp index 6a8b2f5deed..9c4771fc0db 100644 --- a/dbms/src/Storages/MergeTree/MergeTreeData.cpp +++ b/dbms/src/Storages/MergeTree/MergeTreeData.cpp @@ -360,7 +360,7 @@ void MergeTreeData::MergingParams::check(const NamesAndTypesList & columns) cons throw Exception("The column " + version_column + " cannot be used as a version column for storage " + storage + " because it is of type " + column.type->getName() + - " (must use an integer or Date or DateTime)", ErrorCodes::BAD_TYPE_OF_FIELD); + " (must be of an integer type or of type Date or DateTime)", ErrorCodes::BAD_TYPE_OF_FIELD); miss_column = false; break; } From ce9647274a75a211787684bfc9b779168876683a Mon Sep 17 00:00:00 2001 From: proller Date: Tue, 6 Mar 2018 23:04:54 +0300 Subject: [PATCH 7/8] CLICKHOUSE-3586 debian/compat : use version 8 to support old gdb version less 7.9.1 (old gdb have no support of compressed symbols) --- debian/compat | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/debian/compat b/debian/compat index ec635144f60..45a4fb75db8 100644 --- a/debian/compat +++ b/debian/compat @@ -1 +1 @@ -9 +8 From b0a4e1294db0341e67196683c913517c4ed246b2 Mon Sep 17 00:00:00 2001 From: Alexey Zatelepin Date: Tue, 6 Mar 2018 20:35:40 +0000 Subject: [PATCH 8/8] remove obsolete setting from test --- .../test_distributed_ddl/configs/users.d/query_log.xml | 1 - 1 file changed, 1 deletion(-) diff --git a/dbms/tests/integration/test_distributed_ddl/configs/users.d/query_log.xml b/dbms/tests/integration/test_distributed_ddl/configs/users.d/query_log.xml index d8254eeb8af..0f389295bec 100644 --- a/dbms/tests/integration/test_distributed_ddl/configs/users.d/query_log.xml +++ b/dbms/tests/integration/test_distributed_ddl/configs/users.d/query_log.xml @@ -3,7 +3,6 @@ 1 - 1