GitHub user dosubot[bot] added a comment to the discussion: Refactor Pinot sqlglot dialect tests to use AST comparison instead of sub-string matching
<!-- Greeting --> Hi @0eu! I'm [Dosu](https://go.dosu.dev/dosubot) and I’m helping the superset team. <!-- Answer --> Your proposal to refactor Pinot dialect tests to use AST comparison is a solid improvement. Current Pinot and other dialect tests in Superset rely on string-based assertions, which are brittle and sensitive to formatting or case changes, and there’s no established guidance or prior art for AST-based assertions in the codebase yet ([example](https://github.com/apache/superset/pull/35424), [example](https://github.com/apache/superset/blob/master/tests/unit_tests/sql/dialects/firebolt_tests.py)). AST comparison will make the tests more robust and focused on semantic correctness. As you implement this, watch for dialect-specific constructs or sqlglot parsing limitations that might affect AST equivalence, and document any edge cases you find. In your PR, consider including examples where AST comparison clearly improves reliability over string matching, and explain the rationale for reviewers. This will help establish a strong precedent for future dialect test improvements. If you run into tricky cases or want a second opinion on specific test conversions, feel free to ask! <!-- Dosu Comment Footer --> *To reply, just mention [@dosu](https://go.dosu.dev/dosubot).* --- <sup>How did I do? [Good](https://app.dosu.dev/response-feedback/ab2eb21b-ac73-474c-9bb2-9e9e0b439feb?feedback_type=great_response) | [Irrelevant](https://app.dosu.dev/response-feedback/ab2eb21b-ac73-474c-9bb2-9e9e0b439feb?feedback_type=irrelevant_answer) | [Incorrect](https://app.dosu.dev/response-feedback/ab2eb21b-ac73-474c-9bb2-9e9e0b439feb?feedback_type=incorrect_sources) | [Verbose](https://app.dosu.dev/response-feedback/ab2eb21b-ac73-474c-9bb2-9e9e0b439feb?feedback_type=too_verbose) | [Hallucination](https://app.dosu.dev/response-feedback/ab2eb21b-ac73-474c-9bb2-9e9e0b439feb?feedback_type=hallucination) | [Report 🐛](https://app.dosu.dev/response-feedback/ab2eb21b-ac73-474c-9bb2-9e9e0b439feb?feedback_type=bug_report) | [Other](https://app.dosu.dev/response-feedback/ab2eb21b-ac73-474c-9bb2-9e9e0b439feb?feedback_type=other)</sup> [](https://app.dosu.dev/a28d3c7e-a9d3-459e-9fb6-3a6f9ff4f357/ask?utm_source=github) [](https://cursor.com/link/prompt?text=Your%20proposal%20to%20refactor%20Pinot%20dialect%20tests%20to%20use%20AST%20comparison%20is%20a%20solid%20improvement.%20Current%20Pinot%20and%20other%20dialect%20tests%20in%20Superset%20rely%20on%20string-based%20assertions%2C%20which%20are%20brittle%20and%20sensitive%20to%20formatting%20or%20case%20changes%2C%20and%20there%E2%80%99s%20no%20established%20guidance%20or%20prior%20art%20for%20AST-based%20assertions%20in%20the%20codebase%20yet%C2%A0%28%5Bexample%5D%28https%3A//github.com/apache/superset/pull/35424%29%2C%20%5Bexample%5D%28https%3A//github.com/apache/superset/blob/master/tests/unit_tests/sql/dialects/firebolt_tests.py%29%29.%20AST%20comparison%20will%20make%20the%20tests%20more%20robust%20and%20focused%20on%20semantic%20correctness.%0A%0A As%20you%20implement%20this%2C%20watch%20for%20dialect-specific%20constructs%20or%20sqlglot%20parsing%20limitations%20that%20might%20affect%20AST%20equivalence%2C%20and%20document%20any%20edge%20cases%20you%20find.%20In%20your%20PR%2C%20consider%20including%20examples%20where%20AST%20comparison%20clearly%20improves%20reliability%20over%20string%20matching%2C%20and%20explain%20the%20rationale%20for%20reviewers.%20This%20will%20help%20establish%20a%20strong%20precedent%20for%20future%20dialect%20test%20improvements.%0A%0AIf%20you%20run%20into%20tricky%20cases%20or%20want%20a%20second%20opinion%20on%20specific%20test%20conversions%2C%20feel%20free%20to%20ask%21) [](https://go.dosu.dev/discord-bot) [](https://twitter.com/intent/tweet?text=%40dosu_ai%20helped%20me%20solve%20this%20issue!&url=https%3A//github.com/apache/superset/di scussions/36833) GitHub link: https://github.com/apache/superset/discussions/36833#discussioncomment-15337792 ---- This is an automatically sent email for [email protected]. To unsubscribe, please send an email to: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
