You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/11/03 22:13:52 UTC

[GitHub] [iceberg] ddrinka opened a new pull request, #6117: Fix typo in `_ManifestEvalVisitor.visit_equal`

ddrinka opened a new pull request, #6117:
URL: https://github.com/apache/iceberg/pull/6117

   @Fokko here's another runtime bug I'm seeing while playing with this new Manifest evaluator (#5845)


-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] ajantha-bhat commented on a diff in pull request #6117: Fix typo in `_ManifestEvalVisitor.visit_equal`

Posted by GitBox <gi...@apache.org>.
ajantha-bhat commented on code in PR #6117:
URL: https://github.com/apache/iceberg/pull/6117#discussion_r1013615833


##########
python/pyiceberg/expressions/visitors.py:
##########
@@ -526,7 +526,7 @@ def visit_equal(self, term: BoundTerm, literal: Literal[Any]) -> bool:
         if lower > literal.value:
             return ROWS_CANNOT_MATCH
 
-        upper = _from_byte_buffer(term.ref().field.field_type, field.lower_bound)
+        upper = _from_byte_buffer(term.ref().field.field_type, field.upper_bound)

Review Comment:
   good finding 👍🏻. 
   
   nit: maybe good to have unit tests around these. 



-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] Fokko commented on pull request #6117: Fix typo in `_ManifestEvalVisitor.visit_equal`

Posted by GitBox <gi...@apache.org>.
Fokko commented on PR #6117:
URL: https://github.com/apache/iceberg/pull/6117#issuecomment-1303050019

   Thanks for spotting this one @ddrinka It looks like we also need a not-`None` check. I also noticed that this is being fixed in https://github.com/apache/iceberg/pull/6069. So I'll close this one for now. If you need this right away, feel free to open up a new PR (with the check included).


-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] Fokko closed pull request #6117: Fix typo in `_ManifestEvalVisitor.visit_equal`

Posted by GitBox <gi...@apache.org>.
Fokko closed pull request #6117: Fix typo in `_ManifestEvalVisitor.visit_equal`
URL: https://github.com/apache/iceberg/pull/6117


-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] Fokko commented on pull request #6117: Fix typo in `_ManifestEvalVisitor.visit_equal`

Posted by GitBox <gi...@apache.org>.
Fokko commented on PR #6117:
URL: https://github.com/apache/iceberg/pull/6117#issuecomment-1305312653

   Fixed the linting issue, thanks @ddrinka!


-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on pull request #6117: Fix typo in `_ManifestEvalVisitor.visit_equal`

Posted by GitBox <gi...@apache.org>.
rdblue commented on PR #6117:
URL: https://github.com/apache/iceberg/pull/6117#issuecomment-1304947082

   I reopened this because I think it's a good idea to get it in independently. Thanks for finding this, @ddrinka!


-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] Fokko merged pull request #6117: Fix typo in `_ManifestEvalVisitor.visit_equal`

Posted by GitBox <gi...@apache.org>.
Fokko merged PR #6117:
URL: https://github.com/apache/iceberg/pull/6117


-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org