feat(frontend): ban jsonb in aggregation stream key #14442
Conversation
src/frontend/src/optimizer/mod.rs
Outdated
| if let Some(err) = StreamKeyChecker.visit(self.plan.clone()) { | ||
| return Err(ErrorCode::NotSupported( | ||
| err, | ||
| "Use a large Jsonb as part of the stream key is unexpected. If you are sure your Jsonb is small, use `set XXX true`".to_string(), | ||
| ).into()); | ||
| } |
There was a problem hiding this comment.
I think we should check the stream key at least after the logical optimization phase because after column pruning, some unnecessary columns check could be skipped.
There was a problem hiding this comment.
The plan nodes after optimization can be different from the plan after binding and cause inaccurate error messages.
The columns checked are group by order by partition by, which is unlikely to be pruned.
There was a problem hiding this comment.
But for union, it seems hard to say? e.g. select a, b, c from ( select * from t1 union all select * from t2)
There was a problem hiding this comment.
SELECT distinct a,b FROM FROM t;
There was a problem hiding this comment.
fixed an issue about union all
There was a problem hiding this comment.
Agg can be created during subquery unnesting. See the sample query here: #7981 (comment)
Btw is join key a concern as well?
There was a problem hiding this comment.
Yes, this or only handles user explicitly write agg queries. We need a fallback to handle other cases.
Yes, join can be a concern.
src/frontend/src/optimizer/plan_visitor/jsonb_stream_key_checker.rs
Outdated
Show resolved
Hide resolved
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
#14314
Checklist
./risedev check(or alias,./risedev c)Documentation
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.