Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Remove MDB_UNSIGNEDKEY, add a builder for Dbi, add MDB_INTEGERKEY comparators#276

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
at055612 wants to merge62 commits intomaster
base:master
Choose a base branch
Loading
fromgh-249-remove-unsignedkey

Conversation

@at055612
Copy link
Collaborator

@at055612at055612 commentedNov 5, 2025
edited
Loading

Fixes#249
Fixes#267

Two fixes in one PR as both need each other.

at055612and others added30 commitsMarch 6, 2025 16:04
There are now essentially three ways of configuring comparatorswhen creating a Dbi.**null comparator**LMDB will use its own comparator & CursorIterable will call downto mdb_cmp for comparisons between the current cursor key and therange start/stop key.**provided comparator**LMDB will use its own comparator & CursorIterable will use theprovided comparator for comparisons between the current cursorkey and the range start/stop key.**provided comparator with nativeCb==true**LMDB will call back to java for all comparator duties.CursorIterable will use the same provided comparator forcomparisons between the current cursor key and the rangestart/stop key.The methods `getSignedComparator()` and `getUnsignedComparator()`have been made public so users of this library can access them.
Refactor DbiBuilder and Dbi ctor to use DbiFlagSet.
Replace Env#copy(File, CopyFlags...) with copy(File, CopyFlagSet).As there is only one flag this should not be a breaking change.Deprecate Env#txn(Txn, TxnFlags...) as there is nowEnv#txn(Txn)Env#txn(Txn, TxnFlags)Env#txn(Txn, TxnFlagSet)
Also improve javadoc and refactor some tests to use DbiBuilder.Some tests are failing.
@benalexau
Copy link
Member

Thanks@at055612 for this extensive PR.

Just looking over it, would you please remove formatting changes to files where they are not otherwise required, such as extra lines, spaces and hyphen-based section separators. Also thesrc/main/resources binaries are not needed as they are transitively available from the LmdbJava Native artifact.

