feat: validate input references are consistent with the input schema

[xxchan]

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

What's changed and what's your intention?

So that bugs like #7158 can be found earlier in frontend instead of backend.

[codecov]

Codecov Report

Merging #7193 (487a5dd) into main (af29351) will increase coverage by 0.01%.
The diff coverage is 84.93%.

@@            Coverage Diff             @@
##             main    #7193      +/-   ##
==========================================
+ Coverage   73.12%   73.14%   +0.01%     
==========================================
  Files        1055     1056       +1     
  Lines      168949   169022      +73     
==========================================
+ Hits       123545   123625      +80     
+ Misses      45404    45397       -7     

Flag	Coverage Δ
rust	73.14% <84.93%> (+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_visitor/mod.rs	93.02% <ø> (+2.32%)	⬆️
.../src/optimizer/plan_visitor/input_ref_validator.rs	81.96% <81.96%> (ø)
src/frontend/src/optimizer/mod.rs	96.49% <100.00%> (+0.04%)	⬆️
.../frontend/src/optimizer/plan_node/batch_project.rs	97.77% <100.00%> (+0.15%)	⬆️
...frontend/src/optimizer/plan_node/stream_project.rs	91.11% <100.00%> (+0.63%)	⬆️
src/meta/src/manager/cluster.rs	76.86% <0.00%> (-0.25%)	⬇️
src/common/src/cache.rs	97.31% <0.00%> (-0.23%)	⬇️
src/stream/src/executor/aggregation/minput.rs	96.49% <0.00%> (+0.10%)	⬆️
src/storage/src/hummock/compactor/mod.rs	83.41% <0.00%> (+0.15%)	⬆️
src/common/src/types/ordered_float.rs	31.25% <0.00%> (+0.19%)	⬆️
... and 3 more

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

[chenzl25]

LGTM. Should we only enable these checks in debug mode?

[xxchan]

Should we only enable these checks in debug mode?

I was also thinking about this problem and was not sure whether to add cfg!(debug_assertions). Are you mainly worried about performance or safety or other things?

IMHO:

• Overhead is acceptable.
• Violation of the assertion causes bugs. And frontend panic is better than backend panic.

BTW there are already many other assertions and we havn't systematically considered how to use assertions 🤔

[chenzl25]

IMHO

My concern is about performance, because we need to visit the entire plan exprs several times. Most of our CI tests run in debug mode and once some bugs are found we can still catch them.

[xxchan]

My concern is about performance, because we need to visit the entire plan exprs several times.

If so do we also change these to debug_assert 🫣:

        assert!(*plan.distribution() == Distribution::Single, "{}", plan);
        assert!(!has_batch_exchange(plan.clone()), "{}", plan);

Oh the first one should be cheap. The validation in this PR visits all expressions and thus is also more expensive than the second one. 🤔

Yes it's expensive. Let's ban it