apache / lucene

Apache Lucene open-source search software
https://lucene.apache.org/
Apache License 2.0
2.46k stars 979 forks source link

Split IOContext.DEFAULT into WRITE and READ. #13236

Closed jpountz closed 3 months ago

jpountz commented 3 months ago

This effectively removes the DEFAULT IOContext and forces callers to use the existing IOContext.READ context or the new IOContext.WRITE context instead.

This comes with some interesting changes:

jpountz commented 3 months ago

For what it's worth, the change is huge because lots of tests used to use IOContext.DEFAULT and had to be fixed. If you filter the diff to only look at the non-test source folders, this change is pretty contained.

uschindler commented 3 months ago

Hi, I was thinking about it. Why do we need to differentiate between read and write? If you open an IndexInput you read, if you open IndexOutput you're going to write. If you copy, you will do both and this sequential.

How about removing READ instead and name it DEFAULT? The others are FLUSH and MERGE.

In the future we may also have a WriteAdvice enum so we have all coordinates for handling IO.

uschindler commented 3 months ago

Actually my suggestion is now a rather simple rename so should be easy to apply with IDE. The diff should also get shorter.

jpountz commented 3 months ago

That should work as well, let me give it a try.

jpountz commented 3 months ago

@uschindler See #13242.

uschindler commented 3 months ago

Perfect. Let's close this PR as won't fix.

jpountz commented 3 months ago

Closed in favor of #13242.