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.

Sign up for GitHub

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

Jump to bottom

roachtest: sqlsmith/setup=rand-tables/setting=no-mutations failed #62686

Closed
cockroach-teamcity opened this issue Mar 27, 2021 · 4 comments · Fixed by #62893
Closed

roachtest: sqlsmith/setup=rand-tables/setting=no-mutations failed #62686

cockroach-teamcity opened this issue Mar 27, 2021 · 4 comments · Fixed by #62893
Assignees
@mgartner
Labels
branch-master Failures and bugs on the master branch. C-test-failure Broken test (automatically or manually discovered). O-roachtest O-robot Originated from a bot.

Comments

@cockroach-teamcity
Copy link
Member

(roachtest).sqlsmith/setup=rand-tables/setting=no-mutations failed on master@3cfe2a38044b9e0d47b09815658e8634e4f4bfda:

The test failed on branch=master, cloud=gce:
test artifacts and logs in: /home/agent/work/.go/src/github.com/cockroachdb/cockroach/artifacts/sqlsmith/setup=rand-tables/setting=no-mutations/run_1
	sqlsmith.go:226,sqlsmith.go:262,test_runner.go:768: error: pq: internal error: parameter unknown is not float
		stmt:
		SELECT
			count(*) AS col_9954
		FROM
			defaultdb.public.table3@[0] AS tab_4368
		GROUP BY
			tab_4368.col3_2
		HAVING
			st_dfullywithin(tab_4368.col3_2::GEOMETRY, '0103000080010000000D000000B69EA24CD242F141F4A559F67BA5F6C19A2F8FA3DC7AF0419C8E7784A933F3414E0FF2D93E17E7C156511FE87EE5FCC11CB45A4E3ED2E741C0942722B7F7D0C10054C7671E8E9A41D4797FCB1E41F741A06E052316EBDDC11C8098700DBBF84182BA05798ACEF74100DA077053687E415E0F2838028602429C3B88DFBA48EA416045DA4F1C3CBC41F410037434C9FF419E4A16022C9AFA4120989642CE17C041009F2570447AD04174818A4626F8FAC158AB9F50C7F6004240EF0371DF6ECB414F98404580B2FAC1C4E66AE3689BF24152D18747B883E4C19E53C0A6B3E8FFC1B44C1A3BA069F44168CAA9D5010401424C56C6F8754EF9C11C81A971ECEEE3417707383A674EF5C1F36F3081ED0302C2B03092CBF89BD041A416AA1CC7B4FBC1B69EA24CD242F141F4A559F67BA5F6C19A2F8FA3DC7AF041':::GEOMETRY::GEOMETRY, NULL::FLOAT8)::BOOL
		ORDER BY
			tab_4368.col3_2, tab_4368.col3_2 ASC
		LIMIT
			78:::INT8;
More

Artifacts: /sqlsmith/setup=rand-tables/setting=no-mutations

See this test on roachdash
powered by pkg/cmd/internal/issues
@cockroach-teamcity cockroach-teamcity added branch-master Failures and bugs on the master branch. 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 Mar 27, 2021
@asubiotto asubiotto removed the release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. label Mar 29, 2021
@yuzefovich
Copy link
Member

Reduced repro:

CREATE TABLE t (c GEOMETRY NULL, INVERTED INDEX (c ASC));

SELECT
  count(*)
FROM
  t
GROUP BY
  c
HAVING
  st_dfullywithin(
    c::GEOMETRY,
    '0103000080010000000D000000B69EA24CD242F141F4A559F67BA5F6C19A2F8FA3DC7AF0419C8E7784A933F3414E0FF2D93E17E7C156511FE87EE5FCC11CB45A4E3ED2E741C0942722B7F7D0C10054C7671E8E9A41D4797FCB1E41F741A06E052316EBDDC11C8098700DBBF84182BA05798ACEF74100DA077053687E415E0F2838028602429C3B88DFBA48EA416045DA4F1C3CBC41F410037434C9FF419E4A16022C9AFA4120989642CE17C041009F2570447AD04174818A4626F8FAC158AB9F50C7F6004240EF0371DF6ECB414F98404580B2FAC1C4E66AE3689BF24152D18747B883E4C19E53C0A6B3E8FFC1B44C1A3BA069F44168CAA9D5010401424C56C6F8754EF9C11C81A971ECEEE3417707383A674EF5C1F36F3081ED0302C2B03092CBF89BD041A416AA1CC7B4FBC1B69EA24CD242F141F4A559F67BA5F6C19A2F8FA3DC7AF041':::GEOMETRY::GEOMETRY,
    NULL::FLOAT8
  )::BOOL;

