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: ResolveFunctionByOID unimplemented #86075

Closed
cockroach-teamcity opened this issue Aug 13, 2022 · 7 comments · Fixed by #86186
Closed

roachtest/sqlsmith: ResolveFunctionByOID unimplemented #86075

cockroach-teamcity opened this issue Aug 13, 2022 · 7 comments · Fixed by #86186
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 Aug 13, 2022

roachtest.sqlsmith/setup=rand-tables/setting=no-ddl failed with artifacts on master @ 0dd438d3dc0b42543890455945a7a6b42811def1:

test artifacts and logs in: /artifacts/sqlsmith/setup=rand-tables/setting=no-ddl/run_1
	sqlsmith.go:265,sqlsmith.go:325,test_runner.go:896: error: pq: internal error: ResolveFunctionByOID unimplemented
		stmt:
		SELECT
			NULL AS col_84195,
			tab_42278.col1_4 AS col_84196,
			(-0.2449352525117256):::FLOAT8 AS col_84197,
			(SELECT tab_42279.col5_3 AS col_84198 FROM defaultdb.public.table5@[0] AS tab_42279 LIMIT 1:::INT8) AS col_84199
		FROM
			defaultdb.public.table1@[0] AS tab_42278
		ORDER BY
			tab_42278.col1_8 DESC, tab_42278.col1_15 DESC, tab_42278.col1_13;

Parameters: ROACHTEST_cloud=gce , ROACHTEST_cpu=4 , 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-18555

@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 Aug 13, 2022
@cockroach-teamcity cockroach-teamcity added this to the 22.2 milestone Aug 13, 2022
@blathers-crl blathers-crl bot added the T-sql-queries SQL Queries Team label Aug 13, 2022
@yuzefovich yuzefovich changed the title roachtest: sqlsmith/setup=rand-tables/setting=no-ddl failed roachtest/sqlsmith: ResolveFunctionByOID unimplemented Aug 15, 2022
@yuzefovich
Copy link
Member

This looks like a problem with using faketreeeval.DummyEvalPlanner for something on a remote node. I was able to reproduce it on a 3 node local cluster with scattering the tables before the statement:

ERROR: internal error: ResolveFunctionByOID unimplemented
SQLSTATE: XX000
DETAIL: stack trace:
github.com/cockroachdb/cockroach/pkg/sql/faketreeeval/evalctx.go:449: ResolveFunctionByOID()
github.com/cockroachdb/cockroach/pkg/sql/sem/eval/cast.go:946: performIntToOidCast()
github.com/cockroachdb/cockroach/pkg/sql/sem/eval/cast.go:863: performCastWithoutPrecisionTruncation()
github.com/cockroachdb/cockroach/pkg/sql/sem/eval/cast.go:63: PerformCast()
github.com/cockroachdb/cockroach/bazel-out/darwin-fastbuild/bin/pkg/sql/colexec/colexecbase/cast.eg.go:14474: func1()
github.com/cockroachdb/cockroach/pkg/sql/colmem/allocator.go:349: PerformOperation()
github.com/cockroachdb/cockroach/bazel-out/darwin-fastbuild/bin/pkg/sql/colexec/colexecbase/cast.eg.go:14340: Next()
github.com/cockroachdb/cockroach/pkg/sql/colexec/colexecbase/simple_project.go:124: Next()
github.com/cockroachdb/cockroach/pkg/sql/colflow/stats.go:99: next()
github.com/cockroachdb/cockroach/pkg/sql/colexecerror/error.go:91: CatchVectorizedRuntimeError()
github.com/cockroachdb/cockroach/pkg/sql/colflow/stats.go:107: Next()
github.com/cockroachdb/cockroach/pkg/sql/colexec/colexecutils/deselector.go:53: Next()
github.com/cockroachdb/cockroach/pkg/sql/colflow/colrpc/outbox.go:289: func1()
github.com/cockroachdb/cockroach/pkg/sql/colexecerror/error.go:91: CatchVectorizedRuntimeError()
github.com/cockroachdb/cockroach/pkg/sql/colflow/colrpc/outbox.go:280: sendBatches()
github.com/cockroachdb/cockroach/pkg/sql/colflow/colrpc/outbox.go:414: runWithStream()
github.com/cockroachdb/cockroach/pkg/sql/colflow/colrpc/outbox.go:222: Run()
github.com/cockroachdb/cockroach/pkg/sql/colflow/vectorized_flow.go:751: func1()
github.com/cockroachdb/cockroach/pkg/sql/colflow/vectorized_flow.go:1342: 1()
GOROOT/src/runtime/asm_amd64.s:1571: goexit()

Generally speaking, we prohibit the usages of DistSQL when casting to OID, however, the vectorized engine can plan casts in some rare cases on its own, and I think this is where the problem lies:

