Skip to content

Comments

[SPARK-55640][Geo][SQL] Propagate WKB parsing errors for Geometry and Geography#54424

Open
uros-db wants to merge 3 commits intoapache:masterfrom
uros-db:geo-wkb-parse-exceptions
Open

[SPARK-55640][Geo][SQL] Propagate WKB parsing errors for Geometry and Geography#54424
uros-db wants to merge 3 commits intoapache:masterfrom
uros-db:geo-wkb-parse-exceptions

Conversation

@uros-db
Copy link
Contributor

@uros-db uros-db commented Feb 23, 2026

What changes were proposed in this pull request?

WKB reader was implemented for Geometry and Geography, but only using internal exception handling. This PR addresses this by introducing proper user-facing error classes for WKB parsing.

Why are the changes needed?

Propagate the WKB parsing errors properly to the user.

Does this PR introduce any user-facing change?

Yes, users now get proper errors for invalid WKB parsing.

How was this patch tested?

Added new unit tests and end-to-end SQL tests for WKB parsing.

Was this patch authored or co-authored using generative AI tooling?

No.

@uros-db uros-db changed the title [DRAFT][Geo][SQL] Propagate WKB parsing errors for Geometry and Geography [DRAFT][SPARK][Geo][SQL] Propagate WKB parsing errors for Geometry and Geography Feb 23, 2026
@uros-db uros-db changed the title [DRAFT][SPARK][Geo][SQL] Propagate WKB parsing errors for Geometry and Geography [SPARK-55640][Geo][SQL] Propagate WKB parsing errors for Geometry and Geography Feb 23, 2026
@uros-db uros-db marked this pull request as ready for review February 23, 2026 20:30
Copy link
Contributor Author

@uros-db uros-db left a comment

Choose a reason for hiding this comment

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

Please review @cloud-fan @szehon-ho.

ByteBuffer.wrap(bytes).order(DEFAULT_ENDIANNESS).putInt(srid);
System.arraycopy(wkb, 0, bytes, WKB_OFFSET, wkb.length);
return fromBytes(bytes);
} catch (WkbParseException e) {
Copy link
Member

Choose a reason for hiding this comment

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

curious, we do not keep the wkb string as hex, it is by design? I find it useful for debugging, but curious

* Exception thrown when parsing WKB data fails.
*/
class WkbParseException extends RuntimeException {
public class WkbParseException extends RuntimeException {
Copy link
Contributor

Choose a reason for hiding this comment

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

shall we just extend SparkRuntimeException here and specify the error condition name? then we don't need try-catch to rethrow the error

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants