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

ResolveFunctionByOID function not found with optimizer rules disabled #98373

Closed
cockroach-teamcity opened this issue Mar 10, 2023 · 10 comments · Fixed by #99180
Closed

ResolveFunctionByOID function not found with optimizer rules disabled #98373

cockroach-teamcity opened this issue Mar 10, 2023 · 10 comments · Fixed by #99180
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. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. T-sql-queries SQL Queries Team
Milestone

Comments

@cockroach-teamcity
Copy link
Member

cockroach-teamcity commented Mar 10, 2023

roachtest.unoptimized-query-oracle/disable-rules=all/seed-multi-region failed with artifacts on master @ d4a584e49f0b1ca89738376090939d7669c3b3db:

test artifacts and logs in: /artifacts/unoptimized-query-oracle/disable-rules=all/seed-multi-region/run_1
(query_comparison_util.go:251).runOneRoundQueryComparison: internal error while running optimized statement. 1204 statements run: pq: internal error: regproc(): ResolveFunctionByOID unimplemented

Parameters: 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-25232

@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 10, 2023
@cockroach-teamcity cockroach-teamcity added this to the 23.1 milestone Mar 10, 2023
@blathers-crl blathers-crl bot added the T-sql-queries SQL Queries Team label Mar 10, 2023
@msirek
Copy link
Contributor

msirek commented Mar 11, 2023

Reproducible test case:

SET testing_optimizer_random_seed = 4225827507322334784;

SET testing_optimizer_disable_rule_probability = 1.000000;

SET vectorize = off;

CREATE TABLE IF NOT EXISTS seed_mr_table AS
        SELECT
                g::INT2 AS _int2,
                g::INT4 AS _int4,
                g::INT8 AS _int8,
                g::FLOAT8 AS _float8,
                '2001-01-01'::DATE + g AS _date,
                '2001-01-01'::TIMESTAMP + g * '1 day'::INTERVAL AS _timestamp,
                '2001-01-01'::TIMESTAMPTZ + g * '1 day'::INTERVAL AS _timestamptz,
                g * '1 day'::INTERVAL AS _interval,
                g % 2 = 1 AS _bool,
                g::DECIMAL AS _decimal,
                g::STRING AS _string,
                g::STRING::BYTES AS _bytes,
                substring('00000000-0000-0000-0000-' || g::STRING || '00000000000', 1, 36)::UUID AS _uuid
        FROM
                generate_series(1, 5) AS g;

EXPLAIN SELECT
        '=r2':::STRING:::NAME AS "
col857",
        "ta b276"._string AS "co%ql858",
        "ta b276"._int4 AS col859,
        " tab 278"._interval AS col860,
        "ta\v😸b279".tableoid AS col861,
        " tab 278"._interval AS col862,
        "ta\v😸b279"._float8 AS "col%863",
        0.7427599033731295:::FLOAT8 AS c😠ol864,
        regproc("ta\v😸b279"._int2::INT8)::REGPROC AS col865,
        "ta\v😸b279"._int2 AS col866,
        tab277._date AS "col^K867",
        12345.67890123456:::FLOAT8 AS "co^Kl868",
        'aaaaaa':::STRING AS col869,
        'infinity':::DATE AS col870,
        tab277._float8 AS col871,
        4046:::INT8 AS col872,
        (-78):::INT8 AS col873,
        "ta b276"._timestamp AS col874
FROM
        defaultdb.public.seed_mr_table@[0] AS "ta b276"
        JOIN defaultdb.public.seed_mr_table@[0] AS tab277 ON
                        ("ta b276"._int4) = (tab277._int4) AND ("ta b276"._int8) = (tab277._int4)
        JOIN defaultdb.public.seed_mr_table@[0] AS " tab 278"
                FULL JOIN defaultdb.public.seed_mr_table@[0] AS "ta\v😸b279" ON
                                (" tab 278"._interval) = ("ta\v😸b279"._interval)
                                AND (" tab 278".crdb_internal_mvcc_timestamp) = ("ta\v😸b279".crdb_internal_mvcc_timestamp) ON
                        ("ta b276"._int4) = (" tab 278"._int8)
WHERE
        (bool("ta b276"._int4::INT8)::BOOL OR (NOT ('o':::STRING ILIKE "ta\v😸b279"._string)));

ERROR: internal error: regproc(): ResolveFunctionByOID unimplemented
SQLSTATE: XX000
DETAIL: stack trace:
github.com/cockroachdb/cockroach/pkg/sql/faketreeeval/evalctx.go:457: ResolveFunctionByOID()
github.com/cockroachdb/cockroach/pkg/sql/sem/eval/cast.go:1001: performIntToOidCast()
github.com/cockroachdb/cockroach/pkg/sql/sem/eval/cast.go:920: performCastWithoutPrecisionTruncation()
github.com/cockroachdb/cockroach/pkg/sql/sem/eval/cast.go:68: PerformCast()
github.com/cockroachdb/cockroach/pkg/sql/sem/builtins/pg_builtins.go:164: 1()
github.com/cockroachdb/cockroach/pkg/sql/sem/eval/expr.go:479: EvalFuncExpr()
github.com/cockroachdb/cockroach/bazel-out/darwin_arm64-fastbuild/bin/pkg/sql/sem/tree/eval_expr_generated.go:276: Eval()
github.com/cockroachdb/cockroach/pkg/sql/sem/eval/expr.go:26: Expr()
github.com/cockroachdb/cockroach/pkg/sql/execinfrapb/pkg/sql/execinfrapb/expr.go:248: Eval()
github.com/cockroachdb/cockroach/pkg/sql/execinfra/processorsbase.go:275: ProcessRow()
github.com/cockroachdb/cockroach/pkg/sql/execinfra/processorsbase.go:711: ProcessRowHelper()
github.com/cockroachdb/cockroach/pkg/sql/rowexec/filterer.go:104: Next()
github.com/cockroachdb/cockroach/pkg/sql/execinfra/base.go:186: Run()
github.com/cockroachdb/cockroach/pkg/sql/execinfra/processorsbase.go:733: Run()
github.com/cockroachdb/cockroach/pkg/sql/flowinfra/flow.go:447: func1()
GOROOT/src/runtime/asm_arm64.s:1172: goexit()

On v22.2.6 a different error occurs:

ERROR: relation "defaultdb.public.seed_mr_table" does not exist

Removing release blocker label. Can only be reproduced by disabling normalizations.

@msirek msirek removed the release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. label Mar 11, 2023
@msirek msirek changed the title roachtest: unoptimized-query-oracle/disable-rules=all/seed-multi-region failed ResolveFunctionByOID function not found with optimizer rules disabled Mar 12, 2023
@mgartner
Copy link
Collaborator

@otan I think this is related to #97093. Do you mind taking a look?

@otan
Copy link
Contributor

otan commented Mar 14, 2023

Hmm, not sure about this one or at least it's caused by me.

This is evaluated using &DummyEvalPlanner{}, which only happens here:

Planner: &faketreeeval.DummyEvalPlanner{Monitor: monitor},
- in theory this is a problem if you just did ("ta\v😸b279"._int2::INT8)::REGPROC AS col865.

the minimal repro above doesn't work for me:

ERROR: no data source matches prefix: " tab 278" in this context
SQLSTATE: 42P01

@mgartner mgartner added the release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. label Mar 17, 2023
@mgartner
Copy link
Collaborator

Adding release-blocker label until we have a smaller repro and understand this better.

@DrewKimball
Copy link
Collaborator

@msirek was it necessary to use a multiregion cluster to reproduce this? It doesn't reproduce for me on a single-node cluster on current master.

@msirek
Copy link
Contributor

msirek commented Mar 20, 2023

@msirek was it necessary to use a multiregion cluster to reproduce this? It doesn't reproduce for me on a single-node cluster on current master.

Yes, it's possible I was running a multiregion cluster at the time. Let me try reproducing this again.

@msirek
Copy link
Contributor

msirek commented Mar 20, 2023

The repro isn't working for me now, so I'm re-running the full setup script.

@msirek
Copy link
Contributor

msirek commented Mar 20, 2023

I can't reproduce this bug now, even with running the full setup. Not sure what's changed on my machine.
I'm wondering if this is related to #98322, where we don't seem to call the function, and don't error out, but instead return the input to the function as the result.

