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.

Sign up for GitHub

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

Jump to bottom

colexec: add support for remaining binary operators #49463

Closed
9 tasks done
yuzefovich opened this issue May 23, 2020 · 2 comments · Fixed by #85681
Closed
9 tasks done

colexec: add support for remaining binary operators #49463

yuzefovich opened this issue May 23, 2020 · 2 comments · Fixed by #85681
Labels
A-sql-vec SQL vectorized engine C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) meta-issue Contains a list of several other issues. T-sql-queries SQL Queries Team

Comments

@yuzefovich
Copy link
Member

yuzefovich commented May 23, 2020 •
edited
Loading

This issue tracks adding support for the remaining binary expressions in the vectorized engine. Here is what's left:

The second commit from #49761 can serve as a guideline. Here is a rough outline of what's needed to do for each:

  • update execgen.BinaryOpName to introduce the binary operator
  • register the output type of newly supported binary operator in registerBinOpOutputTypes
  • modify getBinOpAssignFunc method of appropriate type customizers to support new tree.BinaryOperator type. This step is the most involved and requires translating the code we have in tree/eval.go to a similar code to be included in the generated code for the vectorized engine. Note that some types (like types.Int) are represented "natively" (i.e. with int64) whereas others (like types.Jsonb) are represented via datum-backed vectors (i.e. with tree.Datum).
  • add some tests to logic_test/vectorize_overloads.

To provide some context on what's happening behind the scenes for the curious: we have defined template files that are used in conjunction with these type customizers in order to generate type-specific code (similar to what generics are for in other languages). If you take a look at _SET_SINGLE_TUPLE_PROJECTION in proj_non_const_opt_tmpl.go, this is where _ASSIGN (which is a "templated function call") will be replaced by the template code that needs to be added in getBinOpAssignFunc. One could look at proj_non_const_ops.eg.go (once cockroach is built) and at the _tmpl file to get some more insight.

Jira issue: CRDB-4225
@yuzefovich yuzefovich added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) meta-issue Contains a list of several other issues. A-sql-vec SQL vectorized engine labels May 23, 2020
This was referenced May 23, 2020
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
yongyanglai added a commit to yongyanglai/cockroach that referenced this issue Jun 1, 2020
@yongyanglai
colexec: add support for Concat binary operator
14f6d44 
Previously, the vectorized engine lacked support for concat binary
operator.
This commit added this support. I implemented concat for bytes
referrering to same operator for row-oriented egine. Since there was
already a "getBinOpAssignFunc" implemented for "datumCustomizer", I
just registered the output type for Concat operator for datum type, so
that Concat operator for datum type would be generated as expected.

Resolves: cockroachdb#49466
See also: cockroachdb#49463

Release note (sql change): None
@yongyanglai yongyanglai mentioned this issue Jun 1, 2020
Merged
craig bot pushed a commit that referenced this issue Jun 2, 2020
@mgartner @yongyanglai
Merge #49453 #49758
455a67b 
49453: sql: store partial index predicate in index descriptor r=mgartner a=mgartner

With this commit, serialized partial index predicates are now stored on
index descriptors. Predicates are dequalified so that database and table
names are not included in column references.

Release note: None

49758: colexec: add support for Concat binary operator r=yuzefovich a=yongyanglai

Previously, the vectorized engine lacked support for concat binary
operator.
This commit added this support. I implemented concat for bytes
referrering to same operator for row-oriented egine. Since there was
already a "getBinOpAssignFunc" implemented for "datumCustomizer", I
just registered the output type for Concat operator for datum type, so
that Concat operator for datum type would be generated as expected.

Resolves: #49466
See also: #49463

Co-authored-by: Marcus Gartner <[email protected]>
Co-authored-by: Yongyang Lai <[email protected]>
@yuzefovich yuzefovich mentioned this issue Jun 6, 2020
Closed
@tancredosouza
Copy link
Contributor

Hi there! Thanks for giving a guideline for how one could approach this.

I had some trouble finding the aforementioned populateBinOpOutputTypes. Should that be registerBinOpOutputTypes instead?

Since this is my first time looking at the source code, I'm still getting familiar with where things are.

Thanks! Kind regards,
Tan.

@yuzefovich
Copy link
Member Author

yuzefovich commented Jun 16, 2020 •
edited
Loading

I had some trouble finding the aforementioned populateBinOpOutputTypes. Should that be registerBinOpOutputTypes instead?

Yes, it should be, my apologies. Thanks for pointing this out!

@yuzefovich yuzefovich mentioned this issue Oct 9, 2020
Closed
@yuzefovich yuzefovich mentioned this issue Oct 17, 2020
Merged
@rafiss rafiss mentioned this issue Oct 22, 2020
Closed
@jlinder jlinder added the T-sql-queries SQL Queries Team label Jun 16, 2021
@yuzefovich yuzefovich mentioned this issue Aug 5, 2022
Merged
@craig craig bot closed this as completed in ce8bfd4 Aug 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-sql-vec SQL vectorized engine C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) meta-issue Contains a list of several other issues. T-sql-queries SQL Queries Team
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

colexecbase: add all casts of native types to strings yuzefovich/cockroach
3 participants
@jlinder @yuzefovich @tancredosouza