r.Root, err = colexecbase.GetCastOperator(

@rafiss
Copy link
Collaborator

rafiss commented Aug 15, 2022

cc @chengxiong-ruan

@chengxiong-ruan
Copy link
Contributor

Thanks for letting know, @rafiss ! And, thanks for the investigation, @yuzefovich
Sorry that the "fake"/"dummy" word in the names made me think that the library was only for tests, so I didn't implement proper function resolution for DummyEvalPlanner. @yuzefovich do you see any cases that user defined functions need to be resolved in the "rare cases" you mentioned? If not, we may just implement it to resolve builtin functions.

@yuzefovich
Copy link
Member

At the moment, I think the bug is in the vectorized execution planning and we don't need to make DummyEvalPlanner actually implement anything.

@chengxiong-ruan
Copy link
Contributor

@yuzefovich
I've also recently run into an issue with some UDF execution logic tests under fakedist.* configs that tries to resolve a UDF but panic because there's no function resolver kinda of thing. I'm not familiar with DistSQL internal and not sure how should we fix this. Maybe we should add UDF resolver to DistSQL?
For example, a logic test like this would fail under fakedist config:

statement ok
CREATE TABLE ab (
a INT PRIMARY KEY,
b INT
)

statement ok
INSERT INTO ab VALUES (1, 1), (2, 2), (3, 3), (4, 1), (5, 1)

statement ok
CREATE FUNCTION one() RETURNS INT LANGUAGE SQL AS 'SELECT 2-1';

query III colnames
SELECT *, one() FROM ab WHERE b = one()
----
a  b  one
1  1  1
4  1  1
5  1  1

Thanks!

@chengxiong-ruan
Copy link
Contributor

oh, @ajwerner mentioned that we just set DistsqlBlocklist in FunctionProperties to true for now.

@yuzefovich
Copy link
Member

Yeah, that's the way to go.

craig bot pushed a commit that referenced this issue Aug 23, 2022
86186: sql: do not distribute queries with subqueries returning OIDs r=yuzefovich a=yuzefovich

If a subquery results in a DOid datum, the datum will get a type
annotation (because DOids are ambiguous) when serializing the
render expression involving the result of the subquery. As a
result, we might need to perform a cast on a remote node which
might fail, thus we prohibit the distribution of the main query.

Fixes: #86075.

Release justification: bug fix.

Release note: None

86357: colmem: improve memory accounting when memory limit is exceeded r=yuzefovich a=yuzefovich

**colmem: improve memory accounting when memory limit is exceeded**

This commit improves the memory accounting when memory limit is
exceeded. Previously, in several operators we could run into a situation
where we perform some allocations and run into a memory limit error
later, which results in those allocations being unaccounted for. In some
cases this is acceptable (when the query results in an error), but in
others the memory error is caught and spilling to disk occurs. In the
latter scenarios we would under-account, and this commit fixes most of
such situations.

Now, each disk-spilling operator instantiates a "limited" allocator that
will grow an unlimited memory account when a memory error is
encountered. The idea is that even though the denied allocations cannot
be registered with the main memory account (meaning the operator has
exceeded its memory limit), we still will draw from the
`--max-sql-memory` pool since the allocations can be live for
non-trivial amount of time. If an error occurs when growing the
unlimited memory account, then that error is returned (not the original
memory error) so that the disk spiller doesn't catch it.

This commit audits all operators in `execplan` to use the limited
allocator where appropriate. The new accounting method is only used in
a handful of places which cover most of the use cases. The goal is to
make this commit backportable whereas the follow-up commit will audit
usages of `AdjustMemoryUsage` and will not be backported.

Addresses: #64906.
Fixes: #86351.
Addresses: https://github.com/cockroachlabs/support/issues/1762.

Release justification: bug fix.

Release note: None

**colmem: audit callers of AdjustMemoryUsage**

This commit audits all callers of `Allocator.AdjustMemoryUsage` to use
the newly-exported `AdjustMemoryUsageAfterAllocation` where applicable
(meaning that if an allocation occurs before the method is called, then
the new method is now used). In many cases this won't result in a change
in the behavior since the allocators are not instantiated with limited
memory accounts, but in some cases it is still useful.

Release justification: bug fix.

Release note: None

86402: externalconn,amazon: support s3 KMS in External Connecetions r=benbardin a=adityamaru

Informs: #84228

Release note (sql change): Users can now
`CREATE EXTERNAL CONNECTION` to represent an `aws-kms`
scheme that represents an AWS KMS resource.

Release justification: low risk change to new functionality to register s3 KMS as a supported External Connection

86613: streamproducer: check the job type for replication stream r=yuzefovich a=yuzefovich

Previously, we would panic if the job id corresponded to a job type
different from the replication stream job, and this is now fixed.

Fixes: #86508.

Release justification: bug fix.

Release note: None

Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: Aditya Maru <[email protected]>
@craig craig bot closed this as completed in #86186 Aug 23, 2022
@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.

4 participants