If this issue can't be reproduced, maybe we should close it as a dup of #98322, and fix that one instead. That one may be worse because it causes incorrect results instead of erroring out the query.

@cucaroach
Copy link
Contributor

Maybe some of these OID functions should be in distsql block list.

@msirek
Copy link
Contributor

msirek commented Mar 21, 2023

OK, here's a logic test that hits the bug in the fakedist-disk config:

statement ok
SET vectorize = off;

statement ok
CREATE TABLE IF NOT EXISTS seed_mr_table AS
        SELECT
                g::INT2 AS _int2,
                g::INT4 AS _int4,
                g::INT8 AS _int8
        FROM
                generate_series(1, 5) AS g;

query T
SELECT
        regproc(_int2::INT8)::REGPROC
FROM
        seed_mr_table@[0]
----

msirek pushed a commit to msirek/cockroach that referenced this issue Mar 22, 2023
…k list

Distributed SQL which executes functions or casts to OID rely on
`planner` receiver functions to execute internal SQL to get information
about the OID from system tables. If these casts occur on a remote
processor, the `planner` is not accessible and a dummy planner is used,
which does not implement these receiver functions. To prevent internal
errors, these casts or problem functions are added to a distSQL block
list by `distSQLExprCheckVisitor`. A cast to an OID is can also be done via
a builtin function of the same name as the target type, e.g. `regproc`. These
builtins do not currently have `DistsqlBlocklist` set, allowing distributed
execution.

The solution is to mark `DistsqlBlocklist` as true for any builtin
function which casts to an OID type.

Fixes cockroachdb#98373

Release note: None
craig bot pushed a commit that referenced this issue Mar 22, 2023
…99239 #99263 #99278

98980: kvcoord: Add metric to keep track of restarted ranges in rangefeed r=miretskiy a=miretskiy

Add a `distsender.rangefeed.restart_ranges` metric to keep track of the number of ranges restarted due to transient error.

Epic: CRDB-25044
Release note: None

99069: storage/cloud: correct the flag name in implicit credentials error message r=rhu713 a=taroface

When `--external-io-disable-implicit-credentials` is set and the user issues a command with `AUTH=implicit`, the resulting error message has the wrong flag name (`disable` is left out). Searching for that flag name in the docs doesn't return any results. The flag name is corrected in this PR.

Release note: none
Release justification: CLI bug

99077: changefeedccl: Allow timeout override r=miretskiy a=miretskiy

Add timeout URL parameter for schema registry URIs. Prior to this change, all schema registry calls used default time out of 3 seconds.  This PR increases the timeout to 30 seconds, and allows timeout to be specified via `timeout=T` URL parameter.

Informs https://github.com/cockroachlabs/support/issues/2173

Release note (enterprise change): AVRO schema registry URI allow additional `timeout=T` query parameter to change the default timeout for contacting schema registry.

99141: storage: CheckSSTConflict fix for nexting over overlapping points r=jbowens a=itsbilal

Previously, the nexting logic around both iterators being at a range key and not a point key was flawed in that we'd miss ext points that were in between
the current and next sst keys, when we'd next both of them. This change addresses that.

It also addresses other miscellaneous corner cases around stats calculations with overlapping sst/engine range keys and point keys. All these bugs were found with the upcoming CheckSSTConflicts randomized test in #98408.

Epic: none

Release note: None

99146: opt: speed up lookup constraint builder r=mgartner a=mgartner

#### opt: add benchmark with many lookup joins

This commit adds an optimizer benchmark that explores many lookup joins.
It explores many potential lookup joins that do not ultimately get added
to the memo, as well as many lookup joins that do get added to the memo.

Release note: None

#### opt: split HasSingleColumnConstValues into two functions

This commit splits HasSingleColumnConstValues into two functions - one
that returns a boolean if a constraint set constrains a single column to
a set of constant, non-null values, and another function that returns
the constant values. The former is more efficient when the only the
boolean is needed.

Release note: None

#### opt: simplify lookup join constraint builder

This commit reduces computation and allocations when attempting to
build lookup join constraints by performing a simple column ID equality
before more complex computations and allocations.

Release note: None

#### opt: reduce allocations when building lookup join constraints

During the construction of lookup join constraints, two allocations of a
`opt.ColList` have been combined into a single allocation, and
allocation of a `memo.FiltersExpr` to store remaining filters is now
only performed if necessary.

Release note: None

These changes offer a nice speedup for the newly added benchmark:

```
name                         old time/op    new time/op    delta
SlowQueries/slow-query-1-10    15.8ms ± 1%    15.7ms ± 1%     ~     (p=0.690 n=5+5)
SlowQueries/slow-query-2-10     220ms ± 0%     219ms ± 0%     ~     (p=0.095 n=5+5)
SlowQueries/slow-query-3-10    63.0ms ± 1%    62.4ms ± 0%   -0.98%  (p=0.008 n=5+5)
SlowQueries/slow-query-4-10     1.70s ± 1%     1.38s ± 0%  -19.22%  (p=0.008 n=5+5)

name                         old alloc/op   new alloc/op   delta
SlowQueries/slow-query-1-10    7.04MB ± 0%    6.98MB ± 0%   -0.79%  (p=0.008 n=5+5)
SlowQueries/slow-query-2-10    48.7MB ± 0%    48.7MB ± 0%   -0.11%  (p=0.008 n=5+5)
SlowQueries/slow-query-3-10    45.1MB ± 0%    44.9MB ± 0%   -0.55%  (p=0.008 n=5+5)
SlowQueries/slow-query-4-10     878MB ± 0%     737MB ± 0%  -16.03%  (p=0.008 n=5+5)

name                         old allocs/op  new allocs/op  delta
SlowQueries/slow-query-1-10     76.1k ± 0%     75.8k ± 0%   -0.38%  (p=0.008 n=5+5)
SlowQueries/slow-query-2-10      401k ± 0%      400k ± 0%   -0.25%  (p=0.008 n=5+5)
SlowQueries/slow-query-3-10      390k ± 0%      389k ± 0%   -0.21%  (p=0.008 n=5+5)
SlowQueries/slow-query-4-10     18.2M ± 0%     17.4M ± 0%   -4.44%  (p=0.008 n=5+5)
```

Epic: None


99154: ui: stop polling in stmt fingerprint details page, change default sort on stmts r=maryliag a=xinhaoz

See individual commits.

https://www.loom.com/share/17569db4a0c04a968dabbc4421d429bf

99169: kv: unflake TestDelegateSnapshot r=kvoli a=andrewbaptist

Fixes: #96841
Fixes: #96525

Previously this test would assume that all snapshots came from the sending of snapshots through the AdminChangeReplicasRequest which end up as type OTHER. However occassionally we get a spurious raft snapshot which makes this test flaky. This change ignores any raft snapshots that are sent.

Epic: none
Release note: None

99172: upgrades: hardcode descriptors in system_rbr_indexes r=JeffSwenson a=JeffSwenson

Previously, if a change was made to the system.sql_instances, system.lease, or system.sqlliveness bootstrap schema, it would change the behavior of the upgrade attached to the V23_1_SystemRbrReadNew version gate.

Now, the content of the descriptors is hard coded in the upgrade so that the behavior is not accidentally changed in the future.

Fixes: #99074

Release note: None

99180: builtins: add builtin functions which cast to OID to the distSQL block list r=michae2,cucaroach a=msirek

Distributed SQL which executes functions or casts to OID rely on `planner` receiver functions to execute internal SQL to get information about the OID from system tables. If these casts occur on a remote processor, the `planner` is not accessible and a dummy planner is used, which does not implement these receiver functions. To prevent internal errors, these casts or problem functions are added to a distSQL block list by `distSQLExprCheckVisitor`. A cast to an OID can also be done via a builtin function of the same name as the target type, e.g. `regproc`. These builtins do not currently have `DistsqlBlocklist` set, allowing distributed execution.

The solution is to mark `DistsqlBlocklist` as true for any builtin function which casts to an OID type.

Fixes #98373

Release note: None

99239: appstats: fix percentile greater than max latency r=maryliag a=maryliag

Part Of #99070
When an execution happens, its latency is added to a stream and then ordered so percentiles can be queried.
When getting the percentile values, we don't have the timestamp of when each value was added, meaning when we query the stream we could be getting values from a previous aggregation timestamp, if the current windows has very few executions (the stream has a limit, so we only have the most recent execution, but if the statement is not run frequently this stream can have old data).

The way this information is stored will need to be changed, but for now a patchy solution was added so we don't have the case where we show percentiles greater than the actual max.

Release note (bug fix): Add a check so percentiles are never greater than the max latency value.

99263: roachtest: copyfrom fix cluster package install r=aliher1911 a=aliher1911

When installing packages, look on cluster remoteness as proxy for arch instead of roachtest runtime which is runs different arch.

Epic: none

Release note: None

99278: sql: fix update helper optional from clause r=rytaft a=lyang24

fixes #98662

sanity testing:
output
<img width="265" alt="Screen Shot 2023-03-22 at 1 01 10 PM" src="https://user-images.githubusercontent.com/20375035/227027849-34f34bb4-d52b-4de4-8a5b-456ee8b27f1b.png">
sample sql
<img width="874" alt="Screen Shot 2023-03-22 at 1 10 15 PM" src="https://user-images.githubusercontent.com/20375035/227027874-3f6515f4-fdbe-4c64-9d6f-03eb9b5c67f3.png">


Release note (sql change): fix helper message on update sql to correctly position the optional from cause.

Co-authored-by: Yevgeniy Miretskiy <[email protected]>
Co-authored-by: Ryan Kuo <[email protected]>
Co-authored-by: Bilal Akhtar <[email protected]>
Co-authored-by: Marcus Gartner <[email protected]>
Co-authored-by: Xin Hao Zhang <[email protected]>
Co-authored-by: Andrew Baptist <[email protected]>
Co-authored-by: Jeff <[email protected]>
Co-authored-by: Mark Sirek <[email protected]>
Co-authored-by: maryliag <[email protected]>
Co-authored-by: Oleg Afanasyev <[email protected]>
Co-authored-by: Eric.Yang <[email protected]>
@craig craig bot closed this as completed in b837c30 Mar 22, 2023
blathers-crl bot pushed a commit that referenced this issue Mar 22, 2023
…k list

Distributed SQL which executes functions or casts to OID rely on
`planner` receiver functions to execute internal SQL to get information
about the OID from system tables. If these casts occur on a remote
processor, the `planner` is not accessible and a dummy planner is used,
which does not implement these receiver functions. To prevent internal
errors, these casts or problem functions are added to a distSQL block
list by `distSQLExprCheckVisitor`. A cast to an OID is can also be done via
a builtin function of the same name as the target type, e.g. `regproc`. These
builtins do not currently have `DistsqlBlocklist` set, allowing distributed
execution.

The solution is to mark `DistsqlBlocklist` as true for any builtin
function which casts to an OID type.

Fixes #98373

Release note: None
msirek pushed a commit to msirek/cockroach that referenced this issue Mar 23, 2023
This moves the EXPLAIN test of cockroachdb#98373 from logictest to execbuilder.

Informs cockroachdb#98373

Release note: None
craig bot pushed a commit that referenced this issue Mar 23, 2023
99396: execbuilder: move explain test out of logictest r=msirek a=msirek

This moves the EXPLAIN test of #98373 from logictest to execbuilder.

Informs #98373

Release note: None

Co-authored-by: Mark Sirek <[email protected]>
msirek pushed a commit to msirek/cockroach that referenced this issue Mar 26, 2023
…k list

Distributed SQL which executes functions or casts to OID rely on
`planner` receiver functions to execute internal SQL to get information
about the OID from system tables. If these casts occur on a remote
processor, the `planner` is not accessible and a dummy planner is used,
which does not implement these receiver functions. To prevent internal
errors, these casts or problem functions are added to a distSQL block
list by `distSQLExprCheckVisitor`. A cast to an OID is can also be done via
a builtin function of the same name as the target type, e.g. `regproc`. These
builtins do not currently have `DistsqlBlocklist` set, allowing distributed
execution.

The solution is to mark `DistsqlBlocklist` as true for any builtin
function which casts to an OID type.

Fixes cockroachdb#98373

Release note: None
msirek pushed a commit to msirek/cockroach that referenced this issue Mar 26, 2023
This moves the EXPLAIN test of cockroachdb#98373 from logictest to execbuilder.

Informs cockroachdb#98373

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. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. T-sql-queries SQL Queries Team
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

7 participants