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

Add type coercion to top-down fix expression types #2150

Closed
wants to merge 5 commits into from

Conversation

max-hoffman
Copy link
Contributor

@max-hoffman max-hoffman commented Nov 17, 2023

Setup an outline for how top-down hinting can help type coerce expressions that otherwise lack the context to know what types they need to be. The benefits of this compared to type casting within individual expression Evals:

  • Controlling types upfront should aid correctness, for example for aggregation functions that return different types depending on the context, or INSERTs/UPDATEs that expect specific type inputs, or bindvar substitution
  • Adding type casts while we have info at the AST layer should generalize to more expression types than adding type casts in individual execution functions
  • Front-loading type information should make validation easier to add at binding, which is preferable to after analysis and is probably easier than doing another tree walk

fixes: dolthub/dolt#7018

@max-hoffman max-hoffman changed the title Add type coercion to normalize expressions Add type coercion to top-down fix expression types Nov 17, 2023
@max-hoffman max-hoffman requested review from zachmu and jycor November 17, 2023 19:42
Copy link
Contributor

@jycor jycor left a comment

Choose a reason for hiding this comment

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

LGTM

@max-hoffman
Copy link
Contributor Author

closing this for now, refer here for background: https://docs.google.com/document/d/1AaoIGaoLktCFtQeqJLzJsQMKYL12K1GvnyVTq-WkeTg/edit

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.

Unexpected Results about Floating-point Type Casting
2 participants