Skip to content

Commit

Permalink
KIKIMR-18545: do not set not_null for default values (#3512) (#3548)
Browse files Browse the repository at this point in the history
  • Loading branch information
qrort authored Apr 8, 2024
1 parent d14df37 commit 295aafd
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 16 deletions.
22 changes: 16 additions & 6 deletions ydb/core/kqp/ut/opt/kqp_not_null_ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1966,8 +1966,8 @@ Y_UNIT_TEST_SUITE(KqpNotNullColumns) {
{"Value3", "Pg('pgint2','',21,0,0)"}
};

const THashMap<std::string_view, bool> columnNullability = {
{"Key1", true},
const THashMap<std::string_view, bool> columnNonNullability = {
{"Key1", false},
{"Key2", false},
{"Value1", false},
{"Value2", false},
Expand All @@ -1977,7 +1977,12 @@ Y_UNIT_TEST_SUITE(KqpNotNullColumns) {
const auto& columns = describeTableResult.GetTableDescription().GetTableColumns();
for (const auto& column : columns) {
UNIT_ASSERT_VALUES_EQUAL_C(column.Type.ToString(), columnTypes.at(column.Name), column.Name);
UNIT_ASSERT_VALUES_EQUAL_C(column.NotNull.value(), columnNullability.at(column.Name), column.Name);
bool isNotNull = columnNonNullability.at(column.Name);
if (isNotNull) {
UNIT_ASSERT_VALUES_EQUAL_C(column.NotNull.value(), true, column.Name);
} else {
UNIT_ASSERT_C(!column.NotNull.has_value() || !column.NotNull.value(), column.Name);
}
}
}

Expand All @@ -1997,16 +2002,21 @@ Y_UNIT_TEST_SUITE(KqpNotNullColumns) {
}
auto describeTableResult = session.DescribeTable("/Root/NotNullCheck").GetValueSync();
UNIT_ASSERT_C(describeTableResult.IsSuccess(), describeTableResult.GetIssues().ToString());
const THashMap<std::string_view, bool> columnNullability = {
const THashMap<std::string_view, bool> columnNonNullability = {
{"1", false},
{"2", true},
{"2", false},
{"3", false},
{"4", true},
};

const auto& columns = describeTableResult.GetTableDescription().GetTableColumns();
for (const auto& column : columns) {
UNIT_ASSERT_VALUES_EQUAL_C(column.NotNull.value(), columnNullability.at(column.Name), column.Name);
bool isNotNull = columnNonNullability.at(column.Name);
if (isNotNull) {
UNIT_ASSERT_VALUES_EQUAL_C(column.NotNull.value(), true, column.Name);
} else {
UNIT_ASSERT_C(!column.NotNull.has_value() || !column.NotNull.value(), column.Name);
}
}

{
Expand Down
2 changes: 0 additions & 2 deletions ydb/core/tx/schemeshard/ut_export/ut_export.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,6 @@ Y_UNIT_TEST_SUITE(TExportToS3Tests) {
}
}
}
not_null: false
from_literal {
type {
optional_type {
Expand All @@ -389,7 +388,6 @@ columns {
}
}
}
not_null: false
from_literal {
type {
optional_type {
Expand Down
10 changes: 6 additions & 4 deletions ydb/core/ydb_convert/table_description.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -367,13 +367,15 @@ static Ydb::Type* AddColumn(Ydb::Table::ColumnMeta* newColumn, const TColumn& co
pg->set_oid(NPg::PgTypeIdFromTypeDesc(typeDesc));
pg->set_typlen(0);
pg->set_typmod(0);
if (column.GetNotNull()) {
newColumn->set_not_null(column.GetNotNull());
}
} else {
NYql::NProto::TypeIds protoType;
if (!NYql::NProto::TypeIds_Parse(column.GetType(), &protoType)) {
throw NYql::TErrorException(NKikimrIssues::TIssuesIds::DEFAULT_ERROR)
<< "Got invalid type: " << column.GetType() << " for column: " << column.GetName();
}

if (column.GetNotNull()) {
columnType = newColumn->mutable_type();
} else {
Expand All @@ -389,7 +391,6 @@ static Ydb::Type* AddColumn(Ydb::Table::ColumnMeta* newColumn, const TColumn& co
NMiniKQL::ExportPrimitiveTypeToProto(protoType, *columnType);
}
}
newColumn->set_not_null(column.GetNotNull());
return columnType;
}

Expand All @@ -407,13 +408,15 @@ Ydb::Type* AddColumn<NKikimrSchemeOp::TColumnDescription>(Ydb::Table::ColumnMeta
pg->set_oid(NPg::PgTypeIdFromTypeDesc(typeDesc));
pg->set_typlen(0);
pg->set_typmod(0);
if (column.GetNotNull()) {
newColumn->set_not_null(column.GetNotNull());
}
} else {
NYql::NProto::TypeIds protoType;
if (!NYql::NProto::TypeIds_Parse(column.GetType(), &protoType)) {
throw NYql::TErrorException(NKikimrIssues::TIssuesIds::DEFAULT_ERROR)
<< "Got invalid type: " << column.GetType() << " for column: " << column.GetName();
}

if (column.GetNotNull()) {
columnType = newColumn->mutable_type();
} else {
Expand All @@ -429,7 +432,6 @@ Ydb::Type* AddColumn<NKikimrSchemeOp::TColumnDescription>(Ydb::Table::ColumnMeta
NMiniKQL::ExportPrimitiveTypeToProto(protoType, *columnType);
}
}
newColumn->set_not_null(column.GetNotNull());
switch (column.GetDefaultValueCase()) {
case NKikimrSchemeOp::TColumnDescription::kDefaultFromLiteral: {
auto fromLiteral = newColumn->mutable_from_literal();
Expand Down
4 changes: 0 additions & 4 deletions ydb/services/ydb/ydb_ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1262,7 +1262,6 @@ columns {
}
}
}
not_null: false
}
columns {
name: "Value"
Expand All @@ -1273,7 +1272,6 @@ columns {
}
}
}
not_null: false
}
primary_key: "Key"
partitioning_settings {
Expand Down Expand Up @@ -1601,7 +1599,6 @@ columns {
}
}
}
not_null: false
}
columns {
name: "IValue"
Expand All @@ -1612,7 +1609,6 @@ columns {
}
}
}
not_null: false
}
primary_key: "Key"
indexes {
Expand Down

0 comments on commit 295aafd

Please sign in to comment.