Skip to content

Enable validation pipeline and update all test expected outputs#5215

Draft
yuancu wants to merge 1 commit intoopensearch-project:feature/validationfrom
yuancu:subpr-c
Draft

Enable validation pipeline and update all test expected outputs#5215
yuancu wants to merge 1 commit intoopensearch-project:feature/validationfrom
yuancu:subpr-c

Conversation

@yuancu
Copy link
Collaborator

@yuancu yuancu commented Mar 9, 2026

Summary

Sub-PR 3/4 for #4892 — splitting the large PR into reviewable pieces targeting feature/validation.

This PR enables the validation pipeline by wiring validate() into QueryService's execute and explain flows, and updates all test expected outputs for the structural and type changes caused by the RelNode ↔ SqlNode round-trip.

What's included

Enablement:

  • QueryServiceexecuteWithCalcite() and explainWithCalcite() now call validate(relNode, context) before convertToCalcitePlan()

Test Expected Output Updates (~546 YAML files):

  • All explain plan YAML files updated for structural changes (field reordering, projection restructuring, sort index changes) caused by the validation round-trip
  • JSON → YAML migration for several explain tests (~78 JSON files removed, replaced by YAML equivalents)

Integration Test Updates:

  • CalciteExplainIT, CalcitePPLExplainIT, ExplainITassertJsonEqualsassertYamlEqualsIgnoreId
  • CalcitePPLBasicITtestBetweenWithMixedTypes (was error, now works with implicit cast)
  • CalciteWhereCommandITtestInWithMixedType (was error, now works)
  • CalciteMultisearchCommandIT / CalcitePPLAppendCommandIT — Schema type changes (string → timestamp for UDT fields)
  • CalcitePPLAggregationIT — Exception type change
  • CalcitePPLPatternsIT@Ignore for tests pending [FEATURE] Enhance patterns command output structure for easy type validation #4968
  • CalciteBinCommandIT, CalciteNoMvCommandIT — Error message updates
  • PPLClickBenchIT — ClickBench plan updates + q41 special handling
  • WhereCommandIT — Removed incompatible type test (moved to Calcite-specific test)
  • DateTimeFunctionIT — Interval semantics fix
  • MatcherUtils — New assertYamlEqualsIgnoreId(List, String) overload

PPL Unit Test Updates:

  • Plan structure updates across CalcitePPLAggregationTest, CalcitePPLChartTest, CalcitePPLJoinTest, CalcitePPLStreamstatsTest, CalcitePPLTimechartTest, CalcitePPLTransposeTest, etc.

Reviewer Note

~546 of 583 files are mechanical YAML expected output updates. If CI passes, those are correct. Focus review on the ~37 source/test files.

Dependency

Signed-off-by: Yuanchun Shen yuanchu@amazon.com

@github-actions
Copy link
Contributor

github-actions bot commented Mar 9, 2026

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit ce09da4.

'Diff too large, requires skip by maintainers after manual review'


Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.


⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 9, 2026

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit a494a06.

'Diff too large, requires skip by maintainers after manual review'


Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.


⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.

This PR enables the validation pipeline and updates all expected test
outputs to match the validated query plans:

- Enable PplTypeCoercion and PplValidator in the Calcite pipeline
- Update ~500 expected output files for explain tests across calcite/,
  calcite_no_pushdown/, and ppl/ directories
- Fix CalciteBinCommandIT and CalcitePPLBasicIT test assertions
- Update yamlRestTest expected outputs
- Includes validation infrastructure (sub-PR A) and operand type
  checkers (sub-PR B) as prerequisites

Signed-off-by: Yuanchun Shen <yuanchu@amazon.com>
@github-actions
Copy link
Contributor

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit c9fc356.

'Diff too large, requires skip by maintainers after manual review'


Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.


⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.

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.

1 participant