Skip to content

Add tests for coalesce's error semantics #33110

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 22, 2025

Conversation

ggevay
Copy link
Contributor

@ggevay ggevay commented Jul 22, 2025

This is a follow-up to #33103 and #33109, adding tests that show the current behavior.

Note that these tests are not meant to set the current behavior in stone. They are just there to

  • let us know if our behavior changes (although we might easily accept changes to it, but it's still better to know about them than to not know about them).
  • These tests (together with their comments) are my best guess of what the error semantics specification will mandate for these queries, so writing them down in tests might help in working towards a specification.

(We can incrementally add more tests in this file while we work on error semantics.)

Motivation

Tips for reviewer

Checklist

  • This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
  • If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.

@ggevay ggevay requested review from mgree and frankmcsherry July 22, 2025 14:33
@ggevay ggevay added the T-testing Theme: tests or test infrastructure label Jul 22, 2025
Copy link
Contributor

@frankmcsherry frankmcsherry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, and thank you!

@ggevay ggevay force-pushed the error-semantics-tests branch from 8b3c195 to 1100d3c Compare July 22, 2025 14:36
@ggevay ggevay enabled auto-merge July 22, 2025 14:36
@ggevay ggevay merged commit 4edcd26 into MaterializeInc:main Jul 22, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-testing Theme: tests or test infrastructure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants