Skip to content

Commit

Permalink
Fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
@shnikd
shnikd committed Jul 17, 2024
1 parent e56b5aa commit dbf83b3
Show file tree
Hide file tree
Showing 9 changed files with 157 additions and 136 deletions.
2 changes: 1 addition & 1 deletion ydb/core/kqp/provider/yql_kikimr_exec.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ namespace {
TAlterSequenceSettings alterSequenceSettings;
alterSequenceSettings.Name = TString(alterSequence.Sequence());
alterSequenceSettings.SequenceSettings = ParseSequenceSettings(alterSequence.SequenceSettings());
if (TString(alterSequence.ValueType()) == "Null") {
if (TString(alterSequence.ValueType()) != "Null") {
alterSequenceSettings.SequenceSettings.DataType = TString(alterSequence.ValueType());
}

Expand Down
61 changes: 60 additions & 1 deletion ydb/core/kqp/ut/pg/kqp_pg_ut.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2522,6 +2522,7 @@ Y_UNIT_TEST_SUITE(KqpPg) {
UNIT_ASSERT_VALUES_EQUAL(sequenceDescription.GetCache(), 3);
UNIT_ASSERT_VALUES_EQUAL(sequenceDescription.GetIncrement(), 2);
UNIT_ASSERT_VALUES_EQUAL(sequenceDescription.GetCycle(), true);
UNIT_ASSERT_VALUES_EQUAL(sequenceDescription.GetDataType(), "pgint4");
}

{
Expand Down Expand Up @@ -2555,6 +2556,7 @@ Y_UNIT_TEST_SUITE(KqpPg) {
UNIT_ASSERT_VALUES_EQUAL(sequenceDescription.GetCache(), 3);
UNIT_ASSERT_VALUES_EQUAL(sequenceDescription.GetIncrement(), 5);
UNIT_ASSERT_VALUES_EQUAL(sequenceDescription.GetCycle(), false);
UNIT_ASSERT_VALUES_EQUAL(sequenceDescription.GetDataType(), "pgint2");
}

{
Expand All @@ -2564,13 +2566,27 @@ Y_UNIT_TEST_SUITE(KqpPg) {
const auto queryAlter = R"(
--!syntax_pg
ALTER SEQUENCE IF EXISTS seq
MAXVALUE WITH 65000;
MAXVALUE 65000;
)";

auto resultAlter = session.ExecuteQuery(queryAlter, NYdb::NQuery::TTxControl::NoTx()).ExtractValueSync();
UNIT_ASSERT(!resultAlter.IsSuccess());
}

{
auto session = client.GetSession().GetValueSync().GetSession();
auto id = session.GetId();

const auto queryAlter = R"(
--!syntax_pg
ALTER SEQUENCE IF EXISTS seq
MAXVALUE 32000;
)";

auto resultAlter = session.ExecuteQuery(queryAlter, NYdb::NQuery::TTxControl::NoTx()).ExtractValueSync();
UNIT_ASSERT_C(resultAlter.IsSuccess(), resultAlter.GetIssues().ToString());
}

{
auto session = client.GetSession().GetValueSync().GetSession();
auto id = session.GetId();
Expand All @@ -2584,6 +2600,49 @@ Y_UNIT_TEST_SUITE(KqpPg) {
auto resultAlter = session.ExecuteQuery(queryAlter, NYdb::NQuery::TTxControl::NoTx()).ExtractValueSync();
UNIT_ASSERT(!resultAlter.IsSuccess());
}

{
auto session = client.GetSession().GetValueSync().GetSession();
auto id = session.GetId();

const auto queryAlter = R"(
--!syntax_pg
ALTER SEQUENCE IF EXISTS seq
as bigint;
)";

auto resultAlter = session.ExecuteQuery(queryAlter, NYdb::NQuery::TTxControl::NoTx()).ExtractValueSync();
UNIT_ASSERT_C(resultAlter.IsSuccess(), resultAlter.GetIssues().ToString());
}

{
auto session = client.GetSession().GetValueSync().GetSession();
auto id = session.GetId();

const auto queryAlter = R"(
--!syntax_pg
ALTER SEQUENCE IF EXISTS seq
as integer;
MAXVALUE 2147483647;
)";

auto resultAlter = session.ExecuteQuery(queryAlter, NYdb::NQuery::TTxControl::NoTx()).ExtractValueSync();
UNIT_ASSERT(!resultAlter.IsSuccess());
}

{
auto session = client.GetSession().GetValueSync().GetSession();
auto id = session.GetId();

const auto queryAlter = R"(
--!syntax_pg
ALTER SEQUENCE IF EXISTS seq
MAXVALUE 2147483647;
)";

auto resultAlter = session.ExecuteQuery(queryAlter, NYdb::NQuery::TTxControl::NoTx()).ExtractValueSync();
UNIT_ASSERT_C(resultAlter.IsSuccess(), resultAlter.GetIssues().ToString());
}
}

