-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Feature/move long array group varint to backward codecs 15113 #15186
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Feature/move long array group varint to backward codecs 15113 #15186
Conversation
- Move deprecated long[] group varint methods from core to backward-codecs - Remove GroupVIntUtil.readGroupVInts(DataInput, long[], int) from core - Remove GroupVIntUtil.writeGroupVInts(DataOutput, byte[], long[], int) from core - Remove DataOutput.writeGroupVInts(long[], int) from core - Add GroupVIntUtil class in backward-codecs with long[] methods - Add DataOutputUtil class in backward-codecs for long[] DataOutput operations - Add comprehensive tests for the moved functionality - Prevents accidental usage of legacy long[] methods in benchmarks - Maintains backward compatibility for codecs that need long[] functionality Fixes apache#15113
Add changelog entry documenting the move of long[] group varint methods to backward-codecs module as required by contributing guidelines.
- Update existing PostingsUtil files in lucene912 and lucene99 to use backward-codecs GroupVIntUtil - Remove deprecated testGroupVIntOverflow from test framework - Fix TestGroupVIntUtil to use ByteBuffersDataOutput/Input correctly - All tests now pass with Java 24 - Implementation is fully functional and tested Resolves remaining compilation errors from apache#15113
- Removed lucene/backward-codecs/src/test/org/apache/lucene/backward_codecs/util/TestGroupVIntUtil.java - Keeping the correct version at lucene/backward-codecs/src/test/java/org/apache/lucene/backward_codecs/util/TestGroupVIntUtil.java - This follows proper Maven directory structure with src/test/java/ path
- Remove trailing spaces from blank lines in TestGroupVIntUtil.java - Fix formatting in BaseDirectoryTestCase.java - Resolves checkGoogleJavaFormat validation errors
- Fixes TestModularLayer.testAllExportedPackagesInSync test failure - The util package contains GroupVIntUtil and related backward compatibility utilities - Required for proper module system compliance in Java 9+ environments
- Add package-info.java for org.apache.lucene.backward_codecs.util package - Add Javadoc comment for GroupVIntUtil.MAX_LENGTH_PER_GROUP field - Resolves Javadoc validation errors in CI builds
} | ||
|
||
@Deprecated | ||
public void testGroupVIntOverflow() throws IOException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we move this test to class TestGroupVIntUtil
instead of removing it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, move this test to TestGroupVIntUtil. In addition as the code here does no longer access any internals of a specific directory implementation (there is no implementations anymore), so just test the overflow on the standard test directory impl used by tests!
In short: Move test to test class above an instead of getDirectory(createTempDir("testGroupVIntOverflow"))
use LuceneTestCase#newDirectory()
.
} | ||
|
||
@Deprecated | ||
public void testGroupVIntOverflow() throws IOException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, move this test to TestGroupVIntUtil. In addition as the code here does no longer access any internals of a specific directory implementation (there is no implementations anymore), so just test the overflow on the standard test directory impl used by tests!
In short: Move test to test class above an instead of getDirectory(createTempDir("testGroupVIntOverflow"))
use LuceneTestCase#newDirectory()
.
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package org.apache.lucene.backward_codecs.util; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe move this class next to store package (to not open another package).
* @param limit the number of values to write. | ||
* @lucene.experimental | ||
*/ | ||
public static void writeGroupVInts(DataOutput out, long[] values, int limit) throws IOException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move that to GroupVIntUtil
.
exports org.apache.lucene.backward_codecs.lucene103; | ||
exports org.apache.lucene.backward_codecs.packed; | ||
exports org.apache.lucene.backward_codecs.store; | ||
exports org.apache.lucene.backward_codecs.util; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see below: move the classes to another package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in addition, we do not need to open the package as it shozuld not be used from outside backwards_codecs!
Description
Fixes: Move long[] group varint to backward-codecs #15113