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

MDEV-35117 Improve error message on unexpected geometries for st_dist… #3716

Open
wants to merge 1 commit into
base: 10.5
Choose a base branch
from

Conversation

DaveGosselin-MariaDB
Copy link
Member

…ance_sphere

When invoking st_distance_sphere with unexpected geometries, the error message now given is:
ERROR HY000: Internal error: Point or multipoint geometries expected
This commit fixes a few formatting issues in the affected function.

Copy link
Member

@grooverdan grooverdan left a comment

Choose a reason for hiding this comment

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

Its a bit perverse using ER_INTERNAL_ERROR as this is the only SQL layer expose of this error message (excluding DDL, debug mode and partitioning things, and things within storage engines that propagate up).

I'd suggest using ER_GIS_UNSUPPORTED_ARGUMENT and passing the function name to both uses of ER_INTERNAL_ERROR.

The current changed error doesn't list which function has errors with Point/Multipoint geometries. The KB page on this is clear on expected arguments if ER_GIS_UNSUPPORTED_ARGUMENT is used, though could be expanded with an example.

Changing error messages always risks incompatibility, but for this case I consider the original so bad a choice, (its not even internal), that its justified.

If a second opinion says to keep this code, then ok, but append " for function st_distance_sphere" to the error text to be a bit clear as this detail may get lost in more complicated queries.

Also add test case for negative radius.

@DaveGosselin-MariaDB
Copy link
Member Author

Thanks for the pointer, I didn't want to change the error code because of compatibility but agree that it's not a good error code. I'll update the PR to use ER_GIS_UNSUPPORTED_ARGUMENT and update the KB page with an example in the case of an error.

@DaveGosselin-MariaDB
Copy link
Member Author

DaveGosselin-MariaDB commented Dec 19, 2024

Just noting that gis-precise has a test case for negative radius already:

SELECT ST_DISTANCE_SPHERE(ST_GEOMFROMTEXT('POINT(0 0)'), ST_GEOMFROMTEXT('POINT(1 1)'), -1) as result;

@DaveGosselin-MariaDB
Copy link
Member Author

In the same function there's a third instance of ER_INTERNAL_ERROR but I didn't change it because it appears legitimate. However, I changed the other two that you mentioned.

…ance_sphere

When invoking st_distance_sphere with unexpected geometries, the error
message now given is:
  ERROR HY000: Internal error: Point or multipoint geometries expected
This commit fixes a few formatting issues in the affected function.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants