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: internal error: building declarative schema change targets for CREATE FUNCTION #105259

Closed
cockroach-teamcity opened this issue Jun 21, 2023 · 9 comments · Fixed by #106868
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
Projects
Milestone

Comments

@cockroach-teamcity
Copy link
Member

cockroach-teamcity commented Jun 21, 2023

roachtest.sqlsmith/setup=rand-tables/setting=no-mutations failed with artifacts on master @ 1eb628e8c8a7eb1cbf9bfa2bd6c31982c25cbba0:

(sqlsmith.go:258).func3: error: pq: internal error: building declarative schema change targets for CREATE FUNCTION: ReturnType called on TypedExpr with empty typeAnnotation. Was the underlying Expr type-checked before asserting a type of TypedExpr?
stmt:
CREATE FUNCTION func🙁428(IN p0 GEOGRAPHY, IN p1 REGCLASS, IN p2 BYTES, IN p3 rand_typ_1, IN p4 TIMESTAMP, IN p5 TSQUERY, IN p6 GEOGRAPHY, IN p7 OIDVECTOR, IN p8 DECIMAL)
	RETURNS SETOF RECORD
	RETURNS NULL ON NULL INPUT
	VOLATILE
	LANGUAGE SQL
	AS $funcbody$SELECT 3026675214:::OID AS "co l28019", p2 AS "|col28020", "tab�😺11710"."col""3_4" AS col28021, "tab�😺11710".col3_0 AS col28022, "tab�😺11710"."cOl3_7" AS col28023 FROM defaultdb.public."tabl\\x96e3"@"tabl\\x96e3_col3_3_col""3_4_expr_col3_5_co%ql3_1_co"" l\g3_10_col3_2_key" AS "tab�😺11710" WHERE true ORDER BY "tab�😺11710".col3_0 DESC NULLS LAST, "tab�😺11710".col3_5 NULLS FIRST, "tab�😺11710"."co'l3_6" ASC NULLS LAST LIMIT 2:::INT8;
