You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2022/11/24 14:46:39 UTC
[GitHub] [lucene] rmuir opened a new pull request, #11974: fix wrong serialization by ShapeDocValues
rmuir opened a new pull request, #11974:
URL: https://github.com/apache/lucene/pull/11974
Here's a fix, and also enabling the error-prone check. But I feel like some sort of testcase should have been failing here all along?
Closes #11973
--
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@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] nknize commented on pull request #11974: fix wrong serialization by ShapeDocValues
Posted by GitBox <gi...@apache.org>.
nknize commented on PR #11974:
URL: https://github.com/apache/lucene/pull/11974#issuecomment-1334422558
Yeah, nice catch @rmuir!! Thanks for enabling the logical assignment error-prone check. I had no idea that was disabled!
> I feel like some sort of testcase should have been failing here all along?
💯!
No test cases failed here because the edge membership boolean is only used in `CONTAINS` queries, which [isn't yet supported](https://github.com/apache/lucene/blob/0a9bb6e2ace2e07512d67dc346ca5d19d473c538/lucene/core/src/java/org/apache/lucene/document/BaseShapeDocValuesQuery.java#L48). Either way, I agree w/ you that we should have an explicit check that the serialization matches what's expected from the original shape. I'll write up a simple one to ensure coverage here. Thanks for catching this!
--
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@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #11974: fix wrong serialization by ShapeDocValues
Posted by GitBox <gi...@apache.org>.
rmuir commented on PR #11974:
URL: https://github.com/apache/lucene/pull/11974#issuecomment-1333827718
@nknize any thoughts? sorry i haven't had time to dig into the guts of this, but would be good to get this check enabled
--
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@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] dweiss commented on pull request #11974: fix wrong serialization by ShapeDocValues
Posted by GitBox <gi...@apache.org>.
dweiss commented on PR #11974:
URL: https://github.com/apache/lucene/pull/11974#issuecomment-1326621733
Wow. Nice catch.
--
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@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir merged pull request #11974: fix wrong serialization by ShapeDocValues
Posted by GitBox <gi...@apache.org>.
rmuir merged PR #11974:
URL: https://github.com/apache/lucene/pull/11974
--
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@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #11974: fix wrong serialization by ShapeDocValues
Posted by GitBox <gi...@apache.org>.
rmuir commented on PR #11974:
URL: https://github.com/apache/lucene/pull/11974#issuecomment-1334646881
@nknize thank you for looking.
one other related suggestion i have for this code in the future would be to change code such as this:
```
header |= 0x01;
```
to something like this:
```
// named constants grouped together which kinda documents the "format" so it is a bit easier
// these names are just examples and might not be the best ones :)
static final int HAS_LEFT_SUBTREE = 1 << 0;
static final int HAS_RIGHT_SUBTREE = 1 << 1;
...
header |= HAS_LEFT_SUBTREE;
```
--
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@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org