aihuaxu commented on PR #12611: URL: https://github.com/apache/iceberg/pull/12611#issuecomment-2755506786
> @aihuaxu > > Since this is first PR, need to discuss few things before in hand to ensure the direction is correct. Hence, marking the pr as draft for now. > > Added a new method `sanitizeVariant(VariantValue value, long now, int today)` to sanitize the variant fields only for `BoundPredicate` expression code path by calling from `sanitize(Type type, Object value, long now, int today)`. Need to do the same for `UnboundPredicate` by calling another (not yet added) variant of sanitize variant `sanitizeVariant(Literal<?> literal, long now, int today)` by calling from https://github.com/apache/iceberg/blob/main/api/src/main/java/org/apache/iceberg/expressions/ExpressionUtil.java#L551 . It requires us to add a new class `VariantLiteral` similar to any other `Literal` for other types like int, float etc. Is this correct? Can you confirm this? > > Also, need to add the test as well. Sorry for the late reply. I'm not exactly sure if unbound is needed. Agree that we can start with the simple case. Note in `sanitize(Type type, Object value, long now, int today)`, the variant can be primitive, array or object. I guess it would be cleaner to use visitor pattern to handle that. -- 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: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
