Skip to content

YDB FQ: avoid outdated syntax "SELECT * FROM cluster.db.table" #6901

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

Closed
wants to merge 7 commits into from
Closed

YDB FQ: avoid outdated syntax "SELECT * FROM cluster.db.table" #6901

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f94e9e8
Reproduced on preprod
vitalyisaev2 Jul 20, 2024
64672e3
Handle Scheme error
vitalyisaev2 Jul 20, 2024
508fc80
Make database_name mandatory
vitalyisaev2 Jul 20, 2024
06ce877
Drop commented out lines
vitalyisaev2 Jul 20, 2024
82f6ce3
Fix style
vitalyisaev2 Jul 20, 2024
17caeb7
MySQL binary type mapping
vitalyisaev2 Jul 20, 2024
4b27fa8
Fix ydb/tests/fq/generic
vitalyisaev2 Jul 21, 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
4 changes: 4 additions & 0 deletions ydb/library/yql/providers/generic/connector/libcpp/error.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@ 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_BAD_REQUEST;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Нужно в NDqProto::StatusIds::StatusCode::StatusIds_StatusCode_SCHEME_ERROR мапить

default:
// FIXME: remove me after debug
Cout << "CRAB: " << ::Ydb::StatusIds::StatusCode_Name(error.status()) << Endl;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Это нужно убрать

ythrow yexception() << "Unexpected YDB status code: " << ::Ydb::StatusIds::StatusCode_Name(error.status());
}
}
Expand Down
2 changes: 1 addition & 1 deletion ydb/library/yql/providers/generic/connector/tests/datasource/clickhouse/docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.17@sha256:3763344ab70f9a6b8c1546f15c0b31465590e8ac6636be15ca2d29c4f4cd9b19
image: ghcr.io/ydb-platform/fq-connector-go:v0.5.0@sha256:6d3cec43478bef88dda195cd38c10e4df719c8ce6d13c9bd288c7ec40410e9d8
ports:
- 2130
volumes:
Expand Down
2 changes: 1 addition & 1 deletion ydb/library/yql/providers/generic/connector/tests/datasource/mysql/docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
services:
fq-connector-go:
container_name: fq-tests-my-fq-connector-go
image: ghcr.io/ydb-platform/fq-connector-go:v0.4.17@sha256:3763344ab70f9a6b8c1546f15c0b31465590e8ac6636be15ca2d29c4f4cd9b19
image: ghcr.io/ydb-platform/fq-connector-go:v0.5.0@sha256:6d3cec43478bef88dda195cd38c10e4df719c8ce6d13c9bd288c7ec40410e9d8
ports:
- 2130
volumes:
Expand Down
4 changes: 2 additions & 2 deletions ydb/library/yql/providers/generic/connector/tests/datasource/mysql/select_positive.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,12 +140,12 @@ def _primitive_types(self) -> Sequence[TestCase]:
# Don't be surprised - binary types look like UTF8 strings in Go
Column(
name='col_22_binary',
ydb_type=makeOptionalYdbTypeFromTypeID(Type.UTF8),
ydb_type=makeOptionalYdbTypeFromTypeID(Type.STRING),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Интересно как это связано с текущим ревью. Или это просто рефакторинг?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Тип MySQL Binary мапится в тип YDB String

https://github.com/ydb-platform/fq-connector-go/blob/17998b3542c7719295af44268b0e0dcdb0c2774a/docs/type_mapping_table.md?plain=1#L53C1-L53C265

data_source_type=DataSourceType(my=mysql.Binary()),
),
Column(
name='col_23_varbinary',
ydb_type=makeOptionalYdbTypeFromTypeID(Type.UTF8),
ydb_type=makeOptionalYdbTypeFromTypeID(Type.STRING),
data_source_type=DataSourceType(my=mysql.VarBinary()),
),
Column(
Expand Down
2 changes: 1 addition & 1 deletion ydb/library/yql/providers/generic/connector/tests/datasource/postgresql/docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.17@sha256:3763344ab70f9a6b8c1546f15c0b31465590e8ac6636be15ca2d29c4f4cd9b19
image: ghcr.io/ydb-platform/fq-connector-go:v0.5.0@sha256:6d3cec43478bef88dda195cd38c10e4df719c8ce6d13c9bd288c7ec40410e9d8
ports:
- 2130
volumes:
Expand Down
2 changes: 1 addition & 1 deletion ydb/library/yql/providers/generic/connector/tests/datasource/ydb/docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.17@sha256:3763344ab70f9a6b8c1546f15c0b31465590e8ac6636be15ca2d29c4f4cd9b19
image: ghcr.io/ydb-platform/fq-connector-go:v0.5.0@sha256:6d3cec43478bef88dda195cd38c10e4df719c8ce6d13c9bd288c7ec40410e9d8
ports:
- 2130
volumes:
Expand Down
2 changes: 1 addition & 1 deletion ydb/library/yql/providers/generic/connector/tests/join/docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.17@sha256:3763344ab70f9a6b8c1546f15c0b31465590e8ac6636be15ca2d29c4f4cd9b19
image: ghcr.io/ydb-platform/fq-connector-go:v0.5.0@sha256:6d3cec43478bef88dda195cd38c10e4df719c8ce6d13c9bd288c7ec40410e9d8
ports:
- 2130
volumes:
Expand Down
37 changes: 28 additions & 9 deletions ydb/library/yql/providers/generic/provider/yql_generic_cluster_config.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand Down Expand Up @@ -333,9 +329,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,
Expand Down Expand Up @@ -422,6 +430,17 @@ namespace NYql {
}
}

// 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'");
Expand Down
9 changes: 1 addition & 8 deletions ydb/library/yql/providers/generic/provider/yql_generic_dq_integration.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,13 +269,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
Expand All @@ -288,7 +281,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.
Expand Down
34 changes: 2 additions & 32 deletions ydb/library/yql/providers/generic/provider/yql_generic_load_meta.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,38 +378,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:
Expand Down
2 changes: 1 addition & 1 deletion ydb/tests/fq/generic/docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.17@sha256:3763344ab70f9a6b8c1546f15c0b31465590e8ac6636be15ca2d29c4f4cd9b19
image: ghcr.io/ydb-platform/fq-connector-go:v0.5.0@sha256:6d3cec43478bef88dda195cd38c10e4df719c8ce6d13c9bd288c7ec40410e9d8
ports:
- "2130"
postgresql:
Expand Down
Loading