From cf6500b753f6b859edab48aa6b7f1c62dc552dcd Mon Sep 17 00:00:00 2001 From: Grigorii Papashvili Date: Sun, 14 Jul 2024 00:01:03 +0000 Subject: [PATCH 01/10] reverting changes to check github CI --- .../providers/generic/connector/api/common/data_source.proto | 2 -- 1 file changed, 2 deletions(-) diff --git a/ydb/library/yql/providers/generic/connector/api/common/data_source.proto b/ydb/library/yql/providers/generic/connector/api/common/data_source.proto index 7be6bd8d7b27..278cd95a7fae 100644 --- a/ydb/library/yql/providers/generic/connector/api/common/data_source.proto +++ b/ydb/library/yql/providers/generic/connector/api/common/data_source.proto @@ -35,7 +35,6 @@ enum EDataSourceKind { MYSQL = 5; MS_SQL_SERVER = 6; GREENPLUM = 7; - ORACLE = 8; } // EProtocol generalizes various kinds of network protocols supported by different databases. @@ -99,6 +98,5 @@ message TDataSourceInstance { TClickhouseDataSourceOptions ch_options = 8; TS3DataSourceOptions s3_options = 9; TGreenplumDataSourceOptions gp_options = 10; - TOracleDataSourceOptions ora_options = 11; } } From 3de47000baf423609648e0b435e5b7453b50d89b Mon Sep 17 00:00:00 2001 From: Grigorii Papashvili Date: Sun, 14 Jul 2024 09:10:16 +0000 Subject: [PATCH 02/10] setting back oracle in proto --- .../providers/generic/connector/api/common/data_source.proto | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ydb/library/yql/providers/generic/connector/api/common/data_source.proto b/ydb/library/yql/providers/generic/connector/api/common/data_source.proto index 278cd95a7fae..7be6bd8d7b27 100644 --- a/ydb/library/yql/providers/generic/connector/api/common/data_source.proto +++ b/ydb/library/yql/providers/generic/connector/api/common/data_source.proto @@ -35,6 +35,7 @@ enum EDataSourceKind { MYSQL = 5; MS_SQL_SERVER = 6; GREENPLUM = 7; + ORACLE = 8; } // EProtocol generalizes various kinds of network protocols supported by different databases. @@ -98,5 +99,6 @@ message TDataSourceInstance { TClickhouseDataSourceOptions ch_options = 8; TS3DataSourceOptions s3_options = 9; TGreenplumDataSourceOptions gp_options = 10; + TOracleDataSourceOptions ora_options = 11; } } From 462dfcfce57165a8eee4e56b26e07dcf3846c33e Mon Sep 17 00:00:00 2001 From: Grigorii Papashvili Date: Tue, 16 Jul 2024 08:36:35 +0000 Subject: [PATCH 03/10] adding new exernal datasource oracle with new field - SERVICE_NAME --- .../external_sources/external_data_source.cpp | 2 +- .../external_source_factory.cpp | 4 ++++ .../external_data_source/manager.cpp | 1 + .../db_id_async_resolver/db_async_resolver.h | 7 ++++++- .../actors/yql_generic_provider_factories.cpp | 2 +- .../provider/yql_generic_cluster_config.cpp | 21 ++++++++++++++++++- .../provider/yql_generic_dq_integration.cpp | 5 +++++ .../provider/yql_generic_load_meta.cpp | 18 ++++++++++++++++ 8 files changed, 56 insertions(+), 4 deletions(-) diff --git a/ydb/core/external_sources/external_data_source.cpp b/ydb/core/external_sources/external_data_source.cpp index d44c8ca6dbb1..c14b2d7b3d0b 100644 --- a/ydb/core/external_sources/external_data_source.cpp +++ b/ydb/core/external_sources/external_data_source.cpp @@ -37,7 +37,7 @@ struct TExternalDataSource : public IExternalSource { } bool IsRDBMSDataSource(const TProtoStringType& sourceType) const { - return IsIn({"Greenplum", "PostgreSQL", "MySQL", "MsSQLServer", "Clickhouse"}, sourceType); + return IsIn({"Greenplum", "PostgreSQL", "MySQL", "MsSQLServer", "ClickHouse", "Oracle"}, sourceType); } virtual void ValidateExternalDataSource(const TString& externalDataSourceDescription) const override { diff --git a/ydb/core/external_sources/external_source_factory.cpp b/ydb/core/external_sources/external_source_factory.cpp index c0be11d62eab..8c1e1c9bc94f 100644 --- a/ydb/core/external_sources/external_source_factory.cpp +++ b/ydb/core/external_sources/external_source_factory.cpp @@ -70,6 +70,10 @@ IExternalSourceFactory::TPtr CreateExternalSourceFactory(const std::vector(name, readActorFactory); factory.RegisterLookupSource(name, lookupActorFactory); } diff --git a/ydb/library/yql/providers/generic/provider/yql_generic_cluster_config.cpp b/ydb/library/yql/providers/generic/provider/yql_generic_cluster_config.cpp index 0c0e769184c0..b8feb881bb34 100644 --- a/ydb/library/yql/providers/generic/provider/yql_generic_cluster_config.cpp +++ b/ydb/library/yql/providers/generic/provider/yql_generic_cluster_config.cpp @@ -139,6 +139,24 @@ namespace NYql { clusterConfig.mutable_datasourceoptions()->insert({TString("schema"), TString(it->second)}); } + void ParseServiceName(const THashMap& properties, + NYql::TGenericClusterConfig& clusterConfig) { + auto it = properties.find("service_name"); + if (it == properties.cend()) { + // TODO: required property? copied from ParserSchema + // ythrow yexception() << "missing 'SERVICE_NAME' value"; + return; + } + + if (!it->second) { + // TODO: required property? copied from ParserSchema + // ythrow yexception() << "invalid 'SERVICE_NAME' value: '" << it->second << "'"; + return; + } + + clusterConfig.mutable_datasourceoptions()->insert({TString("service_name"), TString(it->second)}); + } + void ParseMdbClusterId(const THashMap& properties, NYql::TGenericClusterConfig& clusterConfig) { auto it = properties.find("mdb_cluster_id"); @@ -192,7 +210,7 @@ namespace NYql { NYql::TGenericClusterConfig& clusterConfig) { using namespace NConnector::NApi; - if (IsIn({EDataSourceKind::GREENPLUM, EDataSourceKind::YDB, EDataSourceKind::MYSQL, EDataSourceKind::MS_SQL_SERVER}, clusterConfig.GetKind())) { + if (IsIn({EDataSourceKind::GREENPLUM, EDataSourceKind::YDB, EDataSourceKind::MYSQL, EDataSourceKind::MS_SQL_SERVER, EDataSourceKind::ORACLE}, clusterConfig.GetKind())) { clusterConfig.SetProtocol(EProtocol::NATIVE); return; } @@ -268,6 +286,7 @@ namespace NYql { ParseUseTLS(properties, clusterConfig); ParseDatabaseName(properties, clusterConfig); ParseSchema(properties, clusterConfig); + ParseServiceName(properties, clusterConfig); ParseMdbClusterId(properties, clusterConfig); ParseDatabaseId(properties, clusterConfig); ParseSourceType(properties, clusterConfig); diff --git a/ydb/library/yql/providers/generic/provider/yql_generic_dq_integration.cpp b/ydb/library/yql/providers/generic/provider/yql_generic_dq_integration.cpp index 19d8a5694d21..8954937a78a2 100644 --- a/ydb/library/yql/providers/generic/provider/yql_generic_dq_integration.cpp +++ b/ydb/library/yql/providers/generic/provider/yql_generic_dq_integration.cpp @@ -35,6 +35,8 @@ namespace NYql { return "GreenplumGeneric"; case NYql::NConnector::NApi::MS_SQL_SERVER: return "MsSQLServerGeneric"; + case NYql::NConnector::NApi::ORACLE: + return "OracleGeneric"; default: ythrow yexception() << "Data source kind is unknown or not specified"; } @@ -214,6 +216,9 @@ namespace NYql { case NConnector::NApi::MS_SQL_SERVER: properties["SourceType"] = "MsSQLServer"; break; + case NConnector::NApi::ORACLE: + properties["SourceType"] = "Oracle"; + break; case NConnector::NApi::DATA_SOURCE_KIND_UNSPECIFIED: break; default: diff --git a/ydb/library/yql/providers/generic/provider/yql_generic_load_meta.cpp b/ydb/library/yql/providers/generic/provider/yql_generic_load_meta.cpp index a5becd6add88..bae09232b8e6 100644 --- a/ydb/library/yql/providers/generic/provider/yql_generic_load_meta.cpp +++ b/ydb/library/yql/providers/generic/provider/yql_generic_load_meta.cpp @@ -327,6 +327,20 @@ namespace NYql { request.set_schema(schema); } + template + void GetServiceName(T& request, const TGenericClusterConfig& clusterConfig) { + TString serviceName; + const auto it = clusterConfig.GetDataSourceOptions().find("service_name"); + if (it != clusterConfig.GetDataSourceOptions().end()) { + serviceName = it->second; + } + if (!serviceName) { + serviceName = ""; + } + + request.set_service_name(serviceName); + } + void FillDataSourceOptions(NConnector::NApi::TDescribeTableRequest& request, const TGenericClusterConfig& clusterConfig) { const auto dataSourceKind = clusterConfig.GetKind(); switch (dataSourceKind) { @@ -346,6 +360,10 @@ namespace NYql { auto* options = request.mutable_data_source_instance()->mutable_pg_options(); SetSchema(*options, clusterConfig); } break; + case NYql::NConnector::NApi::ORACLE: { + auto* options = request.mutable_data_source_instance()->mutable_ora_options(); + GetServiceName(*options, clusterConfig); + } break; default: ythrow yexception() << "Unexpected data source kind: '" << NYql::NConnector::NApi::EDataSourceKind_Name(dataSourceKind) From 8e190ee929fe1046203f9a2e34f28e1111b6a78f Mon Sep 17 00:00:00 2001 From: Grigorii Papashvili Date: Tue, 16 Jul 2024 12:38:58 +0000 Subject: [PATCH 04/10] review changes --- .../provider/yql_generic_cluster_config.cpp | 15 +++++++++++---- .../generic/provider/yql_generic_load_meta.cpp | 11 ++--------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/ydb/library/yql/providers/generic/provider/yql_generic_cluster_config.cpp b/ydb/library/yql/providers/generic/provider/yql_generic_cluster_config.cpp index b8feb881bb34..f7faead2a77c 100644 --- a/ydb/library/yql/providers/generic/provider/yql_generic_cluster_config.cpp +++ b/ydb/library/yql/providers/generic/provider/yql_generic_cluster_config.cpp @@ -143,14 +143,10 @@ namespace NYql { NYql::TGenericClusterConfig& clusterConfig) { auto it = properties.find("service_name"); if (it == properties.cend()) { - // TODO: required property? copied from ParserSchema - // ythrow yexception() << "missing 'SERVICE_NAME' value"; return; } if (!it->second) { - // TODO: required property? copied from ParserSchema - // ythrow yexception() << "invalid 'SERVICE_NAME' value: '" << it->second << "'"; return; } @@ -415,6 +411,17 @@ namespace NYql { } } + // Oracle: + // * always set service_name for oracle; + if (clusterConfig.GetKind() == NConnector::NApi::ORACLE) { + if (!clusterConfig.HasServiceName()) { + return ValidationError( + clusterConfig, + context, + "For Oracle databases you must set service, but you have not set it"); + } + } + // check required fields if (!clusterConfig.GetName()) { return ValidationError(clusterConfig, context, "empty field 'Name'"); diff --git a/ydb/library/yql/providers/generic/provider/yql_generic_load_meta.cpp b/ydb/library/yql/providers/generic/provider/yql_generic_load_meta.cpp index bae09232b8e6..ab94ffd9b131 100644 --- a/ydb/library/yql/providers/generic/provider/yql_generic_load_meta.cpp +++ b/ydb/library/yql/providers/generic/provider/yql_generic_load_meta.cpp @@ -327,18 +327,11 @@ namespace NYql { request.set_schema(schema); } - template - void GetServiceName(T& request, const TGenericClusterConfig& clusterConfig) { - TString serviceName; + void GetServiceName(NYql::NConnector::NApi::TOracleDataSourceOptions& request, const TGenericClusterConfig& clusterConfig) { const auto it = clusterConfig.GetDataSourceOptions().find("service_name"); if (it != clusterConfig.GetDataSourceOptions().end()) { - serviceName = it->second; - } - if (!serviceName) { - serviceName = ""; + request.set_service_name(it->second); } - - request.set_service_name(serviceName); } void FillDataSourceOptions(NConnector::NApi::TDescribeTableRequest& request, const TGenericClusterConfig& clusterConfig) { From aadb517bd105c0b875f0ff5845ef8e0adb063893 Mon Sep 17 00:00:00 2001 From: Grigorii Papashvili Date: Tue, 16 Jul 2024 13:00:41 +0000 Subject: [PATCH 05/10] validate fixes --- .../providers/generic/provider/yql_generic_cluster_config.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ydb/library/yql/providers/generic/provider/yql_generic_cluster_config.cpp b/ydb/library/yql/providers/generic/provider/yql_generic_cluster_config.cpp index f7faead2a77c..597f289e3385 100644 --- a/ydb/library/yql/providers/generic/provider/yql_generic_cluster_config.cpp +++ b/ydb/library/yql/providers/generic/provider/yql_generic_cluster_config.cpp @@ -414,7 +414,7 @@ namespace NYql { // Oracle: // * always set service_name for oracle; if (clusterConfig.GetKind() == NConnector::NApi::ORACLE) { - if (!clusterConfig.HasServiceName()) { + if (!clusterConfig.GetDataSourceOptions().contains("service_name")) { return ValidationError( clusterConfig, context, From 0d3e90a48fc5fb251e881d17200f2942382717de Mon Sep 17 00:00:00 2001 From: Grigorii Papashvili Date: Tue, 16 Jul 2024 15:51:27 +0000 Subject: [PATCH 06/10] style fixes --- .../providers/generic/provider/yql_generic_cluster_config.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ydb/library/yql/providers/generic/provider/yql_generic_cluster_config.cpp b/ydb/library/yql/providers/generic/provider/yql_generic_cluster_config.cpp index 597f289e3385..dc74479e4d5a 100644 --- a/ydb/library/yql/providers/generic/provider/yql_generic_cluster_config.cpp +++ b/ydb/library/yql/providers/generic/provider/yql_generic_cluster_config.cpp @@ -140,7 +140,7 @@ namespace NYql { } void ParseServiceName(const THashMap& properties, - NYql::TGenericClusterConfig& clusterConfig) { + NYql::TGenericClusterConfig& clusterConfig) { auto it = properties.find("service_name"); if (it == properties.cend()) { return; From 73facdf6e377be0d35b3d083a59b06feacc7240e Mon Sep 17 00:00:00 2001 From: Grigorii Papashvili Date: Wed, 17 Jul 2024 11:14:09 +0000 Subject: [PATCH 07/10] adding oracle external datasource validation --- ydb/core/external_sources/external_data_source.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ydb/core/external_sources/external_data_source.cpp b/ydb/core/external_sources/external_data_source.cpp index c14b2d7b3d0b..ef159ab8e794 100644 --- a/ydb/core/external_sources/external_data_source.cpp +++ b/ydb/core/external_sources/external_data_source.cpp @@ -53,10 +53,15 @@ struct TExternalDataSource : public IExternalSource { ythrow TExternalSourceException() << "Unsupported property: " << key; } - if (IsRDBMSDataSource(proto.GetSourceType()) && !proto.GetProperties().GetProperties().contains("database_name")){ + if (IsRDBMSDataSource(proto.GetSourceType()) && !proto.GetProperties().GetProperties().contains("database_name")) { ythrow TExternalSourceException() << proto.GetSourceType() << " source must provide database_name"; } + // oracle must have property service_name + if (proto.GetSourceType() && !proto.GetProperties().GetProperties().contains("service_name")) { + ythrow TExternalSourceException() << proto.GetSourceType() << " source must provide service_name"; + } + ValidateHostname(HostnamePatterns, proto.GetLocation()); } From a8afa6abe6b469f4e4882ab2c2756b6ff78cf7c3 Mon Sep 17 00:00:00 2001 From: Grigorii Papashvili Date: Wed, 17 Jul 2024 11:14:56 +0000 Subject: [PATCH 08/10] renaming ora_options to oracle_options --- .../yql/providers/generic/provider/yql_generic_load_meta.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ydb/library/yql/providers/generic/provider/yql_generic_load_meta.cpp b/ydb/library/yql/providers/generic/provider/yql_generic_load_meta.cpp index ab94ffd9b131..341568e82b71 100644 --- a/ydb/library/yql/providers/generic/provider/yql_generic_load_meta.cpp +++ b/ydb/library/yql/providers/generic/provider/yql_generic_load_meta.cpp @@ -354,7 +354,7 @@ namespace NYql { SetSchema(*options, clusterConfig); } break; case NYql::NConnector::NApi::ORACLE: { - auto* options = request.mutable_data_source_instance()->mutable_ora_options(); + auto* options = request.mutable_data_source_instance()->mutable_oracle_options(); GetServiceName(*options, clusterConfig); } break; From 69ab9bdc904b9f1564d869db770b6d3fbb8e910f Mon Sep 17 00:00:00 2001 From: Grigorii Papashvili Date: Wed, 17 Jul 2024 11:21:11 +0000 Subject: [PATCH 09/10] renaming ora_options to oracle_options --- .../providers/generic/connector/api/common/data_source.proto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ydb/library/yql/providers/generic/connector/api/common/data_source.proto b/ydb/library/yql/providers/generic/connector/api/common/data_source.proto index 7be6bd8d7b27..692a26f491ce 100644 --- a/ydb/library/yql/providers/generic/connector/api/common/data_source.proto +++ b/ydb/library/yql/providers/generic/connector/api/common/data_source.proto @@ -99,6 +99,6 @@ message TDataSourceInstance { TClickhouseDataSourceOptions ch_options = 8; TS3DataSourceOptions s3_options = 9; TGreenplumDataSourceOptions gp_options = 10; - TOracleDataSourceOptions ora_options = 11; + TOracleDataSourceOptions oracle_options = 11; } } From 376081ca2102a7439671d56d29730915ff8f3eda Mon Sep 17 00:00:00 2001 From: Grigorii Papashvili Date: Wed, 17 Jul 2024 13:17:46 +0000 Subject: [PATCH 10/10] fixing oracle property validation --- ydb/core/external_sources/external_data_source.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ydb/core/external_sources/external_data_source.cpp b/ydb/core/external_sources/external_data_source.cpp index ef159ab8e794..9de3908fc759 100644 --- a/ydb/core/external_sources/external_data_source.cpp +++ b/ydb/core/external_sources/external_data_source.cpp @@ -58,7 +58,7 @@ struct TExternalDataSource : public IExternalSource { } // oracle must have property service_name - if (proto.GetSourceType() && !proto.GetProperties().GetProperties().contains("service_name")) { + if (proto.GetSourceType() == "Oracle" && !proto.GetProperties().GetProperties().contains("service_name")) { ythrow TExternalSourceException() << proto.GetSourceType() << " source must provide service_name"; }