Skip to content

sql: fix crash when using a partial index with predicate that has a UDF#171000

Open
dils2k wants to merge 1 commit into
cockroachdb:masterfrom
dils2k:udf-predicate-partial-index
Open

sql: fix crash when using a partial index with predicate that has a UDF#171000
dils2k wants to merge 1 commit into
cockroachdb:masterfrom
dils2k:udf-predicate-partial-index

Conversation

@dils2k

@dils2k dils2k commented May 27, 2026

Copy link
Copy Markdown
Contributor

Using a partial index with predicate that references a UDF produces a Project expression that was not handled during partial index contruction in the optimizer, causing an error. This commit fixes it by handling Project expressions and also inlining additional expressions.

Epic: CRDB-55454
Release note: None
Fixes: #155488

@dils2k dils2k requested a review from a team as a code owner May 27, 2026 12:37
@dils2k dils2k requested review from DrewKimball and removed request for a team May 27, 2026 12:37
@trunk-io

trunk-io Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

Merging to master in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

@blathers-crl

blathers-crl Bot commented May 27, 2026

Copy link
Copy Markdown

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@blathers-crl blathers-crl Bot added the O-community Originated from the community label May 27, 2026
@cockroach-teamcity

Copy link
Copy Markdown
Member

This change is Reviewable

@dils2k dils2k force-pushed the udf-predicate-partial-index branch from cd4ad88 to d123a66 Compare May 27, 2026 12:38
@blathers-crl

blathers-crl Bot commented May 27, 2026

Copy link
Copy Markdown

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

Thank you for updating your pull request.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@DrewKimball DrewKimball left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach looks good to me. We'll need tests for both changes, though.

@DrewKimball reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on dils2k).

@dils2k

dils2k commented May 31, 2026

Copy link
Copy Markdown
Contributor Author

@DrewKimball Actually I was thinking about this approach and it turns out we will get an error every time a partial index predicate has a non-inlinable UDF. For example, if we use a bitwise "and" instead of a module operator, then we will get the same error:

CREATE FUNCTION is_even(i INT) RETURNS BOOL AS $$
  -- NOTE: bitwise operator, not module
  SELECT i & 1 = 0;
$$ LANGUAGE SQL IMMUTABLE;

CREATE TABLE t (
  k INT PRIMARY KEY,
  a INT,
  INDEX (a) WHERE is_even(k)
);

CREATE INDEX partial_idx ON t(a) where is_even(k);

-- ERROR: index "t_a_idx" is a partial index that does not contain all the rows needed to execute this query SQLSTATE:
-- 42809

Should I also include bitwise operations in CanInline or we should reconsider this approach, because I think there are more operations that could trigger this error, for example consider this:

CREATE FUNCTION is_cat(s varchar) RETURNS BOOL AS $$
  SELECT lower(s) = 'cat';
$$ LANGUAGE SQL IMMUTABLE;

CREATE TABLE tt ( a varchar );

CREATE INDEX partial_idx ON tt(a) WHERE is_cat(a);

-- ERROR: index "partial_idx" is a partial index that does not contain all the rows needed to execute this query
-- SQLSTATE: 42809

@DrewKimball

Copy link
Copy Markdown
Collaborator

@dils2k I think we can safely add simple operators like bitwise AND to that list, maybe in separate commit or PR. You're right that in the general case, this isn't enough to prevent is a partial index that does not contain all the rows needed, but that seems OK for now, since still an improvement over the internal error. We could open a new issue to track that limitation. Also, I wonder if marking the UDF as VOLATILE works as a workaround?

@dils2k dils2k force-pushed the udf-predicate-partial-index branch from d123a66 to 775b0bc Compare July 1, 2026 23:44
@blathers-crl

blathers-crl Bot commented Jul 1, 2026

Copy link
Copy Markdown

Thank you for updating your pull request.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@dils2k dils2k force-pushed the udf-predicate-partial-index branch 3 times, most recently from d1e18cb to 088c7eb Compare July 2, 2026 00:19
Using a partial index with predicate that references a UDF produces
a Project expression that was not handled during partial index
contruction in the optimizer, causing an error. This commit fixes it by
handling Project expressions and also inlining additional expressions.

Epic: CRDB-55454
Release note: None
Fixes: cockroachdb#155488
@dils2k dils2k force-pushed the udf-predicate-partial-index branch from 088c7eb to 4cdca27 Compare July 2, 2026 00:30
@dils2k

dils2k commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

@DrewKimball PTAL when you have time. There were places in the tests where division was (AFAIU) expected to be a non-inlinable operation, I replaced it with a bitwise AND in those cases.

@dils2k dils2k requested a review from DrewKimball July 2, 2026 00:38
@dils2k

dils2k commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

I wonder if marking the UDF as VOLATILE works as a workaround?

I receive this error:

ERROR: is_even(): volatile functions are not allowed in INDEX PREDICATE
SQLSTATE: 0A000

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

O-community Originated from the community

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sql: internal error when creating partial index with predicate that references a UDF

3 participants