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

roachtest/sqlsmith: a bug with propagating precise integer width into the filter #70738

Closed
cockroach-teamcity opened this issue Sep 25, 2021 · 5 comments · Fixed by #74882
Closed
Assignees
Labels
C-test-failure Broken test (automatically or manually discovered). O-roachtest O-robot Originated from a bot. T-sql-queries SQL Queries Team

Comments

@cockroach-teamcity
Copy link
Member

cockroach-teamcity commented Sep 25, 2021

roachtest.sqlsmith/setup=seed-vec/setting=vec failed with artifacts on release-21.2 @ 24021ba163e4ac438b169d575cf1527a4aae394d:

create table t (_interval interval, _int4 int4, _int8 int8, index (_int8));
insert into t values ('1 day'::interval, 1, 1);
select t2.* from t@t__int8_idx as t1 join t@t__int8_idx as t2 on t1._int8 = t2._int4 where (t2._interval / t1._int8) in ('1 day':::INTERVAL);
Reproduce

See: roachtest README

/cc @cockroachdb/sql-queries

This test on roachdash | Improve this report!

@cockroach-teamcity cockroach-teamcity added branch-release-21.2 C-test-failure Broken test (automatically or manually discovered). O-roachtest O-robot Originated from a bot. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. labels Sep 25, 2021
@blathers-crl blathers-crl bot added the T-sql-queries SQL Queries Team label Sep 25, 2021
@yuzefovich
Copy link
Member

The bug is present on 21.1.9 but not on 20.2.16, removing the release-blocker label.

@yuzefovich yuzefovich removed the release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. label Sep 25, 2021
@yuzefovich yuzefovich changed the title roachtest: sqlsmith/setup=seed-vec/setting=vec failed roachtest/sqlsmith: a bug with propagating precise integer width into the filter Sep 27, 2021
@yuzefovich
Copy link
Member

EXPLAIN tells us that the filter divides by INT4:

  • hash join
  │ equality: (_int8) = (_int4)
  │
  ├── • scan
  │     missing stats
  │     table: t@t__int8_idx
  │     spans: FULL SCAN
  │
  └── • filter
      │ filter: (_interval / _int4) = '1 day'
      │
      └── • index join
          │ table: t@primary
          │
          └── • scan
                missing stats
                table: t@t__int8_idx
                spans: FULL SCAN

yet during the execution we divide by INT8:

ERROR: internal error: unexpected error from the vectorized engine: interface conversion: coldata.Column is coldata.Int32s, not coldata.Int64s
SQLSTATE: XX000
DETAIL: stack trace:
github.com/cockroachdb/cockroach/pkg/sql/colexecerror/error.go:88: func1()
runtime/panic.go:965: gopanic()
runtime/iface.go:261: panicdottypeE()
runtime/iface.go:271: panicdottypeI()
github.com/cockroachdb/cockroach/pkg/col/coldata/vec.go:243: Int64()
github.com/cockroachdb/cockroach/pkg/sql/colexec/colexecproj/proj_non_const_ops.eg.go:18189: func1()
github.com/cockroachdb/cockroach/pkg/sql/colmem/allocator.go:301: PerformOperation()
github.com/cockroachdb/cockroach/pkg/sql/colexec/colexecproj/proj_non_const_ops.eg.go:18179: Next()
github.com/cockroachdb/cockroach/pkg/sql/colexec/colexecsel/selection_ops.eg.go:9176: Next()
github.com/cockroachdb/cockroach/pkg/sql/colexec/colexecbase/simple_project.go:125: Next()
...

My guess is that we're missing the propagation of the integer's width during planning.

@mgartner
Copy link
Collaborator

It looks like the width of the integer is propagated correctly, we just don't have a projDivIntervalInt32Op. There is only projDivIntervalInt64Op, so that it used by default in proj_non_const_ops.eg.go. I haven't yet figured out why the operator for interval / int4 and interval / int2 are missing from the generated code - there are operators for decimal / int[2,4,8]: projDivDecimalInt16Op, projDivDecimalInt32Op, projDivDecimalInt64Op.

@yuzefovich
Copy link
Member

I see, indeed. We forgot to define some combinations in registerBinOpOutputTypes in execgen/overloads_bin.go.

@mgartner
Copy link
Collaborator

It's rare to come across this case because most INTERVAL / INT[2,4] expressions are retyped to INTERVAL / INT8 here. I think the fix is simple: #74882.

