Skip to content
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.

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

changefeedccl: support the resolved option with format=parquet #104283

Merged
merged 1 commit into from
Jun 5, 2023

Conversation

jayshrivastava
Copy link
Contributor

@jayshrivastava jayshrivastava commented Jun 2, 2023

Previously, format=parquet and resolved could not be used
together when running changefeeds. This change adds support for
this.

The release note is being left intentionally blank for a future
commit.

Informs: #103129
Release note: None

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@jayshrivastava jayshrivastava force-pushed the parquet-resolved branch 5 times, most recently from bc6ff2d to 1ac4580 Compare June 5, 2023 15:55
@jayshrivastava jayshrivastava changed the title changefeedccl: support the resolved option with format=parquet changefeedccl: support the resolved option with format=parquet Jun 5, 2023
@jayshrivastava jayshrivastava marked this pull request as ready for review June 5, 2023 16:45
@jayshrivastava jayshrivastava requested a review from a team as a code owner June 5, 2023 16:45
@jayshrivastava jayshrivastava requested review from samiskin and removed request for a team June 5, 2023 16:45
Copy link
Contributor

@miretskiy miretskiy left a comment

Choose a reason for hiding this comment

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

I think we should have a short rel note.

Reviewed 9 of 9 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @jayshrivastava and @samiskin)


pkg/ccl/changefeedccl/parquet_sink_cloudstorage.go line 149 at r1 (raw file):

	var writer *parquet.Writer

	writer, err = newParquetWriter(sch, &buf, parquetSink.wrapped.testingKnobs)

can we check -- when we construct this sink -- if resolved option is set, and if so, construct writer once?


pkg/ccl/changefeedccl/parquet_test.go line 176 at r1 (raw file):

		sqlDB.Exec(t, `CREATE TABLE foo (a INT PRIMARY KEY)`)

		foo := feed(t, f, `CREATE CHANGEFEED FOR foo WITH format=parquet, resolved`)

you probably want to specify resolved= some small value... otherwise, this test would run for 30 seconds or so?

Previously, `format=parquet` and `resolved` could not be used
together when running changefeeds. This change adds support for
this.

The release note is being left intentionally blank for a future
commit.

Informs: cockroachdb#103129
Release note: None
@jayshrivastava jayshrivastava force-pushed the parquet-resolved branch 2 times, most recently from 8720056 to 07101ad Compare June 5, 2023 19:57
@jayshrivastava
Copy link
Contributor Author

bors r=miretskiy

@jayshrivastava jayshrivastava mentioned this pull request Jun 5, 2023
13 tasks
@craig
Copy link
Contributor

craig bot commented Jun 5, 2023

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Jun 5, 2023

Build succeeded:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants