From a8dcf15b416f46674fbb758d4f9e978f1c836126 Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Mon, 8 Jul 2024 15:54:27 +0530 Subject: [PATCH 01/15] Minor refactors --- .../apache/druid/error/DruidException.java | 14 +- .../MapOfColumnsRowsAndColumns.java | 7 + .../rowsandcols/column/IntArrayColumn.java | 2 +- .../rowsandcols/column/LongArrayColumn.java | 201 ++++++++++++++++++ .../ColumnBasedFrameRowsAndColumns.java | 5 - .../concrete/ColumnHolderRACColumn.java | 2 +- .../DefaultColumnSelectorFactoryMaker.java | 1 + .../druid/segment/AutoTypeColumnMerger.java | 6 +- .../org/apache/druid/segment/data/VByte.java | 36 ++++ .../druid/segment/serde/MetaSerdeHelper.java | 6 +- .../druid/segment/serde/cell/IOIterator.java | 6 + 11 files changed, 274 insertions(+), 12 deletions(-) create mode 100644 processing/src/main/java/org/apache/druid/query/rowsandcols/column/LongArrayColumn.java diff --git a/processing/src/main/java/org/apache/druid/error/DruidException.java b/processing/src/main/java/org/apache/druid/error/DruidException.java index a04f3f6512cf..3aa49d33f8a4 100644 --- a/processing/src/main/java/org/apache/druid/error/DruidException.java +++ b/processing/src/main/java/org/apache/druid/error/DruidException.java @@ -24,6 +24,7 @@ import org.apache.druid.java.util.common.StringUtils; import javax.annotation.concurrent.NotThreadSafe; +import java.util.Arrays; import java.util.LinkedHashMap; import java.util.Map; @@ -130,6 +131,8 @@ @NotThreadSafe public class DruidException extends RuntimeException { + public static final String CLASS_NAME_STR = DruidException.class.getName(); + /** * Starts building a "general" DruidException targeting the specified persona. * @@ -467,7 +470,7 @@ public DruidException build(String formatMe, Object... vals) public DruidException build(Throwable cause, String formatMe, Object... vals) { - return new DruidException( + final DruidException retVal = new DruidException( cause, errorCode, targetPersona, @@ -475,6 +478,15 @@ public DruidException build(Throwable cause, String formatMe, Object... vals) StringUtils.nonStrictFormat(formatMe, vals), deserialized ); + + StackTraceElement[] stackTrace = retVal.getStackTrace(); + int firstNonDruidExceptionIndex = 0; + while (stackTrace[firstNonDruidExceptionIndex].getClassName().startsWith(CLASS_NAME_STR)) { + ++firstNonDruidExceptionIndex; + } + retVal.setStackTrace(Arrays.copyOfRange(stackTrace, firstNonDruidExceptionIndex, stackTrace.length)); + + return retVal; } } diff --git a/processing/src/main/java/org/apache/druid/query/rowsandcols/MapOfColumnsRowsAndColumns.java b/processing/src/main/java/org/apache/druid/query/rowsandcols/MapOfColumnsRowsAndColumns.java index 121e4863bcdc..42972f9340da 100644 --- a/processing/src/main/java/org/apache/druid/query/rowsandcols/MapOfColumnsRowsAndColumns.java +++ b/processing/src/main/java/org/apache/druid/query/rowsandcols/MapOfColumnsRowsAndColumns.java @@ -25,6 +25,7 @@ import org.apache.druid.query.rowsandcols.column.Column; import org.apache.druid.query.rowsandcols.column.DoubleArrayColumn; import org.apache.druid.query.rowsandcols.column.IntArrayColumn; +import org.apache.druid.query.rowsandcols.column.LongArrayColumn; import org.apache.druid.query.rowsandcols.column.ObjectArrayColumn; import org.apache.druid.query.rowsandcols.semantic.AppendableRowsAndColumns; import org.apache.druid.segment.column.ColumnType; @@ -170,6 +171,12 @@ public Builder add(String name, int[] vals) return add(name, new IntArrayColumn(vals)); } + @SuppressWarnings("unused") + public Builder add(String name, long[] vals) + { + return add(name, new LongArrayColumn(vals)); + } + public Builder add(String name, double[] vals) { return add(name, new DoubleArrayColumn(vals)); diff --git a/processing/src/main/java/org/apache/druid/query/rowsandcols/column/IntArrayColumn.java b/processing/src/main/java/org/apache/druid/query/rowsandcols/column/IntArrayColumn.java index 673cebf0e2e4..07c083d7d9f8 100644 --- a/processing/src/main/java/org/apache/druid/query/rowsandcols/column/IntArrayColumn.java +++ b/processing/src/main/java/org/apache/druid/query/rowsandcols/column/IntArrayColumn.java @@ -195,7 +195,7 @@ public FindResult findString(int startIndex, int endIndex, String val) @Override public FindResult findComplex(int startIndex, int endIndex, Object val) { - return findDouble(startIndex, endIndex, (int) Numbers.tryParseLong(val, 0)); + return findInt(startIndex, endIndex, (int) Numbers.tryParseLong(val, 0)); } } } diff --git a/processing/src/main/java/org/apache/druid/query/rowsandcols/column/LongArrayColumn.java b/processing/src/main/java/org/apache/druid/query/rowsandcols/column/LongArrayColumn.java new file mode 100644 index 000000000000..6374c145b94f --- /dev/null +++ b/processing/src/main/java/org/apache/druid/query/rowsandcols/column/LongArrayColumn.java @@ -0,0 +1,201 @@ +/* + * 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.druid.query.rowsandcols.column; + +import org.apache.druid.java.util.common.ISE; +import org.apache.druid.java.util.common.Numbers; +import org.apache.druid.query.rowsandcols.util.FindResult; +import org.apache.druid.segment.column.ColumnType; + +import javax.annotation.Nonnull; +import javax.annotation.Nullable; +import java.util.Arrays; + +public class LongArrayColumn implements Column +{ + private final long[] vals; + + public LongArrayColumn( + long[] vals + ) + { + this.vals = vals; + } + + @Nonnull + @Override + public ColumnAccessor toAccessor() + { + return new MyColumnAccessor(); + } + + @Nullable + @SuppressWarnings("unchecked") + @Override + public T as(Class clazz) + { + if (VectorCopier.class.equals(clazz)) { + return (T) (VectorCopier) (into, intoStart) -> { + if (Integer.MAX_VALUE - vals.length < intoStart) { + throw new ISE( + "too many rows!!! intoStart[%,d], vals.length[%,d] combine to exceed max_int", + intoStart, + vals.length + ); + } + for (int i = 0; i < vals.length; ++i) { + into[intoStart + i] = vals[i]; + } + }; + } + if (ColumnValueSwapper.class.equals(clazz)) { + return (T) (ColumnValueSwapper) (lhs, rhs) -> { + long tmp = vals[lhs]; + vals[lhs] = vals[rhs]; + vals[rhs] = tmp; + }; + } + return null; + } + + private class MyColumnAccessor implements BinarySearchableAccessor + { + @Override + public ColumnType getType() + { + return ColumnType.LONG; + } + + @Override + public int numRows() + { + return vals.length; + } + + @Override + public boolean isNull(int rowNum) + { + return false; + } + + @Override + public Object getObject(int rowNum) + { + return vals[rowNum]; + } + + @Override + public double getDouble(int rowNum) + { + return vals[rowNum]; + } + + @Override + public float getFloat(int rowNum) + { + return vals[rowNum]; + } + + @Override + public long getLong(int rowNum) + { + return vals[rowNum]; + } + + @Override + public int getInt(int rowNum) + { + return (int) vals[rowNum]; + } + + @Override + public int compareRows(int lhsRowNum, int rhsRowNum) + { + return Long.compare(vals[lhsRowNum], vals[rhsRowNum]); + } + + + @Override + public FindResult findNull(int startIndex, int endIndex) + { + return FindResult.notFound(endIndex); + } + + @Override + public FindResult findDouble(int startIndex, int endIndex, double val) + { + return findLong(startIndex, endIndex, (int) val); + } + + @Override + public FindResult findFloat(int startIndex, int endIndex, float val) + { + return findLong(startIndex, endIndex, (int) val); + } + + @Override + public FindResult findLong(int startIndex, int endIndex, long val) + { + if (vals[startIndex] == val) { + int end = startIndex + 1; + + while (end < endIndex && vals[end] == val) { + ++end; + } + return FindResult.found(startIndex, end); + } + + int i = Arrays.binarySearch(vals, startIndex, endIndex, val); + if (i > 0) { + int foundStart = i; + int foundEnd = i + 1; + + while (foundStart - 1 >= startIndex && vals[foundStart - 1] == val) { + --foundStart; + } + + while (foundEnd < endIndex && vals[foundEnd] == val) { + ++foundEnd; + } + + return FindResult.found(foundStart, foundEnd); + } else { + return FindResult.notFound(-(i + 1)); + } + } + + public FindResult findInt(int startIndex, int endIndex, int val) + { + return findLong(startIndex, endIndex, val); + } + + @Override + public FindResult findString(int startIndex, int endIndex, String val) + { + return findLong(startIndex, endIndex, (int) Numbers.tryParseLong(val, 0)); + } + + @Override + public FindResult findComplex(int startIndex, int endIndex, Object val) + { + return findLong(startIndex, endIndex, Numbers.tryParseLong(val, 0)); + } + } +} diff --git a/processing/src/main/java/org/apache/druid/query/rowsandcols/concrete/ColumnBasedFrameRowsAndColumns.java b/processing/src/main/java/org/apache/druid/query/rowsandcols/concrete/ColumnBasedFrameRowsAndColumns.java index ada3da164ea1..71c2541b387c 100644 --- a/processing/src/main/java/org/apache/druid/query/rowsandcols/concrete/ColumnBasedFrameRowsAndColumns.java +++ b/processing/src/main/java/org/apache/druid/query/rowsandcols/concrete/ColumnBasedFrameRowsAndColumns.java @@ -28,7 +28,6 @@ import org.apache.druid.java.util.common.Intervals; import org.apache.druid.query.rowsandcols.RowsAndColumns; import org.apache.druid.query.rowsandcols.column.Column; -import org.apache.druid.query.rowsandcols.semantic.WireTransferable; import org.apache.druid.segment.CloseableShapeshifter; import org.apache.druid.segment.StorageAdapter; import org.apache.druid.segment.column.ColumnType; @@ -80,7 +79,6 @@ public Column findColumn(String name) } } return colCache.get(name); - } @SuppressWarnings("unchecked") @@ -91,9 +89,6 @@ public T as(Class clazz) if (StorageAdapter.class.equals(clazz)) { return (T) new FrameStorageAdapter(frame, FrameReader.create(signature), Intervals.ETERNITY); } - if (WireTransferable.class.equals(clazz)) { - return (T) this; - } return null; } diff --git a/processing/src/main/java/org/apache/druid/query/rowsandcols/concrete/ColumnHolderRACColumn.java b/processing/src/main/java/org/apache/druid/query/rowsandcols/concrete/ColumnHolderRACColumn.java index ed4f8ead52e8..d68f8872bf46 100644 --- a/processing/src/main/java/org/apache/druid/query/rowsandcols/concrete/ColumnHolderRACColumn.java +++ b/processing/src/main/java/org/apache/druid/query/rowsandcols/concrete/ColumnHolderRACColumn.java @@ -91,7 +91,7 @@ public int numRows() public boolean isNull(int rowNum) { offset.set(rowNum); - return valueSelector.isNull(); + return valueSelector.getObject() == null; } @Nullable diff --git a/processing/src/main/java/org/apache/druid/query/rowsandcols/semantic/DefaultColumnSelectorFactoryMaker.java b/processing/src/main/java/org/apache/druid/query/rowsandcols/semantic/DefaultColumnSelectorFactoryMaker.java index 3c6d3cc08c9b..2eda694ec101 100644 --- a/processing/src/main/java/org/apache/druid/query/rowsandcols/semantic/DefaultColumnSelectorFactoryMaker.java +++ b/processing/src/main/java/org/apache/druid/query/rowsandcols/semantic/DefaultColumnSelectorFactoryMaker.java @@ -210,6 +210,7 @@ public PassThroughColumnValueSelector( break; case ARRAY: myClazz = List.class; + break; default: throw DruidException.defensive("this class cannot handle type [%s]", columnAccessor.getType()); } diff --git a/processing/src/main/java/org/apache/druid/segment/AutoTypeColumnMerger.java b/processing/src/main/java/org/apache/druid/segment/AutoTypeColumnMerger.java index 5d1198f5460a..801eaf112a5f 100644 --- a/processing/src/main/java/org/apache/druid/segment/AutoTypeColumnMerger.java +++ b/processing/src/main/java/org/apache/druid/segment/AutoTypeColumnMerger.java @@ -22,7 +22,7 @@ import com.google.common.base.Preconditions; import com.google.common.collect.Iterators; import com.google.common.collect.PeekingIterator; -import org.apache.druid.java.util.common.ISE; +import org.apache.druid.error.DruidException; import org.apache.druid.java.util.common.io.Closer; import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.segment.column.ColumnDescriptor; @@ -212,7 +212,7 @@ public void writeMergedValueDictionary(List adapters) throws I ); break; default: - throw new ISE( + throw DruidException.defensive( "How did we get here? Column [%s] with type [%s] does not have specialized serializer", name, logicalType @@ -349,7 +349,7 @@ public boolean hasOnlyNulls() @Override public ColumnDescriptor makeColumnDescriptor() { - ColumnDescriptor.Builder descriptorBuilder = new ColumnDescriptor.Builder(); + ColumnDescriptor.Builder descriptorBuilder = ColumnDescriptor.builder(); final NestedCommonFormatColumnPartSerde partSerde = NestedCommonFormatColumnPartSerde.serializerBuilder() diff --git a/processing/src/main/java/org/apache/druid/segment/data/VByte.java b/processing/src/main/java/org/apache/druid/segment/data/VByte.java index 749382cc001e..7886ae7f070a 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/VByte.java +++ b/processing/src/main/java/org/apache/druid/segment/data/VByte.java @@ -19,7 +19,9 @@ package org.apache.druid.segment.data; +import java.io.IOException; import java.nio.ByteBuffer; +import java.nio.channels.WritableByteChannel; public class VByte { @@ -58,6 +60,40 @@ public static int readInt(ByteBuffer buffer) return v; } + public static int writeInt(WritableByteChannel out, int val) throws IOException + { + final byte[] bytes = new byte[5]; + final int numBytes; + if (val < (1 << 7)) { + bytes[0] = (byte) (val | (1 << 7)); + numBytes = 1; + } else if (val < (1 << 14)) { + bytes[0] = extract7bits(0, val); + bytes[1] = (byte) (extract7bitsmaskless(1, (val)) | (1 << 7)); + numBytes = 2; + } else if (val < (1 << 21)) { + bytes[0] = extract7bits(0, val); + bytes[1] = extract7bits(1, val); + bytes[2] = (byte) (extract7bitsmaskless(2, (val)) | (1 << 7)); + numBytes = 3; + } else if (val < (1 << 28)) { + bytes[0] = extract7bits(0, val); + bytes[1] = extract7bits(1, val); + bytes[2] = extract7bits(2, val); + bytes[3] = (byte) (extract7bitsmaskless(3, (val)) | (1 << 7)); + numBytes = 4; + } else { + bytes[0] = extract7bits(0, val); + bytes[1] = extract7bits(1, val); + bytes[2] = extract7bits(2, val); + bytes[3] = extract7bits(3, val); + bytes[4] = (byte) (extract7bitsmaskless(4, (val)) | (1 << 7)); + numBytes = 5; + } + out.write(ByteBuffer.wrap(bytes, 0, numBytes)); + return numBytes; + } + /** * Write a variable byte (vbyte) encoded integer to a {@link ByteBuffer} at the current position, advancing the buffer * position by the number of bytes required to represent the integer, between 1 and 5 bytes. diff --git a/processing/src/main/java/org/apache/druid/segment/serde/MetaSerdeHelper.java b/processing/src/main/java/org/apache/druid/segment/serde/MetaSerdeHelper.java index 113821cee150..08e97e3e5017 100644 --- a/processing/src/main/java/org/apache/druid/segment/serde/MetaSerdeHelper.java +++ b/processing/src/main/java/org/apache/druid/segment/serde/MetaSerdeHelper.java @@ -112,7 +112,11 @@ public void writeTo(WritableByteChannel channel, T x) throws IOException public int size(T x) { - return fieldWriters.stream().mapToInt(w -> w.size(x)).sum(); + int retVal = 0; + for (FieldWriter fieldWriter : fieldWriters) { + retVal += fieldWriter.size(x); + } + return retVal; } public interface FieldWriter diff --git a/processing/src/main/java/org/apache/druid/segment/serde/cell/IOIterator.java b/processing/src/main/java/org/apache/druid/segment/serde/cell/IOIterator.java index 887f1fb65ac6..3931601dd4f3 100644 --- a/processing/src/main/java/org/apache/druid/segment/serde/cell/IOIterator.java +++ b/processing/src/main/java/org/apache/druid/segment/serde/cell/IOIterator.java @@ -22,6 +22,12 @@ import java.io.Closeable; import java.io.IOException; +/** + * An Iterator-like interface that is intentionally not extending Iterator. This is because it is Closeable + * and we never want to lose track of the fact that the object needs to be closed. + * + * @param + */ public interface IOIterator extends Closeable { boolean hasNext() throws IOException; From 57e9b02a7b50fe12c685125e57b22afa18c69601 Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Mon, 8 Jul 2024 16:10:29 +0530 Subject: [PATCH 02/15] Refactor write to return index of value written --- .../apache/druid/frame/read/FrameReader.java | 6 +-- .../apache/druid/guice/JsonConfigurator.java | 4 +- .../StringUtf8DictionaryEncodedColumn.java | 25 ++++++------ .../druid/segment/data/DictionaryWriter.java | 31 +++++++++++++- .../data/EncodedStringDictionaryWriter.java | 6 +-- .../segment/data/FixedIndexedWriter.java | 40 +++++++++---------- .../segment/data/FrontCodedIndexedWriter.java | 13 ++++-- .../data/FrontCodedIntArrayIndexedWriter.java | 37 +++++++++++++---- .../segment/data/GenericIndexedWriter.java | 7 ++-- .../MMappedQueryableSegmentizerFactory.java | 23 ++++++++++- .../segment/nested/DictionaryIdLookup.java | 2 +- 11 files changed, 136 insertions(+), 58 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/frame/read/FrameReader.java b/processing/src/main/java/org/apache/druid/frame/read/FrameReader.java index 8ddf99325d39..46a848fb6b15 100644 --- a/processing/src/main/java/org/apache/druid/frame/read/FrameReader.java +++ b/processing/src/main/java/org/apache/druid/frame/read/FrameReader.java @@ -20,6 +20,7 @@ package org.apache.druid.frame.read; import com.google.common.base.Preconditions; +import org.apache.druid.error.DruidException; import org.apache.druid.frame.Frame; import org.apache.druid.frame.field.FieldReader; import org.apache.druid.frame.field.FieldReaders; @@ -31,7 +32,6 @@ import org.apache.druid.frame.segment.row.FrameCursorFactory; import org.apache.druid.frame.write.FrameWriterUtils; import org.apache.druid.java.util.common.IAE; -import org.apache.druid.java.util.common.ISE; import org.apache.druid.segment.CursorFactory; import org.apache.druid.segment.column.ColumnCapabilities; import org.apache.druid.segment.column.ColumnType; @@ -44,7 +44,7 @@ /** * Embeds the logic to read frames with a given {@link RowSignature}. - * + *

* Stateless and immutable. */ public class FrameReader @@ -146,7 +146,7 @@ public CursorFactory makeCursorFactory(final Frame frame) case ROW_BASED: return new FrameCursorFactory(frame, this, fieldReaders); default: - throw new ISE("Unrecognized frame type [%s]", frame.type()); + throw DruidException.defensive("Unrecognized frame type [%s]", frame.type()); } } diff --git a/processing/src/main/java/org/apache/druid/guice/JsonConfigurator.java b/processing/src/main/java/org/apache/druid/guice/JsonConfigurator.java index 1e4f18dc1cd7..ed7e79df3f17 100644 --- a/processing/src/main/java/org/apache/druid/guice/JsonConfigurator.java +++ b/processing/src/main/java/org/apache/druid/guice/JsonConfigurator.java @@ -236,9 +236,9 @@ private static void hieraricalPutValue( // to configure ParametrizedUriEmitterConfig object. So skipping xxx=yyy key-value pair when configuring Emitter // doesn't make any difference. That is why we just log this situation, instead of throwing an exception. log.info( - "Skipping %s property: one of it's prefixes is also used as a property key. Prefix: %s", + "Skipping property [%s]: one of it's prefixes [%s] is also used as a property key.", originalProperty, - propertyPrefix + nestedKey ); return; } diff --git a/processing/src/main/java/org/apache/druid/segment/column/StringUtf8DictionaryEncodedColumn.java b/processing/src/main/java/org/apache/druid/segment/column/StringUtf8DictionaryEncodedColumn.java index fe8ade4a9ed6..c3ebde1854c0 100644 --- a/processing/src/main/java/org/apache/druid/segment/column/StringUtf8DictionaryEncodedColumn.java +++ b/processing/src/main/java/org/apache/druid/segment/column/StringUtf8DictionaryEncodedColumn.java @@ -528,11 +528,11 @@ public Indexed getStringDictionary() /** * Base type for a {@link SingleValueDimensionVectorSelector} for a dictionary encoded {@link ColumnType#STRING} * built around a {@link ColumnarInts}. Dictionary not included - BYO dictionary lookup methods. - * + *

* Assumes that all implementations return true for {@link #supportsLookupNameUtf8()}. */ public abstract static class StringSingleValueDimensionVectorSelector - implements SingleValueDimensionVectorSelector, IdLookup + implements SingleValueDimensionVectorSelector, IdLookup { private final ColumnarInts column; private final ReadableVectorOffset offset; @@ -540,8 +540,8 @@ public abstract static class StringSingleValueDimensionVectorSelector private int id = ReadableVectorInspector.NULL_ID; public StringSingleValueDimensionVectorSelector( - ColumnarInts column, - ReadableVectorOffset offset + ColumnarInts column, + ReadableVectorOffset offset ) { this.column = column; @@ -601,11 +601,11 @@ public int getMaxVectorSize() /** * Base type for a {@link MultiValueDimensionVectorSelector} for a dictionary encoded {@link ColumnType#STRING} * built around a {@link ColumnarMultiInts}. Dictionary not included - BYO dictionary lookup methods. - * + *

* Assumes that all implementations return true for {@link #supportsLookupNameUtf8()}. */ public abstract static class StringMultiValueDimensionVectorSelector - implements MultiValueDimensionVectorSelector, IdLookup + implements MultiValueDimensionVectorSelector, IdLookup { private final ColumnarMultiInts multiValueColumn; private final ReadableVectorOffset offset; @@ -614,8 +614,8 @@ public abstract static class StringMultiValueDimensionVectorSelector private int id = ReadableVectorInspector.NULL_ID; public StringMultiValueDimensionVectorSelector( - ColumnarMultiInts multiValueColumn, - ReadableVectorOffset offset + ColumnarMultiInts multiValueColumn, + ReadableVectorOffset offset ) { this.multiValueColumn = multiValueColumn; @@ -670,6 +670,7 @@ public IdLookup idLookup() { return this; } + @Override public int getCurrentVectorSize() { @@ -697,8 +698,8 @@ public abstract static class StringVectorObjectSelector implements VectorObjectS private int id = ReadableVectorInspector.NULL_ID; public StringVectorObjectSelector( - ColumnarInts column, - ReadableVectorOffset offset + ColumnarInts column, + ReadableVectorOffset offset ) { this.column = column; @@ -757,8 +758,8 @@ public abstract static class MultiValueStringVectorObjectSelector implements Vec private int id = ReadableVectorInspector.NULL_ID; public MultiValueStringVectorObjectSelector( - ColumnarMultiInts multiValueColumn, - ReadableVectorOffset offset + ColumnarMultiInts multiValueColumn, + ReadableVectorOffset offset ) { this.multiValueColumn = multiValueColumn; diff --git a/processing/src/main/java/org/apache/druid/segment/data/DictionaryWriter.java b/processing/src/main/java/org/apache/druid/segment/data/DictionaryWriter.java index 170f6975f28e..5901e2e13205 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/DictionaryWriter.java +++ b/processing/src/main/java/org/apache/druid/segment/data/DictionaryWriter.java @@ -28,12 +28,41 @@ public interface DictionaryWriter extends Serializer { boolean isSorted(); + /** + * Prepares the writer for writing + * + * @throws IOException if there is a problem with IO + */ void open() throws IOException; - void write(@Nullable T objectToWrite) throws IOException; + /** + * Writes an object to the dictionary. + *

+ * Returns the index of the value that was just written. This is defined as the `int` value that can be passed + * into {@link #get} such that it will return the same value back. + * + * @param objectToWrite object to be written to the dictionary + * @return index of the value that was just written + * @throws IOException if there is a problem with IO + */ + int write(@Nullable T objectToWrite) throws IOException; + /** + * Returns an object that has already been written via the {@link #write} method. + * + * @param dictId index of the object to return + * @return the object identified by the given index + * @throws IOException if there is a problem with IO + */ @Nullable T get(int dictId) throws IOException; + /** + * Returns the number of items that have been written so far in this dictionary. Any number lower than this + * cardinality can be passed into {@link #get} and a value will be returned. If a value greater than or equal to + * the cardinality is passed into {@link #get} all sorts of things could happen, but likely none of them are good. + * + * @return the number of items that have been written so far + */ int getCardinality(); } diff --git a/processing/src/main/java/org/apache/druid/segment/data/EncodedStringDictionaryWriter.java b/processing/src/main/java/org/apache/druid/segment/data/EncodedStringDictionaryWriter.java index 371a73bebd74..1ca4590d0bb9 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/EncodedStringDictionaryWriter.java +++ b/processing/src/main/java/org/apache/druid/segment/data/EncodedStringDictionaryWriter.java @@ -58,9 +58,9 @@ public void open() throws IOException } @Override - public void write(@Nullable String objectToWrite) throws IOException + public int write(@Nullable String objectToWrite) throws IOException { - delegate.write(StringUtils.toUtf8Nullable(NullHandling.emptyToNullIfNeeded(objectToWrite))); + return delegate.write(StringUtils.toUtf8Nullable(NullHandling.emptyToNullIfNeeded(objectToWrite))); } @Nullable @@ -93,4 +93,4 @@ public void writeTo(WritableByteChannel channel, FileSmoosher smoosher) throws I channel.write(ByteBuffer.wrap(new byte[]{encodingStrategy.getId()})); delegate.writeTo(channel, smoosher); } -} +} \ No newline at end of file diff --git a/processing/src/main/java/org/apache/druid/segment/data/FixedIndexedWriter.java b/processing/src/main/java/org/apache/druid/segment/data/FixedIndexedWriter.java index b1b473b3419d..1c77e73354ca 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/FixedIndexedWriter.java +++ b/processing/src/main/java/org/apache/druid/segment/data/FixedIndexedWriter.java @@ -20,8 +20,8 @@ package org.apache.druid.segment.data; import org.apache.druid.common.config.NullHandling; +import org.apache.druid.error.DruidException; import org.apache.druid.io.Channels; -import org.apache.druid.java.util.common.ISE; import org.apache.druid.java.util.common.io.smoosh.FileSmoosher; import org.apache.druid.segment.column.TypeStrategy; import org.apache.druid.segment.writeout.SegmentWriteOutMedium; @@ -46,14 +46,16 @@ public class FixedIndexedWriter implements DictionaryWriter private final Comparator comparator; private final ByteBuffer scratch; private final ByteBuffer readBuffer; - private int numWritten; + private final boolean isSorted; + private final int width; + + private int cardinality = 0; + @Nullable private WriteOutBytes valuesOut = null; private boolean hasNulls = false; - private boolean isSorted; @Nullable private T prevObject = null; - private final int width; public FixedIndexedWriter( SegmentWriteOutMedium segmentWriteOutMedium, @@ -87,7 +89,7 @@ public void open() throws IOException @Override public int getCardinality() { - return hasNulls ? numWritten + 1 : numWritten; + return cardinality; } @Override @@ -97,28 +99,31 @@ public long getSerializedSize() } @Override - public void write(@Nullable T objectToWrite) throws IOException + public int write(@Nullable T objectToWrite) throws IOException { if (prevObject != null && isSorted && comparator.compare(prevObject, objectToWrite) >= 0) { - throw new ISE( + throw DruidException.defensive( "Values must be sorted and unique. Element [%s] with value [%s] is before or equivalent to [%s]", - numWritten, + cardinality, objectToWrite, prevObject ); } if (objectToWrite == null) { + if (cardinality != 0) { + throw DruidException.defensive("Null must come first, got it at cardinality[%,d]!=0", cardinality); + } hasNulls = true; - return; + return cardinality++; } scratch.clear(); typeStrategy.write(scratch, objectToWrite, width); scratch.flip(); Channels.writeFully(valuesOut, scratch); - numWritten++; prevObject = objectToWrite; + return cardinality++; } @Override @@ -141,7 +146,7 @@ public void writeTo( scratch.flip(); Channels.writeFully(channel, scratch); scratch.clear(); - scratch.putInt(numWritten); + scratch.putInt(hasNulls ? cardinality - 1 : cardinality); // we don't actually write the null entry, so subtract 1 scratch.flip(); Channels.writeFully(channel, scratch); valuesOut.writeTo(channel); @@ -166,7 +171,7 @@ public T get(int index) throws IOException public Iterator getIterator() { final ByteBuffer iteratorBuffer = ByteBuffer.allocate(width * PAGE_SIZE).order(readBuffer.order()); - final int totalCount = hasNulls ? 1 + numWritten : numWritten; + final int totalCount = cardinality; final int startPos = hasNulls ? 1 : 0; return new Iterator() @@ -197,13 +202,8 @@ private void readPage() { iteratorBuffer.clear(); try { - if (numWritten - (pos - startPos) < PAGE_SIZE) { - int size = (numWritten - (pos - startPos)) * width; - iteratorBuffer.limit(size); - valuesOut.readFully((long) (pos - startPos) * width, iteratorBuffer); - } else { - valuesOut.readFully((long) (pos - startPos) * width, iteratorBuffer); - } + iteratorBuffer.limit(Math.min(PAGE_SIZE, (cardinality - pos) * width)); + valuesOut.readFully((long) (pos - startPos) * width, iteratorBuffer); iteratorBuffer.clear(); } catch (IOException e) { @@ -212,4 +212,4 @@ private void readPage() } }; } -} +} \ No newline at end of file diff --git a/processing/src/main/java/org/apache/druid/segment/data/FrontCodedIndexedWriter.java b/processing/src/main/java/org/apache/druid/segment/data/FrontCodedIndexedWriter.java index c24d2e55d71d..be4a2ca821c6 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/FrontCodedIndexedWriter.java +++ b/processing/src/main/java/org/apache/druid/segment/data/FrontCodedIndexedWriter.java @@ -21,6 +21,7 @@ import com.google.common.primitives.Ints; import org.apache.druid.common.config.NullHandling; +import org.apache.druid.error.DruidException; import org.apache.druid.io.Channels; import org.apache.druid.java.util.common.IAE; import org.apache.druid.java.util.common.ISE; @@ -102,7 +103,7 @@ public void open() throws IOException } @Override - public void write(@Nullable byte[] value) throws IOException + public int write(@Nullable byte[] value) throws IOException { if (prevObject != null && compareNullableUtf8UsingJavaStringOrdering(prevObject, value) >= 0) { throw new ISE( @@ -114,8 +115,11 @@ public void write(@Nullable byte[] value) throws IOException } if (value == null) { + if (numWritten != 0) { + throw DruidException.defensive("Null must come first, got it at cardinality[%,d]!=0", numWritten); + } hasNulls = true; - return; + return 0; } // if the bucket buffer is full, write the bucket @@ -143,8 +147,9 @@ public void write(@Nullable byte[] value) throws IOException bucketBuffer[numWritten % bucketSize] = value; - ++numWritten; + int retVal = numWritten++; prevObject = value; + return retVal + (hasNulls ? 1 : 0); } @@ -480,4 +485,4 @@ static ByteBuffer getFromBucketV1(ByteBuffer buffer, int offset, int bucketSize) } return ByteBuffer.wrap(valueBytes); } -} +} \ No newline at end of file diff --git a/processing/src/main/java/org/apache/druid/segment/data/FrontCodedIntArrayIndexedWriter.java b/processing/src/main/java/org/apache/druid/segment/data/FrontCodedIntArrayIndexedWriter.java index 8116882191b0..954d8dc22050 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/FrontCodedIntArrayIndexedWriter.java +++ b/processing/src/main/java/org/apache/druid/segment/data/FrontCodedIntArrayIndexedWriter.java @@ -21,6 +21,7 @@ import com.google.common.primitives.Ints; import org.apache.druid.common.config.NullHandling; +import org.apache.druid.error.DruidException; import org.apache.druid.io.Channels; import org.apache.druid.java.util.common.IAE; import org.apache.druid.java.util.common.ISE; @@ -81,6 +82,10 @@ public class FrontCodedIntArrayIndexedWriter implements DictionaryWriter private boolean isClosed = false; private boolean hasNulls = false; + private int readCachedBucket = -1; + @Nullable + private ByteBuffer readBufferCache = null; + public FrontCodedIntArrayIndexedWriter( SegmentWriteOutMedium segmentWriteOutMedium, ByteOrder byteOrder, @@ -107,7 +112,7 @@ public void open() throws IOException } @Override - public void write(@Nullable int[] value) throws IOException + public int write(@Nullable int[] value) throws IOException { if (prevObject != null && ARRAY_COMPARATOR.compare(prevObject, value) >= 0) { @@ -120,8 +125,11 @@ public void write(@Nullable int[] value) throws IOException } if (value == null) { + if (numWritten != 0) { + throw DruidException.defensive("Null must come first, got it at numWritten[%,d]!=0", numWritten); + } hasNulls = true; - return; + return 0; } // if the bucket buffer is full, write the bucket @@ -147,8 +155,9 @@ public void write(@Nullable int[] value) throws IOException bucketBuffer[numWritten % bucketSize] = value; - ++numWritten; + int retVal = numWritten++; prevObject = value; + return retVal + (hasNulls ? 1 : 0); } @@ -206,6 +215,11 @@ public int[] get(int index) throws IOException return bucketBuffer[relativeIndex]; } else { final int bucket = adjustedIndex >> div; + if (readCachedBucket == bucket) { + readBufferCache.position(0); + return getFromBucket(readBufferCache, relativeIndex); + } + long startOffset; if (bucket == 0) { startOffset = 0; @@ -217,10 +231,17 @@ public int[] get(int index) throws IOException if (currentBucketSize == 0) { return null; } - final ByteBuffer bucketBuffer = ByteBuffer.allocate(currentBucketSize).order(byteOrder); - valuesOut.readFully(startOffset, bucketBuffer); - bucketBuffer.clear(); - return getFromBucket(bucketBuffer, relativeIndex); + if (readBufferCache == null || readBufferCache.capacity() < currentBucketSize) { + readBufferCache = ByteBuffer.allocate(currentBucketSize).order(byteOrder); + } + readBufferCache.clear(); + readBufferCache.limit(currentBucketSize); + valuesOut.readFully(startOffset, readBufferCache); + + readCachedBucket = bucket; + + readBufferCache.position(0); + return getFromBucket(readBufferCache, relativeIndex); } } @@ -412,4 +433,4 @@ int[] getFromBucket(ByteBuffer buffer, int offset) } return value; } -} +} \ No newline at end of file diff --git a/processing/src/main/java/org/apache/druid/segment/data/GenericIndexedWriter.java b/processing/src/main/java/org/apache/druid/segment/data/GenericIndexedWriter.java index 8b38125322ba..f6fc17c8ec88 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/GenericIndexedWriter.java +++ b/processing/src/main/java/org/apache/druid/segment/data/GenericIndexedWriter.java @@ -242,7 +242,7 @@ void setIntMaxForCasting(final int intMaxForCasting) } @Override - public void write(@Nullable T objectToWrite) throws IOException + public int write(@Nullable T objectToWrite) throws IOException { if (objectsSorted && prevObject != null && strategy.compare(prevObject, objectToWrite) >= 0) { objectsSorted = false; @@ -263,7 +263,7 @@ public void write(@Nullable T objectToWrite) throws IOException // Increment number of values written. Important to do this after the check above, since numWritten is // accessed during "initializeHeaderOutLong" to determine the length of the header. - ++numWritten; + int retVal = numWritten++; if (!requireMultipleFiles) { headerOut.writeInt(checkedCastNonnegativeLongToInt(valuesOut.size())); @@ -280,6 +280,7 @@ public void write(@Nullable T objectToWrite) throws IOException if (objectsSorted) { prevObject = objectToWrite; } + return retVal; } @Nullable @@ -535,4 +536,4 @@ private int checkedCastNonnegativeLongToInt(final long n) throw new IAE("Value out of nonnegative int range"); } } -} +} \ No newline at end of file diff --git a/processing/src/main/java/org/apache/druid/segment/loading/MMappedQueryableSegmentizerFactory.java b/processing/src/main/java/org/apache/druid/segment/loading/MMappedQueryableSegmentizerFactory.java index d3124cb122f9..9cf8a577a856 100644 --- a/processing/src/main/java/org/apache/druid/segment/loading/MMappedQueryableSegmentizerFactory.java +++ b/processing/src/main/java/org/apache/druid/segment/loading/MMappedQueryableSegmentizerFactory.java @@ -20,6 +20,7 @@ package org.apache.druid.segment.loading; import com.fasterxml.jackson.annotation.JacksonInject; +import com.fasterxml.jackson.annotation.JsonCreator; import com.google.common.base.Preconditions; import org.apache.druid.segment.IndexIO; import org.apache.druid.segment.QueryableIndexSegment; @@ -34,9 +35,29 @@ */ public class MMappedQueryableSegmentizerFactory implements SegmentizerFactory { + /** + * A static method to make a segmentizer factory that can be serialized by Jackson. This exists because the object + * doesn't actually need the IndexIO object in order to be serialized by Jackson, but the public constructor + * *does* require it. We leave the public constructor alone because if indexIO is null, this SegmentizerFactory + * is largely useless, so we just create this one static method to enable creating the object for this singular + * use case. + * + * @return a SegmentizerFactory that can be used to be serialized by Jackson + */ + @SuppressWarnings("unused") + public static MMappedQueryableSegmentizerFactory makeForSerialization() + { + return new MMappedQueryableSegmentizerFactory(); + } private final IndexIO indexIO; + private MMappedQueryableSegmentizerFactory() + { + this.indexIO = null; + } + + @JsonCreator public MMappedQueryableSegmentizerFactory(@JacksonInject IndexIO indexIO) { this.indexIO = Preconditions.checkNotNull(indexIO, "Null IndexIO"); @@ -52,4 +73,4 @@ public Segment factorize(DataSegment dataSegment, File parentDir, boolean lazy, throw new SegmentLoadingException(e, "%s", e.getMessage()); } } -} +} \ No newline at end of file diff --git a/processing/src/main/java/org/apache/druid/segment/nested/DictionaryIdLookup.java b/processing/src/main/java/org/apache/druid/segment/nested/DictionaryIdLookup.java index f4176db220cd..44c37da01f26 100644 --- a/processing/src/main/java/org/apache/druid/segment/nested/DictionaryIdLookup.java +++ b/processing/src/main/java/org/apache/druid/segment/nested/DictionaryIdLookup.java @@ -51,7 +51,7 @@ /** * Value to dictionary id lookup, backed with memory mapped dictionaries populated lazily by the supplied - * @link DictionaryWriter}. + * {@link DictionaryWriter}. */ public final class DictionaryIdLookup implements Closeable { From cb79da452e905d7674bba2dc2975bfef764f2451 Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Mon, 8 Jul 2024 16:26:58 +0530 Subject: [PATCH 03/15] Refactor compressionFactory --- .../druid/segment/data/CompressionFactory.java | 9 +++++++++ .../segment/data/DeltaLongEncodingReader.java | 6 ++++++ .../segment/data/LongsLongEncodingReader.java | 6 ++++++ .../segment/data/LongsLongEncodingWriter.java | 14 +++++++++++++- .../segment/data/TableLongEncodingReader.java | 6 ++++++ .../segment/serde/cell/StorableBuffer.java | 18 ++++++++++++++++++ 6 files changed, 58 insertions(+), 1 deletion(-) diff --git a/processing/src/main/java/org/apache/druid/segment/data/CompressionFactory.java b/processing/src/main/java/org/apache/druid/segment/data/CompressionFactory.java index dde6a440d9ea..98e28380b1ef 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/CompressionFactory.java +++ b/processing/src/main/java/org/apache/druid/segment/data/CompressionFactory.java @@ -233,6 +233,13 @@ public interface LongEncodingWriter void write(long value) throws IOException; + default void write(long[] values, int offset, int length) throws IOException + { + for (int i = offset; i < length; ++i) { + write(values[i]); + } + } + /** * Flush the unwritten content to the current output. */ @@ -294,6 +301,8 @@ public interface LongEncodingReader * various duplicates. */ LongEncodingReader duplicate(); + + LongEncodingStrategy getStrategy(); } public static Supplier getLongSupplier( diff --git a/processing/src/main/java/org/apache/druid/segment/data/DeltaLongEncodingReader.java b/processing/src/main/java/org/apache/druid/segment/data/DeltaLongEncodingReader.java index 435aa2ddfd1a..b7feb3b1dd34 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/DeltaLongEncodingReader.java +++ b/processing/src/main/java/org/apache/druid/segment/data/DeltaLongEncodingReader.java @@ -82,4 +82,10 @@ public CompressionFactory.LongEncodingReader duplicate() { return new DeltaLongEncodingReader(buffer.duplicate(), base, bitsPerValue); } + + @Override + public CompressionFactory.LongEncodingStrategy getStrategy() + { + return CompressionFactory.LongEncodingStrategy.AUTO; + } } diff --git a/processing/src/main/java/org/apache/druid/segment/data/LongsLongEncodingReader.java b/processing/src/main/java/org/apache/druid/segment/data/LongsLongEncodingReader.java index 2ed0459121af..7df866f22c7f 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/LongsLongEncodingReader.java +++ b/processing/src/main/java/org/apache/druid/segment/data/LongsLongEncodingReader.java @@ -71,4 +71,10 @@ public CompressionFactory.LongEncodingReader duplicate() { return new LongsLongEncodingReader(buffer.getByteBuffer(), buffer.getTypeByteOrder()); } + + @Override + public CompressionFactory.LongEncodingStrategy getStrategy() + { + return CompressionFactory.LongEncodingStrategy.LONGS; + } } diff --git a/processing/src/main/java/org/apache/druid/segment/data/LongsLongEncodingWriter.java b/processing/src/main/java/org/apache/druid/segment/data/LongsLongEncodingWriter.java index 2aeb194d9a8a..f2b198b7e7a8 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/LongsLongEncodingWriter.java +++ b/processing/src/main/java/org/apache/druid/segment/data/LongsLongEncodingWriter.java @@ -22,7 +22,6 @@ import org.apache.druid.segment.writeout.WriteOutBytes; import javax.annotation.Nullable; - import java.io.IOException; import java.io.OutputStream; import java.nio.ByteBuffer; @@ -75,6 +74,19 @@ public void write(long value) throws IOException } } + @Override + public void write(long[] values, int offset, int length) throws IOException + { + if (outBuffer != null) { + outBuffer.asLongBuffer().put(values, offset, length); + outBuffer.position(outBuffer.position() + (length * Long.BYTES)); + } else { + for (int i = offset; i < length; ++i) { + write(values[i]); + } + } + } + @Override public void flush() { diff --git a/processing/src/main/java/org/apache/druid/segment/data/TableLongEncodingReader.java b/processing/src/main/java/org/apache/druid/segment/data/TableLongEncodingReader.java index 6a5e17b1080b..7e9b1fdc927e 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/TableLongEncodingReader.java +++ b/processing/src/main/java/org/apache/druid/segment/data/TableLongEncodingReader.java @@ -88,4 +88,10 @@ public CompressionFactory.LongEncodingReader duplicate() { return new TableLongEncodingReader(buffer.duplicate(), table, bitsPerValue); } + + @Override + public CompressionFactory.LongEncodingStrategy getStrategy() + { + return CompressionFactory.LongEncodingStrategy.AUTO; + } } diff --git a/processing/src/main/java/org/apache/druid/segment/serde/cell/StorableBuffer.java b/processing/src/main/java/org/apache/druid/segment/serde/cell/StorableBuffer.java index f228a81904fc..7a3bcea371e0 100644 --- a/processing/src/main/java/org/apache/druid/segment/serde/cell/StorableBuffer.java +++ b/processing/src/main/java/org/apache/druid/segment/serde/cell/StorableBuffer.java @@ -48,6 +48,24 @@ public int getSerializedSize() } }; + static StorableBuffer fromBytes(byte[] bytes) + { + return new StorableBuffer() + { + @Override + public void store(ByteBuffer byteBuffer) + { + byteBuffer.put(bytes); + } + + @Override + public int getSerializedSize() + { + return bytes.length; + } + }; + } + void store(ByteBuffer byteBuffer); int getSerializedSize(); From 615bf3290b9e71326b7cee7d583d67466bed932f Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Tue, 9 Jul 2024 09:21:44 +0530 Subject: [PATCH 04/15] Fix checkstyle --- .../druid/segment/data/EncodedStringDictionaryWriter.java | 2 +- .../java/org/apache/druid/segment/data/FixedIndexedWriter.java | 2 +- .../org/apache/druid/segment/data/FrontCodedIndexedWriter.java | 2 +- .../druid/segment/data/FrontCodedIntArrayIndexedWriter.java | 2 +- .../org/apache/druid/segment/data/GenericIndexedWriter.java | 2 +- .../segment/loading/MMappedQueryableSegmentizerFactory.java | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/segment/data/EncodedStringDictionaryWriter.java b/processing/src/main/java/org/apache/druid/segment/data/EncodedStringDictionaryWriter.java index 1ca4590d0bb9..799ed3766f28 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/EncodedStringDictionaryWriter.java +++ b/processing/src/main/java/org/apache/druid/segment/data/EncodedStringDictionaryWriter.java @@ -93,4 +93,4 @@ public void writeTo(WritableByteChannel channel, FileSmoosher smoosher) throws I channel.write(ByteBuffer.wrap(new byte[]{encodingStrategy.getId()})); delegate.writeTo(channel, smoosher); } -} \ No newline at end of file +} diff --git a/processing/src/main/java/org/apache/druid/segment/data/FixedIndexedWriter.java b/processing/src/main/java/org/apache/druid/segment/data/FixedIndexedWriter.java index 1c77e73354ca..42ca16b78f42 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/FixedIndexedWriter.java +++ b/processing/src/main/java/org/apache/druid/segment/data/FixedIndexedWriter.java @@ -212,4 +212,4 @@ private void readPage() } }; } -} \ No newline at end of file +} diff --git a/processing/src/main/java/org/apache/druid/segment/data/FrontCodedIndexedWriter.java b/processing/src/main/java/org/apache/druid/segment/data/FrontCodedIndexedWriter.java index be4a2ca821c6..707e3894793a 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/FrontCodedIndexedWriter.java +++ b/processing/src/main/java/org/apache/druid/segment/data/FrontCodedIndexedWriter.java @@ -485,4 +485,4 @@ static ByteBuffer getFromBucketV1(ByteBuffer buffer, int offset, int bucketSize) } return ByteBuffer.wrap(valueBytes); } -} \ No newline at end of file +} diff --git a/processing/src/main/java/org/apache/druid/segment/data/FrontCodedIntArrayIndexedWriter.java b/processing/src/main/java/org/apache/druid/segment/data/FrontCodedIntArrayIndexedWriter.java index 954d8dc22050..50e350f3d640 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/FrontCodedIntArrayIndexedWriter.java +++ b/processing/src/main/java/org/apache/druid/segment/data/FrontCodedIntArrayIndexedWriter.java @@ -433,4 +433,4 @@ int[] getFromBucket(ByteBuffer buffer, int offset) } return value; } -} \ No newline at end of file +} diff --git a/processing/src/main/java/org/apache/druid/segment/data/GenericIndexedWriter.java b/processing/src/main/java/org/apache/druid/segment/data/GenericIndexedWriter.java index f6fc17c8ec88..a87a61843fac 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/GenericIndexedWriter.java +++ b/processing/src/main/java/org/apache/druid/segment/data/GenericIndexedWriter.java @@ -536,4 +536,4 @@ private int checkedCastNonnegativeLongToInt(final long n) throw new IAE("Value out of nonnegative int range"); } } -} \ No newline at end of file +} diff --git a/processing/src/main/java/org/apache/druid/segment/loading/MMappedQueryableSegmentizerFactory.java b/processing/src/main/java/org/apache/druid/segment/loading/MMappedQueryableSegmentizerFactory.java index 9cf8a577a856..4afd7691fdf6 100644 --- a/processing/src/main/java/org/apache/druid/segment/loading/MMappedQueryableSegmentizerFactory.java +++ b/processing/src/main/java/org/apache/druid/segment/loading/MMappedQueryableSegmentizerFactory.java @@ -73,4 +73,4 @@ public Segment factorize(DataSegment dataSegment, File parentDir, boolean lazy, throw new SegmentLoadingException(e, "%s", e.getMessage()); } } -} \ No newline at end of file +} From d09706c336a30879be98d35e8a765bcc07a7ea72 Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Wed, 10 Jul 2024 22:53:46 +0530 Subject: [PATCH 05/15] Remove some unused code --- .../rowsandcols/column/LongArrayColumn.java | 5 +-- .../segment/data/CompressionFactory.java | 2 ++ .../org/apache/druid/segment/data/VByte.java | 36 ------------------- .../MMappedQueryableSegmentizerFactory.java | 21 ----------- .../segment/serde/cell/StorableBuffer.java | 18 ---------- 5 files changed, 5 insertions(+), 77 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/query/rowsandcols/column/LongArrayColumn.java b/processing/src/main/java/org/apache/druid/query/rowsandcols/column/LongArrayColumn.java index 6374c145b94f..4f2a055ed25b 100644 --- a/processing/src/main/java/org/apache/druid/query/rowsandcols/column/LongArrayColumn.java +++ b/processing/src/main/java/org/apache/druid/query/rowsandcols/column/LongArrayColumn.java @@ -141,13 +141,13 @@ public FindResult findNull(int startIndex, int endIndex) @Override public FindResult findDouble(int startIndex, int endIndex, double val) { - return findLong(startIndex, endIndex, (int) val); + return findLong(startIndex, endIndex, (long) val); } @Override public FindResult findFloat(int startIndex, int endIndex, float val) { - return findLong(startIndex, endIndex, (int) val); + return findLong(startIndex, endIndex, (long) val); } @Override @@ -181,6 +181,7 @@ public FindResult findLong(int startIndex, int endIndex, long val) } } + @SuppressWarnings("unused") public FindResult findInt(int startIndex, int endIndex, int val) { return findLong(startIndex, endIndex, val); diff --git a/processing/src/main/java/org/apache/druid/segment/data/CompressionFactory.java b/processing/src/main/java/org/apache/druid/segment/data/CompressionFactory.java index 98e28380b1ef..91ec70b7f171 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/CompressionFactory.java +++ b/processing/src/main/java/org/apache/druid/segment/data/CompressionFactory.java @@ -233,6 +233,7 @@ public interface LongEncodingWriter void write(long value) throws IOException; + @SuppressWarnings("unused") default void write(long[] values, int offset, int length) throws IOException { for (int i = offset; i < length; ++i) { @@ -302,6 +303,7 @@ public interface LongEncodingReader */ LongEncodingReader duplicate(); + @SuppressWarnings("unused") LongEncodingStrategy getStrategy(); } diff --git a/processing/src/main/java/org/apache/druid/segment/data/VByte.java b/processing/src/main/java/org/apache/druid/segment/data/VByte.java index 7886ae7f070a..749382cc001e 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/VByte.java +++ b/processing/src/main/java/org/apache/druid/segment/data/VByte.java @@ -19,9 +19,7 @@ package org.apache.druid.segment.data; -import java.io.IOException; import java.nio.ByteBuffer; -import java.nio.channels.WritableByteChannel; public class VByte { @@ -60,40 +58,6 @@ public static int readInt(ByteBuffer buffer) return v; } - public static int writeInt(WritableByteChannel out, int val) throws IOException - { - final byte[] bytes = new byte[5]; - final int numBytes; - if (val < (1 << 7)) { - bytes[0] = (byte) (val | (1 << 7)); - numBytes = 1; - } else if (val < (1 << 14)) { - bytes[0] = extract7bits(0, val); - bytes[1] = (byte) (extract7bitsmaskless(1, (val)) | (1 << 7)); - numBytes = 2; - } else if (val < (1 << 21)) { - bytes[0] = extract7bits(0, val); - bytes[1] = extract7bits(1, val); - bytes[2] = (byte) (extract7bitsmaskless(2, (val)) | (1 << 7)); - numBytes = 3; - } else if (val < (1 << 28)) { - bytes[0] = extract7bits(0, val); - bytes[1] = extract7bits(1, val); - bytes[2] = extract7bits(2, val); - bytes[3] = (byte) (extract7bitsmaskless(3, (val)) | (1 << 7)); - numBytes = 4; - } else { - bytes[0] = extract7bits(0, val); - bytes[1] = extract7bits(1, val); - bytes[2] = extract7bits(2, val); - bytes[3] = extract7bits(3, val); - bytes[4] = (byte) (extract7bitsmaskless(4, (val)) | (1 << 7)); - numBytes = 5; - } - out.write(ByteBuffer.wrap(bytes, 0, numBytes)); - return numBytes; - } - /** * Write a variable byte (vbyte) encoded integer to a {@link ByteBuffer} at the current position, advancing the buffer * position by the number of bytes required to represent the integer, between 1 and 5 bytes. diff --git a/processing/src/main/java/org/apache/druid/segment/loading/MMappedQueryableSegmentizerFactory.java b/processing/src/main/java/org/apache/druid/segment/loading/MMappedQueryableSegmentizerFactory.java index 4afd7691fdf6..d3124cb122f9 100644 --- a/processing/src/main/java/org/apache/druid/segment/loading/MMappedQueryableSegmentizerFactory.java +++ b/processing/src/main/java/org/apache/druid/segment/loading/MMappedQueryableSegmentizerFactory.java @@ -20,7 +20,6 @@ package org.apache.druid.segment.loading; import com.fasterxml.jackson.annotation.JacksonInject; -import com.fasterxml.jackson.annotation.JsonCreator; import com.google.common.base.Preconditions; import org.apache.druid.segment.IndexIO; import org.apache.druid.segment.QueryableIndexSegment; @@ -35,29 +34,9 @@ */ public class MMappedQueryableSegmentizerFactory implements SegmentizerFactory { - /** - * A static method to make a segmentizer factory that can be serialized by Jackson. This exists because the object - * doesn't actually need the IndexIO object in order to be serialized by Jackson, but the public constructor - * *does* require it. We leave the public constructor alone because if indexIO is null, this SegmentizerFactory - * is largely useless, so we just create this one static method to enable creating the object for this singular - * use case. - * - * @return a SegmentizerFactory that can be used to be serialized by Jackson - */ - @SuppressWarnings("unused") - public static MMappedQueryableSegmentizerFactory makeForSerialization() - { - return new MMappedQueryableSegmentizerFactory(); - } private final IndexIO indexIO; - private MMappedQueryableSegmentizerFactory() - { - this.indexIO = null; - } - - @JsonCreator public MMappedQueryableSegmentizerFactory(@JacksonInject IndexIO indexIO) { this.indexIO = Preconditions.checkNotNull(indexIO, "Null IndexIO"); diff --git a/processing/src/main/java/org/apache/druid/segment/serde/cell/StorableBuffer.java b/processing/src/main/java/org/apache/druid/segment/serde/cell/StorableBuffer.java index 7a3bcea371e0..f228a81904fc 100644 --- a/processing/src/main/java/org/apache/druid/segment/serde/cell/StorableBuffer.java +++ b/processing/src/main/java/org/apache/druid/segment/serde/cell/StorableBuffer.java @@ -48,24 +48,6 @@ public int getSerializedSize() } }; - static StorableBuffer fromBytes(byte[] bytes) - { - return new StorableBuffer() - { - @Override - public void store(ByteBuffer byteBuffer) - { - byteBuffer.put(bytes); - } - - @Override - public int getSerializedSize() - { - return bytes.length; - } - }; - } - void store(ByteBuffer byteBuffer); int getSerializedSize(); From ee8766b8a3e56e1fdc21ffbad27cfb74175f5af0 Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Wed, 10 Jul 2024 23:29:19 +0530 Subject: [PATCH 06/15] Fix broken test --- .../main/java/org/apache/druid/error/DruidException.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/error/DruidException.java b/processing/src/main/java/org/apache/druid/error/DruidException.java index 3aa49d33f8a4..f0846e6c6f54 100644 --- a/processing/src/main/java/org/apache/druid/error/DruidException.java +++ b/processing/src/main/java/org/apache/druid/error/DruidException.java @@ -481,10 +481,14 @@ public DruidException build(Throwable cause, String formatMe, Object... vals) StackTraceElement[] stackTrace = retVal.getStackTrace(); int firstNonDruidExceptionIndex = 0; - while (stackTrace[firstNonDruidExceptionIndex].getClassName().startsWith(CLASS_NAME_STR)) { + while ( + firstNonDruidExceptionIndex < stackTrace.length && + stackTrace[firstNonDruidExceptionIndex].getClassName().startsWith(CLASS_NAME_STR)) { ++firstNonDruidExceptionIndex; } - retVal.setStackTrace(Arrays.copyOfRange(stackTrace, firstNonDruidExceptionIndex, stackTrace.length)); + if (firstNonDruidExceptionIndex < stackTrace.length) { + retVal.setStackTrace(Arrays.copyOfRange(stackTrace, firstNonDruidExceptionIndex, stackTrace.length)); + } return retVal; } From 70b13dadef8370a41f178d3c747a728cf50dd4d7 Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Fri, 12 Jul 2024 14:09:26 +0530 Subject: [PATCH 07/15] Add tests --- .../column/BinarySearchableAccessor.java | 4 + .../column/ConstantObjectColumn.java | 10 ++- .../rowsandcols/column/DoubleArrayColumn.java | 14 +-- .../rowsandcols/column/IntArrayColumn.java | 14 +-- .../rowsandcols/column/LongArrayColumn.java | 16 ++-- .../column/LongArrayColumnTest.java | 87 +++++++++++++++++++ .../AppendableRowsAndColumnsTest.java | 5 +- .../segment/data/FrontCodedIndexedTest.java | 2 +- 8 files changed, 128 insertions(+), 24 deletions(-) create mode 100644 processing/src/test/java/org/apache/druid/query/rowsandcols/column/LongArrayColumnTest.java diff --git a/processing/src/main/java/org/apache/druid/query/rowsandcols/column/BinarySearchableAccessor.java b/processing/src/main/java/org/apache/druid/query/rowsandcols/column/BinarySearchableAccessor.java index 4eddcc77f1c5..f7c339b2080d 100644 --- a/processing/src/main/java/org/apache/druid/query/rowsandcols/column/BinarySearchableAccessor.java +++ b/processing/src/main/java/org/apache/druid/query/rowsandcols/column/BinarySearchableAccessor.java @@ -22,6 +22,10 @@ import org.apache.druid.java.util.common.ISE; import org.apache.druid.query.rowsandcols.util.FindResult; +/** + * The implementations of this interface will not validate that things are sorted for the binary search, it assumes that + * they must be. As such, behavior are undefined if the column is not actually sorted. + */ public interface BinarySearchableAccessor extends ColumnAccessor { static BinarySearchableAccessor fromColumn(Column col) diff --git a/processing/src/main/java/org/apache/druid/query/rowsandcols/column/ConstantObjectColumn.java b/processing/src/main/java/org/apache/druid/query/rowsandcols/column/ConstantObjectColumn.java index 28a7c3dd10db..01af9b07536b 100644 --- a/processing/src/main/java/org/apache/druid/query/rowsandcols/column/ConstantObjectColumn.java +++ b/processing/src/main/java/org/apache/druid/query/rowsandcols/column/ConstantObjectColumn.java @@ -19,7 +19,7 @@ package org.apache.druid.query.rowsandcols.column; -import org.apache.druid.java.util.common.ISE; +import org.apache.druid.error.DruidException; import org.apache.druid.query.rowsandcols.util.FindResult; import org.apache.druid.segment.column.ColumnType; @@ -55,7 +55,13 @@ public T as(Class clazz) if (VectorCopier.class.equals(clazz)) { return (T) (VectorCopier) (into, intoStart) -> { if (Integer.MAX_VALUE - numRows < intoStart) { - throw new ISE("too many rows!!! intoStart[%,d], numRows[%,d] combine to exceed max_int", intoStart, numRows); + throw DruidException.forPersona(DruidException.Persona.USER) + .ofCategory(DruidException.Category.CAPACITY_EXCEEDED) + .build( + "too many rows!!! intoStart[%,d], vals.length[%,d] combine to exceed max_int", + intoStart, + numRows + ); } Arrays.fill(into, intoStart, intoStart + numRows, obj); }; diff --git a/processing/src/main/java/org/apache/druid/query/rowsandcols/column/DoubleArrayColumn.java b/processing/src/main/java/org/apache/druid/query/rowsandcols/column/DoubleArrayColumn.java index 9c3b799d30e8..58e9626266c6 100644 --- a/processing/src/main/java/org/apache/druid/query/rowsandcols/column/DoubleArrayColumn.java +++ b/processing/src/main/java/org/apache/druid/query/rowsandcols/column/DoubleArrayColumn.java @@ -19,7 +19,7 @@ package org.apache.druid.query.rowsandcols.column; -import org.apache.druid.java.util.common.ISE; +import org.apache.druid.error.DruidException; import org.apache.druid.java.util.common.Numbers; import org.apache.druid.query.rowsandcols.util.FindResult; import org.apache.druid.segment.column.ColumnType; @@ -54,11 +54,13 @@ public T as(Class clazz) if (VectorCopier.class.equals(clazz)) { return (T) (VectorCopier) (into, intoStart) -> { if (Integer.MAX_VALUE - vals.length < intoStart) { - throw new ISE( - "too many rows!!! intoStart[%,d], vals.length[%,d] combine to exceed max_int", - intoStart, - vals.length - ); + throw DruidException.forPersona(DruidException.Persona.USER) + .ofCategory(DruidException.Category.CAPACITY_EXCEEDED) + .build( + "too many rows!!! intoStart[%,d], vals.length[%,d] combine to exceed max_int", + intoStart, + vals.length + ); } for (int i = 0; i < vals.length; ++i) { into[intoStart + i] = vals[i]; diff --git a/processing/src/main/java/org/apache/druid/query/rowsandcols/column/IntArrayColumn.java b/processing/src/main/java/org/apache/druid/query/rowsandcols/column/IntArrayColumn.java index 07c083d7d9f8..dc4e34b3357b 100644 --- a/processing/src/main/java/org/apache/druid/query/rowsandcols/column/IntArrayColumn.java +++ b/processing/src/main/java/org/apache/druid/query/rowsandcols/column/IntArrayColumn.java @@ -19,7 +19,7 @@ package org.apache.druid.query.rowsandcols.column; -import org.apache.druid.java.util.common.ISE; +import org.apache.druid.error.DruidException; import org.apache.druid.java.util.common.Numbers; import org.apache.druid.query.rowsandcols.util.FindResult; import org.apache.druid.segment.column.ColumnType; @@ -54,11 +54,13 @@ public T as(Class clazz) if (VectorCopier.class.equals(clazz)) { return (T) (VectorCopier) (into, intoStart) -> { if (Integer.MAX_VALUE - vals.length < intoStart) { - throw new ISE( - "too many rows!!! intoStart[%,d], vals.length[%,d] combine to exceed max_int", - intoStart, - vals.length - ); + throw DruidException.forPersona(DruidException.Persona.USER) + .ofCategory(DruidException.Category.CAPACITY_EXCEEDED) + .build( + "too many rows!!! intoStart[%,d], vals.length[%,d] combine to exceed max_int", + intoStart, + vals.length + ); } for (int i = 0; i < vals.length; ++i) { into[intoStart + i] = vals[i]; diff --git a/processing/src/main/java/org/apache/druid/query/rowsandcols/column/LongArrayColumn.java b/processing/src/main/java/org/apache/druid/query/rowsandcols/column/LongArrayColumn.java index 4f2a055ed25b..1c054999eb2f 100644 --- a/processing/src/main/java/org/apache/druid/query/rowsandcols/column/LongArrayColumn.java +++ b/processing/src/main/java/org/apache/druid/query/rowsandcols/column/LongArrayColumn.java @@ -19,7 +19,7 @@ package org.apache.druid.query.rowsandcols.column; -import org.apache.druid.java.util.common.ISE; +import org.apache.druid.error.DruidException; import org.apache.druid.java.util.common.Numbers; import org.apache.druid.query.rowsandcols.util.FindResult; import org.apache.druid.segment.column.ColumnType; @@ -54,11 +54,13 @@ public T as(Class clazz) if (VectorCopier.class.equals(clazz)) { return (T) (VectorCopier) (into, intoStart) -> { if (Integer.MAX_VALUE - vals.length < intoStart) { - throw new ISE( - "too many rows!!! intoStart[%,d], vals.length[%,d] combine to exceed max_int", - intoStart, - vals.length - ); + throw DruidException.forPersona(DruidException.Persona.USER) + .ofCategory(DruidException.Category.CAPACITY_EXCEEDED) + .build( + "too many rows!!! intoStart[%,d], vals.length[%,d] combine to exceed max_int", + intoStart, + vals.length + ); } for (int i = 0; i < vals.length; ++i) { into[intoStart + i] = vals[i]; @@ -190,7 +192,7 @@ public FindResult findInt(int startIndex, int endIndex, int val) @Override public FindResult findString(int startIndex, int endIndex, String val) { - return findLong(startIndex, endIndex, (int) Numbers.tryParseLong(val, 0)); + return findLong(startIndex, endIndex, Numbers.tryParseLong(val, 0)); } @Override diff --git a/processing/src/test/java/org/apache/druid/query/rowsandcols/column/LongArrayColumnTest.java b/processing/src/test/java/org/apache/druid/query/rowsandcols/column/LongArrayColumnTest.java new file mode 100644 index 000000000000..c4b6cb9a1ccb --- /dev/null +++ b/processing/src/test/java/org/apache/druid/query/rowsandcols/column/LongArrayColumnTest.java @@ -0,0 +1,87 @@ +/* + * 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.druid.query.rowsandcols.column; + +import org.apache.druid.query.rowsandcols.util.FindResult; +import org.junit.Assert; +import org.junit.Test; + +public class LongArrayColumnTest +{ + @Test + public void testLongArrayColumnWithLongValues() + { + Column column = new LongArrayColumn(new long[]{0, 1, 2, 3, 4, 5, 6, 7, 8, 9}); + ColumnAccessor accessor = column.toAccessor(); + + for (int i = 0; i < 10; i++) { + Assert.assertFalse(accessor.isNull(i)); + Assert.assertEquals(i, accessor.getLong(i)); + Assert.assertEquals((long) i, accessor.getObject(i)); + Assert.assertEquals(i, accessor.getDouble(i), 0); + Assert.assertEquals(i, accessor.getInt(i)); + } + } + + @Test + public void testFindLong() + { + Column column = new LongArrayColumn(new long[] {1, 1, 1, 3, 5, 5, 6, 7, 8, 9}); + BinarySearchableAccessor accessor = (BinarySearchableAccessor) column.toAccessor(); + + FindResult findResult = accessor.findLong(0, accessor.numRows(), 1); + Assert.assertTrue(findResult.wasFound()); + Assert.assertEquals(0, findResult.getStartRow()); + Assert.assertEquals(3, findResult.getEndRow()); + + findResult = accessor.findLong(0, accessor.numRows(), 6); + Assert.assertTrue(findResult.wasFound()); + Assert.assertEquals(6, findResult.getStartRow()); + Assert.assertEquals(7, findResult.getEndRow()); + + Assert.assertFalse(accessor.findLong(0, accessor.numRows(), 2).wasFound()); + Assert.assertFalse(accessor.findLong(0, 3, 9).wasFound()); + } + + @Test + public void testOtherTypeFinds() + { + Column column = new LongArrayColumn(new long[] {0, 1, 2, 3, 4, 5, Long.MAX_VALUE}); + BinarySearchableAccessor accessor = (BinarySearchableAccessor) column.toAccessor(); + + FindResult findResult = accessor.findNull(0, accessor.numRows()); + Assert.assertFalse(findResult.wasFound()); // Always false for long array columns + + findResult = accessor.findDouble(0, accessor.numRows(), 3.0); + Assert.assertTrue(findResult.wasFound()); + Assert.assertEquals(3, findResult.getStartRow()); + Assert.assertEquals(4, findResult.getEndRow()); + + findResult = accessor.findString(0, accessor.numRows(), "4"); + Assert.assertTrue(findResult.wasFound()); + Assert.assertEquals(4, findResult.getStartRow()); + Assert.assertEquals(5, findResult.getEndRow()); + + findResult = accessor.findComplex(0, accessor.numRows(), String.valueOf(Long.MAX_VALUE)); + Assert.assertTrue(findResult.wasFound()); + Assert.assertEquals(6, findResult.getStartRow()); + Assert.assertEquals(7, findResult.getEndRow()); + } +} diff --git a/processing/src/test/java/org/apache/druid/query/rowsandcols/semantic/AppendableRowsAndColumnsTest.java b/processing/src/test/java/org/apache/druid/query/rowsandcols/semantic/AppendableRowsAndColumnsTest.java index e5d6eb1faa79..a305e98ff9e5 100644 --- a/processing/src/test/java/org/apache/druid/query/rowsandcols/semantic/AppendableRowsAndColumnsTest.java +++ b/processing/src/test/java/org/apache/druid/query/rowsandcols/semantic/AppendableRowsAndColumnsTest.java @@ -24,6 +24,7 @@ import org.apache.druid.query.rowsandcols.MapOfColumnsRowsAndColumns; import org.apache.druid.query.rowsandcols.RowsAndColumns; import org.apache.druid.query.rowsandcols.column.IntArrayColumn; +import org.apache.druid.query.rowsandcols.column.LongArrayColumn; import org.apache.druid.query.rowsandcols.column.ObjectArrayColumn; import org.apache.druid.segment.column.ColumnType; import org.junit.Assert; @@ -48,7 +49,7 @@ public void testAppendableRowsAndColumns() RowsAndColumns rac = make(MapOfColumnsRowsAndColumns.fromMap( ImmutableMap.of( "colA", new IntArrayColumn(new int[]{1, 2, 3, 4, 5, 6, 7, 8, 9, 10}), - "colB", new IntArrayColumn(new int[]{4, -4, 3, -3, 4, 82, -90, 4, 0, 0}) + "colB", new LongArrayColumn(new long[]{4, -4, 3, -3, 4, 82, -90, 4, 0, 0}) ) )); @@ -58,7 +59,7 @@ public void testAppendableRowsAndColumns() new RowsAndColumnsHelper() .expectColumn("colA", new int[]{1, 2, 3, 4, 5, 6, 7, 8, 9, 10}) - .expectColumn("colB", new int[]{4, -4, 3, -3, 4, 82, -90, 4, 0, 0}) + .expectColumn("colB", new long[]{4, -4, 3, -3, 4, 82, -90, 4, 0, 0}) .expectColumn("newCol", new int[]{1, 2, 3, 4, 5, 6, 7, 8, 9, 10}) .allColumnsRegistered() .validate(appender); diff --git a/processing/src/test/java/org/apache/druid/segment/data/FrontCodedIndexedTest.java b/processing/src/test/java/org/apache/druid/segment/data/FrontCodedIndexedTest.java index 8b055188e634..c1312731b913 100644 --- a/processing/src/test/java/org/apache/druid/segment/data/FrontCodedIndexedTest.java +++ b/processing/src/test/java/org/apache/druid/segment/data/FrontCodedIndexedTest.java @@ -436,7 +436,7 @@ private static long persistToBuffer( while (sortedStrings.hasNext()) { final String next = sortedStrings.next(); final byte[] nextBytes = StringUtils.toUtf8Nullable(next); - writer.write(nextBytes); + Assert.assertEquals(index, writer.write(nextBytes)); if (nextBytes == null) { Assert.assertNull(writer.get(index)); } else { From c80eaf7d3289728b99a05d66bea3f5c4d9185967 Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Sun, 14 Jul 2024 23:19:19 +0530 Subject: [PATCH 08/15] Additional changes --- .../org/apache/druid/segment/data/BitmapSerde.java | 13 +++++++++++++ .../segment/data/RoaringBitmapSerdeFactory.java | 2 +- .../druid/segment/data/BitmapSerdeFactoryTest.java | 9 +++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/processing/src/main/java/org/apache/druid/segment/data/BitmapSerde.java b/processing/src/main/java/org/apache/druid/segment/data/BitmapSerde.java index aa80186e10dd..b25862b17495 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/BitmapSerde.java +++ b/processing/src/main/java/org/apache/druid/segment/data/BitmapSerde.java @@ -20,7 +20,10 @@ package org.apache.druid.segment.data; import com.fasterxml.jackson.annotation.JsonTypeName; +import org.apache.druid.collections.bitmap.BitmapFactory; +import org.apache.druid.collections.bitmap.ConciseBitmapFactory; import org.apache.druid.collections.bitmap.RoaringBitmapFactory; +import org.apache.druid.error.DruidException; public class BitmapSerde { @@ -48,4 +51,14 @@ public static BitmapSerdeFactory createLegacyFactory() { return new LegacyBitmapSerdeFactory(); } + + public static BitmapSerdeFactory forBitmapFactory(BitmapFactory factory) + { + if (factory instanceof RoaringBitmapFactory) { + return new DefaultBitmapSerdeFactory(); + } else if (factory instanceof ConciseBitmapFactory) { + return new ConciseBitmapSerdeFactory(); + } + throw DruidException.defensive("Unknown type of bitmapFactory [%s]", factory.getClass()); + } } diff --git a/processing/src/main/java/org/apache/druid/segment/data/RoaringBitmapSerdeFactory.java b/processing/src/main/java/org/apache/druid/segment/data/RoaringBitmapSerdeFactory.java index ea6bb9bd994d..f7cf9bdbb00b 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/RoaringBitmapSerdeFactory.java +++ b/processing/src/main/java/org/apache/druid/segment/data/RoaringBitmapSerdeFactory.java @@ -84,7 +84,7 @@ public ImmutableBitmap fromByteBuffer(ByteBuffer buffer, int numBytes) @Override public byte[] toBytes(@Nullable ImmutableBitmap val) { - if (val == null || val.size() == 0) { + if (val == null || val.isEmpty()) { return new byte[]{}; } return val.toBytes(); diff --git a/processing/src/test/java/org/apache/druid/segment/data/BitmapSerdeFactoryTest.java b/processing/src/test/java/org/apache/druid/segment/data/BitmapSerdeFactoryTest.java index f799d35059b2..34b93f101652 100644 --- a/processing/src/test/java/org/apache/druid/segment/data/BitmapSerdeFactoryTest.java +++ b/processing/src/test/java/org/apache/druid/segment/data/BitmapSerdeFactoryTest.java @@ -20,6 +20,8 @@ package org.apache.druid.segment.data; import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.druid.collections.bitmap.ConciseBitmapFactory; +import org.apache.druid.collections.bitmap.RoaringBitmapFactory; import org.apache.druid.jackson.DefaultObjectMapper; import org.junit.Assert; import org.junit.Test; @@ -46,4 +48,11 @@ public void testDeserialization() throws Exception Assert.assertTrue(mapper.readValue("{\"type\":\"concise\"}", BitmapSerdeFactory.class) instanceof ConciseBitmapSerdeFactory); Assert.assertTrue(mapper.readValue("{\"type\":\"BitmapSerde$SomeRandomClass\"}", BitmapSerdeFactory.class) instanceof RoaringBitmapSerdeFactory); } + + @Test + public void testForBitmapFactory() + { + Assert.assertTrue(BitmapSerde.forBitmapFactory(new RoaringBitmapFactory()) instanceof BitmapSerde.DefaultBitmapSerdeFactory); + Assert.assertTrue(BitmapSerde.forBitmapFactory(new ConciseBitmapFactory()) instanceof ConciseBitmapSerdeFactory); + } } From 396c71f24faaec64fba5016345f573431f2853ee Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Mon, 15 Jul 2024 19:14:22 +0530 Subject: [PATCH 09/15] Switch to throwing an exception for invalid find calls --- .../apache/druid/error/NotYetImplemented.java | 72 +++++++++++++++++++ .../rowsandcols/column/DoubleArrayColumn.java | 6 +- .../rowsandcols/column/IntArrayColumn.java | 6 +- .../rowsandcols/column/LongArrayColumn.java | 6 +- .../column/LongArrayColumnTest.java | 11 +-- 5 files changed, 84 insertions(+), 17 deletions(-) create mode 100644 processing/src/main/java/org/apache/druid/error/NotYetImplemented.java diff --git a/processing/src/main/java/org/apache/druid/error/NotYetImplemented.java b/processing/src/main/java/org/apache/druid/error/NotYetImplemented.java new file mode 100644 index 000000000000..3e8df191e32c --- /dev/null +++ b/processing/src/main/java/org/apache/druid/error/NotYetImplemented.java @@ -0,0 +1,72 @@ +/* + * 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.druid.error; + +/** + * A failure class that is used to indicate that something is just not implemented yet. This is useful when a + * developer builds something and they intentionally do not implement a specific branch of code or type of object. + *

+ * The lack of implementation is not necessarily a statement that it SHOULDN'T be implemented, it's just an indication + * that it has not YET been implemented. When one of these exceptions is seen, it is usually an indication that it is + * now time to actually implement the path that was previously elided. + *

+ * Often times, the code path wasn't implemented because the developer thought that it wasn't actually possible to + * see it executed. So, collecting and providing information about why the particular path got executed is often + * extremely helpful in understanding why it happened and accelerating the implementation of what the correct behavior + * should be. + */ +public class NotYetImplemented extends DruidException.Failure +{ + public static DruidException ex(String msg, Object... args) + { + return ex(null, msg, args); + } + + public static DruidException ex(Throwable t, String msg, Object... args) + { + return DruidException.fromFailure(new NotYetImplemented(t, msg, args)); + } + + private final Throwable t; + private final String msg; + private final Object[] args; + + public NotYetImplemented(Throwable t, String msg, Object[] args) + { + super("notYetImplemented"); + this.t = t; + this.msg = msg; + this.args = args; + } + + + @Override + protected DruidException makeException(DruidException.DruidExceptionBuilder bob) + { + bob = bob.forPersona(DruidException.Persona.DEVELOPER) + .ofCategory(DruidException.Category.DEFENSIVE); + + if (t == null) { + return bob.build(msg, args); + } else { + return bob.build(t, msg, args); + } + } +} diff --git a/processing/src/main/java/org/apache/druid/query/rowsandcols/column/DoubleArrayColumn.java b/processing/src/main/java/org/apache/druid/query/rowsandcols/column/DoubleArrayColumn.java index 58e9626266c6..72517851f33c 100644 --- a/processing/src/main/java/org/apache/druid/query/rowsandcols/column/DoubleArrayColumn.java +++ b/processing/src/main/java/org/apache/druid/query/rowsandcols/column/DoubleArrayColumn.java @@ -20,7 +20,7 @@ package org.apache.druid.query.rowsandcols.column; import org.apache.druid.error.DruidException; -import org.apache.druid.java.util.common.Numbers; +import org.apache.druid.error.NotYetImplemented; import org.apache.druid.query.rowsandcols.util.FindResult; import org.apache.druid.segment.column.ColumnType; @@ -185,13 +185,13 @@ public FindResult findLong(int startIndex, int endIndex, long val) @Override public FindResult findString(int startIndex, int endIndex, String val) { - return findDouble(startIndex, endIndex, Numbers.tryParseDouble(val, 0)); + throw NotYetImplemented.ex("findString is not currently supported for DoubleArrayColumns"); } @Override public FindResult findComplex(int startIndex, int endIndex, Object val) { - return findDouble(startIndex, endIndex, Numbers.tryParseDouble(val, 0)); + throw NotYetImplemented.ex("findComplex is not currently supported for DoubleArrayColumns"); } } } diff --git a/processing/src/main/java/org/apache/druid/query/rowsandcols/column/IntArrayColumn.java b/processing/src/main/java/org/apache/druid/query/rowsandcols/column/IntArrayColumn.java index dc4e34b3357b..05c8acd0673f 100644 --- a/processing/src/main/java/org/apache/druid/query/rowsandcols/column/IntArrayColumn.java +++ b/processing/src/main/java/org/apache/druid/query/rowsandcols/column/IntArrayColumn.java @@ -20,7 +20,7 @@ package org.apache.druid.query.rowsandcols.column; import org.apache.druid.error.DruidException; -import org.apache.druid.java.util.common.Numbers; +import org.apache.druid.error.NotYetImplemented; import org.apache.druid.query.rowsandcols.util.FindResult; import org.apache.druid.segment.column.ColumnType; @@ -191,13 +191,13 @@ public FindResult findInt(int startIndex, int endIndex, int val) @Override public FindResult findString(int startIndex, int endIndex, String val) { - return findInt(startIndex, endIndex, (int) Numbers.tryParseLong(val, 0)); + throw NotYetImplemented.ex("findString is not currently supported for IntArrayColumns"); } @Override public FindResult findComplex(int startIndex, int endIndex, Object val) { - return findInt(startIndex, endIndex, (int) Numbers.tryParseLong(val, 0)); + throw NotYetImplemented.ex("findComplex is not currently supported for IntArrayColumns"); } } } diff --git a/processing/src/main/java/org/apache/druid/query/rowsandcols/column/LongArrayColumn.java b/processing/src/main/java/org/apache/druid/query/rowsandcols/column/LongArrayColumn.java index 1c054999eb2f..d9828c37e2b9 100644 --- a/processing/src/main/java/org/apache/druid/query/rowsandcols/column/LongArrayColumn.java +++ b/processing/src/main/java/org/apache/druid/query/rowsandcols/column/LongArrayColumn.java @@ -20,7 +20,7 @@ package org.apache.druid.query.rowsandcols.column; import org.apache.druid.error.DruidException; -import org.apache.druid.java.util.common.Numbers; +import org.apache.druid.error.NotYetImplemented; import org.apache.druid.query.rowsandcols.util.FindResult; import org.apache.druid.segment.column.ColumnType; @@ -192,13 +192,13 @@ public FindResult findInt(int startIndex, int endIndex, int val) @Override public FindResult findString(int startIndex, int endIndex, String val) { - return findLong(startIndex, endIndex, Numbers.tryParseLong(val, 0)); + throw NotYetImplemented.ex("findString is not currently supported for LongArrayColumns"); } @Override public FindResult findComplex(int startIndex, int endIndex, Object val) { - return findLong(startIndex, endIndex, Numbers.tryParseLong(val, 0)); + throw NotYetImplemented.ex("findComplex is not currently supported for LongArrayColumns"); } } } diff --git a/processing/src/test/java/org/apache/druid/query/rowsandcols/column/LongArrayColumnTest.java b/processing/src/test/java/org/apache/druid/query/rowsandcols/column/LongArrayColumnTest.java index c4b6cb9a1ccb..e4300552a208 100644 --- a/processing/src/test/java/org/apache/druid/query/rowsandcols/column/LongArrayColumnTest.java +++ b/processing/src/test/java/org/apache/druid/query/rowsandcols/column/LongArrayColumnTest.java @@ -74,14 +74,9 @@ public void testOtherTypeFinds() Assert.assertEquals(3, findResult.getStartRow()); Assert.assertEquals(4, findResult.getEndRow()); - findResult = accessor.findString(0, accessor.numRows(), "4"); + findResult = accessor.findFloat(0, accessor.numRows(), 1.0fgi); Assert.assertTrue(findResult.wasFound()); - Assert.assertEquals(4, findResult.getStartRow()); - Assert.assertEquals(5, findResult.getEndRow()); - - findResult = accessor.findComplex(0, accessor.numRows(), String.valueOf(Long.MAX_VALUE)); - Assert.assertTrue(findResult.wasFound()); - Assert.assertEquals(6, findResult.getStartRow()); - Assert.assertEquals(7, findResult.getEndRow()); + Assert.assertEquals(1, findResult.getStartRow()); + Assert.assertEquals(2, findResult.getEndRow()); } } From ceafa0940e8006d550d2802be8f0b05f7acc78a5 Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Mon, 15 Jul 2024 19:20:41 +0530 Subject: [PATCH 10/15] Remove comment --- .../druid/query/rowsandcols/column/LongArrayColumnTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/processing/src/test/java/org/apache/druid/query/rowsandcols/column/LongArrayColumnTest.java b/processing/src/test/java/org/apache/druid/query/rowsandcols/column/LongArrayColumnTest.java index e4300552a208..38a53a17a332 100644 --- a/processing/src/test/java/org/apache/druid/query/rowsandcols/column/LongArrayColumnTest.java +++ b/processing/src/test/java/org/apache/druid/query/rowsandcols/column/LongArrayColumnTest.java @@ -74,7 +74,7 @@ public void testOtherTypeFinds() Assert.assertEquals(3, findResult.getStartRow()); Assert.assertEquals(4, findResult.getEndRow()); - findResult = accessor.findFloat(0, accessor.numRows(), 1.0fgi); + findResult = accessor.findFloat(0, accessor.numRows(), 1.0f); Assert.assertTrue(findResult.wasFound()); Assert.assertEquals(1, findResult.getStartRow()); Assert.assertEquals(2, findResult.getEndRow()); From 33732acab0199e906913874173b3b6c0d4db30d5 Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Sat, 20 Jul 2024 22:09:36 +0530 Subject: [PATCH 11/15] Minor changes --- .../org/apache/druid/guice/JsonConfigurator.java | 2 +- .../druid/segment/data/LongsLongEncodingWriter.java | 13 ------------- 2 files changed, 1 insertion(+), 14 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/guice/JsonConfigurator.java b/processing/src/main/java/org/apache/druid/guice/JsonConfigurator.java index ed7e79df3f17..8a53dbffabf0 100644 --- a/processing/src/main/java/org/apache/druid/guice/JsonConfigurator.java +++ b/processing/src/main/java/org/apache/druid/guice/JsonConfigurator.java @@ -238,7 +238,7 @@ private static void hieraricalPutValue( log.info( "Skipping property [%s]: one of it's prefixes [%s] is also used as a property key.", originalProperty, - nestedKey + propertyPrefix ); return; } diff --git a/processing/src/main/java/org/apache/druid/segment/data/LongsLongEncodingWriter.java b/processing/src/main/java/org/apache/druid/segment/data/LongsLongEncodingWriter.java index f2b198b7e7a8..728a50aa2fc4 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/LongsLongEncodingWriter.java +++ b/processing/src/main/java/org/apache/druid/segment/data/LongsLongEncodingWriter.java @@ -74,19 +74,6 @@ public void write(long value) throws IOException } } - @Override - public void write(long[] values, int offset, int length) throws IOException - { - if (outBuffer != null) { - outBuffer.asLongBuffer().put(values, offset, length); - outBuffer.position(outBuffer.position() + (length * Long.BYTES)); - } else { - for (int i = offset; i < length; ++i) { - write(values[i]); - } - } - } - @Override public void flush() { From 496376a3f98137020c476ea158d1ac58d8cde15f Mon Sep 17 00:00:00 2001 From: Benedict Jin Date: Mon, 22 Jul 2024 17:42:23 +0800 Subject: [PATCH 12/15] Update processing/src/main/java/org/apache/druid/error/NotYetImplemented.java --- .../src/main/java/org/apache/druid/error/NotYetImplemented.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/processing/src/main/java/org/apache/druid/error/NotYetImplemented.java b/processing/src/main/java/org/apache/druid/error/NotYetImplemented.java index 3e8df191e32c..f87b9d411549 100644 --- a/processing/src/main/java/org/apache/druid/error/NotYetImplemented.java +++ b/processing/src/main/java/org/apache/druid/error/NotYetImplemented.java @@ -27,7 +27,7 @@ * that it has not YET been implemented. When one of these exceptions is seen, it is usually an indication that it is * now time to actually implement the path that was previously elided. *

- * Often times, the code path wasn't implemented because the developer thought that it wasn't actually possible to + * Oftentimes, the code path wasn't implemented because the developer thought that it wasn't actually possible to * see it executed. So, collecting and providing information about why the particular path got executed is often * extremely helpful in understanding why it happened and accelerating the implementation of what the correct behavior * should be. From ab36689a0b53edac1b2a393d8ec46324405949fe Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Tue, 23 Jul 2024 15:16:46 +0530 Subject: [PATCH 13/15] Address review comments --- .../src/main/java/org/apache/druid/error/DruidException.java | 4 ---- .../main/java/org/apache/druid/error/NotYetImplemented.java | 5 ----- .../druid/query/rowsandcols/column/DoubleArrayColumn.java | 4 ++-- .../druid/query/rowsandcols/column/IntArrayColumn.java | 4 ++-- .../druid/query/rowsandcols/column/LongArrayColumn.java | 4 ++-- 5 files changed, 6 insertions(+), 15 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/error/DruidException.java b/processing/src/main/java/org/apache/druid/error/DruidException.java index f0846e6c6f54..cbe00d5b70dd 100644 --- a/processing/src/main/java/org/apache/druid/error/DruidException.java +++ b/processing/src/main/java/org/apache/druid/error/DruidException.java @@ -24,7 +24,6 @@ import org.apache.druid.java.util.common.StringUtils; import javax.annotation.concurrent.NotThreadSafe; -import java.util.Arrays; import java.util.LinkedHashMap; import java.util.Map; @@ -486,9 +485,6 @@ public DruidException build(Throwable cause, String formatMe, Object... vals) stackTrace[firstNonDruidExceptionIndex].getClassName().startsWith(CLASS_NAME_STR)) { ++firstNonDruidExceptionIndex; } - if (firstNonDruidExceptionIndex < stackTrace.length) { - retVal.setStackTrace(Arrays.copyOfRange(stackTrace, firstNonDruidExceptionIndex, stackTrace.length)); - } return retVal; } diff --git a/processing/src/main/java/org/apache/druid/error/NotYetImplemented.java b/processing/src/main/java/org/apache/druid/error/NotYetImplemented.java index f87b9d411549..b283034fab33 100644 --- a/processing/src/main/java/org/apache/druid/error/NotYetImplemented.java +++ b/processing/src/main/java/org/apache/druid/error/NotYetImplemented.java @@ -34,11 +34,6 @@ */ public class NotYetImplemented extends DruidException.Failure { - public static DruidException ex(String msg, Object... args) - { - return ex(null, msg, args); - } - public static DruidException ex(Throwable t, String msg, Object... args) { return DruidException.fromFailure(new NotYetImplemented(t, msg, args)); diff --git a/processing/src/main/java/org/apache/druid/query/rowsandcols/column/DoubleArrayColumn.java b/processing/src/main/java/org/apache/druid/query/rowsandcols/column/DoubleArrayColumn.java index 72517851f33c..18cb8ad9c5a8 100644 --- a/processing/src/main/java/org/apache/druid/query/rowsandcols/column/DoubleArrayColumn.java +++ b/processing/src/main/java/org/apache/druid/query/rowsandcols/column/DoubleArrayColumn.java @@ -185,13 +185,13 @@ public FindResult findLong(int startIndex, int endIndex, long val) @Override public FindResult findString(int startIndex, int endIndex, String val) { - throw NotYetImplemented.ex("findString is not currently supported for DoubleArrayColumns"); + throw NotYetImplemented.ex(null, "findString is not currently supported for DoubleArrayColumns"); } @Override public FindResult findComplex(int startIndex, int endIndex, Object val) { - throw NotYetImplemented.ex("findComplex is not currently supported for DoubleArrayColumns"); + throw NotYetImplemented.ex(null, "findComplex is not currently supported for DoubleArrayColumns"); } } } diff --git a/processing/src/main/java/org/apache/druid/query/rowsandcols/column/IntArrayColumn.java b/processing/src/main/java/org/apache/druid/query/rowsandcols/column/IntArrayColumn.java index 05c8acd0673f..4a9d7c2c5b92 100644 --- a/processing/src/main/java/org/apache/druid/query/rowsandcols/column/IntArrayColumn.java +++ b/processing/src/main/java/org/apache/druid/query/rowsandcols/column/IntArrayColumn.java @@ -191,13 +191,13 @@ public FindResult findInt(int startIndex, int endIndex, int val) @Override public FindResult findString(int startIndex, int endIndex, String val) { - throw NotYetImplemented.ex("findString is not currently supported for IntArrayColumns"); + throw NotYetImplemented.ex(null, "findString is not currently supported for IntArrayColumns"); } @Override public FindResult findComplex(int startIndex, int endIndex, Object val) { - throw NotYetImplemented.ex("findComplex is not currently supported for IntArrayColumns"); + throw NotYetImplemented.ex(null, "findComplex is not currently supported for IntArrayColumns"); } } } diff --git a/processing/src/main/java/org/apache/druid/query/rowsandcols/column/LongArrayColumn.java b/processing/src/main/java/org/apache/druid/query/rowsandcols/column/LongArrayColumn.java index d9828c37e2b9..bddf235eeb86 100644 --- a/processing/src/main/java/org/apache/druid/query/rowsandcols/column/LongArrayColumn.java +++ b/processing/src/main/java/org/apache/druid/query/rowsandcols/column/LongArrayColumn.java @@ -192,13 +192,13 @@ public FindResult findInt(int startIndex, int endIndex, int val) @Override public FindResult findString(int startIndex, int endIndex, String val) { - throw NotYetImplemented.ex("findString is not currently supported for LongArrayColumns"); + throw NotYetImplemented.ex(null, "findString is not currently supported for LongArrayColumns"); } @Override public FindResult findComplex(int startIndex, int endIndex, Object val) { - throw NotYetImplemented.ex("findComplex is not currently supported for LongArrayColumns"); + throw NotYetImplemented.ex(null, "findComplex is not currently supported for LongArrayColumns"); } } } From f58ffbae1e46ede254362a7b4b2d924157bc2804 Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Wed, 24 Jul 2024 12:16:28 +0530 Subject: [PATCH 14/15] Revert accidental change --- .../main/java/org/apache/druid/error/DruidException.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/error/DruidException.java b/processing/src/main/java/org/apache/druid/error/DruidException.java index cbe00d5b70dd..3aa49d33f8a4 100644 --- a/processing/src/main/java/org/apache/druid/error/DruidException.java +++ b/processing/src/main/java/org/apache/druid/error/DruidException.java @@ -24,6 +24,7 @@ import org.apache.druid.java.util.common.StringUtils; import javax.annotation.concurrent.NotThreadSafe; +import java.util.Arrays; import java.util.LinkedHashMap; import java.util.Map; @@ -480,11 +481,10 @@ public DruidException build(Throwable cause, String formatMe, Object... vals) StackTraceElement[] stackTrace = retVal.getStackTrace(); int firstNonDruidExceptionIndex = 0; - while ( - firstNonDruidExceptionIndex < stackTrace.length && - stackTrace[firstNonDruidExceptionIndex].getClassName().startsWith(CLASS_NAME_STR)) { + while (stackTrace[firstNonDruidExceptionIndex].getClassName().startsWith(CLASS_NAME_STR)) { ++firstNonDruidExceptionIndex; } + retVal.setStackTrace(Arrays.copyOfRange(stackTrace, firstNonDruidExceptionIndex, stackTrace.length)); return retVal; } From f9befcd549cc706dacdc24ee8ea539be4e488c4c Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Wed, 24 Jul 2024 12:36:35 +0530 Subject: [PATCH 15/15] Refactor --- .../apache/druid/error/DruidException.java | 8 +++- .../org/apache/druid/error/ExceptionTest.java | 48 +++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) create mode 100644 processing/src/test/java/org/apache/druid/error/ExceptionTest.java diff --git a/processing/src/main/java/org/apache/druid/error/DruidException.java b/processing/src/main/java/org/apache/druid/error/DruidException.java index 3aa49d33f8a4..f6af298d40eb 100644 --- a/processing/src/main/java/org/apache/druid/error/DruidException.java +++ b/processing/src/main/java/org/apache/druid/error/DruidException.java @@ -481,10 +481,14 @@ public DruidException build(Throwable cause, String formatMe, Object... vals) StackTraceElement[] stackTrace = retVal.getStackTrace(); int firstNonDruidExceptionIndex = 0; - while (stackTrace[firstNonDruidExceptionIndex].getClassName().startsWith(CLASS_NAME_STR)) { + while ( + firstNonDruidExceptionIndex < stackTrace.length + && stackTrace[firstNonDruidExceptionIndex].getClassName().startsWith(CLASS_NAME_STR)) { ++firstNonDruidExceptionIndex; } - retVal.setStackTrace(Arrays.copyOfRange(stackTrace, firstNonDruidExceptionIndex, stackTrace.length)); + if (firstNonDruidExceptionIndex > 0) { + retVal.setStackTrace(Arrays.copyOfRange(stackTrace, firstNonDruidExceptionIndex, stackTrace.length)); + } return retVal; } diff --git a/processing/src/test/java/org/apache/druid/error/ExceptionTest.java b/processing/src/test/java/org/apache/druid/error/ExceptionTest.java new file mode 100644 index 000000000000..bf587e4cbd9a --- /dev/null +++ b/processing/src/test/java/org/apache/druid/error/ExceptionTest.java @@ -0,0 +1,48 @@ +/* + * 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.druid.error; + +import org.junit.Assert; +import org.junit.Test; + +import java.util.Map; + +public class ExceptionTest +{ + @Test + public void testNoCause() + { + DruidException exception = DruidException.defensive().build("defensive"); + StackTraceElement[] stackTrace = exception.getStackTrace(); + for (StackTraceElement stackTraceElement : stackTrace) { + Assert.assertFalse(stackTraceElement.getClassName().startsWith(DruidException.CLASS_NAME_STR)); + } + } + + @Test + public void testNoStacktrace() + { + ErrorResponse errorResponse = new ErrorResponse(Forbidden.exception()); + final Map asMap = errorResponse.getAsMap(); + DruidException exception = ErrorResponse.fromMap(asMap).getUnderlyingException(); + Assert.assertTrue(exception.getCause() instanceof DruidException); + Assert.assertEquals(0, exception.getCause().getStackTrace().length); + } +}