Skip to content
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

fix AS OF clause panic for certain expressions #2779

Merged
merged 8 commits into from
Dec 4, 2024
Merged

Conversation

jycor
Copy link
Contributor

@jycor jycor commented Dec 4, 2024

We attempt to parse eval AS OF expressions in the builder (because we assume it is going to be a literal), but Subqueries cannot be evaluated until after they have gone through the analyzer.

partially addresses: dolthub/dolt#8635

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, just some questions about whether it's easy to extend the fix to be a bit safer

@@ -615,6 +615,8 @@ func (b *Builder) buildAsOfExpr(inScope *scope, time ast.Expr) sql.Expression {
err := sql.ErrInvalidAsOfExpression.New(v)
b.handleErr(err)
}
case *ast.Subquery:
Copy link
Contributor

Choose a reason for hiding this comment

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

is there anyway we could easily extend this a bit? should the default be an error? would we need a whitelist of acceptable exprs? longer blacklist of non-acceptable exprs?

@jycor jycor merged commit 6ffe4b8 into main Dec 4, 2024
8 checks passed
@jycor jycor deleted the james/asof-subquery branch December 4, 2024 21:58
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.

2 participants