Fix points writing with no values #13378
Conversation
There was a problem hiding this comment.
@ChrisHegarty Thanks for the detailed explanation and fix, makes sense
There was a problem hiding this comment.
Thanks, the fix looks good to me. This makes me want to also improve the test from #13369 to add an assertNull(onlyReader.getPointValues("field"));, could you do it as part of this PR? The connection with your PR is that we're not writing empty fields, so getting a PointValues instance should return null, not an empty instance.
++ that makes sense. Nice to assert this. Done. |
This commit updates the writer to handle the case where there are no values. Previously (before apache#13369), there was a check that there were some points values before trying to write, this is no longer the case. The code in writeFieldNDims has an assumption that the values is not empty - an empty values will result in calculating a negative number of splits, and a negate array size to hold the splits. The fix is trivial, return null when values is empty - null is an allowable return value from this method. Note: writeField1Dim is able to handle an empty values.
This commit updates the writer to handle the case where there are no values.
Previously (before #13369), there was a check that there were some points values before trying to write, this is no longer the case. The code in
writeFieldNDimshas an assumption that the values is not empty - an empty values will result in calculating a negative number of splits, and a negate array size to hold the splits.The fix is trivial, return null when values is empty - null is an allowable return value from this method. Note:
writeField1Dimis able to handle an empty values.After this change both the newly failing test,
TestIndexWriterExceptions2.testBasics, and the test added for #13369,testExceptionJustBeforeFlushWithPointValues, pass successfully several thousands of times. No new test is added, astestBasicsalready covers this.closes #13377