Skip to content

colexec: operate on pointer to apd.Decimal rather than value#74469

Closed
yuzefovich wants to merge 5 commits intocockroachdb:masterfrom
yuzefovich:decimal-pointer
Closed

colexec: operate on pointer to apd.Decimal rather than value#74469
yuzefovich wants to merge 5 commits intocockroachdb:masterfrom
yuzefovich:decimal-pointer

Conversation

@yuzefovich
Copy link
Copy Markdown
Member

No description provided.

When we introduced the hash aggregation with partial order support, we
mistakenly removed the ordered aggregation from `aggTypes` slice that is
used in some tests as well as in the benchmarks. This is now fixed.

Release note: None
This commit removes a couple of `execgen.COPYVAL` calls that were
redundant because the first and the second argument are the same. These
calls are redundant because we already performed the same call right
after calling `Get` from the original vector and we will perform a deep
copy when calling `Set` next.

Release note: None
This commit reduces the size of the hash aggregates by removing the
reference to the well-typed column (i.e. a concrete unwrapped
`coldata.Vec`, something like `[]int64`). This is possible because the
hash aggregates only access the concrete column once, in `Flush`, so
there is no point in storing the concrete column as we do for the
ordered aggregates. We still perform the interface dispatch call only
once - previously it was in `SetOutput`, now it is in `Flush`. This
should be a non-trivial improvement since the hash aggregation uses
a separate aggregation function object for each bucket.

This change also allows us to remove the overriding of `SetOutput`
method implimentation provided by the base struct from the hash and
window aggregates.

Release note: None
This commit removes some of the slice element assignments of the form
`col[idx] = ...` in favor of its equivalent `col.Set(idx, ...)`. This
makes it easier to change the type of the element.

Release note: None
@yuzefovich yuzefovich added the do-not-merge bors won't merge a PR with this label. label Jan 5, 2022
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

To be filled if necessary.

Release note: None
@yuzefovich
Copy link
Copy Markdown
Member Author

This is no longer needed.

@yuzefovich yuzefovich closed this Jan 7, 2022
@yuzefovich yuzefovich deleted the decimal-pointer branch January 7, 2022 04:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge bors won't merge a PR with this label.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants