add external data source public API for description #14438

jepett0 · 2025-02-11T12:43:05Z

Add public API to describe external data sources and external tables.

Changelog entry

Changelog category

Not for changelog (changelog entry is not required)

Additional information

external data sources: describe #14436

dorooleg · 2025-02-11T12:55:14Z

ydb/public/api/protos/ydb_table.proto

+    Ydb.Scheme.Entry self = 1;
+    optional string source_type = 2;
+    optional string location = 3;
+    optional string installation = 4;


Installation сейчас не используется, предлагаю когда имплементация появится, то добавить это поле

dorooleg · 2025-02-11T12:55:49Z

ydb/public/api/protos/ydb_table.proto

+    optional string data_source_path = 3;
+    optional string location = 4;
+    repeated ColumnMeta columns = 5;
+    optional bytes content = 6;


Content лучше развернуть в какой нибудь json/map<string, string> или же any с конкретными protobuf

Развернул в map<string, string>.

Для полноты:

content - это NKikimrExternalSources::TGeneral, что видно здесь

в TGeneral помимо map<string, string> лежит ещё и location, но он был добавлен для валидации wildcard'ов, возможно, его и не нужно в description'е отдавать (тем более, в description external table уже есть location (см. поле номер 4)) @dorooleg, @GrigoriyPA

dorooleg · 2025-02-11T12:56:53Z

ydb/public/api/protos/ydb_table.proto

+    optional string source_type = 2;
+    optional string location = 3;
+    optional string installation = 4;
+    optional ExternalAuthorization authorization = 5;


Возможно authorization стоит развернуть и переместить в properties. Между этими именами не будет пересечений это гарантируется на уровне ddl сейчас

Развернул Authorization в общий map<string, string> properties. Кажется, такое описание лучше ложится на:

YQL для создания external data source. Смотри доку, там, по сути, CREATE EXTERNAL DATA SOURCE и куча структурно не связанных друг с другом параметров в WITH блоке. Такая независимость может быть удобна и предусмотрена by design (@dorooleg ?), чтобы легче было добавлять новые внешние источники данных.

внутреннее представление этих параметров перед разворачиванием их в TModifyScheme (парсер сложил всё, что было в WITH блоке в map of features и мы оттуда достаём по имени параметр за параметром)

Иначе говоря, для создания external data source через публичный API (на данных момент это исключительно YQL) нужно собрать такую вот map of features. Поэтому естественно здесь, в describe, отдавать именно её.

Дополнительное усложнение в виде выделения ExternalAuthorization кажется избыточным, потому что такой структуры нет в CREATE EXTERNAL DATA SOURCE (она есть только на уровне приватных протобуфов SchemeShard'а), пользователь о ней не знает и в документации авторизация никак не выделена среди остальных свойств external data source

CyberROFL · 2025-02-11T13:01:23Z

ydb/public/api/protos/ydb_table.proto

+}
+
+message ExternalDataSourceProperties {
+    map<string, string> properties = 1;


Почему это упаковано в отдельное сообщение?

Когда это было на уровне ModifyScheme SS, то была идея это сделать отдельным protobuf чтобы можно было какие-то типизированные настройки добавлять. Но пока что не пригодилось)

Выложил на верхний уровень DescribeExternalDataSourceResult'а и объединил с ExternalAuthorization, которые тоже развернул в map<string, string>. Подробнее в этом комменте

CyberROFL · 2025-02-11T13:02:47Z

ydb/public/api/protos/ydb_table.proto

