YDB FQ: avoid outdated syntax "SELECT * FROM cluster.db.table" (copy …

…of PR ydb-platform#6901) (ydb-platform#6945) Co-authored-by: Vitaly Isaev <vitalyisaev@ydb.tech>
dorooleg · Jul 23, 2024 · aa344c4 · aa344c4
1 parent 9210980
commit aa344c4
Show file tree

Hide file tree

Showing 9 changed files with 49 additions and 54 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<NConnector::NApi::EDataSourceKind> managedDatabaseKinds{
+        NConnector::NApi::EDataSourceKind::CLICKHOUSE,
+        NConnector::NApi::EDataSourceKind::GREENPLUM,
+        NConnector::NApi::EDataSourceKind::MYSQL,
         NConnector::NApi::EDataSourceKind::POSTGRESQL,
+        NConnector::NApi::EDataSourceKind::YDB,
+    };
+
+    static const TSet<NConnector::NApi::EDataSourceKind> 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
@@ -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
@@ -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
@@ -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: