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

exec: float 'NaN' comparisons are incorrect #38751

Closed
solongordon opened this issue Jul 9, 2019 · 1 comment · Fixed by #38881
Closed

exec: float 'NaN' comparisons are incorrect #38751

solongordon opened this issue Jul 9, 2019 · 1 comment · Fixed by #38881
Assignees
Labels
A-sql-vec SQL vectorized engine C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@solongordon
Copy link
Contributor

A few of the float logic test fail for local-vec:

--- FAIL: TestLogic (0.61s)
    test_log_scope.go:77: test logs captured to: /var/folders/9q/5bgz_tb55tdfxrfz_bnp6p6r0000gn/T/logTestLogic230065997
    test_log_scope.go:58: use -show-logs to present logs inline
    --- FAIL: TestLogic/local-vec (0.00s)
        --- FAIL: TestLogic/local-vec/float (0.58s)
            logic.go:2338:

                testdata/logic_test/float:43: select f, f > 'NaN' from p@{FORCE_INDEX=primary} where f > 'NaN' ORDER BY f
                expected:
                    -Inf  true
                    -1    true
                    0     true
                    1     true
                    +Inf  true

                but found (query options: "") :


            logic.go:2338:

                testdata/logic_test/float:52: select f, f > 'NaN' from p@{FORCE_INDEX=p_f_key} where f > 'NaN' ORDER BY f
                expected:
                    -Inf  true
                    -1    true
                    0     true
                    1     true
                    +Inf  true

                but found (query options: "") :
                    -Inf  false
                    -1    false
                    0     false
                    1     false
                    +Inf  false
@solongordon solongordon added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-sql-vec SQL vectorized engine labels Jul 9, 2019
@solongordon
Copy link
Contributor Author

This appears to affect sorting as well:

testdata/logic_test/order_by:312: SELECT x FROM nan ORDER BY x
                expected:
                    NaN
                    NaN
                    -1
                    1

                but found (query options: "") :
                    NaN
                    -1
                    1
                    NaN

solongordon added a commit to solongordon/cockroach that referenced this issue Jul 15, 2019
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 cockroachdb#38751

Release note: None
solongordon added a commit to solongordon/cockroach that referenced this issue Jul 16, 2019
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 cockroachdb#38751

Release note: None
craig bot pushed a commit that referenced this issue 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>
@craig craig bot closed this as completed in #38881 Jul 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sql-vec SQL vectorized engine C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant