From aa344c488b75a93a47844595f3350bd2fe78d890 Mon Sep 17 00:00:00 2001 From: Grigorii Papashvili Date: Tue, 23 Jul 2024 09:27:58 +0300 Subject: [PATCH 1/2] YDB FQ: avoid outdated syntax "SELECT * FROM cluster.db.table" (copy of PR #6901) (#6945) Co-authored-by: Vitaly Isaev --- .../generic/connector/libcpp/error.cpp | 2 + .../datasource/clickhouse/docker-compose.yml | 2 +- .../datasource/postgresql/docker-compose.yml | 2 +- .../tests/datasource/ydb/docker-compose.yml | 2 +- .../connector/tests/join/docker-compose.yml | 2 +- .../provider/yql_generic_cluster_config.cpp | 48 +++++++++++++++---- .../provider/yql_generic_dq_integration.cpp | 9 +--- .../provider/yql_generic_load_meta.cpp | 34 +------------ ydb/tests/fq/generic/docker-compose.yml | 2 +- 9 files changed, 49 insertions(+), 54 deletions(-) diff --git a/ydb/library/yql/providers/generic/connector/libcpp/error.cpp b/ydb/library/yql/providers/generic/connector/libcpp/error.cpp index 3f6bd3b7a448..a050e1f8ef27 100644 --- a/ydb/library/yql/providers/generic/connector/libcpp/error.cpp +++ b/ydb/library/yql/providers/generic/connector/libcpp/error.cpp @@ -37,6 +37,8 @@ namespace NYql::NConnector { return NDqProto::StatusIds::StatusCode::StatusIds_StatusCode_UNSUPPORTED; case ::Ydb::StatusIds::StatusCode::StatusIds_StatusCode_NOT_FOUND: return NDqProto::StatusIds::StatusCode::StatusIds_StatusCode_BAD_REQUEST; + case ::Ydb::StatusIds::StatusCode::StatusIds_StatusCode_SCHEME_ERROR: + return NDqProto::StatusIds::StatusCode::StatusIds_StatusCode_SCHEME_ERROR; default: ythrow yexception() << "Unexpected YDB status code: " << ::Ydb::StatusIds::StatusCode_Name(error.status()); } diff --git a/ydb/library/yql/providers/generic/connector/tests/datasource/clickhouse/docker-compose.yml b/ydb/library/yql/providers/generic/connector/tests/datasource/clickhouse/docker-compose.yml index afa9f3d4d3c2..dd58ce80b4df 100644 --- a/ydb/library/yql/providers/generic/connector/tests/datasource/clickhouse/docker-compose.yml +++ b/ydb/library/yql/providers/generic/connector/tests/datasource/clickhouse/docker-compose.yml @@ -12,7 +12,7 @@ services: - 8123 fq-connector-go: container_name: fq-tests-ch-fq-connector-go - image: ghcr.io/ydb-platform/fq-connector-go:v0.4.9@sha256:3a1fe086be50c0edbae2c2b284aee5ce76bd056d7f46cb460919ec37a6f8ab5c + image: ghcr.io/ydb-platform/fq-connector-go:v0.5.0@sha256:6d3cec43478bef88dda195cd38c10e4df719c8ce6d13c9bd288c7ec40410e9d8 ports: - 2130 volumes: diff --git a/ydb/library/yql/providers/generic/connector/tests/datasource/postgresql/docker-compose.yml b/ydb/library/yql/providers/generic/connector/tests/datasource/postgresql/docker-compose.yml index 146f50dd07b0..de6e29ea8bf4 100644 --- a/ydb/library/yql/providers/generic/connector/tests/datasource/postgresql/docker-compose.yml +++ b/ydb/library/yql/providers/generic/connector/tests/datasource/postgresql/docker-compose.yml @@ -1,7 +1,7 @@ services: fq-connector-go: container_name: fq-tests-pg-fq-connector-go - image: ghcr.io/ydb-platform/fq-connector-go:v0.4.9@sha256:3a1fe086be50c0edbae2c2b284aee5ce76bd056d7f46cb460919ec37a6f8ab5c + image: ghcr.io/ydb-platform/fq-connector-go:v0.5.0@sha256:6d3cec43478bef88dda195cd38c10e4df719c8ce6d13c9bd288c7ec40410e9d8 ports: - 2130 volumes: diff --git a/ydb/library/yql/providers/generic/connector/tests/datasource/ydb/docker-compose.yml b/ydb/library/yql/providers/generic/connector/tests/datasource/ydb/docker-compose.yml index 4b691f155d54..37a2dec71d1d 100644 --- a/ydb/library/yql/providers/generic/connector/tests/datasource/ydb/docker-compose.yml +++ b/ydb/library/yql/providers/generic/connector/tests/datasource/ydb/docker-compose.yml @@ -5,7 +5,7 @@ services: echo \"$$(dig fq-tests-ydb-ydb +short) fq-tests-ydb-ydb\" >> /etc/hosts; cat /etc/hosts; /opt/ydb/bin/fq-connector-go server -c /opt/ydb/cfg/fq-connector-go.yaml" container_name: fq-tests-ydb-fq-connector-go - image: ghcr.io/ydb-platform/fq-connector-go:v0.4.9@sha256:3a1fe086be50c0edbae2c2b284aee5ce76bd056d7f46cb460919ec37a6f8ab5c + image: ghcr.io/ydb-platform/fq-connector-go:v0.5.0@sha256:6d3cec43478bef88dda195cd38c10e4df719c8ce6d13c9bd288c7ec40410e9d8 ports: - 2130 volumes: diff --git a/ydb/library/yql/providers/generic/connector/tests/join/docker-compose.yml b/ydb/library/yql/providers/generic/connector/tests/join/docker-compose.yml index efb69b50eee6..8383a480bf6f 100644 --- a/ydb/library/yql/providers/generic/connector/tests/join/docker-compose.yml +++ b/ydb/library/yql/providers/generic/connector/tests/join/docker-compose.yml @@ -12,7 +12,7 @@ services: - 8123 fq-connector-go: container_name: fq-tests-join-fq-connector-go - image: ghcr.io/ydb-platform/fq-connector-go:v0.4.9@sha256:3a1fe086be50c0edbae2c2b284aee5ce76bd056d7f46cb460919ec37a6f8ab5c + image: ghcr.io/ydb-platform/fq-connector-go:v0.5.0@sha256:6d3cec43478bef88dda195cd38c10e4df719c8ce6d13c9bd288c7ec40410e9d8 ports: - 2130 volumes: 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..0eeae111939b 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 @@ -107,14 +107,12 @@ namespace NYql { NYql::TGenericClusterConfig& clusterConfig) { auto it = properties.find("database_name"); if (it == properties.cend()) { - // TODO: make this property required during https://st.yandex-team.ru/YQ-2494 - // ythrow yexception() << "missing 'DATABASE_NAME' value"; + // DATABASE_NAME is a mandatory field for the most of databases, + // however, managed YDB does not require it, so we have to accept empty values here. return; } if (!it->second) { - // TODO: make this property required during https://st.yandex-team.ru/YQ-2494 - // ythrow yexception() << "invalid 'DATABASE_NAME' value: '" << it->second << "'"; return; } @@ -125,14 +123,12 @@ namespace NYql { NYql::TGenericClusterConfig& clusterConfig) { auto it = properties.find("schema"); if (it == properties.cend()) { - // TODO: make this property required during https://st.yandex-team.ru/YQ-2494 - // ythrow yexception() << "missing 'SCHEMA' value"; + // SCHEMA is optional field return; } if (!it->second) { - // TODO: make this property required during https://st.yandex-team.ru/YQ-2494 - // ythrow yexception() << "invalid 'SCHEMA' value: '" << it->second << "'"; + // SCHEMA is optional field return; } @@ -318,9 +314,21 @@ namespace NYql { } static const TSet managedDatabaseKinds{ + NConnector::NApi::EDataSourceKind::CLICKHOUSE, + NConnector::NApi::EDataSourceKind::GREENPLUM, + NConnector::NApi::EDataSourceKind::MYSQL, NConnector::NApi::EDataSourceKind::POSTGRESQL, + NConnector::NApi::EDataSourceKind::YDB, + }; + + static const TSet traditionalRelationalDatabaseKinds{ NConnector::NApi::EDataSourceKind::CLICKHOUSE, - NConnector::NApi::EDataSourceKind::YDB}; + NConnector::NApi::EDataSourceKind::GREENPLUM, + NConnector::NApi::EDataSourceKind::MS_SQL_SERVER, + NConnector::NApi::EDataSourceKind::MYSQL, + NConnector::NApi::EDataSourceKind::ORACLE, + NConnector::NApi::EDataSourceKind::POSTGRESQL, + }; void ValidateGenericClusterConfig( const NYql::TGenericClusterConfig& clusterConfig, @@ -396,6 +404,28 @@ namespace NYql { } } + // Oracle: + // * always set service_name for oracle; + if (clusterConfig.GetKind() == NConnector::NApi::ORACLE) { + if (!clusterConfig.GetDataSourceOptions().contains("service_name")) { + return ValidationError( + clusterConfig, + context, + "For Oracle databases you must set service, but you have not set it"); + } + } + + // All the databases with exception to managed YDB: + // * DATABASE_NAME is mandatory field + if (traditionalRelationalDatabaseKinds.contains(clusterConfig.GetKind())) { + if (!clusterConfig.GetDatabaseName()) { + return ValidationError( + clusterConfig, + context, + "You must provide database name explicitly"); + } + } + // check required fields if (!clusterConfig.GetName()) { return ValidationError(clusterConfig, context, "empty field 'Name'"); 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..24fab4c67637 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 @@ -264,13 +264,6 @@ namespace NYql { const auto& clusterConfig = State_->Configuration->ClusterNamesToClusterConfigs[clusterName]; const auto& endpoint = clusterConfig.endpoint(); - // for backward compability full path can be used (cluster_name.`db_name.table`) - // TODO: simplify during https://st.yandex-team.ru/YQ-2494 - TStringBuf db, dbTable; - if (!TStringBuf(table).TrySplit('.', db, dbTable)) { - dbTable = table; - } - YQL_CLOG(INFO, ProviderGeneric) << "Filling lookup source settings" << ": cluster: " << clusterName @@ -283,7 +276,7 @@ namespace NYql { } Generic::TLookupSource source; - source.set_table(TString(dbTable)); + source.set_table(table); *source.mutable_data_source_instance() = tableMeta.value()->DataSourceInstance; // Managed YDB supports access via IAM token. 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..994d1cc6e81d 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 @@ -367,38 +367,8 @@ namespace NYql { void FillTablePath(NConnector::NApi::TDescribeTableRequest& request, const TGenericClusterConfig& clusterConfig, const TString& tablePath) { - // for backward compability full path can be used (cluster_name.`db_name.table`) - // TODO: simplify during https://st.yandex-team.ru/YQ-2494 - const auto dataSourceKind = clusterConfig.GetKind(); - const auto& dbNameFromConfig = clusterConfig.GetDatabaseName(); - TStringBuf dbNameTarget, tableName; - auto isFullPath = TStringBuf(tablePath).TrySplit('.', dbNameTarget, tableName); - - if (!dbNameFromConfig.empty()) { - dbNameTarget = dbNameFromConfig; - if (!isFullPath) { - tableName = tablePath; - } - } else if (!isFullPath) { - tableName = tablePath; - switch (dataSourceKind) { - case NYql::NConnector::NApi::CLICKHOUSE: - dbNameTarget = "default"; - break; - case NYql::NConnector::NApi::POSTGRESQL: - dbNameTarget = "postgres"; - break; - case NYql::NConnector::NApi::MS_SQL_SERVER: - dbNameTarget = "mssqlserver"; - break; - default: - ythrow yexception() << "You must provide database name explicitly for data source kind: '" - << NYql::NConnector::NApi::EDataSourceKind_Name(dataSourceKind) << "'"; - } - } // else take database name from table path - - request.mutable_data_source_instance()->set_database(TString(dbNameTarget)); - request.set_table(TString(tableName)); + request.mutable_data_source_instance()->set_database(clusterConfig.GetDatabaseName()); + request.set_table(tablePath); } private: diff --git a/ydb/tests/fq/generic/docker-compose.yml b/ydb/tests/fq/generic/docker-compose.yml index 8b8d86f4ddb3..ce60fdfe7d63 100644 --- a/ydb/tests/fq/generic/docker-compose.yml +++ b/ydb/tests/fq/generic/docker-compose.yml @@ -15,7 +15,7 @@ services: echo \"$$(dig tests-fq-generic-ydb +short) tests-fq-generic-ydb\" >> /etc/hosts; cat /etc/hosts; /opt/ydb/bin/fq-connector-go server -c /opt/ydb/cfg/fq-connector-go.yaml" container_name: tests-fq-generic-fq-connector-go - image: ghcr.io/ydb-platform/fq-connector-go:v0.4.9@sha256:3a1fe086be50c0edbae2c2b284aee5ce76bd056d7f46cb460919ec37a6f8ab5c + image: ghcr.io/ydb-platform/fq-connector-go:v0.5.0@sha256:6d3cec43478bef88dda195cd38c10e4df719c8ce6d13c9bd288c7ec40410e9d8 ports: - "2130" postgresql: From eb3b7dff41dfa44a42d624e263040a4a7bf7b017 Mon Sep 17 00:00:00 2001 From: Oleg Doronin Date: Tue, 23 Jul 2024 14:56:47 +0000 Subject: [PATCH 2/2] cleanup --- .../generic/provider/yql_generic_cluster_config.cpp | 12 ------------ 1 file changed, 12 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 0eeae111939b..4f773517111c 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 @@ -326,7 +326,6 @@ namespace NYql { NConnector::NApi::EDataSourceKind::GREENPLUM, NConnector::NApi::EDataSourceKind::MS_SQL_SERVER, NConnector::NApi::EDataSourceKind::MYSQL, - NConnector::NApi::EDataSourceKind::ORACLE, NConnector::NApi::EDataSourceKind::POSTGRESQL, }; @@ -404,17 +403,6 @@ namespace NYql { } } - // Oracle: - // * always set service_name for oracle; - if (clusterConfig.GetKind() == NConnector::NApi::ORACLE) { - if (!clusterConfig.GetDataSourceOptions().contains("service_name")) { - return ValidationError( - clusterConfig, - context, - "For Oracle databases you must set service, but you have not set it"); - } - } - // All the databases with exception to managed YDB: // * DATABASE_NAME is mandatory field if (traditionalRelationalDatabaseKinds.contains(clusterConfig.GetKind())) {