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: costfuzz/unoptimized-query-oracle make sure randomly generated aggregate functions are deterministic #87353

Closed
cockroach-teamcity opened this issue Sep 3, 2022 · 5 comments · Fixed by #87591
Assignees
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. T-sql-queries SQL Queries Team
Milestone

Comments

@cockroach-teamcity
Copy link
Member

cockroach-teamcity commented Sep 3, 2022

roachtest.costfuzz failed with artifacts on master @ 4dcb32c0346e20a95847763f89b9b0796d9ed4dc:

		  |   		"000000000000000000000000,000000000000000000000000000000000000000",
		  |   		"000000000000000,",
		  | - 		"27251",
		  | + 		"53",
		  |   	}, ""),
		  |   	"000000000000000000000000,01,17781",
		  |   	strings.Join({
		  |   		"000000000000000000000000,010101,",
		  | - 		"20823",
		  | + 		"32054",
		  |   	}, ""),
		  |   	strings.Join({
		  |   		"000000000000000000000000,0101010101,",
		  | - 		"16645",
		  | + 		"20823",
		  |   	}, ""),
		  |   	strings.Join({
		  |   		"000000000000000000000000,01010101010101,",
		  | - 		"32054",
		  | + 		"16645",
		  |   	}, ""),
		  |   	"000011000001110101101000,00,15723",
		  |   	"000100011101010110001110,00,17831",
		  |   	... // 30 identical elements
		  |   	"110111011101100011110001,01,69",
		  |   	"111011000001000100101101,01,0",
		  | - 	"111111111111111111111111,00,17",
		  | + 	"111111111111111111111111,00,17833",
		  |   	strings.Join({
		  |   		"111111111111111111111111,000000,17",
		  | - 		"833",
		  |   	}, ""),
		  |   	... // 6 identical and 59 modified elements
		  |   }
		  | sql: SELECT
		  | 	tab_4700.col2_0 AS col_15194,
		  | 	string_agg(tab_4700.col2_6::STRING, tab_4700.col2_6::STRING ORDER BY tab_4700.col2_6::STRING ASC, tab_4700.col2_6::STRING) OVER (PARTITION BY tab_4700.col2_6, tab_4700.col2_0 ORDER BY tab_4700.col2_6, tab_4700.col2_6::STRING ASC, tab_4700.col2_6::STRING ROWS BETWEEN CURRENT ROW AND 8049193815714677612:::INT8 FOLLOWING)::STRING
		  | 		AS col_15195,
		  | 	tab_4700.col2_1 AS col_15196
		  | FROM
		  | 	defaultdb.public.table2@[0] AS tab_4700
		  | WHERE
		  | 	true
		  | GROUP BY
		  | 	tab_4700.col2_1, tab_4700.col2_0, tab_4700.col2_6
		  | HAVING
		  | 	bool_and(true::BOOL)::BOOL
		  | ORDER BY
		  | 	tab_4700.col2_6 ASC
		Error types: (1) *withstack.withStack (2) *errutil.withPrefix (3) *withstack.withStack (4) *errutil.leafError

Parameters: ROACHTEST_cloud=gce , ROACHTEST_cpu=4 , ROACHTEST_ssd=0

Help

See: roachtest README

See: How To Investigate (internal)

Same failure on other branches

/cc @cockroachdb/sql-queries

This test on roachdash | Improve this report!

Jira issue: CRDB-19289

@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 Sep 3, 2022
@cockroach-teamcity cockroach-teamcity added this to the 22.2 milestone Sep 3, 2022
@blathers-crl blathers-crl bot added the T-sql-queries SQL Queries Team label Sep 3, 2022
@cockroach-teamcity

This comment was marked as duplicate.

@mgartner
Copy link
Collaborator

mgartner commented Sep 6, 2022

Tried reducing the failure above with reduce but running into bugs in reduce: #87418 and #87419. I'll try manually reducing.

@mgartner
Copy link
Collaborator

mgartner commented Sep 6, 2022

Reduced reproduction:

