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

fix(sink): sink exec get params from fe #6922

Merged
merged 6 commits into from
Dec 16, 2022
Merged

fix(sink): sink exec get params from fe #6922

merged 6 commits into from
Dec 16, 2022

Conversation

st1page
Copy link
Contributor

@st1page st1page commented Dec 15, 2022

I hereby agree to the terms of the Singularity Data, Inc. Contributor License Agreement.

What's changed and what's your intention?

In the past, we assume the SinkExecutor's input must be a Materialize executor. SinkExecutor in Compute Node depends on that and uses its upstream input's schema and pk_indices to build the sinkImpl. But after we support creating a sink with query, we can not simply use the upstream executor's schema and pk_indices as the sink's property.
it is just a workaround fix for this release. It will be like #6899 finally.

Checklist

  • I have written necessary rustdoc comments
  • I have added necessary unit tests and integration tests
  • All checks passed in ./risedev check (or alias, ./risedev c)

@st1page st1page marked this pull request as ready for review December 15, 2022 16:04
@github-actions github-actions bot added the type/fix Bug fix label Dec 15, 2022
@codecov
Copy link

codecov bot commented Dec 15, 2022

Codecov Report

Merging #6922 (719c15a) into main (1ae051b) will increase coverage by 0.01%.
The diff coverage is 26.66%.

@@            Coverage Diff             @@
##             main    #6922      +/-   ##
==========================================
+ Coverage   73.15%   73.17%   +0.01%     
==========================================
  Files        1035     1035              
  Lines      165556   165574      +18     
==========================================
+ Hits       121120   121153      +33     
+ Misses      44436    44421      -15     
Flag Coverage Δ
rust 73.17% <26.66%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/frontend/src/optimizer/plan_node/stream.rs 13.96% <0.00%> (-0.16%) ⬇️
src/stream/src/executor/sink.rs 0.00% <0.00%> (ø)
src/stream/src/from_proto/sink.rs 0.00% <0.00%> (ø)
...rc/frontend/src/optimizer/plan_node/stream_sink.rs 80.43% <100.00%> (+6.14%) ⬆️
src/stream/src/executor/aggregation/minput.rs 96.39% <0.00%> (-0.11%) ⬇️
src/batch/src/executor/group_top_n.rs 74.85% <0.00%> (+6.43%) ⬆️
src/common/src/catalog/schema.rs 85.62% <0.00%> (+7.50%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@mergify mergify bot merged commit bf87df6 into main Dec 16, 2022
@mergify mergify bot deleted the sts/fix_sink_schema branch December 16, 2022 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/fix Bug fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants