Skip to content

Commit f0bda0c

Browse files
EmgarikoEmgariko
authored
Unifying column names validation for both row and column tables #14722 (#15453)
1 parent bc8199d commit f0bda0c

File tree

2 files changed

+194
-1
lines changed

2 files changed

+194
-1
lines changed

‎ydb/core/tx/schemeshard/olap/columns/update.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#include "update.h"
22
#include <ydb/core/tx/schemeshard/schemeshard_info_types.h>
3+
#include <ydb/core/tx/schemeshard/schemeshard_utils.h>
34
#include <yql/essentials/minikql/mkql_type_ops.h>
45
#include <ydb/core/scheme/scheme_types_proto.h>
56
#include <ydb/core/scheme_types/scheme_type_registry.h>
@@ -53,6 +54,10 @@ bool TOlapColumnBase::ParseFromRequest(const NKikimrSchemeOp::TOlapColumnDescrip
5354
return false;
5455
}
5556
Name = columnSchema.GetName();
57+
if (!IsValidColumnName(Name, false)) {
58+
errors.AddError(Sprintf("Invalid name for column '%s'", Name.data()));
59+
return false;
60+
}
5661
NotNullFlag = columnSchema.GetNotNull();
5762
TypeName = columnSchema.GetType();
5863
StorageId = columnSchema.GetStorageId();

‎ydb/core/tx/schemeshard/ut_olap/ut_olap.cpp

Lines changed: 189 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,20 @@ static const TString defaultStoreSchema = R"(
3030
}
3131
)";
3232

33+
static const TString invalidStoreSchema = R"(
34+
Name: "OlapStore"
35+
ColumnShardCount: 1
36+
SchemaPresets {
37+
Name: "default"
38+
Schema {
39+
Columns { Name: "timestamp" Type: "Timestamp" NotNull: true }
40+
Columns { Name: "data" Type: "Utf8" }
41+
Columns { Name: "mess age" Type: "Utf8" }
42+
KeyColumnNames: "timestamp"
43+
}
44+
}
45+
)";
46+
3347
static const TString defaultTableSchema = R"(
3448
Name: "ColumnTable"
3549
ColumnShardCount: 1
@@ -45,7 +59,21 @@ static const TVector<NArrow::NTest::TTestColumn> defaultYdbSchema = {
4559
NArrow::NTest::TTestColumn("data", TTypeInfo(NTypeIds::Utf8) )
4660
};
4761

48-
}}
62+
static const TString tableSchemaFormat = R"(
63+
Name: "TestTable"
64+
Schema {
65+
Columns {
66+
Name: "Id"
67+
Type: "Int32"
68+
NotNull: True
69+
}
70+
Columns {
71+
Name: "%s"
72+
Type: "Utf8"
73+
}
74+
KeyColumnNames: ["Id"]
75+
}
76+
)";
4977

5078
#define DEBUG_HINT (TStringBuilder() << "at line " << __LINE__)
5179

@@ -99,6 +127,7 @@ NKikimrTxDataShard::TEvPeriodicTableStats WaitTableStats(TTestActorRuntime& runt
99127

100128
return stats;
101129
}
130+
}}
102131

103132
Y_UNIT_TEST_SUITE(TOlap) {
104133
Y_UNIT_TEST(CreateStore) {
@@ -1136,3 +1165,162 @@ Y_UNIT_TEST_SUITE(TOlap) {
11361165
CheckQuotaExceedance(runtime, TTestTxConfig::SchemeShard, "/MyRoot/SomeDatabase", false, DEBUG_HINT);
11371166
}
11381167
}
1168+
1169+
Y_UNIT_TEST_SUITE(TOlapNaming) {
1170+
1171+
Y_UNIT_TEST(CreateColumnTableOk) {
1172+
TTestBasicRuntime runtime;
1173+
TTestEnv env(runtime);
1174+
ui64 txId = 100;
1175+
1176+
TString allowedChars = "_-0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz";
1177+
1178+
TString tableSchema = Sprintf(tableSchemaFormat.c_str(), allowedChars.c_str());
1179+
1180+
TestCreateColumnTable(runtime, ++txId, "/MyRoot", tableSchema, {NKikimrScheme::StatusAccepted});
1181+
env.TestWaitNotification(runtime, txId);
1182+
}
1183+
1184+
Y_UNIT_TEST(CreateColumnTableFailed) {
1185+
TTestBasicRuntime runtime;
1186+
TTestEnv env(runtime);
1187+
ui64 txId = 100;
1188+
1189+
TVector<TString> notAllowedNames = {"mess age", "~!@#$%^&*()+=asdfa"};
1190+
1191+
for (const auto& colName: notAllowedNames) {
1192+
TString tableSchema = Sprintf(tableSchemaFormat.c_str(), colName.c_str());
1193+
1194+
TestCreateColumnTable(runtime, ++txId, "/MyRoot", tableSchema, {NKikimrScheme::StatusSchemeError});
1195+
env.TestWaitNotification(runtime, txId);
1196+
}
1197+
}
1198+
1199+
Y_UNIT_TEST(CreateColumnStoreOk) {
1200+
TTestBasicRuntime runtime;
1201+
TTestEnv env(runtime);
1202+
ui64 txId = 100;
1203+
1204+
const TString& storeSchema = defaultStoreSchema;
1205+
1206+
TestCreateOlapStore(runtime, ++txId, "/MyRoot", storeSchema, {NKikimrScheme::StatusAccepted});
1207+
env.TestWaitNotification(runtime, txId);
1208+
1209+
TestLs(runtime, "/MyRoot/OlapStore", false, NLs::PathExist);
1210+
}
1211+
1212+
Y_UNIT_TEST(CreateColumnStoreFailed) {
1213+
TTestBasicRuntime runtime;
1214+
TTestEnv env(runtime);
1215+
ui64 txId = 100;
1216+
1217+
const TString& storeSchema = invalidStoreSchema;
1218+
1219+
TestCreateOlapStore(runtime, ++txId, "/MyRoot", storeSchema, {NKikimrScheme::StatusSchemeError});
1220+
env.TestWaitNotification(runtime, txId);
1221+
1222+
TestLs(runtime, "/MyRoot/OlapStore", false, NLs::PathNotExist);
1223+
}
1224+
1225+
Y_UNIT_TEST(AlterColumnTableOk) {
1226+
TTestBasicRuntime runtime;
1227+
TTestEnvOptions options;
1228+
TTestEnv env(runtime, options);
1229+
ui64 txId = 100;
1230+
1231+
TString tableSchema = Sprintf(tableSchemaFormat.c_str(), "message");
1232+
1233+
TestCreateColumnTable(runtime, ++txId, "/MyRoot", tableSchema, {NKikimrScheme::StatusAccepted});
1234+
env.TestWaitNotification(runtime, txId);
1235+
1236+
TestAlterColumnTable(runtime, ++txId, "/MyRoot", R"(
1237+
Name: "TestTable"
1238+
AlterSchema {
1239+
AddColumns {
1240+
Name: "NewColumn"
1241+
Type: "Int32"
1242+
}
1243+
}
1244+
)");
1245+
env.TestWaitNotification(runtime, txId);
1246+
}
1247+
1248+
Y_UNIT_TEST(AlterColumnTableFailed) {
1249+
TTestBasicRuntime runtime;
1250+
TTestEnvOptions options;
1251+
TTestEnv env(runtime, options);
1252+
ui64 txId = 100;
1253+
1254+
TString tableSchema = Sprintf(tableSchemaFormat.c_str(), "message");
1255+
1256+
TestCreateColumnTable(runtime, ++txId, "/MyRoot", tableSchema, {NKikimrScheme::StatusAccepted});
1257+
env.TestWaitNotification(runtime, txId);
1258+
1259+
TestAlterColumnTable(runtime, ++txId, "/MyRoot", R"(
1260+
Name: "TestTable"
1261+
AlterSchema {
1262+
AddColumns {
1263+
Name: "New Column"
1264+
Type: "Int32"
1265+
}
1266+
}
1267+
)", {NKikimrScheme::StatusSchemeError});
1268+
env.TestWaitNotification(runtime, txId);
1269+
}
1270+
1271+
Y_UNIT_TEST(AlterColumnStoreOk) {
1272+
TTestBasicRuntime runtime;
1273+
TTestEnv env(runtime);
1274+
ui64 txId = 100;
1275+
1276+
const TString& olapSchema = defaultStoreSchema;
1277+
1278+
TestCreateOlapStore(runtime, ++txId, "/MyRoot", olapSchema);
1279+
env.TestWaitNotification(runtime, txId);
1280+
1281+
const TString& tableSchema = defaultTableSchema;
1282+
1283+
TestCreateColumnTable(runtime, ++txId, "/MyRoot/OlapStore", tableSchema);
1284+
env.TestWaitNotification(runtime, txId);
1285+
1286+
TestAlterOlapStore(runtime, ++txId, "/MyRoot", R"(
1287+
Name: "OlapStore"
1288+
AlterSchemaPresets {
1289+
Name: "default"
1290+
AlterSchema {
1291+
AddColumns { Name: "comment" Type: "Utf8" }
1292+
}
1293+
}
1294+
)", {NKikimrScheme::StatusAccepted});
1295+
1296+
env.TestWaitNotification(runtime, txId);
1297+
}
1298+
1299+
Y_UNIT_TEST(AlterColumnStoreFailed) {
1300+
TTestBasicRuntime runtime;
1301+
TTestEnv env(runtime);
1302+
ui64 txId = 100;
1303+
1304+
const TString& olapSchema = defaultStoreSchema;
1305+
1306+
TestCreateOlapStore(runtime, ++txId, "/MyRoot", olapSchema);
1307+
env.TestWaitNotification(runtime, txId);
1308+
1309+
const TString& tableSchema = defaultTableSchema;
1310+
1311+
TestCreateColumnTable(runtime, ++txId, "/MyRoot/OlapStore", tableSchema);
1312+
env.TestWaitNotification(runtime, txId);
1313+
1314+
TestAlterOlapStore(runtime, ++txId, "/MyRoot", R"(
1315+
Name: "OlapStore"
1316+
AlterSchemaPresets {
1317+
Name: "default"
1318+
AlterSchema {
1319+
AddColumns { Name: "mess age" Type: "Utf8" }
1320+
}
1321+
}
1322+
)", {NKikimrScheme::StatusSchemeError});
1323+
1324+
env.TestWaitNotification(runtime, txId);
1325+
}
1326+
}

0 commit comments

Comments
 (0)