SELECT '1971-02-05 22:57:53.000533':::TIMESTAMP AS col😎28024, p7 AS "c	ol28025", st_geogfromgeojson(e'\x01U':::STRING::STRING)::GEOGRAPHY AS "c%pol28026", p2 AS col28027, regclass(p1::OID)::REGCLASS AS "😅c%ol28028", '''yRyhokDfkg''':::TSQUERY AS "co""l28029", p0 AS "co'l28030";
SELECT 4.502830228560990032:::DECIMAL AS col28031, 0:::OID AS "col
28032", 970746649:::OID AS "cOl28033" FROM defaultdb.public.table1@table1_co̯l1_8_key AS tab11711, defaultdb.public."table 2"@"table 2_col2_1_.col2_0_idx" AS tab11712 JOIN defaultdb.public."tabl\\x96e3"@"tabl\\x96e3_col3_3_col""3_4_expr_col3_5_co%ql3_1_co"" l\g3_10_col3_2_key" AS tab11713 ON (tab11712.tableoid) = (tab11713."co%ql3_1") WHERE NULL LIMIT 30:::INT8;
SELECT p5 AS col28034 FROM defaultdb.public."table 2"@"table 2_expr_col2_1_idx" AS "t(a\\U0003910Cb😙11714" LIMIT 15:::INT8;
SELECT " ta\fb\\U000C6A3811715".col1_7 AS col28035, p3 AS "c""ol28036", md5(p2::BYTES)::STRING AS col28037, '01030000E0E6100000010000000C0000004A5DF8BFCDC161C09EB475EDC00B43C0EC903E207176E741C2D3BA46290601C2806078ED2BA43140700B08FBE69A3CC092C881C05E60F8C1F00FBCCB66CA0042C476A3E3020D55401997CA1C81D255C070737C24AE0FEE41FC514311E7F2E84184312C30482E524000B842E0729344C0C0E487AA3DF2E641F64EE57188FFE3C190B8DF05C9845D402A98F570DDD453C0F41C5F828038EA41E0D0898EB28AF041DC555FECDEF85640F8F03BE91CE837C00A3666A59401024228CE0304F255DC4196C4D1869C3F5140C0DADDD73CB807409C77C17770B4E341F0C5D1FD8DEAC2C18823A88CADD25940E057D8E0321D45400C7E5C0CAF1CE6C1753BD810EF9001C2E4F2CDBFD3DF5B405A0AF0075B8353408C0ABA8C073F02C2D0A5635B94F7CEC1CCC3534C70E243403C6F5F6A1ED64140AA22CA483075F7C1D272DE6ED691EAC1A7ADAC83672F66C0BC21F37D32894540B89928E0B2C9E841DA103C055E12EAC14A5DF8BFCDC161C09EB475EDC00B43C0EC903E207176E741C2D3BA46290601C2':::GEOGRAPHY AS col28038, p5 AS "Col28039", " ta\fb\\U000C6A3811715"."""col1_2" AS "col�28040" FROM defaultdb.public.table1@[0] AS " ta\fb\\U000C6A3811715" ORDER BY " ta\fb\\U000C6A3811715"."'co
l😶1_6" ASC NULLS FIRST;
SELECT '0102000020E61000000900000045601746204661C0C199104CA07D49C0E3DE781FAA875DC0CBDB682CF6F948C0C8BD1EF641EF38C04E7082AE1AD93FC000A8111949B23D402A62B99E1D5A52C0C264C25626F862402C4C9FE585C83FC0E09C7F0D735A59409C3BC58474634A400A7E1BBD15DF5040B6355AA997F84140C0C1BA0A4D052440DCF5A7C1B946504058B86881AD3B58C058343AC593D65040':::GEOGRAPHY AS col28041, 3994915626:::OID AS col28042, NULL AS col_28043, "t%qAb%q11716".😆col3_9 AS _col28044, p8 AS cͥol͢28045 FROM defaultdb.public."tabl\\x96e3"@"tabl\\x96e3_col3_2_expr_😆col3_9_col3_5_co"" l\g3_10_col3_8_col3_3_co'l3_6_co%ql3_1_cOl3_7_idx" AS "t%qAb%q11716" LIMIT 66:::INT8;
SELECT (NULL,) AS "c ol28047" FROM defaultdb.public."tabl\\x96e3"@"tabl\\x96e3_pkey" AS "tab\\x6d11717", defaultdb.public."table 2"@[0] AS tab11718 WHERE (SELECT 'mtyfv':::rand_typ_0 AS col28046 FROM defaultdb.public.table1@table1_co̯l1_8_key AS "ta&b11719" WHERE (NOT false) LIMIT 1:::INT8)::rand_typ_0 IN ('kcvlu':::rand_typ_0,)$funcbody$;
test artifacts and logs in: /artifacts/sqlsmith/setup=rand-tables/setting=no-mutations/run_1

Parameters: ROACHTEST_arch=amd64 , ROACHTEST_cloud=gce , ROACHTEST_cpu=4 , ROACHTEST_encrypted=false , ROACHTEST_ssd=0

Help

See: roachtest README

See: How To Investigate (internal)

/cc @cockroachdb/sql-queries

This test on roachdash | Improve this report!

Jira issue: CRDB-28946

@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. labels Jun 21, 2023
@cockroach-teamcity cockroach-teamcity added this to the 23.1 milestone Jun 21, 2023
@cockroach-teamcity cockroach-teamcity added this to Triage in SQL Queries Jun 21, 2023
@blathers-crl blathers-crl bot added the T-sql-queries SQL Queries Team label Jun 21, 2023
@mgartner mgartner removed this from Triage in SQL Queries Jun 21, 2023
@mgartner mgartner added this to Triage in SQL Foundations via automation Jun 21, 2023
@blathers-crl blathers-crl bot added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label Jun 21, 2023
@yuzefovich yuzefovich changed the title roachtest: sqlsmith/setup=rand-tables/setting=no-mutations failed roachtest/sqlsmith: internal error: building declarative schema change targets for CREATE FUNCTION Jun 21, 2023
@yuzefovich yuzefovich removed the T-sql-queries SQL Queries Team label Jun 21, 2023
@chengxiong-ruan
Copy link
Contributor

A simpler repro:

CREATE TYPE e1 AS ENUM ('a', 'b');

CREATE FUNCTION f() RETURNS INT
LANGUAGE SQL
AS $$
  SELECT 1 WHERE  (SELECT 'a')::e1 IS NOT NULL;
$$;

And, this is also a problem for Views, for example, this would fail with the same error:

CREATE VIEW v AS
  SELECT 1 WHERE (SELECT 'a')::e1 IS NOT NULL;

@chengxiong-ruan
Copy link
Contributor

Looks like this happens when we try to serialize UDT reference into IDs, and it failed to find the type annotations for some reason, and looks like only happens on type casting on query. A even simpler repro:

CREATE VIEW v AS
SELECT (SELECT 'a')::e1;

@chengxiong-ruan
Copy link
Contributor

chengxiong-ruan commented Jun 28, 2023

So the problem is that we do TypeCheck on a subquery during the serialization and the TypeCheck of the subquery only assert on the type annotation. But...looks like we never resolve the type of the subquery during this serialization path...(because it's outside of optbuilder?)

@rafiss
Copy link
Collaborator

rafiss commented Jun 30, 2023

@chengxiong-ruan I'm pretty sure I already resolved this issue for UDFs: #105465 fixes #104242

Do we need something similar for views?

@chengxiong-ruan
Copy link
Contributor

@rafiss I think this is a different issue than you fix :(. the annotation is a different thing than the annotation in your fix I think, I believe I was able to repro the error with your fix on master...

@rafiss
Copy link
Collaborator

rafiss commented Jun 30, 2023

ah yeah you're right. the new thing in this issue is the subquery.

@mgartner
Copy link
Collaborator

This reproduces all the way back to v21.2, but not on v21.1. @chengxiong-ruan are you working on a fix, or would you like me to look into it?

@mgartner
Copy link
Collaborator

I think I have a solution. Should have a PR up soon.

@rafiss rafiss assigned mgartner and unassigned chengxiong-ruan Jul 11, 2023
@rafiss rafiss removed this from Triage in SQL Foundations Jul 11, 2023
@rafiss rafiss added T-sql-queries SQL Queries Team and removed T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) labels Jul 11, 2023
@blathers-crl blathers-crl bot added this to Triage in SQL Queries Jul 11, 2023
@msirek msirek moved this from Triage to Active in SQL Queries Jul 11, 2023
@chengxiong-ruan
Copy link
Contributor

@mgartner no I don't have a proper solution on the schema changer side. Thanks for working on this. It seems tricky to me.

mgartner added a commit to mgartner/cockroach that referenced this issue Jul 11, 2023
Prior to this commit, the optimizer serialized the `CREATE VIEW` query
AST after performing static analysis of the view query. Now, the AST is
plumbed to the planNode responsible for executing this DDL statement.
This is more consistent with how other DML statements are planned, like
`CREATE FUNCTION`. This change will:

1. Allow us to address cockroachdb#105259 in a future commit more easily.
2. Allow for greater flexibility within the execution logic.
3. Reduce redundant computation, like re-parsing and type-checking the
   AST.

Release note: None
mgartner added a commit to mgartner/cockroach that referenced this issue Jul 11, 2023
This commit fixes a bug that was caused when attempting to re-type-check
a view's query that contains a subquery. This type-checking occurred
outside of optbuilder. However, the logic for type-checking subqueries
is only implemented within optbuilder, so it failed.

This commit fixes the issue by avoid the duplicate type-checking
altogether. It should not be necessary, but it serviced two important
functions:

1. Serializing user-defined types as their descriptor IDs. This is now
   accomplished by formatting the view query's AST with the
   `FmtSerializable` option.
2. Checking for cross-database type references. This is now done with a
   new function that traverses the view query's AST without performing
   type-checking. In the future, this should happen while optbuilder
   performs static analysis, and I've left a TODO that mentions this.

This commit does not address a similar bug that affects UDFs. It will be
fixed in a future commit.

Informs cockroachdb#105259

Release note (bug fix): A bug has been fixed that caused internal errors
when using user-defined types in views that have subqueries. This bug
was present since version v21.2.
mgartner added a commit to mgartner/cockroach that referenced this issue Jul 12, 2023
This commit fixes a bug that was caused when attempting to re-type-check
a view's query that contains a subquery. This type-checking occurred
outside of optbuilder. However, the logic for type-checking subqueries
is only implemented within optbuilder, so it failed.

This commit fixes the issue by avoid the duplicate type-checking
altogether. It should not be necessary, but it serviced two important
functions:

1. Serializing user-defined types as their descriptor IDs. This is now
   accomplished by formatting the view query's AST with the
   `FmtSerializable` option.
2. Checking for cross-database type references. This is now done with a
   new function that traverses the view query's AST without performing
   type-checking. In the future, this should happen while optbuilder
   performs static analysis, and I've left a TODO that mentions this.

This commit does not address a similar bug that affects UDFs. It will be
fixed in a future commit.

Informs cockroachdb#105259

Release note (bug fix): A bug has been fixed that caused internal errors
when using user-defined types in views that have subqueries. This bug
was present since version v21.2.
mgartner added a commit to mgartner/cockroach that referenced this issue Jul 14, 2023
This commit fixes a bug that was caused when attempting to re-type-check
a view's query that contains a subquery. This type-checking occurred
outside of optbuilder. However, the logic for type-checking subqueries
is only implemented within optbuilder, so it failed.

Fixes cockroachdb#105259

Release note (bug fix): A bug has been fixed that caused internal errors
when using user-defined types in views and user-defined functions that
have subqueries. This bug was present when using views since version
v21.2. It was present when using user-defined functions since v23.1.
craig bot pushed a commit that referenced this issue Jul 17, 2023
106824: Revert "roachprod: provide workaround for long-running AWS clusters" r=herkolategan a=renatolabs

This reverts commit e6cf25d.

The oldest `aws` cluster at the time of this commit is ~17d old, long after the hostname validation was introduced.

Since we now have an easy way to update roachprod (`roachprod update`), it should be straightforward for anyone to be running updated versions of roachprod that include hostname validation.

Epic: none

Release note: None

106868: sql: do not type-check subqueries outside of optbuilder r=mgartner a=mgartner

This commit fixes a bug that was caused when attempting to re-type-check
a view's query that contains a subquery. This type-checking occurred
outside of optbuilder. However, the logic for type-checking subqueries
is only implemented within optbuilder, so it failed.

Fixes #105259

Release note (bug fix): A bug has been fixed that caused internal errors
when using user-defined types in views and user-defined functions that
have subqueries. This bug was present when using views since version
v21.2. It was present when using user-defined functions since v23.1.


106871: ts: correctly set Columnar flag on memory context r=yuzefovich a=yuzefovich

This flag is used to determine the size of the slab, and it wasn't set properly since 2018. The impact is that for non-rollup we'll be able to retrieve more samples in a single operation because the columnar slab size is smaller (in a test I observed 4.5kb vs 14.5kv for the row format slab).

Epic: None

Release note: None

106881: bench, collector: remove usages of base.TODOTestTenantDisabled r=yuzefovich a=yuzefovich

We have four variants of running cockroach in benchmarks, two of them create tenants on their own, so now they are marked as explicitly controlling tenants. Eventually, we'll want to transition `benchmarkCockroach` and `benchmarkMultinodeCockroach` to use tenants too, and that work is tracked by #83461.

This commit also removes a single usage of the TODO value in `util/tracing/collector` where the test itself creates tenants.

Addresses: #76378.
Epic: CRDB-18499

Release note: None

106898: serverccl: mark TestAdminAPIDataDistributionPartitioning as problematic r=yuzefovich a=knz

Informs #76378.
Epic: CRDB-18499.
Followup issue: #106897.

Release note: None

106921: github-pull-request-make: stress if test was unskipped via `skip` pkg r=rail a=rickystewart

Also remove unnecessary retries that were added because `yarn install` was flaky (we don't do `yarn install` as part of the build any more).

Epic: none
Release note: None
Closes: #106914

Co-authored-by: Renato Costa <renato@cockroachlabs.com>
Co-authored-by: Marcus Gartner <marcus@cockroachlabs.com>
Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net>
Co-authored-by: Ricky Stewart <ricky@cockroachlabs.com>
@craig craig bot closed this as completed in bd38886 Jul 17, 2023
SQL Queries automation moved this from Active to Done Jul 17, 2023
mgartner added a commit to mgartner/cockroach that referenced this issue Jul 21, 2023
This commit fixes a bug that was caused when attempting to re-type-check
a view's query that contains a subquery. This type-checking occurred
outside of optbuilder. However, the logic for type-checking subqueries
is only implemented within optbuilder, so it failed.

Fixes cockroachdb#105259

Release note (bug fix): A bug has been fixed that caused internal errors
when using user-defined types in views that have subqueries. This bug
was present when using views since version v21.2.
mgartner added a commit to mgartner/cockroach that referenced this issue Jul 24, 2023
This commit fixes a bug that was caused when attempting to re-type-check
a view's query that contains a subquery. This type-checking occurred
outside of optbuilder. However, the logic for type-checking subqueries
is only implemented within optbuilder, so it failed.

Fixes cockroachdb#105259

Release note (bug fix): A bug has been fixed that caused internal errors
when using user-defined types in views and user-defined functions that
have subqueries. This bug was present when using views since version
v21.2. It was present when using user-defined functions since v23.1.
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.

5 participants