Re: [PR] Promote PerThreadPKLookup to core as PrimaryKeyLookup [lucene]
github-actions[bot] commented on PR #16128: URL: https://github.com/apache/lucene/pull/16128#issuecomment-5137829979 This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the [email protected] list. Thank you for your contribution! -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] Promote PerThreadPKLookup to core as PrimaryKeyLookup [lucene]
mikemccand commented on code in PR #16128:
URL: https://github.com/apache/lucene/pull/16128#discussion_r3587079202
##
lucene/core/src/java/org/apache/lucene/index/PrimaryKeyLookup.java:
##
@@ -0,0 +1,185 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.apache.lucene.index;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.lucene.index.IndexReader.CacheHelper;
+import org.apache.lucene.util.Bits;
+import org.apache.lucene.util.BytesRef;
+
+/**
+ * Class to do efficient primary-key lookups. Instances are not thread-safe.
Each calling thread
+ * must own its own instance, and an instance must only ever be used from the
thread that
+ * constructed it. When assertions are enabled this invariant is checked on
every lookup and reopen
+ * call. Do not use this if a term may appear in more than one document! It
will only return the
+ * first one it finds.
+ *
+ * @lucene.experimental
+ */
+public class PrimaryKeyLookup {
+
+ /** Name of the primary-key field. */
+ protected final String idFieldName;
+
+ /** Per-segment {@link TermsEnum}. */
+ protected final TermsEnum[] termsEnums;
+
+ /** Per-segment {@link PostingsEnum}. */
+ protected final PostingsEnum[] postingsEnums;
+
+ /** Per-segment live-docs bitset. */
+ protected final Bits[] liveDocs;
+
+ /** Per-segment {@code docBase} used to translate leaf docIDs to absolute
docIDs. */
+ protected final int[] docBases;
+
+ /** Number of populated entries in termsEnums/postingsEnums. */
+ protected final int numEnums;
+
+ /** True if any segment in this lookup has deletions. */
+ protected final boolean hasDeletions;
+
+ /**
+ * Maps a segment core cache key to its index in termsEnums/postingsEnums,
so that reopen can
+ * transfer per-segment enum state to the next generation.
+ */
+ protected final Map enumIndexes;
+
+ // Thread-stickiness guard.
Review Comment:
Maybe add "only checked under assert" or so?
##
lucene/core/src/java/org/apache/lucene/index/PrimaryKeyLookup.java:
##
@@ -0,0 +1,185 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.apache.lucene.index;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.lucene.index.IndexReader.CacheHelper;
+import org.apache.lucene.util.Bits;
+import org.apache.lucene.util.BytesRef;
+
+/**
+ * Class to do efficient primary-key lookups. Instances are not thread-safe.
Each calling thread
+ * must own its own instance, and an instance must only ever be used from the
thread that
+ * constructed it. When assertions are enabled this invariant is checked on
every lookup and reopen
+ * call. Do not use this if a term may appear in more than one document! It
will only return the
+ * first one it finds.
+ *
+ * @lucene.experimental
+ */
+public class PrimaryKeyLookup {
+
+ /** Name of the primary-key field. */
+ protected final String idFieldName;
+
+ /** Per-segment {@link TermsEnum}. */
+ protected final TermsEnum[] termsEnums;
+
+ /** Per-segment {@link PostingsEnum}. */
+ protected final PostingsEnum[] postingsEnums;
+
+ /** Per-segment live-docs bitset. */
+ protected final Bits[] liveDocs;
+
+ /** Per-segment {@code docBase} used to
Re: [PR] Promote PerThreadPKLookup to core as PrimaryKeyLookup [lucene]
mikemccand commented on PR #16128: URL: https://github.com/apache/lucene/pull/16128#issuecomment-4980229748 Sorry @shubhamsrkdev -- I will try to review / merge soon. It's great to make this common use case easy and performant by default for Lucene users. Thank you for following up on this -- it was a spinoff from nightly benchy having its own optimized primary key lookup task. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] Promote PerThreadPKLookup to core as PrimaryKeyLookup [lucene]
github-actions[bot] commented on PR #16128: URL: https://github.com/apache/lucene/pull/16128#issuecomment-4975514264 This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the [email protected] list. Thank you for your contribution! -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] Promote PerThreadPKLookup to core as PrimaryKeyLookup [lucene]
shubhamsrkdev commented on PR #16128: URL: https://github.com/apache/lucene/pull/16128#issuecomment-4771233267 Gentle reminder - if anyone can help merge this it would be great, TIA! -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] Promote PerThreadPKLookup to core as PrimaryKeyLookup [lucene]
shubhamsrkdev commented on PR #16128: URL: https://github.com/apache/lucene/pull/16128#issuecomment-4633213658 Thanks for the review @epotyom ! If this looks ready can anyone help merge (or review) it? TIA! -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] Promote PerThreadPKLookup to core as PrimaryKeyLookup [lucene]
epotyom commented on code in PR #16128:
URL: https://github.com/apache/lucene/pull/16128#discussion_r3340431221
##
lucene/core/src/java/org/apache/lucene/index/PrimaryKeyLookup.java:
##
@@ -0,0 +1,175 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.apache.lucene.index;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.lucene.index.IndexReader.CacheHelper;
+import org.apache.lucene.util.Bits;
+import org.apache.lucene.util.BytesRef;
+
+/**
+ * Class to do efficient primary-key lookups. Instances are not thread-safe.
Each calling thread
+ * must own its own instance, and an instance must only ever be used from the
thread that
+ * constructed it. When assertions are enabled this invariant is checked on
every lookup and reopen
+ * call. Do not use this if a term may appear in more than one document! It
will only return the
+ * first one it finds.
+ *
+ * @lucene.experimental
+ */
+public class PrimaryKeyLookup {
Review Comment:
Ah, makes sense to keep as-is then. One nit: `lookup` could be made `final`
since no subclass overrides it.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] Promote PerThreadPKLookup to core as PrimaryKeyLookup [lucene]
shubhamsrkdev commented on code in PR #16128:
URL: https://github.com/apache/lucene/pull/16128#discussion_r3318477962
##
lucene/core/src/java/org/apache/lucene/index/PrimaryKeyLookup.java:
##
@@ -0,0 +1,175 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.apache.lucene.index;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.lucene.index.IndexReader.CacheHelper;
+import org.apache.lucene.util.Bits;
+import org.apache.lucene.util.BytesRef;
+
+/**
+ * Class to do efficient primary-key lookups. Instances are not thread-safe.
Each calling thread
+ * must own its own instance, and an instance must only ever be used from the
thread that
+ * constructed it. When assertions are enabled this invariant is checked on
every lookup and reopen
+ * call. Do not use this if a term may appear in more than one document! It
will only return the
+ * first one it finds.
+ *
+ * @lucene.experimental
+ */
+public class PrimaryKeyLookup {
+
+ /** Name of the primary-key field. */
+ protected final String idFieldName;
+
+ /** Per-segment {@link TermsEnum}. */
+ protected final TermsEnum[] termsEnums;
+
+ /** Per-segment {@link PostingsEnum}. */
+ protected final PostingsEnum[] postingsEnums;
+
+ /** Per-segment live-docs bitset. */
+ protected final Bits[] liveDocs;
+
+ /** Per-segment {@code docBase} used to translate leaf docIDs to absolute
docIDs. */
+ protected final int[] docBases;
+
+ /** Number of populated entries in termsEnums/postingsEnums. */
+ protected final int numEnums;
+
+ /** True if any segment in this lookup has deletions. */
+ protected final boolean hasDeletions;
+
+ /**
+ * Maps a segment core cache key to its index in termsEnums/postingsEnums,
so that reopen can
+ * transfer per-segment enum state to the next generation.
+ */
+ protected final Map enumIndexes;
+
+ // Thread-stickiness guard.
+ private final long ownerThreadId;
+
+ /** Construct a {@code PrimaryKeyLookup} bound to {@code reader}. */
+ public PrimaryKeyLookup(IndexReader reader, String idFieldName) throws
IOException {
+this(reader, idFieldName, Collections.emptyMap(), null, null);
+ }
+
+ /** Construct a {@code PrimaryKeyLookup} instance. */
+ protected PrimaryKeyLookup(
+ IndexReader reader,
+ String idFieldName,
+ Map prevEnumIndexes,
+ TermsEnum[] reusableTermsEnums,
+ PostingsEnum[] reusablePostingsEnums)
+ throws IOException {
+this.idFieldName = idFieldName;
+this.ownerThreadId = Thread.currentThread().threadId();
+
+List leaves = new ArrayList<>(reader.leaves());
+// Larger segments are more likely to have the id, so we sort largest to
smallest by numDocs:
+leaves.sort((c1, c2) -> c2.reader().numDocs() - c1.reader().numDocs());
+
+termsEnums = new TermsEnum[leaves.size()];
+postingsEnums = new PostingsEnum[leaves.size()];
+liveDocs = new Bits[leaves.size()];
+docBases = new int[leaves.size()];
+enumIndexes = new HashMap<>();
+int numEnums = 0;
+boolean hasDeletions = false;
+
+for (LeafReaderContext context : leaves) {
+ LeafReader leafReader = context.reader();
+ CacheHelper cacheHelper = leafReader.getCoreCacheHelper();
+ IndexReader.CacheKey cacheKey = cacheHelper == null ? null :
cacheHelper.getKey();
+
+ if (cacheKey != null && prevEnumIndexes.containsKey(cacheKey)) {
+// Reuse termsEnum, postingsEnum.
+int seg = prevEnumIndexes.get(cacheKey);
+termsEnums[numEnums] = reusableTermsEnums[seg];
+postingsEnums[numEnums] = reusablePostingsEnums[seg];
+ } else {
+// New or empty segment.
+Terms terms = leafReader.terms(idFieldName);
+if (terms != null) {
+ termsEnums[numEnums] = terms.iterator();
+ assert termsEnums[numEnums] != null;
+}
+ }
+
+ if (termsEnums[numEnums] != null) {
+if (cacheKey != null) {
+ enumIndexes.put(cacheKey, numEnums);
+}
+
+docBases[numEnums] = context.docBase;
+liveDocs[numEnums] = leafReader.getLiveDocs();
+
Re: [PR] Promote PerThreadPKLookup to core as PrimaryKeyLookup [lucene]
shubhamsrkdev commented on code in PR #16128:
URL: https://github.com/apache/lucene/pull/16128#discussion_r3325189308
##
lucene/core/src/java/org/apache/lucene/index/PrimaryKeyLookup.java:
##
@@ -0,0 +1,175 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.apache.lucene.index;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.lucene.index.IndexReader.CacheHelper;
+import org.apache.lucene.util.Bits;
+import org.apache.lucene.util.BytesRef;
+
+/**
+ * Class to do efficient primary-key lookups. Instances are not thread-safe.
Each calling thread
+ * must own its own instance, and an instance must only ever be used from the
thread that
+ * constructed it. When assertions are enabled this invariant is checked on
every lookup and reopen
+ * call. Do not use this if a term may appear in more than one document! It
will only return the
+ * first one it finds.
+ *
+ * @lucene.experimental
+ */
+public class PrimaryKeyLookup {
Review Comment:
We can but I think it is extended in `TestIDVersionPostingsFormat.java`
also. So we might have to copy some code from here to there. However we can
make it extend `PrimaryKeyLookup`. Wdyt?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] Promote PerThreadPKLookup to core as PrimaryKeyLookup [lucene]
shubhamsrkdev commented on code in PR #16128:
URL: https://github.com/apache/lucene/pull/16128#discussion_r3318477962
##
lucene/core/src/java/org/apache/lucene/index/PrimaryKeyLookup.java:
##
@@ -0,0 +1,175 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.apache.lucene.index;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.lucene.index.IndexReader.CacheHelper;
+import org.apache.lucene.util.Bits;
+import org.apache.lucene.util.BytesRef;
+
+/**
+ * Class to do efficient primary-key lookups. Instances are not thread-safe.
Each calling thread
+ * must own its own instance, and an instance must only ever be used from the
thread that
+ * constructed it. When assertions are enabled this invariant is checked on
every lookup and reopen
+ * call. Do not use this if a term may appear in more than one document! It
will only return the
+ * first one it finds.
+ *
+ * @lucene.experimental
+ */
+public class PrimaryKeyLookup {
+
+ /** Name of the primary-key field. */
+ protected final String idFieldName;
+
+ /** Per-segment {@link TermsEnum}. */
+ protected final TermsEnum[] termsEnums;
+
+ /** Per-segment {@link PostingsEnum}. */
+ protected final PostingsEnum[] postingsEnums;
+
+ /** Per-segment live-docs bitset. */
+ protected final Bits[] liveDocs;
+
+ /** Per-segment {@code docBase} used to translate leaf docIDs to absolute
docIDs. */
+ protected final int[] docBases;
+
+ /** Number of populated entries in termsEnums/postingsEnums. */
+ protected final int numEnums;
+
+ /** True if any segment in this lookup has deletions. */
+ protected final boolean hasDeletions;
+
+ /**
+ * Maps a segment core cache key to its index in termsEnums/postingsEnums,
so that reopen can
+ * transfer per-segment enum state to the next generation.
+ */
+ protected final Map enumIndexes;
+
+ // Thread-stickiness guard.
+ private final long ownerThreadId;
+
+ /** Construct a {@code PrimaryKeyLookup} bound to {@code reader}. */
+ public PrimaryKeyLookup(IndexReader reader, String idFieldName) throws
IOException {
+this(reader, idFieldName, Collections.emptyMap(), null, null);
+ }
+
+ /** Construct a {@code PrimaryKeyLookup} instance. */
+ protected PrimaryKeyLookup(
+ IndexReader reader,
+ String idFieldName,
+ Map prevEnumIndexes,
+ TermsEnum[] reusableTermsEnums,
+ PostingsEnum[] reusablePostingsEnums)
+ throws IOException {
+this.idFieldName = idFieldName;
+this.ownerThreadId = Thread.currentThread().threadId();
+
+List leaves = new ArrayList<>(reader.leaves());
+// Larger segments are more likely to have the id, so we sort largest to
smallest by numDocs:
+leaves.sort((c1, c2) -> c2.reader().numDocs() - c1.reader().numDocs());
+
+termsEnums = new TermsEnum[leaves.size()];
+postingsEnums = new PostingsEnum[leaves.size()];
+liveDocs = new Bits[leaves.size()];
+docBases = new int[leaves.size()];
+enumIndexes = new HashMap<>();
+int numEnums = 0;
+boolean hasDeletions = false;
+
+for (LeafReaderContext context : leaves) {
+ LeafReader leafReader = context.reader();
+ CacheHelper cacheHelper = leafReader.getCoreCacheHelper();
+ IndexReader.CacheKey cacheKey = cacheHelper == null ? null :
cacheHelper.getKey();
+
+ if (cacheKey != null && prevEnumIndexes.containsKey(cacheKey)) {
+// Reuse termsEnum, postingsEnum.
+int seg = prevEnumIndexes.get(cacheKey);
+termsEnums[numEnums] = reusableTermsEnums[seg];
+postingsEnums[numEnums] = reusablePostingsEnums[seg];
+ } else {
+// New or empty segment.
+Terms terms = leafReader.terms(idFieldName);
+if (terms != null) {
+ termsEnums[numEnums] = terms.iterator();
+ assert termsEnums[numEnums] != null;
+}
+ }
+
+ if (termsEnums[numEnums] != null) {
+if (cacheKey != null) {
+ enumIndexes.put(cacheKey, numEnums);
+}
+
+docBases[numEnums] = context.docBase;
+liveDocs[numEnums] = leafReader.getLiveDocs();
+
Re: [PR] Promote PerThreadPKLookup to core as PrimaryKeyLookup [lucene]
shubhamsrkdev commented on code in PR #16128: URL: https://github.com/apache/lucene/pull/16128#discussion_r3318446426 ## lucene/core/src/java/org/apache/lucene/index/PrimaryKeyLookup.java: ## @@ -0,0 +1,175 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.apache.lucene.index; Review Comment: Hmm, not sure. It uses IndexReader, TermsEnum, PostingsEnum, LeafReaderContext - all in `org.apache.lucene.index`. It doesn't score, doesn't use Query/Weight/Scorer. The original PerThreadPKLookup also lived at the index layer. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] Promote PerThreadPKLookup to core as PrimaryKeyLookup [lucene]
epotyom commented on code in PR #16128:
URL: https://github.com/apache/lucene/pull/16128#discussion_r3303852420
##
lucene/core/src/java/org/apache/lucene/index/PrimaryKeyLookup.java:
##
@@ -0,0 +1,175 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.apache.lucene.index;
Review Comment:
would `org.apache.lucene.search` be a better place for it?
##
lucene/core/src/java/org/apache/lucene/index/PrimaryKeyLookup.java:
##
@@ -0,0 +1,175 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.apache.lucene.index;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.lucene.index.IndexReader.CacheHelper;
+import org.apache.lucene.util.Bits;
+import org.apache.lucene.util.BytesRef;
+
+/**
+ * Class to do efficient primary-key lookups. Instances are not thread-safe.
Each calling thread
+ * must own its own instance, and an instance must only ever be used from the
thread that
+ * constructed it. When assertions are enabled this invariant is checked on
every lookup and reopen
+ * call. Do not use this if a term may appear in more than one document! It
will only return the
+ * first one it finds.
+ *
+ * @lucene.experimental
+ */
+public class PrimaryKeyLookup {
Review Comment:
Can we make it final? The deprecated test class can use composition instead
of inheritance.
##
lucene/core/src/java/org/apache/lucene/index/PrimaryKeyLookup.java:
##
@@ -0,0 +1,175 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.apache.lucene.index;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.lucene.index.IndexReader.CacheHelper;
+import org.apache.lucene.util.Bits;
+import org.apache.lucene.util.BytesRef;
+
+/**
+ * Class to do efficient primary-key lookups. Instances are not thread-safe.
Each calling thread
+ * must own its own instance, and an instance must only ever be used from the
thread that
+ * constructed it. When assertions are enabled this invariant is checked on
every lookup and reopen
+ * call. Do not use this if a term may appear in more than one document! It
will only return the
+ * first one it finds.
+ *
+ * @lucene.experimental
+ */
+public class PrimaryKeyLookup {
+
+ /** Name of the primary-key field. */
+ protected final String idFieldName;
+
+ /** Per-segment {@link TermsEnum}. */
+ protected final TermsEnum[] termsEnums;
+
+ /** Per-segment {@link PostingsEnum}. */
+