CREATE TABLE table2 (col2_0 BIT(24) NULL, col2_1 INT2 NOT NULL, col2_2 TIMETZ NOT NULL, col2_3 TIMESTAMPTZ NOT NULL, col2_4 BIT(2) NOT NULL, col2_5 FLOAT8 NOT NULL, col2_6 STRING NOT NULL AS (lower(CAST(col2_4 AS STRING))) VIRTUAL, col2_7 INT8 NOT NULL AS (col2_1 + 17852:::INT8) STORED, PRIMARY KEY (col2_2, col2_1, col2_3), INDEX (col2_7 ASC, col2_2, col2_5 ASC, col2_4 ASC, col2_6, col2_3 ASC) STORING (col2_0), INDEX (col2_4 ASC, col2_6 ASC, col2_7 DESC, col2_2 ASC, col2_0, col2_5 DESC, col2_1 DESC) WHERE (((((table2.col2_6 != e'\x00':::STRING) AND (table2.col2_3 >= '3000-01-01 00:00:00+00:00':::TIMESTAMPTZ)) AND (table2.col2_1 != (-1):::INT8)) OR (table2.col2_7 != (-2147483648):::INT8)) AND (table2.col2_2 = '00:00:00+15:59:00':::TIMETZ)) AND (table2.col2_5 < 1.7976931348623157e+308:::FLOAT8), INDEX (col2_4) WHERE (((((table2.col2_7 >= 2147483647:::INT8) AND (table2.col2_2 != '00:00:00+15:59:00':::TIMETZ)) AND (table2.col2_6 < 'X':::STRING)) OR (table2.col2_1 <= (-1):::INT8)) OR (table2.col2_5 = (-1.0):::FLOAT8)) AND (table2.col2_3 = '3000-01-01 00:00:00+00:00':::TIMESTAMPTZ), INDEX (col2_2, col2_1 DESC, col2_3 DESC, col2_5, col2_4 ASC) STORING (col2_0, col2_7), UNIQUE (col2_0, col2_2, col2_7, col2_5, col2_1), FAMILY (col2_2), FAMILY (col2_1, col2_4), FAMILY (col2_0, col2_5, col2_7), FAMILY (col2_3));

INSERT
INTO
	defaultdb.public.table2 AS tab_127 (col2_0, col2_1, col2_2, col2_3, col2_4, col2_5)
VALUES
	(
		NULL,
		(-6838):::INT8,
		'21:16:16.722186+08:49:00':::TIMETZ,
		'1973-09-12 06:31:42.000876+00:00':::TIMESTAMPTZ,
		B'00',
		1.234567890123456e+10:::FLOAT8
	);

INSERT
INTO
	defaultdb.public.table2 AS tab_133 (col2_1, col2_2, col2_3, col2_4, col2_5)
VALUES
	((-12):::INT8, '06:28:31.085579-01:02:00':::TIMETZ, '1967-04-07 03:25:45+00:00':::TIMESTAMPTZ, B'00', 0.0:::FLOAT8);


SELECT
  tab_4700.col2_6,
	tab_4700.col2_0 AS col_15194,
	string_agg(tab_4700.col2_6::STRING, tab_4700.col2_6::STRING ORDER BY tab_4700.col2_6::STRING ASC, tab_4700.col2_6::STRING) OVER (PARTITION BY tab_4700.col2_6, tab_4700.col2_0 ORDER BY tab_4700.col2_6, tab_4700.col2_6::STRING ASC, tab_4700.col2_6::STRING ROWS BETWEEN CURRENT ROW AND 8049193815714677612:::INT8 FOLLOWING)::STRING
		AS col_15195,
	tab_4700.col2_1 AS col_15196
FROM
	defaultdb.public.table2@[0] AS tab_4700
WHERE
	true
GROUP BY
	tab_4700.col2_1, tab_4700.col2_0, tab_4700.col2_6
HAVING
	bool_and(true::BOOL)::BOOL
ORDER BY
	tab_4700.col2_6 ASC;

SET testing_optimizer_random_seed = 8664472571050980723;

SET testing_optimizer_cost_perturbation = 1.0;

SELECT
  tab_4700.col2_6,
	tab_4700.col2_0 AS col_15194,
	string_agg(tab_4700.col2_6::STRING, tab_4700.col2_6::STRING ORDER BY tab_4700.col2_6::STRING ASC, tab_4700.col2_6::STRING) OVER (PARTITION BY tab_4700.col2_6, tab_4700.col2_0 ORDER BY tab_4700.col2_6, tab_4700.col2_6::STRING ASC, tab_4700.col2_6::STRING ROWS BETWEEN CURRENT ROW AND 8049193815714677612:::INT8 FOLLOWING)::STRING
		AS col_15195,
	tab_4700.col2_1 AS col_15196