results in

ERROR: internal error: parameter unknown is not float
SQLSTATE: XX000
DETAIL: stack trace:
github.com/cockroachdb/cockroach/pkg/sql/opt/invertedidx/geo.go:149: getDistanceParam()
github.com/cockroachdb/cockroach/pkg/sql/opt/invertedidx/geo.go:186: getSpanExprForGeometryIndex()
github.com/cockroachdb/cockroach/pkg/sql/opt/invertedidx/geo.go:416: extractGeoFilterCondition()
github.com/cockroachdb/cockroach/pkg/sql/opt/invertedidx/geo.go:377: extractInvertedFilterConditionFromLeaf()
github.com/cockroachdb/cockroach/pkg/sql/opt/invertedidx/inverted_index_expr.go:497: extractInvertedFilterCondition()
github.com/cockroachdb/cockroach/pkg/sql/opt/invertedidx/inverted_index_expr.go:125: TryFilterInvertedIndex()
github.com/cockroachdb/cockroach/pkg/sql/opt/xform/select_funcs.go:682: func1()
github.com/cockroachdb/cockroach/pkg/sql/opt/xform/scan_index_iter.go:263: ForEachStartingAfter()
github.com/cockroachdb/cockroach/pkg/sql/opt/xform/scan_index_iter.go:191: ForEach()
github.com/cockroachdb/cockroach/pkg/sql/opt/xform/select_funcs.go:680: GenerateInvertedIndexScans()
github.com/cockroachdb/cockroach/pkg/sql/opt/xform/explorer.og.go:208: exploreSelect()
github.com/cockroachdb/cockroach/pkg/sql/opt/xform/explorer.og.go:20: exploreGroupMember()
github.com/cockroachdb/cockroach/pkg/sql/opt/xform/explorer.go:182: exploreGroup()
github.com/cockroachdb/cockroach/pkg/sql/opt/xform/optimizer.go:464: optimizeGroup()
github.com/cockroachdb/cockroach/pkg/sql/opt/xform/optimizer.go:251: optimizeExpr()
github.com/cockroachdb/cockroach/pkg/sql/opt/xform/optimizer.go:506: optimizeGroupMember()
github.com/cockroachdb/cockroach/pkg/sql/opt/xform/optimizer.go:451: optimizeGroup()
github.com/cockroachdb/cockroach/pkg/sql/opt/xform/optimizer.go:251: optimizeExpr()
github.com/cockroachdb/cockroach/pkg/sql/opt/xform/optimizer.go:506: optimizeGroupMember()
github.com/cockroachdb/cockroach/pkg/sql/opt/xform/optimizer.go:451: optimizeGroup()
github.com/cockroachdb/cockroach/pkg/sql/opt/xform/optimizer.go:225: Optimize()
github.com/cockroachdb/cockroach/pkg/sql/plan_opt.go:512: buildExecMemo()
github.com/cockroachdb/cockroach/pkg/sql/plan_opt.go:194: makeOptimizerPlan()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:937: makeExecPlan()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:818: dispatchToExecutionEngine()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:656: execStmtInOpenState()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:120: execStmt()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1527: func1()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1529: execCmd()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1450: run()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:484: ServeConn()
github.com/cockroachdb/cockroach/pkg/sql/pgwire/conn.go:626: func1()

Having an inverted index seems to be required for the repro. cc @mgartner

@mgartner mgartner self-assigned this Mar 29, 2021
@mgartner
Copy link
Collaborator

It's reproducible with just a WHERE clause too:

CREATE TABLE t (c GEOMETRY NULL, INVERTED INDEX (c ASC));

SELECT count(*) FROM t
WHERE st_dfullywithin(
    c::GEOMETRY,
    ST_GeomFromText('POINT(1 1)'),
    NULL::FLOAT8
)::BOOL;

