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: vectorized engine incorrectly supports dates outside of range #40354

Open
yuzefovich opened this issue Aug 29, 2019 · 2 comments
Open

exec: vectorized engine incorrectly supports dates outside of range #40354

yuzefovich opened this issue Aug 29, 2019 · 2 comments
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. P-3 Issues/test failures with no fix SLA T-sql-queries SQL Queries Team

Comments

@yuzefovich
Copy link
Member

yuzefovich commented Aug 29, 2019

logictest/edge doesn't succeed with auto if we lower the row count threshold to 0 (meaning if the query is streaming, it'll always be executed with the vectorized engine):

root@127.0.0.1:57035/defaultdb> SELECT _date - 1 FROM t WHERE key = 'min';
pq: date is out of range
root@127.0.0.1:57035/defaultdb> explain (vec) SELECT _date - 1 FROM t WHERE key = 'min';
                           text                           
+--------------------------------------------------------+
   │                                                      
   └── Node 1                                             
        └── *distsqlrun.materializer                      
             └── *exec.simpleProjectOp                    
                  └── *exec.projMinusInt64Int64ConstOp    
                       └── *exec.CancelChecker            
                            └── *distsqlrun.colBatchScan  
(7 rows)

Time: 776µs

root@127.0.0.1:57035/defaultdb> set vectorize = off;
SET

Time: 245µs

root@127.0.0.1:57035/defaultdb> SELECT _date - 1 FROM t WHERE key = 'min';
pq: date is out of range
root@127.0.0.1:57035/defaultdb> set vectorize = experimental_on;
SET

Time: 239µs

root@127.0.0.1:57035/defaultdb> SELECT _date - 1 FROM t WHERE key = 'min';
  ?column?   
+-----------+
  -infinity  
(1 row)

Time: 1.771ms

What happens is that Matt added the support for date type exactly like postgres has, but in the vectorized land we're using int64. This should be fixed.

Jira issue: CRDB-5547

@yuzefovich yuzefovich 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 Aug 29, 2019
jordanlewis added a commit to jordanlewis/cockroach that referenced this issue Aug 30, 2019
This test randomly runs binary comparisons against all types with random
data, verifying that the result of Datum.Compare matches the result of
the exec projection.

This found the bug with date infinity matching tracked in cockroachdb#40354, and
immediately found a bug with the timestamp implementation in the
previous commit, so I'm fairly sure it's a useful random test to have
around.

Release note: None
jordanlewis added a commit to jordanlewis/cockroach that referenced this issue Sep 5, 2019
This test randomly runs binary comparisons against all types with random
data, verifying that the result of Datum.Compare matches the result of
the exec projection.

This found the bug with date infinity matching tracked in cockroachdb#40354, and
immediately found a bug with the timestamp implementation in the
previous commit, so I'm fairly sure it's a useful random test to have
around.

Release note: None
yuzefovich pushed a commit to yuzefovich/cockroach that referenced this issue Oct 28, 2019
This test randomly runs binary comparisons against all types with random
data, verifying that the result of Datum.Compare matches the result of
the exec projection.

This found the bug with date infinity matching tracked in cockroachdb#40354, and
immediately found a bug with the timestamp implementation in the
previous commit, so I'm fairly sure it's a useful random test to have
around.

Release note: None
yuzefovich pushed a commit to yuzefovich/cockroach that referenced this issue Oct 29, 2019
This test randomly runs binary comparisons against all types with random
data, verifying that the result of Datum.Compare matches the result of
the exec projection.

This found the bug with date infinity matching tracked in cockroachdb#40354, and
immediately found a bug with the timestamp implementation in the
previous commit, so I'm fairly sure it's a useful random test to have
around.

Release note: None
yuzefovich pushed a commit to yuzefovich/cockroach that referenced this issue Oct 30, 2019
This test randomly runs binary comparisons against all types with random
data, verifying that the result of Datum.Compare matches the result of
the exec projection.

This found the bug with date infinity matching tracked in cockroachdb#40354, and
immediately found a bug with the timestamp implementation in the
previous commit, so I'm fairly sure it's a useful random test to have
around.

Release note: None
craig bot pushed a commit that referenced this issue Oct 30, 2019
41662: colexec: add timestamp type r=yuzefovich a=yuzefovich

This PR is the rebased version of #40250 with a modification of how we're creating `DTimestamp` in `PhysicalTypeColElemToDatum`.

**colexec,coldata: support timestamp type**

This commit adds support for the TIMESTAMP type to the coldata and colexec
packages.

**colexec: add random projection tests for all types**

This test randomly runs binary comparisons against all types with random
data, verifying that the result of Datum.Compare matches the result of
the exec projection.

This found the bug with date infinity matching tracked in #40354, and
immediately found a bug with the timestamp implementation in the
previous commit, so I'm fairly sure it's a useful random test to have
around.

**colserde: add simple serialization for TIMESTAMP**

This commit adds simple (and quite inefficient) serialization
for TIMESTAMP type which uses the provided marshalling and
unmarshalling methods.

Release note: None

Co-authored-by: Jordan Lewis <jordanthelewis@gmail.com>
Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
yuzefovich pushed a commit to yuzefovich/cockroach that referenced this issue Oct 31, 2019
This test randomly runs binary comparisons against all types with random
data, verifying that the result of Datum.Compare matches the result of
the exec projection.

This found the bug with date infinity matching tracked in cockroachdb#40354, and
immediately found a bug with the timestamp implementation in the
previous commit, so I'm fairly sure it's a useful random test to have
around.

Release note: None
yuzefovich pushed a commit to yuzefovich/cockroach that referenced this issue Nov 12, 2019
This test randomly runs binary comparisons against all types with random
data, verifying that the result of Datum.Compare matches the result of
the exec projection.

This found the bug with date infinity matching tracked in cockroachdb#40354, and
immediately found a bug with the timestamp implementation in the
previous commit, so I'm fairly sure it's a useful random test to have
around.

Release note: None
@github-actions
Copy link

github-actions bot commented Jun 4, 2021

We have marked this issue as stale because it has been inactive for
18 months. If this issue is still relevant, removing the stale label
or adding a comment will keep it active. Otherwise, we'll close it in
5 days to keep the issue queue tidy. Thank you for your contribution
to CockroachDB!

@jlinder jlinder added the T-sql-queries SQL Queries Team label Jun 16, 2021
@github-actions
Copy link

We have marked this issue as stale because it has been inactive for
18 months. If this issue is still relevant, removing the stale label
or adding a comment will keep it active. Otherwise, we'll close it in
10 days to keep the issue queue tidy. Thank you for your contribution
to CockroachDB!

@yuzefovich yuzefovich added the P-3 Issues/test failures with no fix SLA label May 2, 2024
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. P-3 Issues/test failures with no fix SLA T-sql-queries SQL Queries Team
Projects
Status: Bugs to Fix
Development

No branches or pull requests

2 participants