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 vector arithmetic #5149

Merged
merged 3 commits into from
Jun 27, 2024
Merged

Add vector arithmetic #5149

merged 3 commits into from
Jun 27, 2024

Conversation

nwt
Copy link
Member

@nwt nwt commented Jun 26, 2024

Implement addition, subtraction, multiplication, division, and modulo for numeric (unsigned integer, signed interger, and floating point) vectors as well as addition for string vectors.

We haven't yet worked out a way to write concise tests for vector operations so test coverage is minimal for now.

Implement addition, subtraction, multiplication, division, and modulo
for numeric (unsigned integer, signed interger, and floating point)
vectors as well as addition for string vectors.

We haven't yet worked out a way to write concise tests for vector
operations so test coverage is minimal for now.
@nwt nwt requested a review from a team June 26, 2024 16:32
}
for lform := vector.Form(0); lform < 3; lform++ {
for rform := vector.Form(0); rform < 3; rform++ {
name := "arith" + opToAlpha[op] + typ + lform.String() + rform.String()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Your naming convention differs from your compare pr in that the generated functions are seperated by underscores (e.g., cmp_EQ_Int_Flat_Flat). I think you should pick one for consistency.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm planning to remove the underscores from gencomparefuncs.go in a separate PR.

}

func (i *Int) Append(v int64) {
i.Values = append(i.Values, v)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do nulls need to be append to here?

Copy link
Member Author

Choose a reason for hiding this comment

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

No need. The model for vector operations whose output is a fixed-size primitive type (like Int, Uint, Float, and Bool) is that we compute a value for every slot in the output vector regardless of nulls in the input vectors and separately compute an output nulls vector (the Nulls *Bool field present in most of the vector types) from the input nulls vectors.

This PR doesn't include that nulls handling and neither does #5146 or any of the existing code under runtime/vam/expr. But it's coming.

@nwt nwt merged commit a1052a0 into main Jun 27, 2024
3 checks passed
@nwt nwt deleted the vector-arithmetic branch June 27, 2024 23:24
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