Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge default values for columns fixes and changes to stable 24 1 #2140

Merged
merged 5 commits into from
Feb 22, 2024
Merged

Merge default values for columns fixes and changes to stable 24 1 #2140

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
012fe7f
don't parametrize default values for columns (#1679)
gridnevvvit Feb 8, 2024
b0361a9
support cast in default values (#1754)
gridnevvvit Feb 9, 2024
0fd6659
fix internal errors during column build when we add not null column (…
gridnevvvit Feb 14, 2024
2d35372
Add default policy to ColumnMeta (#1791)
shnikd Feb 19, 2024
66836e9
disable non-working test
gridnevvvit Feb 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions ydb/core/kqp/common/kqp_resolve.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ struct TTableConstInfo : public TAtomicRefCount<TTableConstInfo> {
ETableKind TableKind = ETableKind::Unknown;
THashMap<TString, TString> Sequences;
THashMap<TString, Ydb::TypedValue> DefaultFromLiteral;
bool IsBuildInProgress = false;

TTableConstInfo() {}
TTableConstInfo(const TString& path) : Path(path) {}
Expand All @@ -52,6 +53,7 @@ struct TTableConstInfo : public TAtomicRefCount<TTableConstInfo> {
NPg::TypeDescFromPgTypeName(phyColumn.GetPgTypeName()));
}
column.NotNull = phyColumn.GetNotNull();
column.IsBuildInProgress = phyColumn.GetIsBuildInProgress();

Columns.emplace(phyColumn.GetId().GetName(), std::move(column));
if (!phyColumn.GetDefaultFromSequence().empty()) {
Expand Down Expand Up @@ -144,19 +146,19 @@ class TKqpTableKeys {
const TMap<TString, NSharding::TShardingBase::TColumn>& GetColumns() const {
return TableConstInfo->Columns;
}

const TVector<TString>& GetKeyColumns() const {
return TableConstInfo->KeyColumns;
}

const TVector<NScheme::TTypeInfo>& GetKeyColumnTypes() const {
return TableConstInfo->KeyColumnTypes;
}

const ETableKind& GetTableKind() const {
return TableConstInfo->TableKind;
}

const THashMap<TString, TString>& GetSequences() const {
return TableConstInfo->Sequences;
}
Expand Down
3 changes: 2 additions & 1 deletion ydb/core/kqp/gateway/kqp_metadata_loader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,8 @@ TTableMetadataResult GetTableMetadataResult(const NSchemeCache::TSchemeCacheNavi
columnDesc.Name, columnDesc.Id, typeName, notNull, columnDesc.PType, columnDesc.PTypeMod,
columnDesc.DefaultFromSequence,
defaultKind,
columnDesc.DefaultFromLiteral
columnDesc.DefaultFromLiteral,
columnDesc.IsBuildInProgress
)
);
if (columnDesc.KeyOrder >= 0) {
Expand Down
4 changes: 2 additions & 2 deletions ydb/core/kqp/opt/kqp_opt_kql.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ TExprBase BuildRowsToDelete(const TKikimrTableDescription& tableData, bool withS
const TPositionHandle pos, TExprContext& ctx, const TIntrusivePtr<TKqpOptimizeContext>& kqpCtx)
{
const auto tableMeta = BuildTableMeta(tableData, pos, ctx);
const auto tableColumns = BuildColumnsList(tableData, pos, ctx, withSystemColumns);
const auto tableColumns = BuildColumnsList(tableData, pos, ctx, withSystemColumns, true /*ignoreWriteOnlyColumns*/);

const auto allRows = BuildReadTable(tableColumns, pos, tableData, false, ctx, kqpCtx);

Expand Down Expand Up @@ -517,7 +517,7 @@ TExprBase BuildDeleteTableWithIndex(const TKiDeleteTable& del, const TKikimrTabl
TExprBase BuildRowsToUpdate(const TKikimrTableDescription& tableData, bool withSystemColumns, const TCoLambda& filter,
const TPositionHandle pos, TExprContext& ctx, const TIntrusivePtr<TKqpOptimizeContext>& kqpCtx)
{
auto kqlReadTable = BuildReadTable(BuildColumnsList(tableData, pos, ctx, withSystemColumns), pos, tableData, false, ctx, kqpCtx);
auto kqlReadTable = BuildReadTable(BuildColumnsList(tableData, pos, ctx, withSystemColumns, true /*ignoreWriteOnlyColumns*/), pos, tableData, false, ctx, kqpCtx);

return Build<TCoFilter>(ctx, pos)
.Input(kqlReadTable)
Expand Down
47 changes: 43 additions & 4 deletions ydb/core/kqp/provider/yql_kikimr_datasink.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,7 @@ class TKikimrDataSink : public TDataProviderBase
TString(dataSink.Cluster()),
key.GetTablePath(), node->Pos(), ctx);

returningColumns = BuildColumnsList(*table, node->Pos(), ctx, sysColumnsEnabled);
returningColumns = BuildColumnsList(*table, node->Pos(), ctx, sysColumnsEnabled, true /*ignoreWriteOnlyColumns*/);

break;
} else {
Expand Down Expand Up @@ -787,7 +787,7 @@ class TKikimrDataSink : public TDataProviderBase
auto replaceIfExists = (settings.Mode.Cast().Value() == "create_or_replace");
auto existringOk = (settings.Mode.Cast().Value() == "create_if_not_exists");

return Build<TKiCreateTable>(ctx, node->Pos())
auto createTable = Build<TKiCreateTable>(ctx, node->Pos())
.World(node->Child(0))
.DataSink(node->Child(1))
.Table().Build(key.GetTablePath())
Expand All @@ -807,8 +807,47 @@ class TKikimrDataSink : public TDataProviderBase
.ExistingOk<TCoAtom>()
.Value(existringOk)
.Build()
.Done()
.Ptr();
.Done();

bool exprEvalNeeded = false;

for(auto item: createTable.Cast<TKiCreateTable>().Columns()) {
auto columnTuple = item.Cast<TExprList>();
if (columnTuple.Size() > 2) {
const auto& columnConstraints = columnTuple.Item(2).Cast<TCoNameValueTuple>();
for(const auto& constraint: columnConstraints.Value().Cast<TCoNameValueTupleList>()) {
if (constraint.Name().Value() != "default")
continue;

YQL_ENSURE(constraint.Value().IsValid());
bool shouldEvaluate = (
constraint.Value().Cast().Ptr()->IsCallable() &&
(constraint.Value().Cast().Ptr()->Content() == "PgCast") &&
(constraint.Value().Cast().Ptr()->ChildrenSize() >= 1) &&
(constraint.Value().Cast().Ptr()->Child(0)->IsCallable()) &&
(constraint.Value().Cast().Ptr()->Child(0)->Content() == "PgConst")
);

if (shouldEvaluate) {
auto evaluatedExpr = ctx.Builder(constraint.Value().Cast().Ptr()->Pos())
.Callable("EvaluateExpr")
.Add(0, constraint.Value().Cast().Ptr())
.Seal()
.Build();

constraint.Ptr()->ChildRef(TCoNameValueTuple::idx_Value) = evaluatedExpr;
exprEvalNeeded = true;
}
}
}
}

if (exprEvalNeeded) {
ctx.Step.Repeat(TExprStep::ExprEval);
}

return createTable.Ptr();

} else if (mode == "alter") {
for (auto setting : settings.Other) {
if (setting.Name().Value() == "intent") {
Expand Down
2 changes: 1 addition & 1 deletion ydb/core/kqp/provider/yql_kikimr_expr_nodes.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ TCoAtomList TKiReadTable::GetSelectColumns(TExprContext& ctx, const TKikimrTable
bool withSystemColumns) const
{
if (Select().Maybe<TCoVoid>()) {
return BuildColumnsList(tableData, Pos(), ctx, withSystemColumns);
return BuildColumnsList(tableData, Pos(), ctx, withSystemColumns, true /*ignoreWriteOnlyColumns*/);
}

return Select().Cast<TCoAtomList>();
Expand Down
6 changes: 5 additions & 1 deletion ydb/core/kqp/provider/yql_kikimr_gateway.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,13 +215,14 @@ struct TKikimrColumnMetadata {
NKikimrKqp::TKqpColumnMetadataProto::EDefaultKind DefaultKind = NKikimrKqp::TKqpColumnMetadataProto::DEFAULT_KIND_UNSPECIFIED;
TString DefaultFromSequence;
Ydb::TypedValue DefaultFromLiteral;
bool IsBuildInProgress = false;

TKikimrColumnMetadata() = default;

TKikimrColumnMetadata(const TString& name, ui32 id, const TString& type, bool notNull,
NKikimr::NScheme::TTypeInfo typeInfo = {}, const TString& typeMod = {}, const TString& defaultFromSequence = {},
NKikimrKqp::TKqpColumnMetadataProto::EDefaultKind defaultKind = NKikimrKqp::TKqpColumnMetadataProto::DEFAULT_KIND_UNSPECIFIED,
const Ydb::TypedValue& defaultFromLiteral = {})
const Ydb::TypedValue& defaultFromLiteral = {}, bool isBuildInProgress = false)
: Name(name)
, Id(id)
, Type(type)
Expand All @@ -231,6 +232,7 @@ struct TKikimrColumnMetadata {
, DefaultKind(defaultKind)
, DefaultFromSequence(defaultFromSequence)
, DefaultFromLiteral(defaultFromLiteral)
, IsBuildInProgress(isBuildInProgress)
{}

explicit TKikimrColumnMetadata(const NKikimrKqp::TKqpColumnMetadataProto* message)
Expand All @@ -242,6 +244,7 @@ struct TKikimrColumnMetadata {
, DefaultKind(message->GetDefaultKind())
, DefaultFromSequence(message->GetDefaultFromSequence())
, DefaultFromLiteral(message->GetDefaultFromLiteral())
, IsBuildInProgress(message->GetIsBuildInProgress())
{
auto typeInfoMod = NKikimr::NScheme::TypeInfoModFromProtoColumnType(message->GetTypeId(),
message->HasTypeInfo() ? &message->GetTypeInfo() : nullptr);
Expand Down Expand Up @@ -279,6 +282,7 @@ struct TKikimrColumnMetadata {
message->SetDefaultFromSequence(DefaultFromSequence);
message->SetDefaultKind(DefaultKind);
message->MutableDefaultFromLiteral()->CopyFrom(DefaultFromLiteral);
message->SetIsBuildInProgress(IsBuildInProgress);
if (columnType.TypeInfo) {
*message->MutableTypeInfo() = *columnType.TypeInfo;
}
Expand Down
6 changes: 5 additions & 1 deletion ydb/core/kqp/provider/yql_kikimr_provider.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,10 +463,14 @@ bool TKikimrKey::Extract(const TExprNode& key) {
}

TCoAtomList BuildColumnsList(const TKikimrTableDescription& table, TPositionHandle pos,
TExprContext& ctx, bool withSystemColumns)
TExprContext& ctx, bool withSystemColumns, bool ignoreWriteOnlyColumns)
{
TVector<TExprBase> columnsToSelect;
for (const auto& pair : table.Metadata->Columns) {
if (pair.second.IsBuildInProgress && ignoreWriteOnlyColumns) {
continue;
}

auto atom = Build<TCoAtom>(ctx, pos)
.Value(pair.second.Name)
.Done();
Expand Down
2 changes: 1 addition & 1 deletion ydb/core/kqp/provider/yql_kikimr_provider_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ TAutoPtr<IGraphTransformer> CreateKiSinkCallableExecutionTransformer(
TAutoPtr<IGraphTransformer> CreateKiSinkPlanInfoTransformer(TIntrusivePtr<IKikimrQueryExecutor> queryExecutor);

NNodes::TCoAtomList BuildColumnsList(const TKikimrTableDescription& table, TPositionHandle pos,
TExprContext& ctx, bool withSystemColumns);
TExprContext& ctx, bool withSystemColumns, bool ignoreWriteOnlyColumns);

const TTypeAnnotationNode* GetReadTableRowType(TExprContext& ctx, const TKikimrTablesData& tablesData,
const TString& cluster, const TString& table, NNodes::TCoAtomList select, bool withSystemColumns = false);
Expand Down
63 changes: 49 additions & 14 deletions ydb/core/kqp/provider/yql_kikimr_type_ann.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,13 @@ class TKiSinkTypeAnnotationTransformer : public TKiSinkVisitorTransformer
THashSet<TString> generateColumnsIfInsertColumnsSet;

for(const auto& [name, info] : table->Metadata->Columns) {
if (info.IsBuildInProgress && rowType->FindItem(name)) {
ctx.AddError(YqlIssue(pos, TIssuesIds::KIKIMR_BAD_REQUEST, TStringBuilder()
<< "Column is under build operation, write operation is not allowed to column: " << name
<< " for table: " << table->Metadata->Name));
return TStatus::Error;
}

if (rowType->FindItem(name)) {
continue;
}
Expand All @@ -431,7 +438,7 @@ class TKiSinkTypeAnnotationTransformer : public TKiSinkVisitorTransformer
}

if (info.IsDefaultKindDefined()) {
if (op == TYdbOperation::Upsert) {
if (op == TYdbOperation::Upsert && !info.IsBuildInProgress) {
generateColumnsIfInsertColumnsSet.emplace(name);
}

Expand Down Expand Up @@ -625,6 +632,13 @@ class TKiSinkTypeAnnotationTransformer : public TKiSinkVisitorTransformer
<< "Column '" << item->GetName() << "' does not exist in table '" << node.Table().Value() << "'."));
return TStatus::Error;
}

if (column->IsBuildInProgress) {
ctx.AddError(YqlIssue(ctx.GetPosition(node.Pos()), TIssuesIds::KIKIMR_BAD_REQUEST, TStringBuilder()
<< "Column '" << item->GetName() << "' is under the build operation '" << node.Table().Value() << "'."));
return TStatus::Error;
}

if (column->NotNull && item->HasOptionalOrNull()) {
if (item->GetItemType()->GetKind() == ETypeAnnotationKind::Pg) {
//no type-level notnull check for pg types.
Expand All @@ -636,6 +650,8 @@ class TKiSinkTypeAnnotationTransformer : public TKiSinkVisitorTransformer
}
}



if (!node.ReturningColumns().Empty()) {
ctx.AddError(TIssue(ctx.GetPosition(node.Pos()), TStringBuilder()
<< "It is not allowed to use returning"));
Expand Down Expand Up @@ -808,7 +824,13 @@ virtual TStatus HandleCreateTable(TKiCreateTable create, TExprContext& ctx) over

columnMeta.SetDefaultFromLiteral();

if (auto pgConst = constraint.Value().Maybe<TCoPgConst>()) {
YQL_ENSURE(constraint.Value().IsValid());
const auto& constrValue = constraint.Value().Cast();
bool isPgNull = constrValue.Ptr()->IsCallable() &&
constrValue.Ptr()->Content() == "PgCast" && constrValue.Ptr()->ChildrenSize() >= 1 &&
constrValue.Ptr()->Child(0)->IsCallable() && constrValue.Ptr()->Child(0)->Content() == "Null";

if (constrValue.Maybe<TCoPgConst>() || isPgNull) {
auto actualPgType = actualType->Cast<TPgExprType>();
YQL_ENSURE(actualPgType);

Expand All @@ -819,25 +841,38 @@ virtual TStatus HandleCreateTable(TKiCreateTable create, TExprContext& ctx) over
return TStatus::Error;
}

TString content = TString(pgConst.Cast().Value().Value());
auto parseResult = NKikimr::NPg::PgNativeBinaryFromNativeText(content, typeDesc);
if (parseResult.Error) {
ctx.AddError(TIssue(ctx.GetPosition(constraint.Pos()),
TStringBuilder() << "Failed to parse default expr for typename " << actualPgType->GetName()
<< ", error reason: " << *parseResult.Error));
return TStatus::Error;
if (isPgNull) {
if (columnMeta.NotNull) {
ctx.AddError(TIssue(ctx.GetPosition(constraint.Pos()), TStringBuilder() << "Default expr " << columnName
<< " is nullable or optional, but column has not null constraint. "));
return TStatus::Error;
}

columnMeta.DefaultFromLiteral.mutable_value()->set_null_flag_value(NProtoBuf::NULL_VALUE);

} else {
YQL_ENSURE(constrValue.Maybe<TCoPgConst>());
auto pgConst = constrValue.Cast<TCoPgConst>();
TString content = TString(pgConst.Value().Value());
auto parseResult = NKikimr::NPg::PgNativeBinaryFromNativeText(content, typeDesc);
if (parseResult.Error) {
ctx.AddError(TIssue(ctx.GetPosition(constraint.Pos()),
TStringBuilder() << "Failed to parse default expr for typename " << actualPgType->GetName()
<< ", error reason: " << *parseResult.Error));
return TStatus::Error;
}

columnMeta.DefaultFromLiteral.mutable_value()->set_bytes_value(parseResult.Str);
}

columnMeta.DefaultFromLiteral.mutable_value()->set_bytes_value(parseResult.Str);
auto* pg = columnMeta.DefaultFromLiteral.mutable_type()->mutable_pg_type();

pg->set_type_name(NKikimr::NPg::PgTypeNameFromTypeDesc(typeDesc));
pg->set_oid(NKikimr::NPg::PgTypeIdFromTypeDesc(typeDesc));
} else if (auto literal = constraint.Value().Maybe<TCoDataCtor>()) {
FillLiteralProto(constraint.Value().Cast<TCoDataCtor>(), columnMeta.DefaultFromLiteral);
} else if (auto literal = constrValue.Maybe<TCoDataCtor>()) {
FillLiteralProto(literal.Cast(), columnMeta.DefaultFromLiteral);
} else {
ctx.AddError(TIssue(ctx.GetPosition(constraint.Pos()),
TStringBuilder() << "Unsupported type of default value " << constraint.Value().Cast().Ptr()->Content()));
TStringBuilder() << "Unsupported type of default value"));
return TStatus::Error;
}

Expand Down
1 change: 1 addition & 0 deletions ydb/core/kqp/query_compiler/kqp_query_compiler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ void FillTable(const TKikimrTableMetadata& tableMeta, THashSet<TStringBuf>&& col
phyColumn.MutableId()->SetId(column->Id);
phyColumn.MutableId()->SetName(column->Name);
phyColumn.SetTypeId(column->TypeInfo.GetTypeId());
phyColumn.SetIsBuildInProgress(column->IsBuildInProgress);
if (column->IsDefaultFromSequence()) {
phyColumn.SetDefaultFromSequence(column->DefaultFromSequence);
} else if (column->IsDefaultFromLiteral()) {
Expand Down
43 changes: 43 additions & 0 deletions ydb/core/kqp/ut/pg/kqp_pg_ut.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3403,7 +3403,50 @@ Y_UNIT_TEST_SUITE(KqpPg) {
)", FormatResultSetYson(result.GetResultSet(0)));
}
}
/*
Y_UNIT_TEST(InsertValuesFromTableWithDefaultAndCast) {
NKikimrConfig::TAppConfig appConfig;
appConfig.MutableTableServiceConfig()->SetEnablePreparedDdl(true);
auto setting = NKikimrKqp::TKqpSetting();
auto serverSettings = TKikimrSettings()
.SetAppConfig(appConfig)
.SetKqpSettings({setting});
TKikimrRunner kikimr(serverSettings.SetWithSampleTables(false));
auto db = kikimr.GetQueryClient();
auto settings = NYdb::NQuery::TExecuteQuerySettings().Syntax(NYdb::NQuery::ESyntax::Pg);
{
auto result = db.ExecuteQuery(R"(
CREATE TABLE t (
a INT,
b int DEFAULT 5::int4,
c int DEFAULT '7'::int4,
d varchar(20) DEFAULT 'foo'::varchar(2),
e int DEFAULT NULL,
f bit varying(5) DEFAULT '1001',
PRIMARY KEY(a)
);
)", NYdb::NQuery::TTxControl::NoTx(), settings).ExtractValueSync();
UNIT_ASSERT_C(result.IsSuccess(), result.GetIssues().ToString());
}
{
auto result = db.ExecuteQuery(R"(
INSERT INTO t VALUES(1);
)", NYdb::NQuery::TTxControl::BeginTx().CommitTx(), settings).ExtractValueSync();
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString());
}
{
auto result = db.ExecuteQuery(R"(
SELECT * FROM t;
)", NYdb::NQuery::TTxControl::BeginTx().CommitTx(), settings).ExtractValueSync();
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString());

UNIT_ASSERT_C(!result.GetResultSets().empty(), "results are empty");
CompareYson(R"(
[["1";"5";"7";"fo";#;"1001"]]
)", FormatResultSetYson(result.GetResultSet(0)));
}
}
*/
Y_UNIT_TEST(InsertValuesFromTableWithDefaultBool) {
NKikimrConfig::TAppConfig appConfig;
appConfig.MutableTableServiceConfig()->SetEnablePreparedDdl(true);
Expand Down
Loading
Loading