Skip to content

Commit d9b11ae

Browse files
SmitaRKulkarniSmitaRKulkarni
committed
Address review comments
1 parent 3a39ca6 commit d9b11ae

File tree

7 files changed

+14
-12
lines changed

7 files changed

+14
-12
lines changed

‎src/Databases/DatabasesCommon.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ void validateCreateQuery(const ASTCreateQuery & query, ContextPtr context)
9999
if (columns.indices)
100100
{
101101
for (const auto & child : columns.indices->children)
102-
IndexDescription::getIndexFromAST(child, columns_desc, context);
102+
IndexDescription::getIndexFromAST(child, columns_desc, /* is_implicitly_created */ false, context);
103103
}
104104
if (columns.constraints)
105105
{

‎src/Interpreters/InterpreterCreateQuery.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -787,7 +787,7 @@ InterpreterCreateQuery::TableProperties InterpreterCreateQuery::getTableProperti
787787
if (create.columns_list->indices)
788788
for (const auto & index : create.columns_list->indices->children)
789789
{
790-
IndexDescription index_desc = IndexDescription::getIndexFromAST(index->clone(), properties.columns, getContext());
790+
IndexDescription index_desc = IndexDescription::getIndexFromAST(index->clone(), properties.columns, /* is_implicitly_created */ false, getContext());
791791
if (properties.indices.has(index_desc.name))
792792
throw Exception(ErrorCodes::ILLEGAL_INDEX, "Duplicated index name {} is not allowed. Please use a different index name", backQuoteIfNeed(index_desc.name));
793793

‎src/Storages/AlterCommands.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -753,7 +753,7 @@ void AlterCommand::apply(StorageInMemoryMetadata & metadata, ContextPtr context)
753753
++insert_it;
754754
}
755755

756-
metadata.secondary_indices.emplace(insert_it, IndexDescription::getIndexFromAST(index_decl, metadata.columns, context));
756+
metadata.secondary_indices.emplace(insert_it, IndexDescription::getIndexFromAST(index_decl, metadata.columns, /* is_implicitly_created */ false, context));
757757
}
758758
else if (type == DROP_INDEX)
759759
{
@@ -1316,7 +1316,7 @@ void AlterCommands::apply(StorageInMemoryMetadata & metadata, ContextPtr context
13161316
{
13171317
try
13181318
{
1319-
index = IndexDescription::getIndexFromAST(index.definition_ast, metadata_copy.columns, context, index.is_implicitly_created);
1319+
index = IndexDescription::getIndexFromAST(index.definition_ast, metadata_copy.columns, index.is_implicitly_created, context);
13201320
}
13211321
catch (Exception & exception)
13221322
{

‎src/Storages/IndicesDescription.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ IndexDescription & IndexDescription::operator=(const IndexDescription & other)
146146
return *this;
147147
}
148148

149-
IndexDescription IndexDescription::getIndexFromAST(const ASTPtr & definition_ast, const ColumnsDescription & columns, ContextPtr context, bool is_implicitly_created)
149+
IndexDescription IndexDescription::getIndexFromAST(const ASTPtr & definition_ast, const ColumnsDescription & columns, bool is_implicitly_created, ContextPtr context)
150150
{
151151
const auto * index_definition = definition_ast->as<ASTIndexDeclaration>();
152152
if (!index_definition)
@@ -222,7 +222,7 @@ IndexDescription IndexDescription::getIndexFromAST(const ASTPtr & definition_ast
222222

223223
void IndexDescription::recalculateWithNewColumns(const ColumnsDescription & new_columns, ContextPtr context)
224224
{
225-
*this = getIndexFromAST(definition_ast, new_columns, context);
225+
*this = getIndexFromAST(definition_ast, new_columns, /* is_implicitly_created */ false, context);
226226
}
227227

228228
bool IndicesDescription::has(const String & name) const
@@ -264,7 +264,7 @@ IndicesDescription IndicesDescription::parse(const String & str, const ColumnsDe
264264
ASTPtr list = parseQuery(parser, str, 0, DBMS_DEFAULT_MAX_PARSER_DEPTH, DBMS_DEFAULT_MAX_PARSER_BACKTRACKS);
265265

266266
for (const auto & index : list->children)
267-
result.emplace_back(IndexDescription::getIndexFromAST(index, columns, context));
267+
result.emplace_back(IndexDescription::getIndexFromAST(index, columns, /* is_implicitly_created */ false, context));
268268

269269
return result;
270270
}
@@ -305,7 +305,7 @@ ASTPtr createImplicitMinMaxIndexAST(const String & column_name)
305305
IndexDescription createImplicitMinMaxIndexDescription(const String & column_name, const ColumnsDescription & columns, ContextPtr context)
306306
{
307307
auto index_ast = createImplicitMinMaxIndexAST(column_name);
308-
return IndexDescription::getIndexFromAST(index_ast, columns, context, /* is_implicitly_created */ true);
308+
return IndexDescription::getIndexFromAST(index_ast, columns, /* is_implicitly_created */ true, context);
309309
}
310310

311311
}

‎src/Storages/IndicesDescription.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,11 @@ struct IndexDescription
5050
/// Index granularity, make sense for skip indices
5151
size_t granularity;
5252

53-
/// This is set when index is created using add_minmax_index_for_numeric_columns and add_minmax_index_for_string_columns
54-
bool is_implicitly_created = false;
53+
/// True if index is created implicitly using settings add_minmax_index_for_numeric_columns or add_minmax_index_for_string_columns
54+
bool is_implicitly_created;
5555

5656
/// Parse index from definition AST
57-
static IndexDescription getIndexFromAST(const ASTPtr & definition_ast, const ColumnsDescription & columns, ContextPtr context, bool is_implicitly_created = false);
57+
static IndexDescription getIndexFromAST(const ASTPtr & definition_ast, const ColumnsDescription & columns, bool is_implicitly_created, ContextPtr context);
5858

5959
IndexDescription() = default;
6060

‎src/Storages/MergeTree/registerStorageMergeTree.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -723,7 +723,7 @@ static StoragePtr create(const StorageFactory::Arguments & args)
723723
{
724724
for (const auto & index : args.query.columns_list->indices->children)
725725
{
726-
metadata.secondary_indices.push_back(IndexDescription::getIndexFromAST(index, columns, context));
726+
metadata.secondary_indices.push_back(IndexDescription::getIndexFromAST(index, columns, /* is_implicitly_created */ false, context));
727727
auto index_name = index->as<ASTIndexDeclaration>()->name;
728728

729729
if (args.mode <= LoadingStrictnessLevel::CREATE &&

‎tests/queries/0_stateless/03632_default_minmax_indices_alter.sql‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
-- Test for issue #75677
2+
13
drop table if exists t;
24

35
create table t (a UInt64, s String) engine = MergeTree order by tuple() settings add_minmax_index_for_numeric_columns = 1;

0 commit comments

Comments
 (0)