tanmayrauth commented on PR #1597:
URL: https://github.com/apache/iceberg-go/pull/1597#issuecomment-5123191435
@laskoviymishka Thanks, I've addressed the non blocking items
- ErrBBoxNotSerializable now wraps ErrNotImplemented instead of
ErrInvalidArgument — it's a contract/not-implemented condition, consistent with
the substrait backstop.
- VisitBoundPredicate godoc now documents the contract: a bbox op with a
visitor that doesn't implement BoundGeospatialExprVisitor[T] panics with a
wrapped ErrNotImplemented, which VisitExpr recovers into a returned error (and
a direct caller must implement the extension or recover).
- Dispatch is now tested directly: a positive case driving BBOX_INTERSECTS
through the extension visitor, plus a negative case with a
BoundBooleanExprVisitor lacking the geo methods asserting errors.Is(err,
ErrNotImplemented).
- Both guards (columnNameTranslator and sanitizeVisitor) now match the
exported BoundBBoxPredicate interface rather than the concrete type, and the
sanitizeVisitor guard is hoisted above the ref := line to mirror the other.
- Retargeted the TestScanPrunesDisjointGeometryFile comment: it proves the
metrics-pruning path end-to-end; the translation/substrait regressions are
pinned by TestBBoxTranslateColumnNames and
TestBBoxPredicateConvertsToTypedError.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
