Skip to content

Conversation

@jycor
Copy link
Contributor

@jycor jycor commented Dec 14, 2023

We have places where we call expression.Eval(nil, nil).
The null context causes a panic when we call Span on it.

This PR just adds a nil check inside the *context.Span() receiver

fixes dolthub/dolt#7154

@jycor jycor requested a review from max-hoffman December 14, 2023 10:02
Copy link
Contributor

@max-hoffman max-hoffman left a comment

Choose a reason for hiding this comment

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

seems OK, is having the nil check inside the sql.Context.Span() and returning a noopSpan for the case impractical?

Copy link
Contributor

@max-hoffman max-hoffman left a comment

Choose a reason for hiding this comment

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

lgtm, i like the improvements

@jycor jycor merged commit 3d4148f into main Dec 14, 2023
@jycor jycor deleted the james/span branch December 14, 2023 19:18
@jycor jycor mentioned this pull request Dec 18, 2023
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.

Panic when Using BETWEEN and CASE WHEN

2 participants