I feel we should defer this PR until 0.10.0:

  • There is unfortunately a risk of introducing regressions given the PR is very large at 6,500 lines and especially given it makes important changes toDbi,Comparator andCursor handling
  • Generally a patch release should have minimal changes so that the stability of the minor version series is increasing with each successive patch
  • While a patch release maintaining full API compatibility can reasonably mark a method as@Deprecated under semantic versioning principles, it is nevertheless much less surprising when incrementing the minor version number
  • Introducing a builder pattern does represent evolution of how we recommend using the API. Again that would be something more ergonomically introduced with a new minor version number
  • I have no objection to a 0.10.0 within a few days of 0.9.2 so that we can surface these changes (and considerIterator performance #270 if it is ready by then). At least that way we know 0.9.2 is a definite reduction in regressions over 0.9.1 and offers a stable point-in-time regardless of any changes in 0.10.0

I have added thegh-249-remove-unsignedkey branch to the benchmarking suite and will update the published reports.

Does this seem reasonable to you?

@benalexaubenalexau mentioned this pull requestNov 7, 2025
@at055612
Copy link
CollaboratorAuthor

at055612 commentedNov 7, 2025
edited
Loading

@benalexau, releasing this in0.10.0 is fine with me. I agree with the reasons you have listed.

I have run the format plugin to sort out the whitespace and removed the separators for static inner classes (personally I think they improve readability but appreciate they are not everybody's cup of tea). Not a fan of some of the decisions that the formatter is making, but formatting is incredibly subjective so having automated opinionated formatting does make life easier in a shared code base.

While this PR adds the Dbi builder, I decided to keep these methods for the really simple cases. The builder can be used for everything else more complicated. Hopefully a reasonable compromise.

publicDbi<T>openDbi(finalbyte[]name,finalDbiFlagSetdbiFlagSet) {  }publicDbi<T>openDbi(finalStringname,finalDbiFlagSetdbiFlagSet) {  }

I have removed the ByteUnits dependency as that repo is no longer being maintained and we were only using a fraction of it and only in tests. I have replaced it with our own ByteUnit class and added new overloadedsetMapSize methods to Env and Env.Builder to make it easier for users to specify larger sizes.

publicBuilder<T>setMapSize(finallongmapSize,finalByteUnitbyteUnit) {    }

importjava.util.List;
importjava.util.stream.Collectors;

classCopyFlagSetTestextendsAbstractFlagSetTest<CopyFlags,CopyFlagSet> {

Check notice

Code scanning / CodeQL

Unused classes and interfaces Note test

Unused class: CopyFlagSetTest is not referenced within this codebase. If not used as an external API it should be removed.

Copilot Autofix

AI about 16 hours ago

The best way to fix the problem without changing the existing functionality is to ensure the class is actually used. Since this is a test class and is likely meant to be discovered and run by a test framework, we should make the classpublic, following standard Java testing conventions. This way, test frameworks like JUnit can find and run the tests inside it, provided the right annotations and structure are in place. The change is minimal and simply involves adding thepublic access modifier to the class declaration insrc/test/java/org/lmdbjava/CopyFlagSetTest.java.


Suggested changeset1
src/test/java/org/lmdbjava/CopyFlagSetTest.java

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git applydiff --git a/src/test/java/org/lmdbjava/CopyFlagSetTest.java b/src/test/java/org/lmdbjava/CopyFlagSetTest.java--- a/src/test/java/org/lmdbjava/CopyFlagSetTest.java+++ b/src/test/java/org/lmdbjava/CopyFlagSetTest.java@@ -20,7 +20,7 @@ import java.util.List; import java.util.stream.Collectors; -class CopyFlagSetTest extends AbstractFlagSetTest<CopyFlags, CopyFlagSet> {+public class CopyFlagSetTest extends AbstractFlagSetTest<CopyFlags, CopyFlagSet> {    @Override   List<CopyFlags> getAllFlags() {EOF
@@ -20,7 +20,7 @@
importjava.util.List;
importjava.util.stream.Collectors;

classCopyFlagSetTestextendsAbstractFlagSetTest<CopyFlags,CopyFlagSet> {
publicclassCopyFlagSetTestextendsAbstractFlagSetTest<CopyFlags,CopyFlagSet> {

@Override
List<CopyFlags>getAllFlags() {
importjava.util.List;
importjava.util.stream.Collectors;

classDbiFlagSetTestextendsAbstractFlagSetTest<DbiFlags,DbiFlagSet> {

Check notice

Code scanning / CodeQL

Unused classes and interfaces Note test

Unused class: DbiFlagSetTest is not referenced within this codebase. If not used as an external API it should be removed.

Copilot Autofix

AI about 16 hours ago

To fix the problem, you should remove the unused class. Specifically:

  • Delete the entireDbiFlagSetTest class fromsrc/test/java/org/lmdbjava/DbiFlagSetTest.java, including all its methods and class definition.
  • Ensure that only the code for this class is removed, and that any license headers or package/import statements not used elsewhere are also removed if the file becomes empty.
  • If this file contains only this class, the whole file should be deleted or left empty.

Suggested changeset1
src/test/java/org/lmdbjava/DbiFlagSetTest.java

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git applydiff --git a/src/test/java/org/lmdbjava/DbiFlagSetTest.java b/src/test/java/org/lmdbjava/DbiFlagSetTest.java--- a/src/test/java/org/lmdbjava/DbiFlagSetTest.java+++ b/src/test/java/org/lmdbjava/DbiFlagSetTest.java@@ -1,59 +1 @@-/*- * Copyright © 2016-2025 The LmdbJava Open Source Project- *- * Licensed under the Apache License, Version 2.0 (the "License");- * you may not use this file except in compliance with the License.- * You may obtain a copy of the License at- *- *     http://www.apache.org/licenses/LICENSE-2.0- *- * Unless required by applicable law or agreed to in writing, software- * distributed under the License is distributed on an "AS IS" BASIS,- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.- * See the License for the specific language governing permissions and- * limitations under the License.- */-package org.lmdbjava;--import java.util.Arrays;-import java.util.Collection;-import java.util.List;-import java.util.stream.Collectors;--class DbiFlagSetTest extends AbstractFlagSetTest<DbiFlags, DbiFlagSet> {--  @Override-  List<DbiFlags> getAllFlags() {-    return Arrays.stream(DbiFlags.values()).collect(Collectors.toList());-  }--  @Override-  DbiFlagSet getEmptyFlagSet() {-    return DbiFlagSet.empty();-  }--  @Override-  AbstractFlagSet.Builder<DbiFlags, DbiFlagSet> getBuilder() {-    return DbiFlagSet.builder();-  }--  @Override-  Class<DbiFlags> getFlagType() {-    return DbiFlags.class;-  }--  @Override-  DbiFlagSet getFlagSet(Collection<DbiFlags> flags) {-    return DbiFlagSet.of(flags);-  }--  @Override-  DbiFlagSet getFlagSet(DbiFlags[] flags) {-    return DbiFlagSet.of(flags);-  }--  @Override-  DbiFlagSet getFlagSet(DbiFlags flag) {-    return DbiFlagSet.of(flag);-  }-}EOF
@@ -1,59 +1 @@
/*
*Copyright ©2016-2025TheLmdbJavaOpenSourceProject
*
*LicensedundertheApacheLicense,Version2.0 (the "License");
*youmaynotusethisfileexceptincompliancewiththeLicense.
*YoumayobtainacopyoftheLicenseat
*
*http://www.apache.org/licenses/LICENSE-2.0
*
*Unlessrequiredbyapplicablelaworagreedtoinwriting,software
*distributedundertheLicenseisdistributedonan"AS IS"BASIS,
*WITHOUTWARRANTIESORCONDITIONSOFANYKIND,eitherexpressorimplied.
*SeetheLicenseforthespecificlanguagegoverningpermissionsand
*limitationsundertheLicense.
*/
packageorg.lmdbjava;

importjava.util.Arrays;
importjava.util.Collection;
importjava.util.List;
importjava.util.stream.Collectors;

classDbiFlagSetTestextendsAbstractFlagSetTest<DbiFlags,DbiFlagSet> {

@Override
List<DbiFlags>getAllFlags() {
returnArrays.stream(DbiFlags.values()).collect(Collectors.toList());
}

@Override
DbiFlagSetgetEmptyFlagSet() {
returnDbiFlagSet.empty();
}

@Override
AbstractFlagSet.Builder<DbiFlags,DbiFlagSet>getBuilder() {
returnDbiFlagSet.builder();
}

@Override
Class<DbiFlags>getFlagType() {
returnDbiFlags.class;
}

@Override
DbiFlagSetgetFlagSet(Collection<DbiFlags>flags) {
returnDbiFlagSet.of(flags);
}

@Override
DbiFlagSetgetFlagSet(DbiFlags[]flags) {
returnDbiFlagSet.of(flags);
}

@Override
DbiFlagSetgetFlagSet(DbiFlagsflag) {
returnDbiFlagSet.of(flag);
}
}
importjava.util.List;
importjava.util.stream.Collectors;

classEnvFlagSetTestextendsAbstractFlagSetTest<EnvFlags,EnvFlagSet> {

Check notice

Code scanning / CodeQL

Unused classes and interfaces Note test

Unused class: EnvFlagSetTest is not referenced within this codebase. If not used as an external API it should be removed.

Copilot Autofix

AI about 16 hours ago

To fix this issue, delete theEnvFlagSetTest class entirely fromsrc/test/java/org/lmdbjava/EnvFlagSetTest.java as it is unused and serves no purpose, thus removing unnecessary clutter from the codebase. Specifically, remove all lines in the file containing the definition of and code for theEnvFlagSetTest class, which comprises the only substantial code in this file. No additional methods, imports, or changes elsewhere are needed.

Suggested changeset1
src/test/java/org/lmdbjava/EnvFlagSetTest.java

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git applydiff --git a/src/test/java/org/lmdbjava/EnvFlagSetTest.java b/src/test/java/org/lmdbjava/EnvFlagSetTest.java--- a/src/test/java/org/lmdbjava/EnvFlagSetTest.java+++ b/src/test/java/org/lmdbjava/EnvFlagSetTest.java@@ -1,59 +1 @@-/*- * Copyright © 2016-2025 The LmdbJava Open Source Project- *- * Licensed under the Apache License, Version 2.0 (the "License");- * you may not use this file except in compliance with the License.- * You may obtain a copy of the License at- *- *     http://www.apache.org/licenses/LICENSE-2.0- *- * Unless required by applicable law or agreed to in writing, software- * distributed under the License is distributed on an "AS IS" BASIS,- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.- * See the License for the specific language governing permissions and- * limitations under the License.- */-package org.lmdbjava;--import java.util.Arrays;-import java.util.Collection;-import java.util.List;-import java.util.stream.Collectors;--class EnvFlagSetTest extends AbstractFlagSetTest<EnvFlags, EnvFlagSet> {--  @Override-  List<EnvFlags> getAllFlags() {-    return Arrays.stream(EnvFlags.values()).collect(Collectors.toList());-  }--  @Override-  EnvFlagSet getEmptyFlagSet() {-    return EnvFlagSet.empty();-  }--  @Override-  AbstractFlagSet.Builder<EnvFlags, EnvFlagSet> getBuilder() {-    return EnvFlagSet.builder();-  }--  @Override-  EnvFlagSet getFlagSet(Collection<EnvFlags> flags) {-    return EnvFlagSet.of(flags);-  }--  @Override-  EnvFlagSet getFlagSet(EnvFlags[] flags) {-    return EnvFlagSet.of(flags);-  }--  @Override-  EnvFlagSet getFlagSet(EnvFlags flag) {-    return EnvFlagSet.of(flag);-  }--  @Override-  Class<EnvFlags> getFlagType() {-    return EnvFlags.class;-  }-}EOF
@@ -1,59 +1 @@
/*
*Copyright ©2016-2025TheLmdbJavaOpenSourceProject
*
*LicensedundertheApacheLicense,Version2.0 (the "License");
*youmaynotusethisfileexceptincompliancewiththeLicense.
*YoumayobtainacopyoftheLicenseat
*
*http://www.apache.org/licenses/LICENSE-2.0
*
*Unlessrequiredbyapplicablelaworagreedtoinwriting,software
*distributedundertheLicenseisdistributedonan"AS IS"BASIS,
*WITHOUTWARRANTIESORCONDITIONSOFANYKIND,eitherexpressorimplied.
*SeetheLicenseforthespecificlanguagegoverningpermissionsand
*limitationsundertheLicense.
*/
packageorg.lmdbjava;

importjava.util.Arrays;
importjava.util.Collection;
importjava.util.List;
importjava.util.stream.Collectors;

classEnvFlagSetTestextendsAbstractFlagSetTest<EnvFlags,EnvFlagSet> {

@Override
List<EnvFlags>getAllFlags() {
returnArrays.stream(EnvFlags.values()).collect(Collectors.toList());
}

@Override
EnvFlagSetgetEmptyFlagSet() {
returnEnvFlagSet.empty();
}

@Override
AbstractFlagSet.Builder<EnvFlags,EnvFlagSet>getBuilder() {
returnEnvFlagSet.builder();
}

@Override
EnvFlagSetgetFlagSet(Collection<EnvFlags>flags) {
returnEnvFlagSet.of(flags);
}

@Override
EnvFlagSetgetFlagSet(EnvFlags[]flags) {
returnEnvFlagSet.of(flags);
}

@Override
EnvFlagSetgetFlagSet(EnvFlagsflag) {
returnEnvFlagSet.of(flag);
}

@Override
Class<EnvFlags>getFlagType() {
returnEnvFlags.class;
}
}
importjava.util.List;
importjava.util.stream.Collectors;

classTxnFlagSetTestextendsAbstractFlagSetTest<TxnFlags,TxnFlagSet> {

Check notice

Code scanning / CodeQL

Unused classes and interfaces Note test

Unused class: TxnFlagSetTest is not referenced within this codebase. If not used as an external API it should be removed.

Copilot Autofix

AI about 15 hours ago

Copilot could not generate an autofix suggestion

Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persistscontact support.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

@at055612at055612

Labels

None yet

Projects

None yet

Milestone

No milestone

4 participants

@at055612@benalexau@stroomdev66

[8]ページ先頭

©2009-2025 Movatter.jp