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

Avoid clock_gettime on pre-10.12 macOS versions #39

Merged

Conversation

ajkr
Copy link

@ajkr ajkr commented Jul 15, 2019

On older macOS like 10.10 we saw the following compiler error:

/go/src/github.com/cockroachdb/cockroach/c-deps/rocksdb/env/env_posix.cc:845:19:
error: use of undeclared identifier 'CLOCK_THREAD_CPUTIME_ID'
    clock_gettime(CLOCK_THREAD_CPUTIME_ID, &ts);
                  ^

According to mac's man clock_gettime: "These functions first appeared in Mac
OSX 10.12". So we should not try to compile it on earlier versions.

Test Plan: verified it compiles now on 10.10. Also did some investigation to
ensure it does not cause regression on macOS 10.12+, although I do not
have access to such an environment to really test.


This change is Reviewable

On older macOS like 10.10 we saw the following compiler error:

```
/go/src/github.com/cockroachdb/cockroach/c-deps/rocksdb/env/env_posix.cc:845:19:
error: use of undeclared identifier 'CLOCK_THREAD_CPUTIME_ID'
    clock_gettime(CLOCK_THREAD_CPUTIME_ID, &ts);
                  ^
```

According to mac's `man clock_gettime`: "These functions first appeared in Mac
OSX 10.12". So we should not try to compile it on earlier versions.

Test Plan: verified it compiles now on 10.10. Also did some investigation to
ensure it does not cause regression on macOS 10.12+, although I do not
have access to such an environment to really test.
@ajkr
Copy link
Author

ajkr commented Jul 15, 2019

Upstream PR is facebook#5570.

@ajkr ajkr requested a review from petermattis July 15, 2019 20:28
Copy link

@petermattis petermattis left a comment

Choose a reason for hiding this comment

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

LGTM

@ajkr ajkr merged commit 8bd5ed5 into cockroachdb:crl-release-6.2.1 Jul 16, 2019
ajkr added a commit to ajkr/cockroach that referenced this pull request Jul 16, 2019
craig bot pushed a commit to cockroachdb/cockroach that referenced this pull request Jul 16, 2019
38767: exec: fix planning of count operator r=yuzefovich a=yuzefovich

Previously, when planning a count operator, we would add it to the
flow and would ignore any post-operator planning (like projections).
Now, this is fixed.

Additionally, this commit fixes slicing within projections operators -
previously, we would always slice up to BatchSize, but the underlying
memory not always has sufficient capacity (for example, count operator
uses a batch with a capacity of 1) which would cause an index out of
bounds.

Fixes: #38752.

Release note: None

38881: exec: fix NaN comparison logic r=solongordon a=solongordon

I added special NaN handling for float comparisons. In SQL, NaNs are
treated as less than any other float value.

Thankfully I'm not seeing a performance hit when I run our sort
benchmarks with float64 values.

Fixes #38751

Release note: None

38891: c-deps: bump rocksdb for macOS build fix r=ajkr a=ajkr

Pick up cockroachdb/rocksdb#39

Release note: None

Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
Co-authored-by: Solon Gordon <solon@cockroachlabs.com>
Co-authored-by: Andrew Kryczka <andrew.kryczka2@gmail.com>
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