craig bot pushed a commit that referenced this issue Jan 18, 2022
74882: execgen: add overloads for INTERVAL / INT4 and INTERVAL / INT2 r=mgartner a=mgartner

Vectorized overloads for `INTERVAL / INT4` and `INTERVAL / INT2`
expressions have been added. The omission of these overloads caused
errors only in rare cases because usually these expressions are retyped
to `INTERVAL / INT8` by the optimizer
[here](https://github.com/cockroachdb/cockroach/blob/583efd3cd9c7198bac935bb81a7a598e5849a269/pkg/sql/opt/optbuilder/scalar.go#L218).

Fixes #70738

Release note (bug fix): A bug has been fixed which caused errors in rare
cases when trying to divide `INTERVAL` values by `INT4` or `INT2`
values.

Co-authored-by: Marcus Gartner <[email protected]>
@craig craig bot closed this as completed in 078c85a Jan 18, 2022
blathers-crl bot pushed a commit that referenced this issue Jan 18, 2022
Vectorized overloads for `INTERVAL / INT4` and `INTERVAL / INT2`
expressions have been added. The omission of these overloads caused
errors only in rare cases because usually these expressions are retyped
to `INTERVAL / INT8` by the optimizer
[here](https://github.com/cockroachdb/cockroach/blob/583efd3cd9c7198bac935bb81a7a598e5849a269/pkg/sql/opt/optbuilder/scalar.go#L218).

Fixes #70738

Release note (bug fix): A bug has been fixed which caused errors in rare
cases when trying to divide `INTERVAL` values by `INT4` or `INT2`
values.
mgartner added a commit that referenced this issue Jan 19, 2022
Vectorized overloads for `INTERVAL / INT4` and `INTERVAL / INT2`
expressions have been added. The omission of these overloads caused
errors only in rare cases because usually these expressions are retyped
to `INTERVAL / INT8` by the optimizer
[here](https://github.com/cockroachdb/cockroach/blob/583efd3cd9c7198bac935bb81a7a598e5849a269/pkg/sql/opt/optbuilder/scalar.go#L218).

Fixes #70738

Release note (bug fix): A bug has been fixed which caused errors in rare
cases when trying to divide `INTERVAL` values by `INT4` or `INT2`
values.
mgartner added a commit that referenced this issue Jan 19, 2022
Vectorized overloads for `INTERVAL / INT4` and `INTERVAL / INT2`
expressions have been added. The omission of these overloads caused
errors only in rare cases because usually these expressions are retyped
to `INTERVAL / INT8` by the optimizer
[here](https://github.com/cockroachdb/cockroach/blob/583efd3cd9c7198bac935bb81a7a598e5849a269/pkg/sql/opt/optbuilder/scalar.go#L218).

Fixes #70738

Release note (bug fix): A bug has been fixed which caused errors in rare
cases when trying to divide `INTERVAL` values by `INT4` or `INT2`
values.
mgartner added a commit to mgartner/cockroach that referenced this issue Jan 19, 2022
Vectorized overloads for `INTERVAL / INT4` and `INTERVAL / INT2`
expressions have been added. The omission of these overloads caused
errors only in rare cases because usually these expressions are retyped
to `INTERVAL / INT8` by the optimizer
[here](https://github.com/cockroachdb/cockroach/blob/583efd3cd9c7198bac935bb81a7a598e5849a269/pkg/sql/opt/optbuilder/scalar.go#L218).

Fixes cockroachdb#70738

Release note (bug fix): A bug has been fixed which caused errors in rare
cases when trying to divide `INTERVAL` values by `INT4` or `INT2`
values.
mgartner added a commit that referenced this issue Jan 20, 2022
Vectorized overloads for `INTERVAL / INT4` and `INTERVAL / INT2`
expressions have been added. The omission of these overloads caused
errors only in rare cases because usually these expressions are retyped
to `INTERVAL / INT8` by the optimizer
[here](https://github.com/cockroachdb/cockroach/blob/583efd3cd9c7198bac935bb81a7a598e5849a269/pkg/sql/opt/optbuilder/scalar.go#L218).

Fixes #70738

Release note (bug fix): A bug has been fixed which caused errors in rare
cases when trying to divide `INTERVAL` values by `INT4` or `INT2`
values.
@mgartner mgartner moved this to Done in SQL Queries Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-test-failure Broken test (automatically or manually discovered). O-roachtest O-robot Originated from a bot. T-sql-queries SQL Queries Team
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants