Enable altering indexImplTable partitioning settings through YQL

jepett0 · jepett0 · commit e49fbccd02be · 2024-05-13T21:42:47.000Z
diff --git a/ydb/core/kqp/provider/yql_kikimr_exec.cpp b/ydb/core/kqp/provider/yql_kikimr_exec.cpp
@@ -27,6 +27,7 @@
 #include <ydb/library/yql/minikql/mkql_program_builder.h>
 
 #include <ydb/core/kqp/provider/yql_kikimr_results.h>
+#include <ydb/core/kqp/gateway/utils/scheme_helpers.h>
 
 namespace NYql {
 namespace {
@@ -1505,6 +1506,49 @@ class TKiSinkCallableExecutionTransformer : public TAsyncCallbackTransformer<TKi
                         default:
                             YQL_ENSURE(false, "Unknown index type: " << (ui32)add_index->type_case());
                     }
+                } else if (name == "alterIndex") {
+                    if (maybeAlter.Cast().Actions().Size() > 1) {
+                        ctx.AddError(
+                            TIssue(ctx.GetPosition(action.Name().Pos()),
+                                   "ALTER INDEX action cannot be combined with any other ALTER TABLE action"
+                            )
+                        );
+                        return SyncError();
+                    }
+                    auto listNode = action.Value().Cast<TCoNameValueTupleList>();
+                    for (const auto& indexSetting : listNode) {
+                        auto settingName = indexSetting.Name().Value();
+                        if (settingName == "indexName") {
+                            auto indexName = indexSetting.Value().Cast<TCoAtom>().StringValue();
+                            auto indexTablePath = NKikimr::NKqp::NSchemeHelpers::CreateIndexTablePath(table.Metadata->Name, indexName);
+                            alterTableRequest.set_path(std::move(indexTablePath));
+                        } else if (settingName == "tableSettings") {
+                            auto tableSettings = indexSetting.Value().Cast<TCoNameValueTupleList>();
+                            for (const auto& tableSetting : tableSettings) {
+                                if (IsPartitioningSetting(tableSetting.Name().Value())) {
+                                    if (!ParsePartitioningSettings(
+                                        *alterTableRequest.mutable_alter_partitioning_settings(), tableSetting, ctx
+                                    )) {
+                                        return SyncError();
+                                    }
+                                } else {
+                                    ctx.AddError(
+                                        TIssue(ctx.GetPosition(tableSetting.Name().Pos()),
+                                               TStringBuilder() << "Unknown index table setting: " << name
+                                        )
+                                    );
+                                    return SyncError();
+                                }
+                            }
+                        } else {
+                            ctx.AddError(
+                                TIssue(ctx.GetPosition(indexSetting.Name().Pos()),
+                                       TStringBuilder() << "Unknown alter index setting: " << settingName
+                                )
+                            );
+                            return SyncError();
+                        }
+                    }
                 } else if (name == "dropIndex") {
                     auto nameNode = action.Value().Cast<TCoAtom>();
                     alterTableRequest.add_drop_indexes(TString(nameNode.Value()));
diff --git a/ydb/core/kqp/provider/yql_kikimr_type_ann.cpp b/ydb/core/kqp/provider/yql_kikimr_type_ann.cpp
@@ -1411,7 +1411,8 @@ virtual TStatus HandleCreateTable(TKiCreateTable create, TExprContext& ctx) over
                     && name != "setTableSettings"
                     && name != "addChangefeed"
                     && name != "dropChangefeed"
-                    && name != "renameIndexTo")
+                    && name != "renameIndexTo"
+                    && name != "alterIndex")
             {
                 ctx.AddError(TIssue(ctx.GetPosition(action.Name().Pos()),
                     TStringBuilder() << "Unknown alter table action: " << name));
diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_alter_table.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_alter_table.cpp
@@ -682,10 +682,18 @@ TVector<ISubOperation::TPtr> CreateConsistentAlterTable(TOperationId id, const T
         return {CreateAlterTable(id, tx)};
     }
 
-    // only for super user use
-    // until correct and safe altering index api is released
+    // Admins can alter indexImplTable unconditionally.
+    // Regular users can only alter allowed fields.
     if (!IsSuperUser(context.UserToken.Get())) {
-        return {CreateAlterTable(id, tx)};
+        std::vector<const google::protobuf::FieldDescriptor*> fields;
+        alter.GetReflection()->ListFields(alter, &fields);
+        for (const auto* field : fields) {
+            if (field->name() != "Name"
+                && field->name() != "PartitionConfig"
+            ) {
+                return {CreateAlterTable(id, tx)};
+            }
+        }
     }
 
     TVector<ISubOperation::TPtr> result;
diff --git a/ydb/library/yql/core/expr_nodes/yql_expr_nodes.json b/ydb/library/yql/core/expr_nodes/yql_expr_nodes.json
@@ -42,7 +42,8 @@
                 {"Index": 0, "Name": "Name", "Type": "TCoAtom"},
                 {"Index": 1, "Name": "Type", "Type": "TCoAtom"},
                 {"Index": 2, "Name": "Columns", "Type": "TCoAtomList"},
-                {"Index": 3, "Name": "DataColumns", "Type": "TCoAtomList"}
+                {"Index": 3, "Name": "DataColumns", "Type": "TCoAtomList"},
+                {"Index": 4, "Name": "TableSettings", "Type": "TCoNameValueTupleList"}
             ]
         },
         {
diff --git a/ydb/library/yql/providers/common/provider/yql_provider.cpp b/ydb/library/yql/providers/common/provider/yql_provider.cpp
@@ -286,6 +286,8 @@ TWriteTableSettings ParseWriteTableSettings(TExprList node, TExprContext& ctx) {
                         index.Columns(item.Value().Cast<TCoAtomList>());
                     } else if (indexItemName == "dataColumns") {
                         index.DataColumns(item.Value().Cast<TCoAtomList>());
+                    } else if (indexItemName == "tableSettings") {
+                        index.TableSettings(item.Value().Cast<TCoNameValueTupleList>());
                     } else {
                         YQL_ENSURE(false, "unknown index item");
                     }
diff --git a/ydb/library/yql/sql/v1/SQLv1.g.in b/ydb/library/yql/sql/v1/SQLv1.g.in
@@ -665,6 +665,7 @@ alter_table_action:
   | alter_table_alter_changefeed
   | alter_table_drop_changefeed
   | alter_table_rename_index_to
+  | alter_table_alter_index
 ;
 
 alter_external_table_stmt: ALTER EXTERNAL TABLE simple_table_ref alter_external_table_action (COMMA alter_external_table_action)*;
@@ -698,6 +699,7 @@ alter_table_rename_index_to: RENAME INDEX an_id TO an_id;
 alter_table_add_changefeed: ADD changefeed;
 alter_table_alter_changefeed: ALTER CHANGEFEED an_id changefeed_alter_settings;
 alter_table_drop_changefeed: DROP CHANGEFEED an_id;
+alter_table_alter_index: ALTER INDEX an_id alter_table_alter_index_action;
 
 column_schema: an_id_schema type_name_or_bind family_relation? opt_column_constraints;
 family_relation: FAMILY an_id;
@@ -755,6 +757,12 @@ split_boundaries:
 
 literal_value_list: LPAREN literal_value (COMMA literal_value)* RPAREN;
 
+alter_table_alter_index_action:
+    alter_table_set_table_setting_uncompat
+  | alter_table_set_table_setting_compat
+  | alter_table_reset_table_setting
+;
+
 drop_table_stmt: DROP (TABLE | TABLESTORE | EXTERNAL TABLE) (IF EXISTS)? simple_table_ref;
 
 create_user_stmt: CREATE USER role_name create_user_option?;
diff --git a/ydb/library/yql/sql/v1/node.h b/ydb/library/yql/sql/v1/node.h
@@ -1022,6 +1022,7 @@ namespace NSQLTranslationV1 {
         EType Type;
         TVector<TIdentifier> IndexColumns;
         TVector<TIdentifier> DataColumns;
+        TTableSettings TableSettings;
     };
 
     struct TChangefeedSettings {
@@ -1072,6 +1073,7 @@ namespace NSQLTranslationV1 {
         TVector<TFamilyEntry> AlterColumnFamilies;
         TTableSettings TableSettings;
         TVector<TIndexDescription> AddIndexes;
+        TVector<TIndexDescription> AlterIndexes;
         TVector<TIdentifier> DropIndexes;
         TMaybe<std::pair<TIdentifier, TIdentifier>> RenameIndexTo;
         TMaybe<TIdentifier> RenameTo;
@@ -1084,7 +1086,7 @@ namespace NSQLTranslationV1 {
             return AddColumns.empty() && DropColumns.empty() && AlterColumns.empty()
                 && AddColumnFamilies.empty() && AlterColumnFamilies.empty()
                 && !TableSettings.IsSet()
-                && AddIndexes.empty() && DropIndexes.empty() && !RenameIndexTo.Defined()
+                && AddIndexes.empty() && AlterIndexes.empty() && DropIndexes.empty() && !RenameIndexTo.Defined()
                 && !RenameTo.Defined()
                 && AddChangefeeds.empty() && AlterChangefeeds.empty() && DropChangefeeds.empty();
         }
diff --git a/ydb/library/yql/sql/v1/query.cpp b/ydb/library/yql/sql/v1/query.cpp
@@ -281,11 +281,28 @@ static INode::TPtr CreateIndexDesc(const TIndexDescription& index, ETableSetting
     }
     const auto& indexType = node.Y(node.Q("indexType"), CreateIndexType(index.Type, node));
     const auto& indexName = node.Y(node.Q("indexName"), BuildQuotedAtom(index.Name.Pos, index.Name.Name));
+    const auto& tableSettings = node.Y(
+        node.Q("tableSettings"),
+        node.Q(CreateTableSettings(index.TableSettings, parsingMode, node))
+    );
     return node.Y(
         node.Q(indexName),
         node.Q(indexType),
         node.Q(node.Y(node.Q("indexColumns"), node.Q(indexColumns))),
         node.Q(node.Y(node.Q("dataColumns"), node.Q(dataColumns))),
+        node.Q(tableSettings)
+    );
+}
+
+static INode::TPtr CreateAlterIndex(const TIndexDescription& index, const INode& node) {
+    const auto& indexName = node.Y(node.Q("indexName"), BuildQuotedAtom(index.Name.Pos, index.Name.Name));
+    const auto& tableSettings = node.Y(
+        node.Q("tableSettings"),
+        node.Q(CreateTableSettings(index.TableSettings, ETableSettingsParsingMode::Alter, node))
+    );
+    return node.Y(
+        node.Q(indexName),
+        node.Q(tableSettings)
     );
 }
 
@@ -1364,6 +1381,11 @@ class TAlterTableNode final: public TAstListNode {
             actions = L(actions, Q(Y(Q("addIndex"), Q(desc))));
         }
 
+        for (const auto& index : Params.AlterIndexes) {
+            const auto& desc = CreateAlterIndex(index, *this);
+            actions = L(actions, Q(Y(Q("alterIndex"), Q(desc))));
+        }
+
         for (const auto& id : Params.DropIndexes) {
             auto indexName = BuildQuotedAtom(id.Pos, id.Name);
             actions = L(actions, Q(Y(Q("dropIndex"), indexName)));
diff --git a/ydb/library/yql/sql/v1/sql_query.cpp b/ydb/library/yql/sql/v1/sql_query.cpp
@@ -1460,6 +1460,14 @@ bool TSqlQuery::AlterTableAction(const TRule_alter_table_action& node, TAlterTab
         AlterTableRenameIndexTo(renameTo, params);
         break;
     }
+    case TRule_alter_table_action::kAltAlterTableAction16: {
+        // ALTER INDEX
+        const auto& rule = node.GetAlt_alter_table_action16().GetRule_alter_table_alter_index1();
+        if (!AlterTableAlterIndex(rule, params)) {
+            return false;
+        }
+        break;
+    }
 
     case TRule_alter_table_action::ALT_NOT_SET:
         AltNotImplemented("alter_table_action", node);
@@ -1683,6 +1691,46 @@ void TSqlQuery::AlterTableRenameIndexTo(const TRule_alter_table_rename_index_to&
     params.RenameIndexTo = std::make_pair(src, dst);
 }
 
+bool TSqlQuery::AlterTableAlterIndex(const TRule_alter_table_alter_index& node, TAlterTableParameters& params) {
+    const auto indexName = IdEx(node.GetRule_an_id3(), *this);
+    params.AlterIndexes.emplace_back(indexName);
+    TTableSettings& indexTableSettings = params.AlterIndexes.back().TableSettings;
+
+    const auto& action = node.GetRule_alter_table_alter_index_action4();
+
+    switch (action.Alt_case()) {
+    case TRule_alter_table_alter_index_action::kAltAlterTableAlterIndexAction1: {
+        // SET setting value
+        const auto& rule = action.GetAlt_alter_table_alter_index_action1().GetRule_alter_table_set_table_setting_uncompat1();
+        if (!AlterTableSetTableSetting(rule, indexTableSettings, params.TableType)) {
+            return false;
+        }
+        break;
+    }
+    case TRule_alter_table_alter_index_action::kAltAlterTableAlterIndexAction2: {
+        // SET (setting1 = value1, ...)
+        const auto& rule = action.GetAlt_alter_table_alter_index_action2().GetRule_alter_table_set_table_setting_compat1();
+        if (!AlterTableSetTableSetting(rule, indexTableSettings, params.TableType)) {
+            return false;
+        }
+        break;
+    }
+    case TRule_alter_table_alter_index_action::kAltAlterTableAlterIndexAction3: {
+        // RESET (setting1, ...)
+        const auto& rule = action.GetAlt_alter_table_alter_index_action3().GetRule_alter_table_reset_table_setting1();
+        if (!AlterTableResetTableSetting(rule, indexTableSettings, params.TableType)) {
+            return false;
+        }
+        break;
+    }
+    case TRule_alter_table_alter_index_action::ALT_NOT_SET:
+        AltNotImplemented("alter_table_alter_index_action", action);
+        return false;
+    }
+
+    return true;
+}
+
 bool TSqlQuery::AlterTableAddChangefeed(const TRule_alter_table_add_changefeed& node, TAlterTableParameters& params) {
     TSqlExpression expr(Ctx, Mode);
     return CreateChangefeed(node.GetRule_changefeed2(), expr, params.AddChangefeeds);
diff --git a/ydb/library/yql/sql/v1/sql_query.h b/ydb/library/yql/sql/v1/sql_query.h
@@ -41,6 +41,7 @@ class TSqlQuery: public TSqlTranslation {
     bool AlterTableAlterChangefeed(const TRule_alter_table_alter_changefeed& node, TAlterTableParameters& params);
     void AlterTableDropChangefeed(const TRule_alter_table_drop_changefeed& node, TAlterTableParameters& params);
     void AlterTableRenameIndexTo(const TRule_alter_table_rename_index_to& node, TAlterTableParameters& params);
+    bool AlterTableAlterIndex(const TRule_alter_table_alter_index& node, TAlterTableParameters& params);
     TNodePtr PragmaStatement(const TRule_pragma_stmt& stmt, bool& success);
     void AddStatementToBlocks(TVector<TNodePtr>& blocks, TNodePtr node);
     bool ParseTableStoreFeatures(std::map<TString, TDeferredAtom> & result, const TRule_alter_table_store_action & actions);

Original file line number	Diff line number	Diff line change
`@@ -42,7 +42,8 @@`
`42`	`42`	`{"Index": 0, "Name": "Name", "Type": "TCoAtom"},`
`43`	`43`	`{"Index": 1, "Name": "Type", "Type": "TCoAtom"},`
`44`	`44`	`{"Index": 2, "Name": "Columns", "Type": "TCoAtomList"},`
`45`		`- {"Index": 3, "Name": "DataColumns", "Type": "TCoAtomList"}`
	`45`	`+ {"Index": 3, "Name": "DataColumns", "Type": "TCoAtomList"},`
	`46`	`+ {"Index": 4, "Name": "TableSettings", "Type": "TCoNameValueTupleList"}`
`46`	`47`	`]`
`47`	`48`	`},`
`48`	`49`	`{`
Original file line number	Diff line number	Diff line change
`@@ -286,6 +286,8 @@ TWriteTableSettings ParseWriteTableSettings(TExprList node, TExprContext& ctx) {`
`286`	`286`	`index.Columns(item.Value().Cast<TCoAtomList>());`
`287`	`287`	`} else if (indexItemName == "dataColumns") {`
`288`	`288`	`index.DataColumns(item.Value().Cast<TCoAtomList>());`
	`289`	`+ } else if (indexItemName == "tableSettings") {`
	`290`	`+ index.TableSettings(item.Value().Cast<TCoNameValueTupleList>());`
`289`	`291`	`} else {`
`290`	`292`	`YQL_ENSURE(false, "unknown index item");`
`291`	`293`	`}`