FROM
	defaultdb.public.table2@[0] AS tab_4700
WHERE
	true
GROUP BY
	tab_4700.col2_1, tab_4700.col2_0, tab_4700.col2_6
HAVING
	bool_and(true::BOOL)::BOOL
ORDER BY
	tab_4700.col2_6 ASC;

Looks like this is not a blocker, and probably due to string_agg not being deterministic.

@mgartner
Copy link
Collaborator

mgartner commented Sep 6, 2022

Further reduced:

CREATE TABLE table2 (
  col2_0 BIT(24) NULL,
  col2_1 INT2 NOT NULL,
  col2_2 TIMETZ NOT NULL,
  col2_4 BIT(2) NOT NULL,
  col2_5 FLOAT8 NOT NULL,
  col2_6 STRING NOT NULL AS (lower(CAST(col2_4 AS STRING))) VIRTUAL,
  col2_7 INT8 NOT NULL AS (col2_1 + 17852:::INT8) STORED,
  PRIMARY KEY (col2_2, col2_1),
  INDEX (col2_7 ASC, col2_2, col2_5 ASC, col2_4 ASC, col2_6) STORING (col2_0)
);

INSERT
INTO
	defaultdb.public.table2 AS tab_127 (col2_0, col2_1, col2_2, col2_4, col2_5)
VALUES
	(
		NULL,
		(-6838):::INT8,
		'21:16:16.722186+08:49:00':::TIMETZ,
		B'00',
		1.234567890123456e+10:::FLOAT8
	);

INSERT
INTO
	defaultdb.public.table2 AS tab_133 (col2_1, col2_2, col2_4, col2_5)
VALUES
	((-12):::INT8, '06:28:31.085579-01:02:00':::TIMETZ, B'00', 0.0:::FLOAT8);


SELECT
	tab_4700.col2_0 AS col_15194,
	string_agg(tab_4700.col2_6::STRING, tab_4700.col2_6::STRING ORDER BY tab_4700.col2_6::STRING ASC, tab_4700.col2_6::STRING) OVER (PARTITION BY tab_4700.col2_6, tab_4700.col2_0 ORDER BY tab_4700.col2_6, tab_4700.col2_6::STRING ASC, tab_4700.col2_6::STRING ROWS BETWEEN CURRENT ROW AND 8049193815714677612:::INT8 FOLLOWING)::STRING
		AS col_15195,
	tab_4700.col2_1 AS col_15196
FROM
	defaultdb.public.table2@[0] AS tab_4700
GROUP BY
	tab_4700.col2_1, tab_4700.col2_0, tab_4700.col2_6
ORDER BY
	tab_4700.col2_6 ASC;

SET testing_optimizer_random_seed = 8664472571050980723;

SET testing_optimizer_cost_perturbation = 1.0;

SELECT
  tab_4700.col2_6,
	tab_4700.col2_0 AS col_15194,
	string_agg(tab_4700.col2_6::STRING, tab_4700.col2_6::STRING ORDER BY tab_4700.col2_6::STRING ASC, tab_4700.col2_6::STRING) OVER (PARTITION BY tab_4700.col2_6, tab_4700.col2_0 ORDER BY tab_4700.col2_6, tab_4700.col2_6::STRING ASC, tab_4700.col2_6::STRING ROWS BETWEEN CURRENT ROW AND 8049193815714677612:::INT8 FOLLOWING)::STRING
		AS col_15195,
	tab_4700.col2_1 AS col_15196
FROM
	defaultdb.public.table2@[0] AS tab_4700
GROUP BY
	tab_4700.col2_1, tab_4700.col2_0, tab_4700.col2_6
ORDER BY
	tab_4700.col2_6 ASC;

@mgartner
Copy link
Collaborator

mgartner commented Sep 6, 2022

I think this is a false positive. The query is not deterministic for all states of table2. I think we need to figure out how to guarantee that string_agg is deterministic. Until then, I've created #87426 to give us some relief from these types of failures.

@mgartner mgartner changed the title roachtest: costfuzz failed roachtest: costfuzz/unoptimized-query-oracle make sure randomly generated aggregate functions are deterministic Sep 6, 2022
@mgartner mgartner removed the release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. label Sep 6, 2022
mgartner added a commit to mgartner/cockroach that referenced this issue Sep 6, 2022
The false-positives of costfuzz and unoptimized-query-oracle caused by
aggregate functions are overwhelming. This commit prevents aggregate
functions from being generated for these tests.

Informs cockroachdb#87353

Release justification: This is a test-only change.

Release note: None
craig bot pushed a commit that referenced this issue Sep 6, 2022
87271: sql: change when txn is checked refresh materialized views r=rafiss a=e-mbrown


Due to the transaction being checked during planning
and not during execution, refresh materialized views
would fail from the client. Now the transaction is checked
during execution .

Release justification: Bug fix for refresh materialized views

Release note: None

87327: bazel: detect go standard library deprecation errors r=rickystewart a=healthy-pod

Previously, `staticcheck` only raised deprecation errors
from non-stdlib packages because the analysis pass used
by the deprecation analyzer is created by `nogo` which doesn't
analyze the standard library packages.

This code change patches `staticcheck` to let it detect deprecation
errors from the standard library and raise them. It also removes two
uses of `tls.Config.PreferServerCipherSuites` because it's deprecated
since Go 1.18 and is a legacy field that is ignored and has no effect.

Closes #84877

Release justification: Non-production code change
Release note: None

87426: sql: disable agg funcs in costfuzz and unoptimized query oracle r=mgartner a=mgartner

The false-positives of costfuzz and unoptimized-query-oracle caused by
aggregate functions are overwhelming. This commit prevents aggregate
functions from being generated for these tests.

Informs #87353

Release justification: This is a test-only change.

Release note: None

87431: server: mark testDrainContext assertion methods as test helpers r=nvanbenschoten a=nvanbenschoten

The error messages in cases like #86974 are not useful otherwise. This change allows us to see where the assertion method was called from.

Release justification: testing only.

Co-authored-by: e-mbrown <[email protected]>
Co-authored-by: healthy-pod <[email protected]>
Co-authored-by: Marcus Gartner <[email protected]>
Co-authored-by: Nathan VanBenschoten <[email protected]>
DrewKimball added a commit to DrewKimball/cockroach that referenced this issue Sep 8, 2022
This commit modifies the sqlsmith logic to add all columns to the `OVER`
clause for all window functions except for the ranking functions. This is
necessary because all other window/aggregate functions can produce different
results for different rows within a peer group. Ordering on all columns
ensures that we are ordering on a key, which restricts each peer group to
one row (and therefore ensures there is only one possible order within each
peer group). This commit also reverts cockroachdb#86742, since window functions should
now be deterministic.

Fixes cockroachdb#87353

Release note: None
DrewKimball added a commit to DrewKimball/cockroach that referenced this issue Sep 9, 2022
This commit modifies the sqlsmith logic to add all columns to the `OVER`
clause for all window functions except for the ranking functions. This is
necessary because all other window/aggregate functions can produce different
results for different rows within a peer group. Ordering on all columns
ensures that we are ordering on a key, which restricts each peer group to
one row (and therefore ensures there is only one possible order within each
peer group). This commit also reverts cockroachdb#87426, since window functions should
now be deterministic.

Fixes cockroachdb#87353

Release note: None
craig bot pushed a commit that referenced this issue Sep 13, 2022
87591: roachtest: make window functions deterministic r=DrewKimball a=DrewKimball

This commit modifies the sqlsmith logic to add all columns to the `OVER` clause for all window functions except for the ranking functions. This is necessary because all other window/aggregate functions can produce different results for different rows within a peer group. Ordering on all columns ensures that we are ordering on a key, which restricts each peer group to one row (and therefore ensures there is only one possible order within each peer group). This commit also reverts #86742, since window functions should now be deterministic.

Fixes #87353

Release note: None

Co-authored-by: DrewKimball <[email protected]>
@craig craig bot closed this as completed in 4137557 Sep 13, 2022
michae2 pushed a commit that referenced this issue Jun 28, 2023
This commit modifies the sqlsmith logic to add all columns to the `OVER`
clause for all window functions except for the ranking functions. This is
necessary because all other window/aggregate functions can produce different
results for different rows within a peer group. Ordering on all columns
ensures that we are ordering on a key, which restricts each peer group to
one row (and therefore ensures there is only one possible order within each
peer group). This commit also reverts #87426, since window functions should
now be deterministic.

Fixes #87353

Release note: None
@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
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. T-sql-queries SQL Queries Team
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants