clintropolis commented on code in PR #14900:
URL: https://github.com/apache/druid/pull/14900#discussion_r1345480513
##########
sql/src/test/java/org/apache/druid/sql/calcite/QueryTestRunner.java:
##########
@@ -667,7 +674,9 @@ public QueryTestRunner(QueryTestBuilder builder)
verifySteps.add(new VerifyNativeQueries(finalExecStep));
}
if (builder.expectedResultsVerifier != null) {
- verifySteps.add(new VerifyResults(finalExecStep));
+ // Don't verify the row signature when MSQ is running, since the
broker receives the task id, and the signature
+ // would be {TASK:STRING} instead of the expected results signature
+ verifySteps.add(new VerifyResults(finalExecStep,
!config.isRunningMSQ()));
Review Comment:
it might be nice to have something that functions equivalently for msq based
tests (if there isn't already) but this doesn't seem like a blocker.
##########
processing/src/main/java/org/apache/druid/frame/field/NumericArrayFieldSelector.java:
##########
@@ -0,0 +1,204 @@
+/*
+ * 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.frame.field;
+
+import org.apache.datasketches.memory.Memory;
+import org.apache.druid.error.DruidException;
+import org.apache.druid.query.monomorphicprocessing.RuntimeShapeInspector;
+import org.apache.druid.segment.ColumnValueSelector;
+
+import javax.annotation.Nullable;
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Base implementation of the column value selector that the concrete numeric
field reader implementations inherit from.
+ * The selector contains the logic to construct an array written by {@link
NumericArrayFieldWriter}, and present it as
+ * a column value selector.
+ *
+ * The inheritors of this class are expected to implement
+ * 1. {@link #getIndividualValueAtMemory} Which extracts the element from the
field where it was written to. Returns
+ * null if the value at that location represents a null element
+ * 2. {@link #getIndividualFieldSize} Which informs the method about the
field size corresponding to each element in
+ * the numeric array's serialized representation
+ *
+ * @param <ElementType> Type of the individual array elements
+ */
+public abstract class NumericArrayFieldSelector<ElementType extends Number>
implements ColumnValueSelector
+{
+ /**
+ * Memory containing the serialized values of the array
+ */
+ protected final Memory memory;
+
+ /**
+ * Pointer to location in the memory. The callers are expected to update the
pointer's position to the start of the
+ * array that they wish to get prior to {@link #getObject()} call.
+ *
+ * Frames read and written using {@link
org.apache.druid.frame.write.FrameWriter} and
+ * {@link org.apache.druid.frame.read.FrameReader} shouldn't worry about
this detail, since they automatically update
+ * and handle the start location
+ */
+ private final ReadableFieldPointer fieldPointer;
+
+ /**
+ * Position last read, for caching the last fetched result
+ */
+ private long currentFieldPosition = -1;
+
+ /**
+ * Value of the row at the location beginning at {@link
#currentFieldPosition}
+ */
+ private final List<ElementType> currentRow = new ArrayList<>();
+
+ /**
+ * Nullity of the row at the location beginning at {@link
#currentFieldPosition}
+ */
+ private boolean currentRowIsNull;
+
+ public NumericArrayFieldSelector(final Memory memory, final
ReadableFieldPointer fieldPointer)
+ {
+ this.memory = memory;
+ this.fieldPointer = fieldPointer;
+ }
+
+ @Override
+ public void inspectRuntimeShape(RuntimeShapeInspector inspector)
+ {
+ // Do nothing
+ }
+
+ @Nullable
+ @Override
+ public Object getObject()
+ {
+ final List<ElementType> currentArray = computeCurrentArray();
Review Comment:
i suppose this is the only real disadvantage of not storing the array length
after the null byte and wanting to spit out `Object[]`. Basically that since we
don't know the length up front we have to make a list and then convert it to
`Object[]` after (though realistically right now it seems like almost
everything that handles arrays handles Object[], List, and some group by stuff,
so its not strictly required, but is better to be consistent).
I'm not sure that the extra bytes per value (could use
`VByte.writeInt`/`VByte.readInt`, well add methods that work on `Memory`
instead of `ByteBuffer` to fit in a little as a byte) are worth the savings in
reads, might be worth measuring someday, though this doesn't really seem like a
blocker to me
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
