xiangfu0 commented on code in PR #19481:
URL: https://github.com/apache/pinot/pull/19481#discussion_r4102585243
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/metadata/SegmentMetadataImpl.java:
##########
@@ -436,11 +473,60 @@ public boolean isSchemaMaterialized() {
return _schema != null;
}
- /// The keys of the column metadata map, i.e. the same names as
`getSchema().getColumnNames()` without building the
- /// schema. Falls back to the explicit schema of a CONSUMING segment, which
has no column metadata map.
+ /// Number of schemas derived from column metadata so far in this JVM. A
load or query path that leaves this
+ /// unchanged did not build any segment's schema.
+ @VisibleForTesting
+ public static long getNumSchemaMaterializations() {
+ return NUM_SCHEMA_MATERIALIZATIONS.get();
+ }
+
+ /// An unmodifiable view of the sorted column name array, i.e. the same
names as `getSchema().getColumnNames()`
+ /// without building the schema. Falls back to the explicit schema of a
CONSUMING segment, which has no column
+ /// metadata. The view is a snapshot: it does not reflect columns added or
removed after this call.
@Override
public NavigableSet<String> getAllColumns() {
Review Comment:
Addressed in 15f6c8a955: the CONSUMING fallback is wrapped in
`Collections.unmodifiableNavigableSet`.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/indexsegment/immutable/ImmutableSegmentImpl.java:
##########
@@ -253,21 +246,14 @@ public ImmutableSegmentImpl(
/// metadata declares them complex (the same rule the eager constructor
applies).
@Nullable
private static Map<String, List<String>>
groupOpenStructChildren(SegmentMetadataImpl segmentMetadata) {
- Map<String, List<String>> children = null;
- Map<String, ColumnMetadata> columnMetadataMap =
segmentMetadata.getColumnMetadataMap();
- for (Map.Entry<String, ColumnMetadata> entry :
columnMetadataMap.entrySet()) {
- if (entry.getValue() instanceof ColumnMetadataImpl impl &&
impl.isMaterializedChild()) {
- if (children == null) {
- children = new HashMap<>();
- }
- children.computeIfAbsent(impl.getParentColumn(), k -> new
ArrayList<>()).add(entry.getKey());
+ Map<String, List<String>> children = new HashMap<>();
+ segmentMetadata.forEachColumn((column, columnMetadata) -> {
+ if (columnMetadata instanceof ColumnMetadataImpl impl &&
impl.isMaterializedChild()) {
+ children.computeIfAbsent(impl.getParentColumn(), k -> new
ArrayList<>()).add(column);
Review Comment:
Keeping as is; load-time only and the result is identical.
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/metadata/SortedStringArraySet.java:
##########
@@ -0,0 +1,313 @@
+/**
+ * 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.pinot.segment.spi.index.metadata;
+
+import java.util.AbstractSet;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.Iterator;
+import java.util.NavigableSet;
+import java.util.NoSuchElementException;
+import java.util.SortedSet;
+import java.util.TreeSet;
+import javax.annotation.Nullable;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+
+/// Unmodifiable [NavigableSet] view of a range of a sorted, duplicate-free
`String[]`, ordered naturally.
+///
+/// Lookups are a binary search over the array, so the whole set costs one
small object rather than a red-black-tree
+/// node per element. That is the point: a server holds the column names of
every loaded segment for the segment's
+/// lifetime, and a wide segment has thousands of them.
+///
+/// The array is referenced, not copied, so the view reflects nothing the
holder does afterwards *except* in-place
+/// writes: [SegmentMetadataImpl] replaces its arrays when its columns change,
which leaves an already-returned view
+/// as the snapshot taken at the time of the call.
+///
+/// `subSet`/`headSet`/`tailSet` are ranges of the same array, and like every
[NavigableSet] range view they reject
+/// an argument outside their own range rather than silently widening it.
+///
+/// Immutable and thread-safe as long as the backing array is not written in
place.
+final class SortedStringArraySet extends AbstractSet<String> implements
NavigableSet<String> {
+ private final String[] _elements;
+ private final int _from;
+ private final int _to;
+ /// The bounds this view was created with, `null` on the side it is
unbounded on. A range view has to reject an
+ /// argument outside its own range, as [NavigableSet] requires, which the
array indices alone cannot tell: an
+ /// exclusive endpoint that is not in the array leaves no trace in them.
+ @Nullable
+ private final String _low;
+ private final boolean _lowInclusive;
+ @Nullable
+ private final String _high;
+ private final boolean _highInclusive;
+
+ SortedStringArraySet(String[] elements) {
+ this(elements, 0, elements.length, null, false, null, false);
+ }
+
+ private SortedStringArraySet(String[] elements, int from, int to, @Nullable
String low, boolean lowInclusive,
+ @Nullable String high, boolean highInclusive) {
+ _elements = elements;
+ _from = from;
+ _to = to;
+ _low = low;
+ _lowInclusive = lowInclusive;
+ _high = high;
+ _highInclusive = highInclusive;
+ }
+
+ @Override
+ public int size() {
+ return _to - _from;
+ }
+
+ @Override
+ public boolean isEmpty() {
+ return _from == _to;
+ }
+
+ @Override
+ public boolean contains(Object o) {
Review Comment:
Addressed in 15f6c8a955 (class Javadoc).
--
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]
