Fix #620: Use only_semantic=True in NewSemanticExceptionTest#621
Merged
javihern98 merged 1 commit intomainfrom Mar 23, 2026
Merged
Fix #620: Use only_semantic=True in NewSemanticExceptionTest#621javihern98 merged 1 commit intomainfrom
javihern98 merged 1 commit intomainfrom
Conversation
- Add only_semantic=not is_runtime_error to NewSemanticExceptionTest so semantic errors (codes not starting with "2") are validated without data execution - Remove deprecated SemanticExceptionTest (no active call sites) - Fix visit_VarID: move join ambiguity resolution outside data guard - Fix visit_HRBinOp: handle filter_comp.data is None in semantic mode - Update test_Fail_GL_67 expected error to 1-1-6-10 Closes #620
mla2001
approved these changes
Mar 23, 2026
Contributor
mla2001
left a comment
mla2001
left a comment
There was a problem hiding this comment.
NewSemanticException Test now looks way simpler 😊
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
NewSemanticExceptionTestnow runs withonly_semantic=Truefor semantic errors (codes not starting with"2"), ensuring semantic checks are validated without relying on data execution side-effectsSemanticExceptionTestmethod (only 2 commented-out call sites remained)visit_VarIDjoin component ambiguity resolution that was gated behind adata is not Nonecheck, preventing it from running in semantic-only modevisit_HRBinOpWHEN handler to gracefully handlefilter_comp.data is Nonein semantic-only modetest_Fail_GL_67expected error from1-1-1-10to1-1-6-10(the correct semantic error)Why
Several semantic error tests were passing for the wrong reason — data-dependent checks in
evaluate()methods would fire before the actual semantic validation, masking the real errors. Running withonly_semantic=Trueexposes the true semantic analysis path and ensures error codes are accurate.Impact/Risk
Low risk. The interpreter logic changes are minimal (reordering existing checks, adding a null guard). The test helper change affects all
NewSemanticExceptionTestcallers but all 4057 tests pass.Notes
This PR is a prerequisite for the DuckDB test routing branch (
cr-route-all-tests-through-run), which depends on these fixes.Closes #620