@mgartner
Copy link
Collaborator

mgartner commented Mar 31, 2021 •
edited
Loading

We don't correctly handle NULL arguments in invertedidx/geo.go. I'll have a PR up soon to fix this panic. And I think we can also add a normalization rule to normalize functions with NULL arguments that do not allow them to be NULL. It looks like we perform a similar rewrite here if the type of the NULL is unknown (e.g. there is no ::FLOAT8 cast).

@mgartner
Copy link
Collaborator

The bug is present in JOINs as well. This test fails:

statement ok
CREATE TABLE t (
  c GEOMETRY,
  INVERTED INDEX (c ASC)
);
INSERT INTO t VALUES (ST_GeomFromText('POINT(1 1)'));

statement ok
SELECT * FROM t t1 JOIN t t2 ON st_dfullywithin(t1.c, t2.c, NULL::FLOAT8)

@mgartner mgartner mentioned this issue Mar 31, 2021
Merged
mgartner added a commit to mgartner/cockroach that referenced this issue Mar 31, 2021
@mgartner
opt: fix geospatial function NULL argument panic
be94f2c 
This commit fixes a panic that occurred when planning inverted index
scans and when performing inverted joins on `GEOMETRY` and `GEOGRAPHY`
inverted indexes for queries containing geospatial functions with an
additional `NULL` constant argument.

This panic only occurred when the constant `NULL` argument was cast to
an acceptable type for the function overload, for example
`ST_DFullyWithin(a, b, NULL::FLOAT8)`. Without the cast, no panic
occured because `tree.FuncExpr.TypeCheck` rewrites the `tree.FuncExpr`
as `tree.DNull` when any of the arguments have an unknown type, such as
an uncast `NULL`. This rewriting happens even before the `tree.FuncExpr`
is built into a `memo.FunctionExpr` by optbuilder.

Fixes cockroachdb#62686

Release note (bug fix): Queries that reference tables with `GEOMETRY` or
`GEOGRAPHY` inverted indexes and that call geospatial functions with
constant `NULL` values cast to a type, like `NULL::FLOAT8`, no longer
error. This bug was present since 20.2.
mgartner added a commit to mgartner/cockroach that referenced this issue Apr 1, 2021
@mgartner
opt: fix geospatial function NULL argument panics
6e2ce57 
This commit fixes panics that occurred when planning inverted index
scans and when performing inverted joins on `GEOMETRY` and `GEOGRAPHY`
inverted indexes for queries containing geospatial functions with an
`NULL` constant argument.

This panic only occurred when the constant `NULL` argument was cast to
an acceptable type for the function overload, for example
`ST_DWithin(a, b, NULL::FLOAT8)` or `ST_DWithin(NULL:GEOMETRY, b, 1)`.
Without the cast, no panic occured because `tree.FuncExpr.TypeCheck`
rewrites a `tree.FuncExpr` as `tree.DNull` when any of the arguments
have an unknown type, such as an uncast `NULL`. This rewriting happens
even before the `tree.FuncExpr` is built into a `memo.FunctionExpr` by
optbuilder.

Fixes cockroachdb#60527
Fixes cockroachdb#62686

Release note (bug fix): Queries that reference tables with `GEOMETRY` or
`GEOGRAPHY` inverted indexes and that call geospatial functions with
constant `NULL` values cast to a type, like `NULL::GEOMETRY` or
`NULL::FLOAT8`, no longer error. This bug was present since 20.2.
mgartner added a commit to mgartner/cockroach that referenced this issue Apr 1, 2021
@mgartner
opt: fix geospatial function NULL argument panics
5d5dca5 
This commit fixes panics that occurred when planning inverted index
scans and when performing inverted joins on `GEOMETRY` and `GEOGRAPHY`
inverted indexes for queries containing geospatial functions with an
`NULL` constant argument.

This panic only occurred when the constant `NULL` argument was cast to
an acceptable type for the function overload, for example
`ST_DWithin(a, b, NULL::FLOAT8)` or `ST_DWithin(NULL:GEOMETRY, b, 1)`.
Without the cast, no panic occured because `tree.FuncExpr.TypeCheck`
rewrites a `tree.FuncExpr` as `tree.DNull` when any of the arguments
have an unknown type, such as an uncast `NULL`. This rewriting happens
even before the `tree.FuncExpr` is built into a `memo.FunctionExpr` by
optbuilder.

