Skip to content

Views: if exists / if not exists for DDL #10831

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

Merged
merged 4 commits into from
Oct 29, 2024
Merged

Views: if exists / if not exists for DDL #10831

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ebf87e4
auto correct: remove trailing whitespace
jepett0 Oct 24, 2024
2fd622a
views: if exists / if not exists for DDL
jepett0 Oct 24, 2024
48d668b
review fix
jepett0 Oct 26, 2024
3a714b5
review fix
jepett0 Oct 29, 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
33 changes: 27 additions & 6 deletions ydb/core/kqp/gateway/behaviour/view/manager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ void FillCreateViewProposal(NKikimrSchemeOp::TModifyScheme& modifyScheme,
const auto pathPair = SplitPathByDb(settings.GetObjectId(), context.GetDatabase());
modifyScheme.SetWorkingDir(pathPair.first);
modifyScheme.SetOperationType(NKikimrSchemeOp::ESchemeOpCreateView);
modifyScheme.SetFailedOnAlreadyExists(!settings.GetExistingOk());

auto& viewDesc = *modifyScheme.MutableCreateView();
viewDesc.SetName(pathPair.second);
Expand All @@ -77,16 +78,20 @@ void FillDropViewProposal(NKikimrSchemeOp::TModifyScheme& modifyScheme,
const auto pathPair = SplitPathByObjectId(settings.GetObjectId());
modifyScheme.SetWorkingDir(pathPair.first);
modifyScheme.SetOperationType(NKikimrSchemeOp::ESchemeOpDropView);
modifyScheme.SetSuccessOnNotExist(settings.GetMissingOk());

auto& drop = *modifyScheme.MutableDrop();
drop.SetName(pathPair.second);
}

NThreading::TFuture<TYqlConclusionStatus> SendSchemeRequest(TEvTxUserProxy::TEvProposeTransaction* request,
TActorSystem* actorSystem,
bool failOnAlreadyExists) {
bool failedOnAlreadyExists,
bool successOnNotExist) {
const auto promiseScheme = NThreading::NewPromise<NKqp::TSchemeOpRequestHandler::TResult>();
IActor* const requestHandler = new TSchemeOpRequestHandler(request, promiseScheme, failOnAlreadyExists);
IActor* const requestHandler = new TSchemeOpRequestHandler(
request, promiseScheme, failedOnAlreadyExists, successOnNotExist
);
actorSystem->Register(requestHandler);
return promiseScheme.GetFuture().Apply([](const NThreading::TFuture<NKqp::TSchemeOpRequestHandler::TResult>& opResult) {
if (opResult.HasValue()) {
Expand All @@ -109,7 +114,12 @@ NThreading::TFuture<TYqlConclusionStatus> CreateView(const NYql::TCreateObjectSe
auto& schemeTx = *proposal->Record.MutableTransaction()->MutableModifyScheme();
FillCreateViewProposal(schemeTx, settings, context.GetExternalData());

return SendSchemeRequest(proposal.Release(), context.GetExternalData().GetActorSystem(), true);
return SendSchemeRequest(
proposal.Release(),
context.GetExternalData().GetActorSystem(),
schemeTx.GetFailedOnAlreadyExists(),
schemeTx.GetSuccessOnNotExist()
);
}

NThreading::TFuture<TYqlConclusionStatus> DropView(const NYql::TDropObjectSettings& settings,
Expand All @@ -122,7 +132,12 @@ NThreading::TFuture<TYqlConclusionStatus> DropView(const NYql::TDropObjectSettin
auto& schemeTx = *proposal->Record.MutableTransaction()->MutableModifyScheme();
FillDropViewProposal(schemeTx, settings);

return SendSchemeRequest(proposal.Release(), context.GetExternalData().GetActorSystem(), false);
return SendSchemeRequest(
proposal.Release(),
context.GetExternalData().GetActorSystem(),
schemeTx.GetFailedOnAlreadyExists(),
schemeTx.GetSuccessOnNotExist()
);
}

void PrepareCreateView(NKqpProto::TKqpSchemeOperation& schemeOperation,
Expand Down Expand Up @@ -214,10 +229,10 @@ NThreading::TFuture<TYqlConclusionStatus> TViewManager::ExecutePrepared(const NK
switch (schemeOperation.GetOperationCase()) {
case NKqpProto::TKqpSchemeOperation::kCreateView:
schemeTx.CopyFrom(schemeOperation.GetCreateView());
return SendSchemeRequest(proposal.Release(), context.GetActorSystem(), true);
break;
case NKqpProto::TKqpSchemeOperation::kDropView:
schemeTx.CopyFrom(schemeOperation.GetDropView());
return SendSchemeRequest(proposal.Release(), context.GetActorSystem(), false);
break;
default:
return NThreading::MakeFuture(TYqlConclusionStatus::Fail(
TStringBuilder()
Expand All @@ -226,6 +241,12 @@ NThreading::TFuture<TYqlConclusionStatus> TViewManager::ExecutePrepared(const NK
)
);
}
return SendSchemeRequest(
proposal.Release(),
context.GetActorSystem(),
schemeTx.GetFailedOnAlreadyExists(),
schemeTx.GetSuccessOnNotExist()
);
}

}
45 changes: 45 additions & 0 deletions ydb/core/kqp/ut/view/view_ut.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,25 @@ Y_UNIT_TEST_SUITE(TCreateAndDropViewTest) {
}
}

Y_UNIT_TEST(CreateViewIfNotExists) {
TKikimrRunner kikimr(TKikimrSettings().SetWithSampleTables(false));
EnableViewsFeatureFlag(kikimr);
auto session = kikimr.GetQueryClient().GetSession().ExtractValueSync().GetSession();

constexpr const char* path = "/Root/TheView";
constexpr const char* queryInView = "SELECT 1";

const TString creationQuery = std::format(R"(
CREATE VIEW IF NOT EXISTS `{}` WITH (security_invoker = true) AS {};
)",
path,
queryInView
);
ExecuteQuery(session, creationQuery);
// an attempt to create a duplicate does not produce an error
ExecuteQuery(session, creationQuery);
}

Y_UNIT_TEST(DropView) {
TKikimrRunner kikimr(TKikimrSettings().SetWithSampleTables(false));
EnableViewsFeatureFlag(kikimr);
Expand Down Expand Up @@ -428,6 +447,32 @@ Y_UNIT_TEST_SUITE(TCreateAndDropViewTest) {
}
}

Y_UNIT_TEST(DropViewIfExists) {
TKikimrRunner kikimr(TKikimrSettings().SetWithSampleTables(false));
EnableViewsFeatureFlag(kikimr);
auto session = kikimr.GetQueryClient().GetSession().ExtractValueSync().GetSession();

constexpr const char* path = "/Root/TheView";
constexpr const char* queryInView = "SELECT 1";

const TString creationQuery = std::format(R"(
CREATE VIEW `{}` WITH (security_invoker = true) AS {};
)",
path,
queryInView
);
ExecuteQuery(session, creationQuery);

const TString dropQuery = std::format(R"(
DROP VIEW IF EXISTS `{}`;
)",
path
);
ExecuteQuery(session, dropQuery);
// an attempt to drop an already deleted view does not produce an error
ExecuteQuery(session, dropQuery);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need more tests to the following cases:

  • DROP VIEW query must fail if there is no such view
  • DROP VIEW query must fail if we are attempting to delete view, but it is not a view (topic, table etc)
  • DROP VIEW IF EXISTS query must fail if we are attempting to delete view, but it is not a view (topic, table etc)

All these cases should work also for ExecuteQuery calls (not only for ExecuteSchemeQuery) - they are newer API

Also the same cases, but for CREATE type

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added all 3 DROP VIEW tests you mentioned.

As for the CREATE VIEW, the test that checks that the CREATE VIEW statement does in fact create a view has already existed, so I haven't added it. However, I have added the other two tests for the CREATE VIEW statement.

}

Y_UNIT_TEST(DropViewInFolder) {
TKikimrRunner kikimr(TKikimrSettings().SetWithSampleTables(false));
EnableViewsFeatureFlag(kikimr);
Expand Down
4 changes: 2 additions & 2 deletions ydb/library/yql/sql/v1/SQLv1.g.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -601,12 +601,12 @@ alter_external_data_source_action:

drop_external_data_source_stmt: DROP EXTERNAL DATA SOURCE (IF EXISTS)? object_ref;

create_view_stmt: CREATE VIEW object_ref
create_view_stmt: CREATE VIEW (IF NOT EXISTS)? object_ref
create_object_features?
AS select_stmt
;

drop_view_stmt: DROP VIEW object_ref;
drop_view_stmt: DROP VIEW (IF EXISTS)? object_ref;

upsert_object_stmt: UPSERT OBJECT object_ref
LPAREN TYPE object_type_ref RPAREN
Expand Down
4 changes: 2 additions & 2 deletions ydb/library/yql/sql/v1/SQLv1Antlr4.g.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -600,12 +600,12 @@ alter_external_data_source_action:

drop_external_data_source_stmt: DROP EXTERNAL DATA SOURCE (IF EXISTS)? object_ref;

create_view_stmt: CREATE VIEW object_ref
create_view_stmt: CREATE VIEW (IF NOT EXISTS)? object_ref
create_object_features?
AS select_stmt
;

drop_view_stmt: DROP VIEW object_ref;
drop_view_stmt: DROP VIEW (IF EXISTS)? object_ref;

upsert_object_stmt: UPSERT OBJECT object_ref
LPAREN TYPE object_type_ref RPAREN
Expand Down
8 changes: 4 additions & 4 deletions ydb/library/yql/sql/v1/format/sql_format_ut.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1539,8 +1539,8 @@ Y_UNIT_TEST(ObfuscatePragma) {

Y_UNIT_TEST(CreateView) {
TCases cases = {
{"creAte vIEw TheView wiTh (security_invoker = trUE) As SELect 1",
"CREATE VIEW TheView WITH (security_invoker = TRUE) AS\nSELECT\n\t1;\n"},
{"creAte vIEw iF nOt EXistS TheView wiTh (security_invoker = trUE) As SELect 1",
"CREATE VIEW IF NOT EXISTS TheView WITH (security_invoker = TRUE) AS\nSELECT\n\t1;\n"},
};

TSetup setup;
Expand All @@ -1549,8 +1549,8 @@ Y_UNIT_TEST(CreateView) {

Y_UNIT_TEST(DropView) {
TCases cases = {
{"dRop viEW theVIEW",
"DROP VIEW theVIEW;\n"},
{"dRop viEW iF EXistS theVIEW",
"DROP VIEW IF EXISTS theVIEW;\n"},
};

TSetup setup;
Expand Down
30 changes: 17 additions & 13 deletions ydb/library/yql/sql/v1/sql_query.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1226,60 +1226,64 @@ bool TSqlQuery::Statement(TVector<TNodePtr>& blocks, const TRule_sql_stmt_core&
break;
}
case TRule_sql_stmt_core::kAltSqlStmtCore42: {
// create_view_stmt: CREATE VIEW name WITH (k = v, ...) AS select_stmt;
// create_view_stmt: CREATE VIEW (IF NOT EXISTS)? name (WITH (k = v, ...))? AS select_stmt;
auto& node = core.GetAlt_sql_stmt_core42().GetRule_create_view_stmt1();
TObjectOperatorContext context(Ctx.Scoped);
if (node.GetRule_object_ref3().HasBlock1()) {
if (!ClusterExpr(node.GetRule_object_ref3().GetBlock1().GetRule_cluster_expr1(),
if (node.GetRule_object_ref4().HasBlock1()) {
if (!ClusterExpr(node.GetRule_object_ref4().GetBlock1().GetRule_cluster_expr1(),
false,
context.ServiceId,
context.Cluster)) {
return false;
}
}

const bool existingOk = node.HasBlock3();

std::map<TString, TDeferredAtom> features;
if (node.HasBlock4()) {
if (!ParseObjectFeatures(features, node.GetBlock4().GetRule_create_object_features1().GetRule_object_features2())) {
if (node.HasBlock5()) {
if (!ParseObjectFeatures(features, node.GetBlock5().GetRule_create_object_features1().GetRule_object_features2())) {
return false;
}
}
if (!ParseViewQuery(features, node.GetRule_select_stmt6())) {
if (!ParseViewQuery(features, node.GetRule_select_stmt7())) {
return false;
}

const TString objectId = Id(node.GetRule_object_ref3().GetRule_id_or_at2(), *this).second;
const TString objectId = Id(node.GetRule_object_ref4().GetRule_id_or_at2(), *this).second;
constexpr const char* TypeId = "VIEW";
AddStatementToBlocks(blocks,
BuildCreateObjectOperation(Ctx.Pos(),
BuildTablePath(Ctx.GetPrefixPath(context.ServiceId, context.Cluster), objectId),
TypeId,
false,
existingOk,
false,
std::move(features),
context));
break;
}
case TRule_sql_stmt_core::kAltSqlStmtCore43: {
// drop_view_stmt: DROP VIEW name;
// drop_view_stmt: DROP VIEW (IF EXISTS)? name;
auto& node = core.GetAlt_sql_stmt_core43().GetRule_drop_view_stmt1();
TObjectOperatorContext context(Ctx.Scoped);
if (node.GetRule_object_ref3().HasBlock1()) {
if (!ClusterExpr(node.GetRule_object_ref3().GetBlock1().GetRule_cluster_expr1(),
if (node.GetRule_object_ref4().HasBlock1()) {
if (!ClusterExpr(node.GetRule_object_ref4().GetBlock1().GetRule_cluster_expr1(),
false,
context.ServiceId,
context.Cluster)) {
return false;
}
}

const TString objectId = Id(node.GetRule_object_ref3().GetRule_id_or_at2(), *this).second;
const bool missingOk = node.HasBlock3();

const TString objectId = Id(node.GetRule_object_ref4().GetRule_id_or_at2(), *this).second;
constexpr const char* TypeId = "VIEW";
AddStatementToBlocks(blocks,
BuildDropObjectOperation(Ctx.Pos(),
BuildTablePath(Ctx.GetPrefixPath(context.ServiceId, context.Cluster), objectId),
TypeId,
false,
missingOk,
{},
context));
break;
Expand Down
46 changes: 32 additions & 14 deletions ydb/library/yql/sql/v1/sql_ut.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2533,8 +2533,8 @@ Y_UNIT_TEST_SUITE(SqlParsingOnly) {
const auto result = SqlToYql(R"(USE plato;
CREATE TABLE table (
pk INT32 NOT NULL,
col String,
INDEX idx GLOBAL USING vector_kmeans_tree
col String,
INDEX idx GLOBAL USING vector_kmeans_tree
ON (col) COVER (col)
WITH (distance=cosine, vector_type=float, vector_dimension=1024,),
PRIMARY KEY (pk))
Expand All @@ -2543,11 +2543,11 @@ Y_UNIT_TEST_SUITE(SqlParsingOnly) {
}

Y_UNIT_TEST(AlterTableAddIndexVector) {
const auto result = SqlToYql(R"(USE plato;
ALTER TABLE table ADD INDEX idx
GLOBAL USING vector_kmeans_tree
const auto result = SqlToYql(R"(USE plato;
ALTER TABLE table ADD INDEX idx
GLOBAL USING vector_kmeans_tree
ON (col) COVER (col)
WITH (distance=cosine, vector_type="float", vector_dimension=1024)
WITH (distance=cosine, vector_type="float", vector_dimension=1024)
)");
UNIT_ASSERT_C(result.IsOk(), result.Issues.ToString());
}
Expand All @@ -2558,11 +2558,11 @@ Y_UNIT_TEST_SUITE(SqlParsingOnly) {
}

Y_UNIT_TEST(AlterTableAddIndexMissedParameter) {
ExpectFailWithError(R"(USE plato;
ALTER TABLE table ADD INDEX idx
GLOBAL USING vector_kmeans_tree
ExpectFailWithError(R"(USE plato;
ALTER TABLE table ADD INDEX idx
GLOBAL USING vector_kmeans_tree
ON (col)
WITH (distance=cosine, vector_type=float)
WITH (distance=cosine, vector_type=float)
)",
"<main>:5:52: Error: vector_dimension should be set\n");
}
Expand Down Expand Up @@ -2790,7 +2790,7 @@ Y_UNIT_TEST_SUITE(SqlParsingOnly) {
auto req = Sprintf(reqTpl, key.c_str(), value.c_str());
auto res = SqlToYql(req);
UNIT_ASSERT(res.Root);

TVerifyLineFunc verifyLine = [&key, &value](const TString& word, const TString& line) {
if (word == "Write") {
UNIT_ASSERT_VALUES_UNEQUAL(TString::npos, line.find("MyReplication"));
Expand Down Expand Up @@ -6716,6 +6716,15 @@ Y_UNIT_TEST_SUITE(TViewSyntaxTest) {
UNIT_ASSERT_C(res.Root, res.Issues.ToString());
}

Y_UNIT_TEST(CreateViewIfNotExistsGrammarSupport) {
NYql::TAstParseResult res = SqlToYql(R"(
USE plato;
CREATE VIEW IF NOT EXISTS TheView WITH (security_invoker = TRUE) AS SELECT 1;
)"
);
UNIT_ASSERT_C(res.Root, res.Issues.ToString());
}

Y_UNIT_TEST(CreateViewFromTable) {
constexpr const char* path = "/PathPrefix/TheView";
constexpr const char* query = R"(
Expand Down Expand Up @@ -6795,6 +6804,15 @@ Y_UNIT_TEST_SUITE(TViewSyntaxTest) {
UNIT_ASSERT_VALUES_EQUAL(elementStat["Write!"], 1);
}

Y_UNIT_TEST(DropViewIfExistsGrammarSupport) {
NYql::TAstParseResult res = SqlToYql(R"(
USE plato;
DROP VIEW IF EXISTS TheView;
)"
);
UNIT_ASSERT_C(res.Root, res.Issues.ToString());
}

Y_UNIT_TEST(CreateViewWithTablePrefix) {
NYql::TAstParseResult res = SqlToYql(R"(
USE plato;
Expand Down Expand Up @@ -6838,7 +6856,7 @@ Y_UNIT_TEST_SUITE(TViewSyntaxTest) {

UNIT_ASSERT_VALUES_EQUAL(elementStat["Write!"], 1);
}

Y_UNIT_TEST(YtAlternativeSchemaSyntax) {
NYql::TAstParseResult res = SqlToYql(R"(
SELECT * FROM plato.Input WITH schema(y Int32, x String not null);
Expand Down Expand Up @@ -6907,7 +6925,7 @@ Y_UNIT_TEST_SUITE(CompactNamedExprs) {
pragma CompactNamedExprs;
pragma ValidateUnusedExprs;

define subquery $x() as
define subquery $x() as
select count(1, 2);
end define;
select 1;
Expand All @@ -6930,7 +6948,7 @@ Y_UNIT_TEST_SUITE(CompactNamedExprs) {
pragma CompactNamedExprs;
pragma DisableValidateUnusedExprs;

define subquery $x() as
define subquery $x() as
select count(1, 2);
end define;
select 1;
Expand Down
Loading
Loading