Enable validation pipeline and update all test expected outputs - #5215

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

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

Conversation

@yuancu

@yuancuyuancu commented Mar 9, 2026

Copy link
Copy Markdown
Collaborator

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:

  • QueryService — executeWithCalcite() 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, ExplainIT — assertJsonEquals → assertYamlEqualsIgnoreId
  • CalcitePPLBasicIT — testBetweenWithMixedTypes (was error, now works with implicit cast)
  • CalciteWhereCommandIT — testInWithMixedType (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

github-actionsBot commented Mar 9, 2026

Copy link
Copy Markdown
Contributor

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.

This was referenced Mar 9, 2026
Draft
Open
@yuancu
yuancu marked this pull request as draft March 9, 2026 02:47
@github-actions

github-actionsBot commented Mar 9, 2026

Copy link
Copy Markdown
Contributor

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.

@yuancu
Enable validation pipeline and update all test expected outputs
c9fc356
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

github-actionsBot commented Mar 10, 2026

Copy link
Copy Markdown
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 freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ps48ps48Awaiting requested review from ps48ps48 is a code owner
@kavithacmkavithacmAwaiting requested review from kavithacm
@derek-hoderek-hoAwaiting requested review from derek-ho
@joshuali925joshuali925Awaiting requested review from joshuali925joshuali925 is a code owner
@dai-chendai-chenAwaiting requested review from dai-chendai-chen is a code owner
@YANG-DBYANG-DBAwaiting requested review from YANG-DB
@mengweiericmengweiericAwaiting requested review from mengweiericmengweieric is a code owner
@vamsimanoharvamsimanoharAwaiting requested review from vamsimanoharvamsimanohar is a code owner
@SwiddisSwiddisAwaiting requested review from SwiddisSwiddis is a code owner
@penghuopenghuoAwaiting requested review from penghuopenghuo is a code owner
@seankao-azseankao-azAwaiting requested review from seankao-az
@MaxKsyunzMaxKsyunzAwaiting requested review from MaxKsyunz
@Yury-FridlyandYury-FridlyandAwaiting requested review from Yury-Fridlyand
@anirudhaanirudhaAwaiting requested review from anirudhaanirudha is a code owner
@forestmveyforestmveyAwaiting requested review from forestmvey
@acarbonettoacarbonettoAwaiting requested review from acarbonettoacarbonetto is a code owner
@GumpacGGumpacGAwaiting requested review from GumpacG
@ykmr1224ykmr1224Awaiting requested review from ykmr1224ykmr1224 is a code owner
@LantaoJinLantaoJinAwaiting requested review from LantaoJinLantaoJin is a code owner
@noChargernoChargerAwaiting requested review from noChargernoCharger is a code owner
@qianheng-awsqianheng-awsAwaiting requested review from qianheng-awsqianheng-aws is a code owner
@RyanL1997RyanL1997Awaiting requested review from RyanL1997RyanL1997 is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@yuancu
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
Enable validation pipeline and update all test expected outputs - #5215
Draft
yuancu wants to merge 1 commit into
opensearch-project:feature/validationfrom 
yuancu:subpr-c
Draft
Enable validation pipeline and update all test expected outputs#5215
yuancu wants to merge 1 commit into
opensearch-project:feature/validationfrom 
yuancu:subpr-c
Conversation
@yuancu
@yuancuyuancu
commented
Mar 9, 2026

Copy link


Copy Markdown

Collaborator
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:
  • QueryService — executeWithCalcite() 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, ExplainIT — assertJsonEquals → assertYamlEqualsIgnoreId
  • CalcitePPLBasicIT — testBetweenWithMixedTypes (was error, now works with implicit cast)
  • CalciteWhereCommandIT — testInWithMixedType (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
github-actionsBot
commented
Mar 9, 2026

Copy link


Copy Markdown

Contributor
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.

This was referenced Mar 9, 2026
 Draft

 Open

@yuancu
yuancu
marked this pull request as draft
March 9, 2026 02:47
@github-actions
github-actionsBot
commented
Mar 9, 2026

Copy link


Copy Markdown

Contributor
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.
@yuancu
Enable validation pipeline and update all test expected outputs
c9fc356
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
github-actionsBot
commented
Mar 10, 2026

Copy link


Copy Markdown

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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@ps48ps48Awaiting requested review from ps48ps48 is a code owner
@kavithacmkavithacmAwaiting requested review from kavithacm
@derek-hoderek-hoAwaiting requested review from derek-ho
@joshuali925joshuali925Awaiting requested review from joshuali925joshuali925 is a code owner
@dai-chendai-chenAwaiting requested review from dai-chendai-chen is a code owner
@YANG-DBYANG-DBAwaiting requested review from YANG-DB
@mengweiericmengweiericAwaiting requested review from mengweiericmengweieric is a code owner
@vamsimanoharvamsimanoharAwaiting requested review from vamsimanoharvamsimanohar is a code owner
@SwiddisSwiddisAwaiting requested review from SwiddisSwiddis is a code owner
@penghuopenghuoAwaiting requested review from penghuopenghuo is a code owner
@seankao-azseankao-azAwaiting requested review from seankao-az
@MaxKsyunzMaxKsyunzAwaiting requested review from MaxKsyunz
@Yury-FridlyandYury-FridlyandAwaiting requested review from Yury-Fridlyand
@anirudhaanirudhaAwaiting requested review from anirudhaanirudha is a code owner
@forestmveyforestmveyAwaiting requested review from forestmvey
@acarbonettoacarbonettoAwaiting requested review from acarbonettoacarbonetto is a code owner
@GumpacGGumpacGAwaiting requested review from GumpacG
@ykmr1224ykmr1224Awaiting requested review from ykmr1224ykmr1224 is a code owner
@LantaoJinLantaoJinAwaiting requested review from LantaoJinLantaoJin is a code owner
@noChargernoChargerAwaiting requested review from noChargernoCharger is a code owner
@qianheng-awsqianheng-awsAwaiting requested review from qianheng-awsqianheng-aws is a code owner
@RyanL1997RyanL1997Awaiting requested review from RyanL1997RyanL1997 is a code owner

At least 1 approving review is required to merge this pull request.


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

1 participant

@yuancu

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Enable validation pipeline and update all test expected outputs - #5215
Draft
yuancu wants to merge 1 commit into
opensearch-project:feature/validationfrom 
yuancu:subpr-c
Draft
Enable validation pipeline and update all test expected outputs#5215
yuancu wants to merge 1 commit into
opensearch-project:feature/validationfrom 
yuancu:subpr-c
Conversation
@yuancu
@yuancuyuancu
commented
Mar 9, 2026

Copy link


Copy Markdown

Collaborator
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:
  • QueryService — executeWithCalcite() 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, ExplainIT — assertJsonEquals → assertYamlEqualsIgnoreId
  • CalcitePPLBasicIT — testBetweenWithMixedTypes (was error, now works with implicit cast)
  • CalciteWhereCommandIT — testInWithMixedType (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
github-actionsBot
commented
Mar 9, 2026

Copy link


Copy Markdown

Contributor
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.

This was referenced Mar 9, 2026
 Draft

 Open

@yuancu
yuancu
marked this pull request as draft
March 9, 2026 02:47
@github-actions
github-actionsBot
commented
Mar 9, 2026

Copy link


Copy Markdown

Contributor
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.
@yuancu
Enable validation pipeline and update all test expected outputs
c9fc356
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
github-actionsBot
commented
Mar 10, 2026

Copy link


Copy Markdown

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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@ps48ps48Awaiting requested review from ps48ps48 is a code owner
@kavithacmkavithacmAwaiting requested review from kavithacm
@derek-hoderek-hoAwaiting requested review from derek-ho
@joshuali925joshuali925Awaiting requested review from joshuali925joshuali925 is a code owner
@dai-chendai-chenAwaiting requested review from dai-chendai-chen is a code owner
@YANG-DBYANG-DBAwaiting requested review from YANG-DB
@mengweiericmengweiericAwaiting requested review from mengweiericmengweieric is a code owner
@vamsimanoharvamsimanoharAwaiting requested review from vamsimanoharvamsimanohar is a code owner
@SwiddisSwiddisAwaiting requested review from SwiddisSwiddis is a code owner
@penghuopenghuoAwaiting requested review from penghuopenghuo is a code owner
@seankao-azseankao-azAwaiting requested review from seankao-az
@MaxKsyunzMaxKsyunzAwaiting requested review from MaxKsyunz
@Yury-FridlyandYury-FridlyandAwaiting requested review from Yury-Fridlyand
@anirudhaanirudhaAwaiting requested review from anirudhaanirudha is a code owner
@forestmveyforestmveyAwaiting requested review from forestmvey
@acarbonettoacarbonettoAwaiting requested review from acarbonettoacarbonetto is a code owner
@GumpacGGumpacGAwaiting requested review from GumpacG
@ykmr1224ykmr1224Awaiting requested review from ykmr1224ykmr1224 is a code owner
@LantaoJinLantaoJinAwaiting requested review from LantaoJinLantaoJin is a code owner
@noChargernoChargerAwaiting requested review from noChargernoCharger is a code owner
@qianheng-awsqianheng-awsAwaiting requested review from qianheng-awsqianheng-aws is a code owner
@RyanL1997RyanL1997Awaiting requested review from RyanL1997RyanL1997 is a code owner

At least 1 approving review is required to merge this pull request.


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

1 participant

@yuancu

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Enable validation pipeline and update all test expected outputs - #5215
Draft
yuancu wants to merge 1 commit into
opensearch-project:feature/validationfrom 
yuancu:subpr-c
Draft
Enable validation pipeline and update all test expected outputs#5215
yuancu wants to merge 1 commit into
opensearch-project:feature/validationfrom 
yuancu:subpr-c
Conversation
@yuancu
@yuancuyuancu
commented
Mar 9, 2026

Copy link


Copy Markdown

Collaborator
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:
  • QueryService — executeWithCalcite() 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, ExplainIT — assertJsonEquals → assertYamlEqualsIgnoreId
  • CalcitePPLBasicIT — testBetweenWithMixedTypes (was error, now works with implicit cast)
  • CalciteWhereCommandIT — testInWithMixedType (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
github-actionsBot
commented
Mar 9, 2026

Copy link


Copy Markdown

Contributor
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.

This was referenced Mar 9, 2026
 Draft

 Open

@yuancu
yuancu
marked this pull request as draft
March 9, 2026 02:47
@github-actions
github-actionsBot
commented
Mar 9, 2026

Copy link


Copy Markdown

Contributor
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.
@yuancu
Enable validation pipeline and update all test expected outputs
c9fc356
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
github-actionsBot
commented
Mar 10, 2026

Copy link


Copy Markdown

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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@ps48ps48Awaiting requested review from ps48ps48 is a code owner
@kavithacmkavithacmAwaiting requested review from kavithacm
@derek-hoderek-hoAwaiting requested review from derek-ho
@joshuali925joshuali925Awaiting requested review from joshuali925joshuali925 is a code owner
@dai-chendai-chenAwaiting requested review from dai-chendai-chen is a code owner
@YANG-DBYANG-DBAwaiting requested review from YANG-DB
@mengweiericmengweiericAwaiting requested review from mengweiericmengweieric is a code owner
@vamsimanoharvamsimanoharAwaiting requested review from vamsimanoharvamsimanohar is a code owner
@SwiddisSwiddisAwaiting requested review from SwiddisSwiddis is a code owner
@penghuopenghuoAwaiting requested review from penghuopenghuo is a code owner
@seankao-azseankao-azAwaiting requested review from seankao-az
@MaxKsyunzMaxKsyunzAwaiting requested review from MaxKsyunz
@Yury-FridlyandYury-FridlyandAwaiting requested review from Yury-Fridlyand
@anirudhaanirudhaAwaiting requested review from anirudhaanirudha is a code owner
@forestmveyforestmveyAwaiting requested review from forestmvey
@acarbonettoacarbonettoAwaiting requested review from acarbonettoacarbonetto is a code owner
@GumpacGGumpacGAwaiting requested review from GumpacG
@ykmr1224ykmr1224Awaiting requested review from ykmr1224ykmr1224 is a code owner
@LantaoJinLantaoJinAwaiting requested review from LantaoJinLantaoJin is a code owner
@noChargernoChargerAwaiting requested review from noChargernoCharger is a code owner
@qianheng-awsqianheng-awsAwaiting requested review from qianheng-awsqianheng-aws is a code owner
@RyanL1997RyanL1997Awaiting requested review from RyanL1997RyanL1997 is a code owner

At least 1 approving review is required to merge this pull request.


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

1 participant

@yuancu

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
Enable validation pipeline and update all test expected outputs - #5215
Draft
yuancu wants to merge 1 commit into
opensearch-project:feature/validationfrom 
yuancu:subpr-c
Draft
Enable validation pipeline and update all test expected outputs#5215
yuancu wants to merge 1 commit into
opensearch-project:feature/validationfrom 
yuancu:subpr-c
Conversation
@yuancu
@yuancuyuancu
commented
Mar 9, 2026

Copy link


Copy Markdown

Collaborator
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:
  • QueryService — executeWithCalcite() 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, ExplainIT — assertJsonEquals → assertYamlEqualsIgnoreId
  • CalcitePPLBasicIT — testBetweenWithMixedTypes (was error, now works with implicit cast)
  • CalciteWhereCommandIT — testInWithMixedType (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
github-actionsBot
commented
Mar 9, 2026

Copy link


Copy Markdown

Contributor
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.

This was referenced Mar 9, 2026
 Draft

 Open

@yuancu
yuancu
marked this pull request as draft
March 9, 2026 02:47
@github-actions
github-actionsBot
commented
Mar 9, 2026

Copy link


Copy Markdown

Contributor
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.
@yuancu
Enable validation pipeline and update all test expected outputs
c9fc356
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
github-actionsBot
commented
Mar 10, 2026

Copy link


Copy Markdown

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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@ps48ps48Awaiting requested review from ps48ps48 is a code owner
@kavithacmkavithacmAwaiting requested review from kavithacm
@derek-hoderek-hoAwaiting requested review from derek-ho
@joshuali925joshuali925Awaiting requested review from joshuali925joshuali925 is a code owner
@dai-chendai-chenAwaiting requested review from dai-chendai-chen is a code owner
@YANG-DBYANG-DBAwaiting requested review from YANG-DB
@mengweiericmengweiericAwaiting requested review from mengweiericmengweieric is a code owner
@vamsimanoharvamsimanoharAwaiting requested review from vamsimanoharvamsimanohar is a code owner
@SwiddisSwiddisAwaiting requested review from SwiddisSwiddis is a code owner
@penghuopenghuoAwaiting requested review from penghuopenghuo is a code owner
@seankao-azseankao-azAwaiting requested review from seankao-az
@MaxKsyunzMaxKsyunzAwaiting requested review from MaxKsyunz
@Yury-FridlyandYury-FridlyandAwaiting requested review from Yury-Fridlyand
@anirudhaanirudhaAwaiting requested review from anirudhaanirudha is a code owner
@forestmveyforestmveyAwaiting requested review from forestmvey
@acarbonettoacarbonettoAwaiting requested review from acarbonettoacarbonetto is a code owner
@GumpacGGumpacGAwaiting requested review from GumpacG
@ykmr1224ykmr1224Awaiting requested review from ykmr1224ykmr1224 is a code owner
@LantaoJinLantaoJinAwaiting requested review from LantaoJinLantaoJin is a code owner
@noChargernoChargerAwaiting requested review from noChargernoCharger is a code owner
@qianheng-awsqianheng-awsAwaiting requested review from qianheng-awsqianheng-aws is a code owner
@RyanL1997RyanL1997Awaiting requested review from RyanL1997RyanL1997 is a code owner

At least 1 approving review is required to merge this pull request.


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

1 participant

@yuancu

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Enable validation pipeline and update all test expected outputs - #5215
Draft
yuancu wants to merge 1 commit into
opensearch-project:feature/validationfrom 
yuancu:subpr-c
Draft
Enable validation pipeline and update all test expected outputs#5215
yuancu wants to merge 1 commit into
opensearch-project:feature/validationfrom 
yuancu:subpr-c
Conversation
@yuancu
@yuancuyuancu
commented
Mar 9, 2026

Copy link


Copy Markdown

Collaborator
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:
  • QueryService — executeWithCalcite() 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, ExplainIT — assertJsonEquals → assertYamlEqualsIgnoreId
  • CalcitePPLBasicIT — testBetweenWithMixedTypes (was error, now works with implicit cast)
  • CalciteWhereCommandIT — testInWithMixedType (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
github-actionsBot
commented
Mar 9, 2026

Copy link


Copy Markdown

Contributor
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.

This was referenced Mar 9, 2026
 Draft

 Open

@yuancu
yuancu
marked this pull request as draft
March 9, 2026 02:47
@github-actions
github-actionsBot
commented
Mar 9, 2026

Copy link


Copy Markdown

Contributor
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.
@yuancu
Enable validation pipeline and update all test expected outputs
c9fc356
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
github-actionsBot
commented
Mar 10, 2026

Copy link


Copy Markdown

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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@ps48ps48Awaiting requested review from ps48ps48 is a code owner
@kavithacmkavithacmAwaiting requested review from kavithacm
@derek-hoderek-hoAwaiting requested review from derek-ho
@joshuali925joshuali925Awaiting requested review from joshuali925joshuali925 is a code owner
@dai-chendai-chenAwaiting requested review from dai-chendai-chen is a code owner
@YANG-DBYANG-DBAwaiting requested review from YANG-DB
@mengweiericmengweiericAwaiting requested review from mengweiericmengweieric is a code owner
@vamsimanoharvamsimanoharAwaiting requested review from vamsimanoharvamsimanohar is a code owner
@SwiddisSwiddisAwaiting requested review from SwiddisSwiddis is a code owner
@penghuopenghuoAwaiting requested review from penghuopenghuo is a code owner
@seankao-azseankao-azAwaiting requested review from seankao-az
@MaxKsyunzMaxKsyunzAwaiting requested review from MaxKsyunz
@Yury-FridlyandYury-FridlyandAwaiting requested review from Yury-Fridlyand
@anirudhaanirudhaAwaiting requested review from anirudhaanirudha is a code owner
@forestmveyforestmveyAwaiting requested review from forestmvey
@acarbonettoacarbonettoAwaiting requested review from acarbonettoacarbonetto is a code owner
@GumpacGGumpacGAwaiting requested review from GumpacG
@ykmr1224ykmr1224Awaiting requested review from ykmr1224ykmr1224 is a code owner
@LantaoJinLantaoJinAwaiting requested review from LantaoJinLantaoJin is a code owner
@noChargernoChargerAwaiting requested review from noChargernoCharger is a code owner
@qianheng-awsqianheng-awsAwaiting requested review from qianheng-awsqianheng-aws is a code owner
@RyanL1997RyanL1997Awaiting requested review from RyanL1997RyanL1997 is a code owner

At least 1 approving review is required to merge this pull request.


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

1 participant

@yuancu

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Enable validation pipeline and update all test expected outputs - #5215
Draft
yuancu wants to merge 1 commit into
opensearch-project:feature/validationfrom 
yuancu:subpr-c
Draft
Enable validation pipeline and update all test expected outputs#5215
yuancu wants to merge 1 commit into
opensearch-project:feature/validationfrom 
yuancu:subpr-c
Conversation
@yuancu
@yuancuyuancu
commented
Mar 9, 2026

Copy link


Copy Markdown

Collaborator
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:
  • QueryService — executeWithCalcite() 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, ExplainIT — assertJsonEquals → assertYamlEqualsIgnoreId
  • CalcitePPLBasicIT — testBetweenWithMixedTypes (was error, now works with implicit cast)
  • CalciteWhereCommandIT — testInWithMixedType (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
github-actionsBot
commented
Mar 9, 2026

Copy link


Copy Markdown

Contributor
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.

This was referenced Mar 9, 2026
 Draft

 Open

@yuancu
yuancu
marked this pull request as draft
March 9, 2026 02:47
@github-actions
github-actionsBot
commented
Mar 9, 2026

Copy link


Copy Markdown

Contributor
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.
@yuancu
Enable validation pipeline and update all test expected outputs
c9fc356
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
github-actionsBot
commented
Mar 10, 2026

Copy link


Copy Markdown

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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@ps48ps48Awaiting requested review from ps48ps48 is a code owner
@kavithacmkavithacmAwaiting requested review from kavithacm
@derek-hoderek-hoAwaiting requested review from derek-ho
@joshuali925joshuali925Awaiting requested review from joshuali925joshuali925 is a code owner
@dai-chendai-chenAwaiting requested review from dai-chendai-chen is a code owner
@YANG-DBYANG-DBAwaiting requested review from YANG-DB
@mengweiericmengweiericAwaiting requested review from mengweiericmengweieric is a code owner
@vamsimanoharvamsimanoharAwaiting requested review from vamsimanoharvamsimanohar is a code owner
@SwiddisSwiddisAwaiting requested review from SwiddisSwiddis is a code owner
@penghuopenghuoAwaiting requested review from penghuopenghuo is a code owner
@seankao-azseankao-azAwaiting requested review from seankao-az
@MaxKsyunzMaxKsyunzAwaiting requested review from MaxKsyunz
@Yury-FridlyandYury-FridlyandAwaiting requested review from Yury-Fridlyand
@anirudhaanirudhaAwaiting requested review from anirudhaanirudha is a code owner
@forestmveyforestmveyAwaiting requested review from forestmvey
@acarbonettoacarbonettoAwaiting requested review from acarbonettoacarbonetto is a code owner
@GumpacGGumpacGAwaiting requested review from GumpacG
@ykmr1224ykmr1224Awaiting requested review from ykmr1224ykmr1224 is a code owner
@LantaoJinLantaoJinAwaiting requested review from LantaoJinLantaoJin is a code owner
@noChargernoChargerAwaiting requested review from noChargernoCharger is a code owner
@qianheng-awsqianheng-awsAwaiting requested review from qianheng-awsqianheng-aws is a code owner
@RyanL1997RyanL1997Awaiting requested review from RyanL1997RyanL1997 is a code owner

At least 1 approving review is required to merge this pull request.


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

1 participant

@yuancu

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
Enable validation pipeline and update all test expected outputs - #5215
Draft
yuancu wants to merge 1 commit into
opensearch-project:feature/validationfrom 
yuancu:subpr-c
Draft
Enable validation pipeline and update all test expected outputs#5215
yuancu wants to merge 1 commit into
opensearch-project:feature/validationfrom 
yuancu:subpr-c
Conversation
@yuancu
@yuancuyuancu
commented
Mar 9, 2026

Copy link


Copy Markdown

Collaborator
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:
  • QueryService — executeWithCalcite() 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, ExplainIT — assertJsonEquals → assertYamlEqualsIgnoreId
  • CalcitePPLBasicIT — testBetweenWithMixedTypes (was error, now works with implicit cast)
  • CalciteWhereCommandIT — testInWithMixedType (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
github-actionsBot
commented
Mar 9, 2026

Copy link


Copy Markdown

Contributor
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.

This was referenced Mar 9, 2026
 Draft

 Open

@yuancu
yuancu
marked this pull request as draft
March 9, 2026 02:47
@github-actions
github-actionsBot
commented
Mar 9, 2026

Copy link


Copy Markdown

Contributor
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.
@yuancu
Enable validation pipeline and update all test expected outputs
c9fc356
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
github-actionsBot
commented
Mar 10, 2026

Copy link


Copy Markdown

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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@ps48ps48Awaiting requested review from ps48ps48 is a code owner
@kavithacmkavithacmAwaiting requested review from kavithacm
@derek-hoderek-hoAwaiting requested review from derek-ho
@joshuali925joshuali925Awaiting requested review from joshuali925joshuali925 is a code owner
@dai-chendai-chenAwaiting requested review from dai-chendai-chen is a code owner
@YANG-DBYANG-DBAwaiting requested review from YANG-DB
@mengweiericmengweiericAwaiting requested review from mengweiericmengweieric is a code owner
@vamsimanoharvamsimanoharAwaiting requested review from vamsimanoharvamsimanohar is a code owner
@SwiddisSwiddisAwaiting requested review from SwiddisSwiddis is a code owner
@penghuopenghuoAwaiting requested review from penghuopenghuo is a code owner
@seankao-azseankao-azAwaiting requested review from seankao-az
@MaxKsyunzMaxKsyunzAwaiting requested review from MaxKsyunz
@Yury-FridlyandYury-FridlyandAwaiting requested review from Yury-Fridlyand
@anirudhaanirudhaAwaiting requested review from anirudhaanirudha is a code owner
@forestmveyforestmveyAwaiting requested review from forestmvey
@acarbonettoacarbonettoAwaiting requested review from acarbonettoacarbonetto is a code owner
@GumpacGGumpacGAwaiting requested review from GumpacG
@ykmr1224ykmr1224Awaiting requested review from ykmr1224ykmr1224 is a code owner
@LantaoJinLantaoJinAwaiting requested review from LantaoJinLantaoJin is a code owner
@noChargernoChargerAwaiting requested review from noChargernoCharger is a code owner
@qianheng-awsqianheng-awsAwaiting requested review from qianheng-awsqianheng-aws is a code owner
@RyanL1997RyanL1997Awaiting requested review from RyanL1997RyanL1997 is a code owner

At least 1 approving review is required to merge this pull request.


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

1 participant

@yuancu