Y_UNIT_TEST(AlterColumnSetDefaultFromSequence) {
Expand Down
67 changes: 5 additions & 62 deletions ydb/core/tx/schemeshard/schemeshard__operation_alter_sequence.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,70 +240,13 @@ std::optional<NKikimrSchemeOp::TSequenceDescription> GetAlterSequenceDescription
dataType = alter.GetDataType();
}

i64 dataTypeMaxValue, dataTypeMinValue;

auto typeName = NMiniKQL::AdaptLegacyYqlType(dataType);
const NScheme::IType* type = typeRegistry.GetType(typeName);
if (type) {
if (!NScheme::NTypeIds::IsYqlType(type->GetTypeId())) {
errStr = Sprintf("Type '%s' specified for sequence '%s' is no longer supported", dataType.data(), sequence.GetName().data());
return std::nullopt;
}

switch (type->GetTypeId()) {
case NScheme::NTypeIds::Int16: {
dataTypeMaxValue = Max<i16>();
dataTypeMinValue = Min<i16>();
break;
}
case NScheme::NTypeIds::Int32: {
dataTypeMaxValue = Max<i32>();
dataTypeMinValue = Min<i32>();
break;
}
case NScheme::NTypeIds::Int64: {
dataTypeMaxValue = Max<i64>();
dataTypeMinValue = Min<i64>();
break;
}
default: {
errStr = Sprintf("Type '%s' specified for sequence '%s' is not supported", dataType.data(), sequence.GetName().data());
return std::nullopt;
}
}
} else {
auto* typeDesc = NPg::TypeDescFromPgTypeName(typeName);
if (!typeDesc) {
errStr = Sprintf("Type '%s' specified for sequence '%s' is not supported", dataType.data(), sequence.GetName().data());
return std::nullopt;
}
if (!pgTypesEnabled) {
errStr = Sprintf("Type '%s' specified for sequence '%s', but support for pg types is disabled (EnableTablePgTypes feature flag is off)", dataType.data(), sequence.GetName().data());
return std::nullopt;
}
switch (NPg::PgTypeIdFromTypeDesc(typeDesc)) {
case INT2OID: {
dataTypeMaxValue = Max<i16>();
dataTypeMinValue = Min<i16>();
break;
}
case INT4OID: {
dataTypeMaxValue = Max<i32>();
dataTypeMinValue = Min<i32>();
break;
}
case INT8OID: {
dataTypeMaxValue = Max<i64>();
dataTypeMinValue = Min<i64>();
break;
}
default: {
errStr = Sprintf("Type '%s' specified for sequence '%s' is not supported", dataType.data(), sequence.GetName().data());
return std::nullopt;
}
}
auto validationResult = ValidateSequenceType(sequence.GetName(), dataType, typeRegistry, pgTypesEnabled, errStr);
if (!validationResult) {
return std::nullopt;
}

auto [dataTypeMinValue, dataTypeMaxValue] = *validationResult;

if (maxValue != Max<i16>() && maxValue != Max<i32>() && maxValue != Max<i64>()) {
if (maxValue > dataTypeMaxValue) {
errStr = Sprintf("MAXVALUE (%ld) is out of range for sequence", maxValue);
Expand Down
72 changes: 9 additions & 63 deletions ydb/core/tx/schemeshard/schemeshard__operation_create_sequence.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,73 +254,18 @@ std::optional<NKikimrSchemeOp::TSequenceDescription> FillSequenceDescription(con

TString dataType;
if (!sequence.HasDataType()) {
errStr = Sprintf("Type is not specified for sequence '%s'", sequence.GetName().data());
return std::nullopt;
dataType = "Int64";
} else {
dataType = sequence.GetDataType();
}

i64 dataTypeMaxValue, dataTypeMinValue;
auto typeName = NMiniKQL::AdaptLegacyYqlType(dataType);
const NScheme::IType* type = typeRegistry.GetType(typeName);
if (type) {
if (!NScheme::NTypeIds::IsYqlType(type->GetTypeId())) {
errStr = Sprintf("Type '%s' specified for sequence '%s' is no longer supported", dataType.data(), sequence.GetName().data());
return std::nullopt;
}

switch (type->GetTypeId()) {
case NScheme::NTypeIds::Int16: {
dataTypeMaxValue = Max<i16>();
dataTypeMinValue = Min<i16>();
break;
}
case NScheme::NTypeIds::Int32: {
dataTypeMaxValue = Max<i32>();
dataTypeMinValue = Min<i32>();
break;
}
case NScheme::NTypeIds::Int64: {
dataTypeMaxValue = Max<i64>();
dataTypeMinValue = Min<i64>();
break;
}
default: {
errStr = Sprintf("Type '%s' specified for sequence '%s' is not supported", dataType.data(), sequence.GetName().data());
return std::nullopt;
}
}
} else {
auto* typeDesc = NPg::TypeDescFromPgTypeName(typeName);
if (!typeDesc) {
errStr = Sprintf("Type '%s' specified for sequence '%s' is not supported", dataType.data(), sequence.GetName().data());
return std::nullopt;
}
if (!pgTypesEnabled) {
errStr = Sprintf("Type '%s' specified for sequence '%s', but support for pg types is disabled (EnableTablePgTypes feature flag is off)", dataType.data(), sequence.GetName().data());
return std::nullopt;
}
switch (NPg::PgTypeIdFromTypeDesc(typeDesc)) {
case INT2OID: {
dataTypeMaxValue = Max<i16>();
dataTypeMinValue = Min<i16>();
break;
}
case INT4OID: {
dataTypeMaxValue = Max<i32>();
dataTypeMinValue = Min<i32>();
break;
}
case INT8OID: {
dataTypeMaxValue = Max<i64>();
dataTypeMinValue = Min<i64>();
break;
}
default: {
errStr = Sprintf("Type '%s' specified for sequence '%s' is not supported", dataType.data(), sequence.GetName().data());
return std::nullopt;
}
}
auto validationResult = ValidateSequenceType(sequence.GetName(), dataType, typeRegistry, pgTypesEnabled, errStr);
if (!validationResult) {
return std::nullopt;
}

auto [dataTypeMinValue, dataTypeMaxValue] = *validationResult;

i64 increment = 0;
if (result.HasIncrement()) {
increment = result.GetIncrement();
Expand Down Expand Up @@ -371,6 +316,7 @@ std::optional<NKikimrSchemeOp::TSequenceDescription> FillSequenceDescription(con
}

result.SetCache(cache);
result.SetDataType(dataType);

return result;
}
Expand Down
70 changes: 70 additions & 0 deletions ydb/core/tx/schemeshard/schemeshard_info_types.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2419,5 +2419,75 @@ bool TSequenceInfo::ValidateCreate(const NKikimrSchemeOp::TSequenceDescription&
return true;
}

// validate type of the sequence
std::optional<std::pair<i64, i64>> ValidateSequenceType(const TString& sequenceName, const TString dataType,
const NScheme::TTypeRegistry& typeRegistry, bool pgTypesEnabled, TString& errStr) {

i64 dataTypeMaxValue, dataTypeMinValue;
auto typeName = NMiniKQL::AdaptLegacyYqlType(dataType);
const NScheme::IType* type = typeRegistry.GetType(typeName);
if (type) {
if (!NScheme::NTypeIds::IsYqlType(type->GetTypeId())) {
errStr = Sprintf("Type '%s' specified for sequence '%s' is no longer supported", dataType.data(), sequenceName.c_str());
return std::nullopt;
}

switch (type->GetTypeId()) {
case NScheme::NTypeIds::Int16: {
dataTypeMaxValue = Max<i16>();
dataTypeMinValue = Min<i16>();
break;
}
case NScheme::NTypeIds::Int32: {
dataTypeMaxValue = Max<i32>();
dataTypeMinValue = Min<i32>();
break;
}
case NScheme::NTypeIds::Int64: {
dataTypeMaxValue = Max<i64>();
dataTypeMinValue = Min<i64>();
break;
}
default: {
errStr = Sprintf("Type '%s' specified for sequence '%s' is not supported", dataType.data(), sequenceName.c_str());
return std::nullopt;
}
}
} else {
auto* typeDesc = NPg::TypeDescFromPgTypeName(typeName);
if (!typeDesc) {
errStr = Sprintf("Type '%s' specified for sequence '%s' is not supported", dataType.data(), sequenceName.c_str());
return std::nullopt;
}
if (!pgTypesEnabled) {
errStr = Sprintf("Type '%s' specified for sequence '%s', but support for pg types is disabled (EnableTablePgTypes feature flag is off)", dataType.data(), sequenceName.c_str());
return std::nullopt;
}
switch (NPg::PgTypeIdFromTypeDesc(typeDesc)) {
case INT2OID: {
dataTypeMaxValue = Max<i16>();
dataTypeMinValue = Min<i16>();
break;
}
case INT4OID: {
dataTypeMaxValue = Max<i32>();
dataTypeMinValue = Min<i32>();
break;
}
case INT8OID: {
dataTypeMaxValue = Max<i64>();
dataTypeMinValue = Min<i64>();
break;
}
default: {
errStr = Sprintf("Type '%s' specified for sequence '%s' is not supported", dataType.data(), sequenceName.c_str());
return std::nullopt;
}
}
}

return {{dataTypeMinValue, dataTypeMaxValue}};
}

} // namespace NSchemeShard
} // namespace NKikimr
3 changes: 3 additions & 0 deletions ydb/core/tx/schemeshard/schemeshard_types.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,4 +162,7 @@ struct TTempDirInfo {
TActorId TempDirOwnerActorId;
};

std::optional<std::pair<i64, i64>> ValidateSequenceType(const TString& sequenceName, const TString dataType,
const NScheme::TTypeRegistry& typeRegistry, bool pgTypesEnabled, TString& errStr);

}
Loading

0 comments on commit dbf83b3

Please sign in to comment.