Skip to content

Commit

Permalink
post-review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
vlad-gogov committed Jan 17, 2025
1 parent f94e573 commit 7a8530f
Show file tree
Hide file tree
Showing 8 changed files with 19 additions and 18 deletions.
5 changes: 3 additions & 2 deletions ydb/core/base/appdata_fwd.h
Original file line number Diff line number Diff line change
Expand Up @@ -297,8 +297,9 @@ inline TAppData* AppData(NActors::TActorSystem* actorSystem) {
}

inline bool HasAppData() {
return !!NActors::TlsActivationContext && NActors::TlsActivationContext->ExecutorThread.ActorSystem &&
NActors::TlsActivationContext->ExecutorThread.ActorSystem->AppData<TAppData>();
return !!!NActors::TlsActivationContext
&& NActors::TlsActivationContext->ExecutorThread.ActorSystem
&& NActors::TlsActivationContext->ExecutorThread.ActorSystem->AppData<TAppData>();
}

inline TAppData& AppDataVerified() {
Expand Down
2 changes: 1 addition & 1 deletion ydb/core/formats/arrow/serializer/utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@ bool SupportsCompressionLevel(const arrow::Compression::type compression, const

std::optional<int> MinimumCompressionLevel(const arrow::Compression::type compression);
std::optional<int> MaximumCompressionLevel(const arrow::Compression::type compression);
} // namespace NKikimr::NArrow
}
12 changes: 6 additions & 6 deletions ydb/core/kqp/ut/common/columnshard.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -169,15 +169,15 @@ namespace NKqp {
<< "` and in right value `" << rhs.GetSerializerClassName() << "`";
return false;
}
if (CompressionType.has_value() && rhs.GetCompressionType().has_value() && CompressionType.value() != rhs.GetCompressionType().value()) {
if (CompressionType.has_value() && rhs.HasCompressionType() && CompressionType.value() != rhs.GetCompressionTypeUnsafe()) {
errorMessage = TStringBuilder() << "different compression type: in left value `"
<< NArrow::CompressionToString(CompressionType.value()) << "` and in right value `"
<< NArrow::CompressionToString(rhs.GetCompressionType().value()) << "`";
<< NArrow::CompressionToString(rhs.GetCompressionTypeUnsafe()) << "`";
return false;
} else if (CompressionType.has_value() && !rhs.GetCompressionType().has_value()) {
} else if (CompressionType.has_value() && !rhs.HasCompressionType()) {
errorMessage = TStringBuilder() << "compression type is set in left value, but not set in right value";
return false;
} else if (!CompressionType.has_value() && rhs.GetCompressionType().has_value()) {
} else if (!CompressionType.has_value() && rhs.HasCompressionType()) {
errorMessage = TStringBuilder() << "compression type is not set in left value, but set in right value";
return false;
}
Expand Down Expand Up @@ -302,8 +302,8 @@ namespace NKqp {
TString TTestHelper::TColumnTableBase::BuildAlterCompressionQuery(const TString& columnName, const TCompression& compression) const {
auto str = TStringBuilder() << "ALTER OBJECT `" << Name << "` (TYPE " << GetObjectType() << ") SET";
str << " (ACTION=ALTER_COLUMN, NAME=" << columnName << ", `SERIALIZER.CLASS_NAME`=`" << compression.GetSerializerClassName() << "`,";
if (compression.GetCompressionType().has_value()) {
auto codec = NArrow::CompressionFromProto(compression.GetCompressionType().value());
if (compression.HasCompressionType()) {
auto codec = NArrow::CompressionFromProto(compression.GetCompressionTypeUnsafe());
Y_VERIFY(codec.has_value());
str << " `COMPRESSION.TYPE`=`" << NArrow::CompressionToString(codec.value()) << "`";
}
Expand Down
2 changes: 1 addition & 1 deletion ydb/core/kqp/ut/common/columnshard.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class TTestHelper {
public:
class TCompression {
YDB_ACCESSOR(TString, SerializerClassName, "ARROW_SERIALIZER");
YDB_ACCESSOR_DEF(std::optional<NKikimrSchemeOp::EColumnCodec>, CompressionType);
YDB_OPT(NKikimrSchemeOp::EColumnCodec, CompressionType);
YDB_ACCESSOR_DEF(std::optional<i32>, CompressionLevel);

public:
Expand Down
8 changes: 4 additions & 4 deletions ydb/core/tx/schemeshard/olap/column_families/schema.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ bool TOlapColumnFamiliesDescription::Validate(const NKikimrSchemeOp::TColumnTabl
}
lastColumnFamilyId = familyProto.GetId();

if (familyProto.HasColumnCodec() && family->GetSerializerContainer().has_value()) {
if (familyProto.HasColumnCodec() && family->HasSerializerContainer()) {
auto serializerProto = ConvertFamilyDescriptionToProtoSerializer(familyProto);
if (serializerProto.IsFail()) {
errors.AddError(serializerProto.GetErrorMessage());
Expand All @@ -130,12 +130,12 @@ bool TOlapColumnFamiliesDescription::Validate(const NKikimrSchemeOp::TColumnTabl
errors.AddError(TStringBuilder() << "can't deserialize column family `" << columnFamilyName << "` from proto ");
return false;
}
if (!family->GetSerializerContainer()->IsEqualTo(serializer)) {
if (!family->GetSerializerContainerUnsafe().IsEqualTo(serializer)) {
errors.AddError(TStringBuilder() << "compression from column family '" << columnFamilyName << "` is not matching schema preset");
return false;
}
} else if ((!familyProto.HasColumnCodec() && family->GetSerializerContainer().has_value()) ||
(familyProto.HasColumnCodec() && !family->GetSerializerContainer().has_value())) {
} else if ((!familyProto.HasColumnCodec() && family->HasSerializerContainer()) ||
(familyProto.HasColumnCodec() && !family->HasSerializerContainer())) {
errors.AddError(TStringBuilder() << "compression is not matching schema preset in column family `" << columnFamilyName << "`");
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion ydb/core/tx/schemeshard/olap/column_families/update.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class TOlapColumnFamlilyDiff {
class TOlapColumnFamlilyAdd {
private:
YDB_READONLY_DEF(TString, Name);
YDB_READONLY_DEF(std::optional<NArrow::NSerialization::TSerializerContainer>, SerializerContainer);
YDB_READONLY_OPT(NArrow::NSerialization::TSerializerContainer, SerializerContainer);

public:
bool ParseFromRequest(const NKikimrSchemeOp::TFamilyDescription& columnFamily, IErrorCollector& errors);
Expand Down
2 changes: 1 addition & 1 deletion ydb/core/tx/schemeshard/olap/columns/schema.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ bool TOlapColumnsDescription::ApplyUpdate(
}
ui32 id = column.GetColumnFamilyId().value();
if (alterColumnFamiliesId.contains(id)) {
column.SetSerializer(columnFamilies.GetByIdVerified(id)->GetSerializerContainer());
column.SetSerializer(columnFamilies.GetByIdVerified(id)->GetSerializerContainerUnsafe());
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions ydb/core/tx/schemeshard/olap/columns/update.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ void TOlapColumnBase::Serialize(NKikimrSchemeOp::TOlapColumnDescription& columnS

bool TOlapColumnBase::ApplySerializerFromColumnFamily(const TOlapColumnFamiliesDescription& columnFamilies, IErrorCollector& errors) {
if (GetColumnFamilyId().has_value()) {
SetSerializer(columnFamilies.GetByIdVerified(GetColumnFamilyId().value())->GetSerializerContainer());
SetSerializer(columnFamilies.GetByIdVerified(GetColumnFamilyId().value())->GetSerializerContainerUnsafe());
} else {
TString familyName = "default";
const TOlapColumnFamily* columnFamily = columnFamilies.GetByName(familyName);
Expand All @@ -210,7 +210,7 @@ bool TOlapColumnBase::ApplySerializerFromColumnFamily(const TOlapColumnFamiliesD
}

ColumnFamilyId = columnFamily->GetId();
SetSerializer(columnFamilies.GetByIdVerified(columnFamily->GetId())->GetSerializerContainer());
SetSerializer(columnFamilies.GetByIdVerified(columnFamily->GetId())->GetSerializerContainerUnsafe());
}
return true;
}
Expand Down

0 comments on commit 7a8530f

Please sign in to comment.