aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMikhail Surin <surinmike@gmail.com>2022-06-10 15:08:12 +0300
committerMikhail Surin <surinmike@gmail.com>2022-06-10 15:08:12 +0300
commitf5bed877e125431228d29c6e832763907035f0df (patch)
tree96a6bd859af66845ef071ba79ca8c1be1720c0c5
parent8737f367c032403e12094d6075c41df0173bb307 (diff)
downloadydb-f5bed877e125431228d29c6e832763907035f0df.tar.gz
Add column name in error message
ref:1278824b682b325d403a8e3b1803ab29b718288f
-rw-r--r--ydb/core/kqp/ut/kqp_yql_ut.cpp26
-rw-r--r--ydb/library/yql/core/yql_expr_type_annotation.cpp72
-rw-r--r--ydb/library/yql/public/issue/yql_issue_manager.cpp30
-rw-r--r--ydb/library/yql/public/issue/yql_issue_manager.h8
-rw-r--r--ydb/services/ydb/ydb_table_ut.cpp1
5 files changed, 103 insertions, 34 deletions
diff --git a/ydb/core/kqp/ut/kqp_yql_ut.cpp b/ydb/core/kqp/ut/kqp_yql_ut.cpp
index 67b87ea306..616fc91aa7 100644
--- a/ydb/core/kqp/ut/kqp_yql_ut.cpp
+++ b/ydb/core/kqp/ut/kqp_yql_ut.cpp
@@ -309,6 +309,32 @@ Y_UNIT_TEST_SUITE(KqpYql) {
[[2u];["Two"]]
])", FormatResultSetYson(result.GetResultSet(0)));
}
+
+ Y_UNIT_TEST_NEW_ENGINE(ColumnTypeMismatch) {
+ TKikimrRunner kikimr;
+ auto db = kikimr.GetTableClient();
+ auto session = db.CreateSession().GetValueSync().GetSession();
+
+ auto params = TParamsBuilder()
+ .AddParam("$key").Uint64(1).Build()
+ .AddParam("$value").Uint64(2).Build()
+ .Build();
+
+ TExecDataQuerySettings settings;
+ auto req = session.ExecuteDataQuery(Q_(R"(
+ DECLARE $key AS Uint64;
+ DECLARE $value AS Uint64;
+
+ REPLACE INTO `KeyValue`
+ (Key, Value)
+ VALUES
+ ($key, $value);
+ )"), TTxControl::BeginTx().CommitTx(), params).ExtractValueSync();
+
+ req.GetIssues().PrintTo(Cerr);
+ UNIT_ASSERT_VALUES_EQUAL(req.GetStatus(), EStatus::GENERIC_ERROR);
+ UNIT_ASSERT_STRING_CONTAINS(req.GetIssues().ToString(), "Failed to convert 'Value': Uint64 to Optional<String>");
+ }
}
} // namespace NKqp
diff --git a/ydb/library/yql/core/yql_expr_type_annotation.cpp b/ydb/library/yql/core/yql_expr_type_annotation.cpp
index f2a0013f5c..c43791b9c9 100644
--- a/ydb/library/yql/core/yql_expr_type_annotation.cpp
+++ b/ydb/library/yql/core/yql_expr_type_annotation.cpp
@@ -85,7 +85,7 @@ TExprNode::TPtr RebuildVariant(const TExprNode::TPtr& node,
}
IGraphTransformer::TStatus TryConvertToImpl(TExprContext& ctx, TExprNode::TPtr& node,
- const TTypeAnnotationNode& sourceType, const TTypeAnnotationNode& expectedType, TConvertFlags flags) {
+ const TTypeAnnotationNode& sourceType, const TTypeAnnotationNode& expectedType, TConvertFlags flags, bool raiseIssues = false) {
if (IsSameAnnotation(sourceType, expectedType)) {
return IGraphTransformer::TStatus::Ok;
@@ -114,7 +114,7 @@ IGraphTransformer::TStatus TryConvertToImpl(TExprContext& ctx, TExprNode::TPtr&
if (expectedType.GetKind() == ETypeAnnotationKind::Optional) {
auto nextType = expectedType.Cast<TOptionalExprType>()->GetItemType();
auto originalNode = node;
- auto status1 = TryConvertToImpl(ctx, node, sourceType, *nextType, flags);
+ auto status1 = TryConvertToImpl(ctx, node, sourceType, *nextType, flags, raiseIssues);
if (status1.Level != IGraphTransformer::TStatus::Error) {
node = ctx.NewCallable(node->Pos(), "Just", { node });
return IGraphTransformer::TStatus::Repeat;
@@ -124,7 +124,7 @@ IGraphTransformer::TStatus TryConvertToImpl(TExprContext& ctx, TExprNode::TPtr&
if (node->IsCallable("Just")) {
auto sourceItemType = sourceType.Cast<TOptionalExprType>()->GetItemType();
auto value = node->HeadRef();
- auto status = TryConvertToImpl(ctx, value, *sourceItemType, *nextType, flags);
+ auto status = TryConvertToImpl(ctx, value, *sourceItemType, *nextType, flags, raiseIssues);
if (status.Level != IGraphTransformer::TStatus::Error) {
node = ctx.NewCallable(node->Pos(), "Just", { value });
return IGraphTransformer::TStatus::Repeat;
@@ -133,7 +133,7 @@ IGraphTransformer::TStatus TryConvertToImpl(TExprContext& ctx, TExprNode::TPtr&
auto sourceItemType = sourceType.Cast<TOptionalExprType>()->GetItemType();
auto arg = ctx.NewArgument(node->Pos(), "item");
auto originalArg = arg;
- auto status = TryConvertToImpl(ctx, arg, *sourceItemType, *nextType, flags);
+ auto status = TryConvertToImpl(ctx, arg, *sourceItemType, *nextType, flags, raiseIssues);
if (status.Level != IGraphTransformer::TStatus::Error) {
auto lambda = ctx.NewLambda(node->Pos(),
ctx.NewArguments(node->Pos(), { originalArg }),
@@ -385,6 +385,10 @@ IGraphTransformer::TStatus TryConvertToImpl(TExprContext& ctx, TExprNode::TPtr&
TExprNode::TPtr field;
if (!pos) {
if (newField->GetItemType()->GetKind() != ETypeAnnotationKind::Optional) {
+ if (raiseIssues) {
+ ctx.AddError(TIssue(node->Pos(ctx), TStringBuilder() <<
+ "Can't find '" << newField->GetName() << "': " << *newField->GetItemType() << " in " << sourceType));
+ }
return IGraphTransformer::TStatus::Error;
}
@@ -404,8 +408,12 @@ IGraphTransformer::TStatus TryConvertToImpl(TExprContext& ctx, TExprNode::TPtr&
}
YQL_ENSURE(field);
- auto status = TryConvertToImpl(ctx, field, *oldType->GetItemType(), *newField->GetItemType(), flags);
+ auto status = TryConvertToImpl(ctx, field, *oldType->GetItemType(), *newField->GetItemType(), flags, raiseIssues);
if (status.Level == IGraphTransformer::TStatus::Error) {
+ if (raiseIssues) {
+ ctx.AddError(TIssue(node->Pos(ctx), TStringBuilder() <<
+ "Failed to convert '" << newField->GetName() << "': " << *oldType->GetItemType() << " to " << *newField->GetItemType()));
+ }
return status;
}
}
@@ -447,6 +455,10 @@ IGraphTransformer::TStatus TryConvertToImpl(TExprContext& ctx, TExprNode::TPtr&
.Seal()
.Build();
} else {
+ if (raiseIssues) {
+ ctx.AddError(TIssue(node->Pos(ctx), TStringBuilder() <<
+ "Can't find '" << newField->GetName() << ": " << *newField->GetItemType() << "' in " << sourceType));
+ }
return IGraphTransformer::TStatus::Error;
}
} else {
@@ -459,8 +471,12 @@ IGraphTransformer::TStatus TryConvertToImpl(TExprContext& ctx, TExprNode::TPtr&
.Seal()
.Build();
- auto status = TryConvertToImpl(ctx, field, *oldType->GetItemType(), *newField->GetItemType(), flags);
+ auto status = TryConvertToImpl(ctx, field, *oldType->GetItemType(), *newField->GetItemType(), flags, raiseIssues);
if (status.Level == IGraphTransformer::TStatus::Error) {
+ if (raiseIssues) {
+ ctx.AddError(TIssue(node->Pos(ctx), TStringBuilder() <<
+ "Failed to convert '" << newField->GetName() << "': " << *oldType->GetItemType() << " to " << *newField->GetItemType()));
+ }
return status;
}
}
@@ -511,7 +527,7 @@ IGraphTransformer::TStatus TryConvertToImpl(TExprContext& ctx, TExprNode::TPtr&
auto status = TryConvertToImpl(
ctx, targetItem,
*fromUnderlying->GetItems()[*fromTargetIndex]->GetItemType(),
- *toUnderlying->GetItems()[*toTargetIndex]->GetItemType(), flags);
+ *toUnderlying->GetItems()[*toTargetIndex]->GetItemType(), flags, raiseIssues);
if (status.Level == IGraphTransformer::TStatus::Error) {
return status;
}
@@ -528,7 +544,7 @@ IGraphTransformer::TStatus TryConvertToImpl(TExprContext& ctx, TExprNode::TPtr&
auto fromElement = item->GetItemType();
auto toElement = toUnderlying->GetItems()[*toIndex]->GetItemType();
auto arg = ctx.NewArgument(TPositionHandle(), "arg");
- auto status1 = TryConvertToImpl(ctx, arg, *fromElement, *toElement, flags);
+ auto status1 = TryConvertToImpl(ctx, arg, *fromElement, *toElement, flags, raiseIssues);
if (status1.Level == IGraphTransformer::TStatus::Error) {
return status1;
}
@@ -558,7 +574,7 @@ IGraphTransformer::TStatus TryConvertToImpl(TExprContext& ctx, TExprNode::TPtr&
auto status = TryConvertToImpl(
ctx, targetItem,
*fromUnderlying->GetItems()[targetIndex],
- *toUnderlying->GetItems()[targetIndex], flags);
+ *toUnderlying->GetItems()[targetIndex], flags, raiseIssues);
if (status.Level == IGraphTransformer::TStatus::Error) {
return status;
}
@@ -569,7 +585,7 @@ IGraphTransformer::TStatus TryConvertToImpl(TExprContext& ctx, TExprNode::TPtr&
}
auto arg = ctx.NewArgument(TPositionHandle(), "arg");
auto status1 = TryConvertToImpl(ctx, arg, *fromUnderlying->GetItems()[i],
- *toUnderlying->GetItems()[i], flags);
+ *toUnderlying->GetItems()[i], flags, raiseIssues);
if (status1.Level == IGraphTransformer::TStatus::Error) {
return status1;
}
@@ -624,7 +640,7 @@ IGraphTransformer::TStatus TryConvertToImpl(TExprContext& ctx, TExprNode::TPtr&
auto arg = ctx.NewArgument(node->Pos(), "item");
auto originalArg = arg;
- auto status = TryConvertToImpl(ctx, arg, *fromElement, *toElement, flags);
+ auto status = TryConvertToImpl(ctx, arg, *fromElement, *toElement, flags, raiseIssues);
if (status.Level == IGraphTransformer::TStatus::Error) {
return status;
}
@@ -659,7 +675,7 @@ IGraphTransformer::TStatus TryConvertToImpl(TExprContext& ctx, TExprNode::TPtr&
auto arg = ctx.NewArgument(node->Pos(), "item");
auto originalArg = arg;
- auto status = TryConvertToImpl(ctx, arg, *fromElement, *toElement, flags);
+ auto status = TryConvertToImpl(ctx, arg, *fromElement, *toElement, flags, raiseIssues);
if (status.Level == IGraphTransformer::TStatus::Error) {
return status;
}
@@ -695,7 +711,7 @@ IGraphTransformer::TStatus TryConvertToImpl(TExprContext& ctx, TExprNode::TPtr&
const auto oldType = from->GetItems()[i];
const auto newType = to->GetItems()[i];
auto value = node->ChildPtr(i);
- if (const auto status = TryConvertToImpl(ctx, value, *oldType, *newType, flags); status.Level == IGraphTransformer::TStatus::Error) {
+ if (const auto status = TryConvertToImpl(ctx, value, *oldType, *newType, flags, raiseIssues); status.Level == IGraphTransformer::TStatus::Error) {
return status;
}
@@ -735,7 +751,7 @@ IGraphTransformer::TStatus TryConvertToImpl(TExprContext& ctx, TExprNode::TPtr&
.Seal()
.Build();
- if (const auto status = TryConvertToImpl(ctx, value, *oldType, *newType, flags); status.Level == IGraphTransformer::TStatus::Error) {
+ if (const auto status = TryConvertToImpl(ctx, value, *oldType, *newType, flags, raiseIssues); status.Level == IGraphTransformer::TStatus::Error) {
return status;
}
@@ -773,7 +789,7 @@ IGraphTransformer::TStatus TryConvertToImpl(TExprContext& ctx, TExprNode::TPtr&
for (ui32 i = node->IsCallable("List") ? 1 : 0; i < node->ChildrenSize(); ++i) {
auto value = node->ChildPtr(i);
- auto status = TryConvertToImpl(ctx, value, *oldItemType, *newItemType, flags);
+ auto status = TryConvertToImpl(ctx, value, *oldItemType, *newItemType, flags, raiseIssues);
if (status.Level == IGraphTransformer::TStatus::Error) {
return status;
}
@@ -791,7 +807,7 @@ IGraphTransformer::TStatus TryConvertToImpl(TExprContext& ctx, TExprNode::TPtr&
auto nextType = to->GetItemType();
auto arg = ctx.NewArgument(node->Pos(), "item");
auto originalArg = arg;
- auto status = TryConvertToImpl(ctx, arg, *from->GetItemType(), *nextType, flags);
+ auto status = TryConvertToImpl(ctx, arg, *from->GetItemType(), *nextType, flags, raiseIssues);
if (status.Level != IGraphTransformer::TStatus::Error) {
auto lambda = ctx.NewLambda(node->Pos(), ctx.NewArguments(node->Pos(), { originalArg }), std::move(arg));
@@ -807,7 +823,7 @@ IGraphTransformer::TStatus TryConvertToImpl(TExprContext& ctx, TExprNode::TPtr&
auto nextType = to->GetItemType();
auto arg = ctx.NewArgument(node->Pos(), "item");
auto originalArg = arg;
- auto status = TryConvertToImpl(ctx, arg, *from->GetItemType(), *nextType, flags);
+ auto status = TryConvertToImpl(ctx, arg, *from->GetItemType(), *nextType, flags, raiseIssues);
if (status.Level != IGraphTransformer::TStatus::Error) {
auto lambda = ctx.NewLambda(node->Pos(), ctx.NewArguments(node->Pos(), { originalArg }), std::move(arg));
@@ -833,8 +849,8 @@ IGraphTransformer::TStatus TryConvertToImpl(TExprContext& ctx, TExprNode::TPtr&
for (ui32 i = node->IsCallable("Dict") ? 1 : 0; i < node->ChildrenSize(); ++i) {
auto valueKey = node->Child(i)->ChildPtr(0);
auto valuePayload = node->Child(i)->ChildPtr(1);
- auto status = TryConvertToImpl(ctx, valueKey, *oldKeyType, *newKeyType, flags);
- status = status.Combine(TryConvertToImpl(ctx, valuePayload, *oldPayloadType, *newPayloadType, flags));
+ auto status = TryConvertToImpl(ctx, valueKey, *oldKeyType, *newKeyType, flags, raiseIssues);
+ status = status.Combine(TryConvertToImpl(ctx, valuePayload, *oldPayloadType, *newPayloadType, flags, raiseIssues));
if (status.Level == IGraphTransformer::TStatus::Error) {
return status;
}
@@ -856,8 +872,8 @@ IGraphTransformer::TStatus TryConvertToImpl(TExprContext& ctx, TExprNode::TPtr&
auto arg = ctx.NewArgument(node->Pos(), "item");
auto key = ctx.NewCallable(node->Pos(), "Nth", { arg, ctx.NewAtom(node->Pos(), "0") });
auto value = ctx.NewCallable(node->Pos(), "Nth", { arg, ctx.NewAtom(node->Pos(), "1") });
- auto status = TryConvertToImpl(ctx, key, *oldKeyType, *newKeyType, flags);
- status = status.Combine(TryConvertToImpl(ctx, value, *oldPayloadType, *newPayloadType, flags));
+ auto status = TryConvertToImpl(ctx, key, *oldKeyType, *newKeyType, flags, raiseIssues);
+ status = status.Combine(TryConvertToImpl(ctx, value, *oldPayloadType, *newPayloadType, flags, raiseIssues));
if (status.Level != IGraphTransformer::TStatus::Error) {
auto body = ctx.NewList(node->Pos(), { key, value });
auto lambda = ctx.NewLambda(node->Pos(), ctx.NewArguments(node->Pos(), { arg }), std::move(body));
@@ -870,7 +886,7 @@ IGraphTransformer::TStatus TryConvertToImpl(TExprContext& ctx, TExprNode::TPtr&
if (from->GetTag() == to->GetTag()) {
auto nextType = to->GetBaseType();
auto arg = ctx.NewCallable(node->Pos(), "Untag", { node, ctx.NewAtom(node->Pos(), from->GetTag()) });
- auto status = TryConvertToImpl(ctx, arg, *from->GetBaseType(), *nextType, flags);
+ auto status = TryConvertToImpl(ctx, arg, *from->GetBaseType(), *nextType, flags, raiseIssues);
if (status.Level != IGraphTransformer::TStatus::Error) {
node = ctx.NewCallable(node->Pos(), "AsTagged", { arg, ctx.NewAtom(node->Pos(), from->GetTag()) });
return IGraphTransformer::TStatus::Repeat;
@@ -3426,12 +3442,14 @@ IGraphTransformer::TStatus TryConvertTo(TExprNode::TPtr& node, const TTypeAnnota
return IGraphTransformer::TStatus::Error;
}
- auto status = TryConvertToImpl(ctx, node, sourceType, expectedType, flags);
- if (status.Level == IGraphTransformer::TStatus::Error) {
- ctx.AddError(TIssue(ctx.GetPosition(node->Pos()), TStringBuilder() << "Failed to convert type: " <<
- sourceType << " to " << expectedType));
+ TIssueScopeGuard guard(ctx.IssueManager, [&] {
+ return MakeIntrusive<TIssue>(ctx.GetPosition(node->Pos()),
+ TStringBuilder() << "Failed to convert type: " << sourceType << " to " << expectedType);
+ });
+ auto status = TryConvertToImpl(ctx, node, sourceType, expectedType, flags, /* raiseIssues */ true);
+ if (status.Level == IGraphTransformer::TStatus::Error) {
+ guard.RaiseIssueForEmptyScope();
}
-
return status;
}
diff --git a/ydb/library/yql/public/issue/yql_issue_manager.cpp b/ydb/library/yql/public/issue/yql_issue_manager.cpp
index 7375d2d30f..08daf0fcdd 100644
--- a/ydb/library/yql/public/issue/yql_issue_manager.cpp
+++ b/ydb/library/yql/public/issue/yql_issue_manager.cpp
@@ -69,29 +69,45 @@ void TIssueManager::LeaveScope() {
}
}
+void TIssueManager::RaiseIssueForEmptyScope() {
+ if (RawIssues_.top().first.Empty()) {
+ TIssuePtr materialized = RawIssues_.top().second();
+ if (auto p = CheckUniqAndLimit(materialized)) {
+ RawIssues_.top().first = p;
+ }
+ }
+}
+
void TIssueManager::LeaveAllScopes() {
while (!RawIssues_.empty()) {
LeaveScope();
}
}
-TIssuePtr TIssueManager::CheckUniqAndLimit(const TIssue& issue) {
- const auto severity = issue.GetSeverity();
+TIssuePtr TIssueManager::CheckUniqAndLimit(TIssuePtr issue) {
+ const auto severity = issue->GetSeverity();
if (OverflowIssues_[severity]) {
return {};
}
- TIssuePtr p = MakeIntrusive<TIssue>(issue);
- if (UniqueIssues_[severity].contains(p)) {
+ if (UniqueIssues_[severity].contains(issue)) {
return {};
}
if (IssueLimit_ && UniqueIssues_[severity].size() == IssueLimit_) {
OverflowIssues_[severity] = MakeIntrusive<TIssue>(TStringBuilder()
- << "Too many " << SeverityToString(issue.GetSeverity()) << " issues");
+ << "Too many " << SeverityToString(issue->GetSeverity()) << " issues");
OverflowIssues_[severity]->Severity = severity;
return {};
}
- UniqueIssues_[severity].insert(p);
- return p;
+ UniqueIssues_[severity].insert(issue);
+ return issue;
+}
+
+TIssuePtr TIssueManager::CheckUniqAndLimit(const TIssue& issue) {
+ const auto severity = issue.GetSeverity();
+ if (OverflowIssues_[severity]) {
+ return {};
+ }
+ return CheckUniqAndLimit(MakeIntrusive<TIssue>(issue));
}
void TIssueManager::RaiseIssue(const TIssue& issue) {
diff --git a/ydb/library/yql/public/issue/yql_issue_manager.h b/ydb/library/yql/public/issue/yql_issue_manager.h
index 9ad5ac7bb4..d9ab4a2821 100644
--- a/ydb/library/yql/public/issue/yql_issue_manager.h
+++ b/ydb/library/yql/public/issue/yql_issue_manager.h
@@ -36,8 +36,11 @@ public:
IssueLimit_ = limit;
}
+ void RaiseIssueForEmptyScope();
+
private:
TIssuePtr CheckUniqAndLimit(const TIssue& issue);
+ TIssuePtr CheckUniqAndLimit(TIssuePtr issue);
struct TIssueHash {
ui64 operator()(const TIssuePtr& p) {
@@ -66,6 +69,11 @@ public:
{
Manager_.AddScope(fn);
}
+
+ void RaiseIssueForEmptyScope() {
+ Manager_.RaiseIssueForEmptyScope();
+ }
+
~TIssueScopeGuard()
{
Manager_.LeaveScope();
diff --git a/ydb/services/ydb/ydb_table_ut.cpp b/ydb/services/ydb/ydb_table_ut.cpp
index c5854ec7be..de43ed56e2 100644
--- a/ydb/services/ydb/ydb_table_ut.cpp
+++ b/ydb/services/ydb/ydb_table_ut.cpp
@@ -197,6 +197,7 @@ Y_UNIT_TEST_SUITE(YdbYqlClient) {
auto ref = R"___(<main>: Error: Type annotation, code: 1030
<main>:2:25: Error: At function: KiWriteTable!
<main>:2:43: Error: Failed to convert type: Struct<'Key':String,'Value':String> to Struct<'Key':Uint32?,'Value':String?>
+ <main>:2:43: Error: Failed to convert 'Key': String to Optional<Uint32>
<main>:2:43: Error: Failed to convert input columns types to scheme types, code: 2031
)___";
UNIT_ASSERT_EQUAL(result.GetIssues().Size(), 1);