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

opt: fix st_distance -> st_dwithin rule to cast distance param as float #54366

Merged
merged 1 commit into from
Sep 15, 2020

Conversation

rytaft
Copy link
Collaborator

@rytaft rytaft commented Sep 14, 2020

Release justification: low-risk updates to new functionality

This commit fixes an error that could occur when the normalization rule was
applied that converts st_distance(g1, g2) <= d to st_dwithin(g1, g2, d).
The error occurred when d, the distance parameter, was some type other than
float. Since st_dwithin requires the distance parameter to have type float,
this commit fixes the problem by casting the parameter to type float.

Fixes #54326

Release note (bug fix): fixed an internal error and/or panic that could occur
when the ST_Distance or ST_MaxDistance functions were compared against a
constant or variable with any type other than float. For example, previously,
a query with the predicate WHERE ST_Distance(g1, g2) < 10::int could cause an
error.

Release justification: low-risk updates to new functionality

This commit fixes an error that could occur when the normalization rule was
applied that converts st_distance(g1, g2) <= d to st_dwithin(g1, g2, d).
The error occurred when d, the distance parameter, was some type other than
float. Since st_dwithin requires the distance parameter to have type float,
this commit fixes the problem by casting the parameter to type float.

Fixes cockroachdb#54326

Release note (bug fix): fixed an internal error and/or panic that could occur
when the ST_Distance or ST_MaxDistance functions were compared against a
constant or variable with any type other than float. For example, previously,
a query with the predicate `WHERE ST_Distance(g1, g2) < 10::int` could cause an
error.
@rytaft rytaft requested review from mgartner, otan, RaduBerinde and a team September 14, 2020 20:57
@rytaft rytaft requested a review from a team as a code owner September 14, 2020 20:57
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@otan otan left a comment

Choose a reason for hiding this comment

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

LGTM, small question

Reviewed 2 of 3 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @mgartner, @otan, @RaduBerinde, and @rytaft)


pkg/sql/opt/norm/testdata/rules/comp, line 849 at r1 (raw file):

# Regression test for #54326. Ensure the distance param is cast to a float.
norm expect=FoldCmpSTDistanceLeft format=(show-scalars,show-types)
SELECT * FROM geom_geog WHERE st_distance(geog, 'point(0.0 0.0)') < 5::int

does this error appropriately for other types, e.g. string?

Copy link
Member

@RaduBerinde RaduBerinde left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @mgartner, @RaduBerinde, and @rytaft)

Copy link
Collaborator Author

@rytaft rytaft left a comment

Choose a reason for hiding this comment

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

TFTRs!

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @mgartner and @otan)


pkg/sql/opt/norm/testdata/rules/comp, line 849 at r1 (raw file):

Previously, otan (Oliver Tan) wrote…

does this error appropriately for other types, e.g. string?

It gives a type checking error (this was already working before): unsupported comparison operator: <float> < <string>.

@rytaft
Copy link
Collaborator Author

rytaft commented Sep 15, 2020

bors r+

@craig
Copy link
Contributor

craig bot commented Sep 15, 2020

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Sep 15, 2020

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Sep 15, 2020

Build succeeded:

@craig craig bot merged commit b007277 into cockroachdb:master Sep 15, 2020
@rafiss rafiss added this to the 20.2 milestone Apr 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"expected *DFloat, found *tree.DInt" SQL panic in st_distance()
5 participants