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

[DNM] vendor: bump cockroachdb/apd #75757

Conversation

nvanbenschoten
Copy link
Member

@nvanbenschoten nvanbenschoten commented Jan 31, 2022

Picks up part of v3.1.0. Draft to show that this passes CI with minimal testing changes.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@nvanbenschoten nvanbenschoten force-pushed the nvanbenschoten/bumpApd3.1.0-part branch from 47be5a1 to fa2569d Compare February 1, 2022 03:56
Picks up part of v3.1.0. Draft to show that this passes CI with no
testing changes.
@nvanbenschoten nvanbenschoten force-pushed the nvanbenschoten/bumpApd3.1.0-part branch from fa2569d to 9d42eca Compare February 1, 2022 05:03
craig bot pushed a commit that referenced this pull request Feb 2, 2022
74612: rangefeedcache,settingswatcher,systemcfgwatcher: lose gossip deps r=ajwerner a=ajwerner

This is a pile of commits which supersedes #69269 and pretty much puts in place all of the pieces to move on #70560. 

75726: sql: refactor pg_has_role to remove privilege.GRANT usage r=RichardJCai a=ecwall

refs #73129

Also combines some layers of privilege checking code.

Release note: None

75770: vendor: bump cockroachdb/apd to v3.1.0, speed up decimal division r=nvanbenschoten a=nvanbenschoten

Picks up two PRs that improved the performance of `Quo`, `Sqrt`, `Cbrt`, `Exp`, `Ln`, `Log`, and `Pow`:
- cockroachdb/apd#114
- cockroachdb/apd#115

Almost all of the testing changes here are due to the rounding behavior in cockroachdb/apd#115. This brings us closer to PG's behavior, but also creates a lot of noise in this diff. To verify that this noise wasn't hiding any correctness regressions caused by the rewrite of `Context.Quo` in the first PR, I created #75757, which only includes the first PR. #75757 passes CI with minimal testing changes. The testing changes that PR did require all have to do with trailing zeros, and most of them are replaced in this PR.

Release note (performance improvement): The performance of many DECIMAL arithmetic operators has been improved by as much as 60%. These operators include division (`/`), `sqrt`, `cbrt`, `exp`, `ln`, `log`, and `pow`.

----

### Speedup on TPC-DS dataset

The TPC-DS dataset is full of decimal columns, so it's a good playground to test this change. Unfortunately, the variance in the runtime performance of the TPC-DS queries themselves is high (many queries varied by 30-40% per attempt), so it was hard to get signal out of them. Instead, I imported the TPC-DS dataset with a scale factor of 10 and ran some custom aggregation queries against the largest table (web_sales, row count = 7,197,566):

```sql
# q1
select sum(ws_wholesale_cost / ws_ext_list_price) from web_sales;

# q2
select sum(ws_wholesale_cost / ws_ext_list_price - sqrt(ws_net_paid_inc_tax)) from web_sales;
```

Here's the difference in runtime of these two queries before and after this change on an `n2-standard-8` instance:

```
name              old s/op   new s/op   delta
TPC-DS/custom/q1  22.4 ± 0%   8.6 ± 0%  -61.33%  (p=0.002 n=6+6)
TPC-DS/custom/q2  91.4 ± 0%  32.1 ± 0%  -64.85%  (p=0.008 n=5+5)
```

75771: colexec: close the ordered sync used by the external sorter r=yuzefovich a=yuzefovich

**colexec: close the ordered sync used by the external sorter**

Previously, we forgot to close the ordered synchronizer that is used by
the external sorter to merge already sorted partitions. This could
result in some tracing spans never being finished and is now fixed.

Release note: None

**colexec: return an error rather than logging it on Close in some cases**

This error eventually will be logged anyway, but we should try to
propagate it up the stack as much as possible.

Release note: None

75807: ui: Add CircleFilled component r=ericharmeling a=ericharmeling

Previously, there was no component for a filled circle icon in the `ui` package.
Recent product designs have requested this icon for the DB Console (see #67510, #73463).
This PR adds a `CircleFilled` component to the `ui` package.

Release note: None

75813: sql: fix flakey TestShowRangesMultipleStores r=ajwerner a=ajwerner

Fixes #75699

Release note: None

75836: dev: add generate protobuf r=ajwerner a=ajwerner

Convenient, fast.

Release note: None

Co-authored-by: Andrew Werner <[email protected]>
Co-authored-by: Evan Wall <[email protected]>
Co-authored-by: Nathan VanBenschoten <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: Eric Harmeling <[email protected]>
@nvanbenschoten nvanbenschoten deleted the nvanbenschoten/bumpApd3.1.0-part branch February 2, 2022 16:55
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