Fixes cockroachdb#60527
Fixes cockroachdb#62686

Release note (bug fix): Queries that reference tables with `GEOMETRY` or
`GEOGRAPHY` inverted indexes and that call geospatial functions with
constant `NULL` values cast to a type, like `NULL::GEOMETRY` or
`NULL::FLOAT8`, no longer error. This bug was present since 20.2.
craig bot pushed a commit that referenced this issue Apr 1, 2021
@mgartner @sumeerbhola
Merge #62893 #62984
a5f2143 
62893: opt: fix geospatial function NULL argument panics r=mgartner a=mgartner

This commit fixes panics that occurred when planning inverted index
scans and when performing inverted joins on `GEOMETRY` and `GEOGRAPHY`
inverted indexes for queries containing geospatial functions with an
`NULL` constant argument.

This panic only occurred when the constant `NULL` argument was cast to
an acceptable type for the function overload, for example
`ST_DWithin(a, b, NULL::FLOAT8)` or `ST_DWithin(NULL:GEOMETRY, b, 1)`.
Without the cast, no panic occured because `tree.FuncExpr.TypeCheck`
rewrites a `tree.FuncExpr` as `tree.DNull` when any of the arguments
have an unknown type, such as an uncast `NULL`. This rewriting happens
even before the `tree.FuncExpr` is built into a `memo.FunctionExpr` by
optbuilder.

Fixes #60527
Fixes #62686

Release note (bug fix): Queries that reference tables with `GEOMETRY` or
`GEOGRAPHY` inverted indexes and that call geospatial functions with
constant `NULL` values cast to a type, like `NULL::GEOMETRY` or
`NULL::FLOAT8`, no longer error. This bug was present since 20.2.


62984: storage: reduce memory allocations in intentInterleavingIter r=sumeerbhola a=sumeerbhola

The memory allocations for computing the bounds for the
EngineIterator were 33% of the memory allocations in
batcheval.Scan in a joinReader benchmark (the rest, as
expected, is in pebbleResults for storing the result
of the scan). These allocations were 6% of the cpu in
MVCCScanToBytes in the same benchmark.

Release note: None

Co-authored-by: Marcus Gartner <[email protected]>
Co-authored-by: sumeerbhola <[email protected]>
@craig craig bot closed this as completed in 894cdb3 Apr 1, 2021
@craig craig bot closed this as completed in #62893 Apr 1, 2021
mgartner added a commit to mgartner/cockroach that referenced this issue Apr 1, 2021
@mgartner
opt: fix geospatial function NULL argument panics
47f002b 
This commit fixes panics that occurred when planning inverted index
scans and when performing inverted joins on `GEOMETRY` and `GEOGRAPHY`
inverted indexes for queries containing geospatial functions with an
`NULL` constant argument.

This panic only occurred when the constant `NULL` argument was cast to
an acceptable type for the function overload, for example
`ST_DWithin(a, b, NULL::FLOAT8)` or `ST_DWithin(NULL:GEOMETRY, b, 1)`.
Without the cast, no panic occured because `tree.FuncExpr.TypeCheck`
rewrites a `tree.FuncExpr` as `tree.DNull` when any of the arguments
have an unknown type, such as an uncast `NULL`. This rewriting happens
even before the `tree.FuncExpr` is built into a `memo.FunctionExpr` by
optbuilder.

Fixes cockroachdb#60527
Fixes cockroachdb#62686

Release note (bug fix): Queries that reference tables with `GEOMETRY` or
`GEOGRAPHY` inverted indexes and that call geospatial functions with
constant `NULL` values cast to a type, like `NULL::GEOMETRY` or
`NULL::FLOAT8`, no longer error. This bug was present since 20.2.
@mgartner mgartner mentioned this issue Apr 1, 2021
Merged
@mgartner mgartner mentioned this issue Apr 2, 2021
Closed
mgartner added a commit to mgartner/cockroach that referenced this issue Apr 2, 2021
@mgartner
opt: fix geospatial function NULL argument panics
5c13b19 
This commit fixes panics that occurred when planning inverted index
scans and when performing inverted joins on `GEOMETRY` and `GEOGRAPHY`
inverted indexes for queries containing geospatial functions with an
`NULL` constant argument.

This panic only occurred when the constant `NULL` argument was cast to
an acceptable type for the function overload, for example
`ST_DWithin(a, b, NULL::FLOAT8)` or `ST_DWithin(NULL:GEOMETRY, b, 1)`.
Without the cast, no panic occured because `tree.FuncExpr.TypeCheck`
rewrites a `tree.FuncExpr` as `tree.DNull` when any of the arguments
have an unknown type, such as an uncast `NULL`. This rewriting happens
even before the `tree.FuncExpr` is built into a `memo.FunctionExpr` by
optbuilder.

Fixes cockroachdb#60527
Fixes cockroachdb#62686

Release note (bug fix): Queries that reference tables with `GEOMETRY` or
`GEOGRAPHY` inverted indexes and that call geospatial functions with
constant `NULL` values cast to a type, like `NULL::GEOMETRY` or
`NULL::FLOAT8`, no longer error. This bug was present since 20.2.
@mgartner mgartner mentioned this issue Apr 2, 2021
Merged
mgartner added a commit to mgartner/cockroach that referenced this issue Apr 2, 2021
@mgartner
opt: fix geospatial function NULL argument panics
af72a3e 
This commit fixes panics that occurred when planning inverted index
scans and when performing inverted joins on `GEOMETRY` and `GEOGRAPHY`
inverted indexes for queries containing geospatial functions with an
`NULL` constant argument.

This panic only occurred when the constant `NULL` argument was cast to
an acceptable type for the function overload, for example
`ST_DWithin(a, b, NULL::FLOAT8)` or `ST_DWithin(NULL:GEOMETRY, b, 1)`.
Without the cast, no panic occured because `tree.FuncExpr.TypeCheck`
rewrites a `tree.FuncExpr` as `tree.DNull` when any of the arguments
have an unknown type, such as an uncast `NULL`. This rewriting happens
even before the `tree.FuncExpr` is built into a `memo.FunctionExpr` by
optbuilder.

Fixes cockroachdb#60527
Fixes cockroachdb#62686

Release note (bug fix): Queries that reference tables with `GEOMETRY` or
`GEOGRAPHY` inverted indexes and that call geospatial functions with
constant `NULL` values cast to a type, like `NULL::GEOMETRY` or
`NULL::FLOAT8`, no longer error. This bug was present since 20.2.
mgartner added a commit to mgartner/cockroach that referenced this issue May 13, 2021
@mgartner
opt: fix geospatial function NULL argument panics
2c02966 
This commit fixes panics that occurred when planning inverted index
scans and when performing inverted joins on `GEOMETRY` and `GEOGRAPHY`
inverted indexes for queries containing geospatial functions with an
`NULL` constant argument.

This panic only occurred when the constant `NULL` argument was cast to
an acceptable type for the function overload, for example
`ST_DWithin(a, b, NULL::FLOAT8)` or `ST_DWithin(NULL:GEOMETRY, b, 1)`.
Without the cast, no panic occured because `tree.FuncExpr.TypeCheck`
rewrites a `tree.FuncExpr` as `tree.DNull` when any of the arguments
have an unknown type, such as an uncast `NULL`. This rewriting happens
even before the `tree.FuncExpr` is built into a `memo.FunctionExpr` by
optbuilder.

Fixes cockroachdb#60527
Fixes cockroachdb#62686

Release note (bug fix): Queries that reference tables with `GEOMETRY` or
`GEOGRAPHY` inverted indexes and that call geospatial functions with
constant `NULL` values cast to a type, like `NULL::GEOMETRY` or
`NULL::FLOAT8`, no longer error. This bug was present since 20.2.
@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
Assignees

@mgartner mgartner

Labels
branch-master Failures and bugs on the master branch. C-test-failure Broken test (automatically or manually discovered). O-roachtest O-robot Originated from a bot.
Projects
SQL Queries
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

opt: fix geospatial function NULL argument panics mgartner/cockroach
4 participants
@mgartner @yuzefovich @asubiotto @cockroach-teamcity