+    message None {
+    }
+
+    message Basic {


У нас это нигде явно не зафиксировано, но мы называем такую аутентификацию "статической": https://ydb.tech/docs/ru/recipes/ydb-sdk/auth-static

Похоже мы разъехались в этом месте, тут уже упоминается как basic https://ydb.tech/docs/ru/concepts/federated_query/clickhouse :(

Названия в данном месте не обязаны совпадать. Это различия между кредами к ydb и кредами к внешней системе. Статические креды-это просто некий класс кредов, можете в инете почитать

Засунул авторизацию в общее поле map<string, string> properties. Теперь уже не важно, Static или Basic. Подробнее в этом комменте

vitalyisaev2 · 2025-02-12T07:00:37Z

ydb/public/api/protos/ydb_table.proto

+message DescribeExternalDataSourceResult {
+    // Description of a generic scheme object.
+    Ydb.Scheme.Entry self = 1;
+    optional string source_type = 2;


А какими могут быть значения этого поля? Это S3/PostgreSQL/MySQL/... ?

Судя по доке:

В качестве источников данных можно использовать:

ClickHouse

PostgreSQL

Подключения к S3 (Object Storage)

Но в коде main на данный момент написано следующее (этот код относится к External Table, смотри следующий коммент):

bool ValidateSourceType(const TString& sourceType, TString& errStr) { // Only object storage supported today if (sourceType != "ObjectStorage") { errStr = "Only ObjectStorage source type supported but got " + sourceType; return false; } return true; }

Ага, кажется, я понимаю - этот код не совсем оттуда. Этот валидатор относится к CREATE EXTERNAL TABLE, и в этой команде действительно единственный поддерживаемый источник - это S3. А вот в CREATE EXTERNAL DATA SOURCE поддерживаемых источников гораздо больше.

Да, сорри, нашёл валидацию external data source. Тут список source type побольше, но полного я не нашёл (тут разобраны только некоторые опции):

bool DataSourceMustHaveDataBaseName(const TProtoStringType& sourceType) const { return IsIn({"Greenplum", "PostgreSQL", "MySQL", "MsSQLServer", "ClickHouse"}, sourceType); }

github-actions · 2025-02-12T07:40:55Z

⚪ 2025-02-12 07:40:54 UTC Pre-commit check linux-x86_64-release-asan for 7c472ce has started.
⚪ 2025-02-12 07:41:08 UTC Artifacts will be uploaded here
⚪ 2025-02-12 07:44:23 UTC ya make is running...
🟡 2025-02-12 09:22:31 UTC Some tests failed, follow the links below. This fail is not in blocking policy yet Going to retry failed tests...

Test history | Ya make output | Test bloat

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
13833	13717	0	66	11	39

⚪ 2025-02-12 09:23:56 UTC ya make is running... (failed tests rerun, try 2)
🟢 2025-02-12 09:35:59 UTC Tests successful.

Test history | Ya make output | Test bloat | Test bloat

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
160 (only retried tests)	122	0	0	3	35

🟢 2025-02-12 09:36:06 UTC Build successful.
🟡 2025-02-12 09:36:33 UTC ydbd size 3.6 GiB changed* by +698.5 KiB, which is >= 100.0 KiB vs main: Warning

ydbd size dash	main: `cdbb0bc`	merge: `7c472ce`	diff	diff %
ydbd size	3 876 203 824 Bytes	3 876 919 104 Bytes	+698.5 KiB	+0.018%
ydbd stripped size	1 357 356 976 Bytes	1 357 633 776 Bytes	+270.3 KiB	+0.020%

^{*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation}

dorooleg · 2025-02-12T10:00:23Z

lgtm

github-actions · 2025-02-12T16:36:05Z

⚪ 2025-02-12 16:36:04 UTC Pre-commit check linux-x86_64-relwithdebinfo for 7c472ce has started.
⚪ 2025-02-12 16:37:47 UTC Artifacts will be uploaded here
⚪ 2025-02-12 16:41:09 UTC ya make is running...
🟡 2025-02-12 18:12:11 UTC Some tests failed, follow the links below. Going to retry failed tests...

Test history | Ya make output | Test bloat

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
28071	25480	0	3	2449	139

⚪ 2025-02-12 18:17:30 UTC ya make is running... (failed tests rerun, try 2)
🟢 2025-02-12 18:29:43 UTC Tests successful.

Test history | Ya make output | Test bloat | Test bloat

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
217 (only retried tests)	90	0	0	0	127

🟢 2025-02-12 18:29:51 UTC Build successful.
🟡 2025-02-12 18:30:11 UTC ydbd size 2.1 GiB changed* by +380.8 KiB, which is >= 100.0 KiB vs main: Warning

ydbd size dash	main: `cdbb0bc`	merge: `7c472ce`	diff	diff %
ydbd size	2 228 945 224 Bytes	2 229 335 192 Bytes	+380.8 KiB	+0.017%
ydbd stripped size	473 261 656 Bytes	473 331 512 Bytes	+68.2 KiB	+0.015%

^{*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation}

add external data source public API for description

72fcd1c

github-actions bot added the not-for-changelog label Feb 11, 2025

This comment was marked as outdated.

Sign in to view

jepett0 requested review from CyberROFL and dorooleg February 11, 2025 12:52

dorooleg reviewed Feb 11, 2025

View reviewed changes

jepett0 requested a review from uzhastik February 11, 2025 12:58

CyberROFL reviewed Feb 11, 2025

View reviewed changes

vitalyisaev2 reviewed Feb 12, 2025

View reviewed changes

make properties a map + make content a map too

810de8b

jepett0 requested review from CyberROFL, dorooleg and vitalyisaev2 February 12, 2025 08:36

jepett0 marked this pull request as ready for review February 12, 2025 08:43

jepett0 requested review from fomichev3000 and spuchin February 12, 2025 08:43

jepett0 mentioned this pull request Feb 12, 2025

External data sources: describe in SDK #14483

Merged

jepett0 linked an issue Feb 12, 2025 that may be closed by this pull request

external data sources: describe #14436

Closed

uzhastik approved these changes Feb 12, 2025

View reviewed changes

CyberROFL approved these changes Feb 12, 2025

View reviewed changes

jepett0 merged commit 4fee494 into ydb-platform:main Feb 12, 2025
12 checks passed

GrigoriyPA mentioned this pull request Feb 13, 2025

YQ-3561 FQrun supported connections / bindings #14552

Merged

lberserq pushed a commit to lberserq/ydb that referenced this pull request Feb 14, 2025

add external data source public API for description (ydb-platform#14438)

6ed8c07

This was referenced Feb 15, 2025

YQ-4114 fixed pragmas for CTAS #14604

Merged

YQ remove anonymous namespace from CA #14636

Merged

blinkov pushed a commit that referenced this pull request Mar 21, 2025

add external data source public API for description (#14438)

9a5c511

add external data source public API for description #14438

add external data source public API for description #14438

Uh oh!

Conversation

jepett0 commented Feb 11, 2025

Changelog entry

Changelog category

Additional information

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jepett0 Feb 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Feb 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dorooleg commented Feb 12, 2025

Uh oh!

github-actions bot commented Feb 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jepett0 Feb 12, 2025 •

edited

Loading

github-actions bot commented Feb 12, 2025 •

edited

Loading

github-actions bot commented Feb 12, 2025 •

edited

Loading