feat(frontend): support create iceberg source #14971

chenzl25 · 2024-02-04T06:19:10Z

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

Related issue Feat: Batch ingest iceberg/file source #14742
Support create iceberg source.
iceberg source could not be used in streaming queries.
iceberg source enumerator only checks source schema against iceberg schema.
Because iceberg source has its own row format and doesn't need to be specified by users explicitly, we could omit it.

Iceberg source Example

create source s
( user_id bigint, user_name string)
with (
    connector = 'iceberg',
    catalog.type = 'storage',
    warehouse.path = 's3://hummock001/',
    s3.endpoint = 'http://127.0.0.1:9301',
    s3.access.key = 'hummockadmin',
    s3.secret.key = 'hummockadmin',
    s3.region = 'us-east-1',
    database.name='demo_db',
    table.name='demo_table'
);

Checklist

I have written necessary rustdoc comments
I have added necessary unit tests and integration tests
I have added test labels as necessary. See details.
I have added fuzzing tests or opened an issue to track them. (Optional, recommended for new SQL features Sqlsmith: Sql feature generation #7934).
My PR contains breaking changes. (If it deprecates some features, please create a tracking issue to remove them in the future).
All checks passed in ./risedev check (or alias, ./risedev c)
My PR changes performance-critical code. (Please run macro/micro-benchmarks and show the results.)

My PR contains critical fixes that are necessary to be merged into the latest release. (Please check out the details)

Documentation

My PR needs documentation updates. (Please use the Release note section below to summarize the impact on users)

Release note

If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.

…iceberg_source

gitguardian · 2024-02-04T06:19:15Z

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secret in your pull request

GitGuardian id	GitGuardian status	Secret	Commit	Filename
9425213	Triggered	Generic Password	`11fdfcd`	ci/scripts/regress-test.sh	View secret

🛠 Guidelines to remediate hardcoded secrets

Understand the implications of revoking this secret by investigating where it is used in your code.
Replace and store your secret safely. Learn here the best practices.
Revoke and rotate this secret.
If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider

following these best practices for managing and storing secrets including API keys and other credentials
install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.

^{🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Our GitHub checks need improvements? Share your feedbacks!}

liurenjie1024

Generally LGTM

src/connector/src/source/iceberg/mod.rs

liurenjie1024 · 2024-02-05T02:29:49Z

src/connector/src/source/base.rs

 pub struct SourceEnumeratorInfo {
    pub source_id: u32,
+    pub source: Option<Source>,


Why we need to add this?

For Iceberg, we need this Source to validate that the schema users provided is validated.

Can we do that in the optimizer stage as other connectors do? During the create source process, we fetch all related info and do checks there

st1page · 2024-02-05T04:03:30Z

src/sqlparser/src/ast/statement.rs

@@ -249,6 +249,18 @@ impl Parser {
            } else {
                ConnectorSchema::native().into()
            })
+        } else if connector.contains("iceberg") {
+            let expected = ConnectorSchema::native();


I am not sure what the format and encode should be here, but it should not be the native. native means the format and encode is the same with Rsingwave's internal representation.
the FORMAT means how to get the operation(Insert/Delete) from the file, the ENCODE is means how to get the rows containment from the bytes, such as parquet/csv/json.

I think actually the issue is that "batch-only source" does not need to resolve the operation from the file... so the streaming connector's “FORMAT” is meaningless for it... And I am not sure about the encode, does it stored in iceberg's catalog?

I also don't think we need encode or format for iceberg.

Yes, for iceberg. We don't need any format or encoding, because they are self-explanatory and users don't need to specify any format and encoding. We could introduce another Format works like Native to meet this requirement.

Too many places expect source format and encoding, so I don't want to make them accept an Option<SourceSchema>. Introduce a trivial format and encoding for iceberg LGTM.

I think actually the issue is that "batch-only source" does not need to resolve the operation from the file... so the streaming connector's “FORMAT” is meaningless for it... And I am not sure about the encode, does it stored in iceberg's catalog?

here we just want to load from the iceberg, all chunks are written as insert I think. format plain should be ok, which means append only. encode can be a random placeholder, eg. iceberg, to mark it as a spec encode and not for common cases.

I am not sure if it can be considered as a kind of format plain... especially considering there could be deletion files in Iceberge. I am thinking if we need ConnectorSchema for the iceberg connector.

st1page

LGTM

src/connector/src/sink/catalog/mod.rs

tabVersion · 2024-02-06T08:47:16Z

src/connector/src/source/base.rs

 pub struct SourceEnumeratorInfo {
    pub source_id: u32,
+    pub source: Option<Source>,


Can we do that in the optimizer stage as other connectors do? During the create source process, we fetch all related info and do checks there

tabVersion · 2024-02-06T08:48:23Z

src/connector/src/source/iceberg/mod.rs

+    fn id(&self) -> SplitId {
+        unimplemented!()
+    }
+
+    fn restore_from_json(_value: JsonbVal) -> anyhow::Result<Self> {
+        unimplemented!()
+    }
+
+    fn encode_to_json(&self) -> JsonbVal {
+        unimplemented!()
+    }
+
+    fn update_with_offset(&mut self, _start_offset: String) -> anyhow::Result<()> {
+        unimplemented!()
+    }


are these going to be impl? or left them here on purpose

Some of them would be implemented for batch-read in a later PR. But most of them related to streaming are left unimplemented on purpose.

src/frontend/src/handler/create_source.rs

liurenjie1024

LGTM

chenzl25 · 2024-02-21T08:01:43Z

We have so many different names for the same aws concepts, and this list may not be exhaustive ...

Not a blocking issue as we can just support many different aliases. But would like to bring it to people's attention.

We have iceberg sink and I just think iceberg source should have similar field names to the sink.

fuyufjh

Batch select from iceberg source is not implemented in this PR, right? If so, how about change the PR title to feat(frontend): support create iceberg source? 😄

The PR itself LGTM.

src/sqlparser/src/ast/statement.rs

tabVersion

LGTM, thanks for the work.

chenzl25 and others added 10 commits February 1, 2024 14:48

support iceberg scan executor

7888b8f

refactor

5e4a47a

Merge branch 'main' into dylan/support_iceberg_scan_executor

11fdfcd

fmt

8f0681b

fix doc

52de44c

Merge remote-tracking branch 'origin/main' into dylan/support_create_…

898edaa

…iceberg_source

Merge remote-tracking branch 'origin/main' into dylan/support_create_…

f86f8ee

…iceberg_source

Merge remote-tracking branch 'origin/main' into dylan/support_create_…

d3a6609

…iceberg_source

support create iceberg source

b5423ea

Merge remote-tracking branch 'origin/main' into dylan/support_create_…

1cada1c

…iceberg_source

github-actions bot added the type/feature label Feb 4, 2024

chenzl25 requested review from tabVersion, st1page, BugenZhao and liurenjie1024 February 4, 2024 06:21

liurenjie1024 reviewed Feb 5, 2024

View reviewed changes

st1page reviewed Feb 5, 2024

View reviewed changes

chenzl25 added 5 commits February 5, 2024 16:26

introduce none format

b60fe1a

resolve conflict

72d7787

fix

6a61245

fix test

5182d50

fix test

db5dcd4

st1page reviewed Feb 6, 2024

View reviewed changes

tabVersion reviewed Feb 6, 2024

View reviewed changes

chenzl25 added 2 commits February 6, 2024 18:27

improve

aaada21

refactor

ea78079

liurenjie1024 approved these changes Feb 18, 2024

View reviewed changes

fmt

5556ad2

fuyufjh self-requested a review February 21, 2024 02:47

chenzl25 requested review from tabVersion and st1page February 21, 2024 06:42

This comment was marked as off-topic.

Sign in to view

fuyufjh approved these changes Feb 21, 2024

View reviewed changes

src/sqlparser/src/ast/statement.rs Show resolved Hide resolved

chenzl25 changed the title ~~feat(batch,stream): support create iceberg source~~ feat(frontend): support create iceberg source Feb 21, 2024

tabVersion approved these changes Feb 21, 2024

View reviewed changes

chenzl25 enabled auto-merge February 21, 2024 15:47

auto-merge was automatically disabled February 22, 2024 06:09
Merge queue setting changed

chenzl25 enabled auto-merge February 22, 2024 09:22

auto-merge was automatically disabled February 23, 2024 01:45
Merge queue setting changed

chenzl25 added this pull request to the merge queue Feb 23, 2024

Merged via the queue into main with commit 0c329e9 Feb 23, 2024
36 of 38 checks passed

chenzl25 deleted the dylan/support_create_iceberg_source branch February 23, 2024 05:24

chenzl25 mentioned this pull request Feb 23, 2024

Feat: Batch ingest iceberg/file source #14742

Open

21 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(frontend): support create iceberg source #14971

feat(frontend): support create iceberg source #14971

chenzl25 commented Feb 4, 2024 •

edited

Loading

gitguardian bot commented Feb 4, 2024 •

edited

Loading

liurenjie1024 left a comment

liurenjie1024 Feb 5, 2024

chenzl25 Feb 5, 2024

tabVersion Feb 6, 2024

st1page Feb 5, 2024

st1page Feb 5, 2024

liurenjie1024 Feb 5, 2024

chenzl25 Feb 5, 2024

chenzl25 Feb 5, 2024

tabVersion Feb 6, 2024

st1page Feb 6, 2024

st1page left a comment

tabVersion Feb 6, 2024

tabVersion Feb 6, 2024

chenzl25 Feb 6, 2024

liurenjie1024 left a comment

This comment was marked as off-topic.

chenzl25 commented Feb 21, 2024

fuyufjh left a comment •

edited

Loading

tabVersion left a comment

feat(frontend): support create iceberg source #14971

feat(frontend): support create iceberg source #14971

Conversation

chenzl25 commented Feb 4, 2024 • edited Loading

What's changed and what's your intention?

Iceberg source Example

Checklist

Documentation

Release note

gitguardian bot commented Feb 4, 2024 • edited Loading

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

liurenjie1024 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

st1page left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liurenjie1024 left a comment

Choose a reason for hiding this comment

This comment was marked as off-topic.

chenzl25 commented Feb 21, 2024

fuyufjh left a comment • edited Loading

Choose a reason for hiding this comment

tabVersion left a comment

Choose a reason for hiding this comment

chenzl25 commented Feb 4, 2024 •

edited

Loading

gitguardian bot commented Feb 4, 2024 •

edited

Loading

fuyufjh left a comment •

edited

Loading