[
https://issues.apache.org/jira/browse/DRILL-8239?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18104223#comment-18104223
]
ASF GitHub Bot commented on DRILL-8239:
---------------------------------------
shfshihuafeng commented on code in PR #2567:
URL: https://github.com/apache/drill/pull/2567#discussion_r3770696057
##########
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java:
##########
@@ -28,12 +28,14 @@
import org.apache.drill.exec.physical.config.Project;
import org.apache.drill.exec.physical.resultSet.ResultSetLoader;
import org.apache.drill.exec.record.AbstractSingleRecordBatch;
+import org.apache.drill.exec.record.MaterializedField;
Review Comment:
nit: The imports for MaterializedField and JsonConverterUtils break the
alphabetical ordering of the other imports.
##########
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java:
##########
@@ -274,8 +338,10 @@ private void setValueCount(int count) {
for (ComplexWriter writer : complexWriters) {
writer.setValueCount(count);
}
- } else if (rsLoader != null) {
- rsLoader.setTargetRowCount(count);
+ } else if (!CollectionUtils.isEmpty(rsLoaders)) {
+ for (ResultSetLoader loader : rsLoaders) {
+ loader.setTargetRowCount(count);
Review Comment:
In my opinion, the loop body calling setTargetRowCount in
ProjectRecordBatch.setValueCount() is dead code.
Two cases exist:
With a ComplexWriter function (convert_fromJSON, split, etc.):
addComplexField() is called → complexWriters is non-null → complexWriters !=
null is true → enters the first branch, else if is short‑circuited.
Without any ComplexWriter function (e.g. select *): complexWriters stays
null → complexWriters != null is false → the else if condition is evaluated.
But addLoader() is also not called → rsLoaders is null →
!CollectionUtils.isEmpty(rsLoaders) is false → the loop body is not entered.
##########
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/conv/JsonConverterUtils.java:
##########
@@ -0,0 +1,180 @@
+/*
+ * 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.drill.exec.expr.fn.impl.conv;
+
+
+import io.netty.buffer.DrillBuf;
+import org.apache.drill.common.exceptions.UserException;
+import org.apache.drill.exec.physical.resultSet.ResultSetLoader;
+import org.apache.drill.exec.physical.resultSet.RowSetLoader;
+import org.apache.drill.exec.server.options.OptionManager;
+import org.apache.drill.exec.store.easy.json.loader.ClosingStreamIterator;
+import org.apache.drill.exec.store.easy.json.loader.JsonLoaderImpl;
+import
org.apache.drill.exec.store.easy.json.loader.JsonLoaderImpl.JsonLoaderBuilder;
+import org.apache.drill.exec.store.easy.json.loader.JsonLoaderOptions;
+import org.apache.drill.exec.vector.complex.fn.DrillBufInputStream;
+
+import java.io.ByteArrayInputStream;
+import java.io.InputStream;
+import java.io.SequenceInputStream;
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
+import java.util.Collections;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class JsonConverterUtils {
+
+ private static final Logger logger =
LoggerFactory.getLogger(JsonConverterUtils.class);
+
+ /**
+ * Field name used to wrap the input value so that the record-oriented JSON
+ * loader can read top-level scalars and arrays (not just objects) uniformly.
+ * The single resulting column carries the converted value and is recognised
+ * and unwrapped by {@code ProjectRecordBatch} (which transfers it directly
to
+ * the output column instead of wrapping the loader's columns in a map).
+ */
+ public static final String WRAP_FIELD = "drill_json_value_wrapper";
+
+ private static final byte[] WRAP_PREFIX =
+ ("{\"" + WRAP_FIELD + "\":").getBytes(StandardCharsets.UTF_8);
+ private static final byte[] WRAP_SUFFIX =
"}".getBytes(StandardCharsets.UTF_8);
+
+ private JsonConverterUtils() {
+ }
+
+ /**
+ * Creates a {@link JsonLoaderImpl} for use in JSON conversion UDFs, using
the
+ * system JSON options from the {@link OptionManager}.
+ *
+ * @param rsLoader The {@link ResultSetLoader} used in the UDF
+ * @param options The {@link OptionManager} used in the UDF. This is used
to extract the global JSON options
+ * @param stream An input stream containing the input JSON data
+ * @return A {@link JsonLoaderImpl} for use in the UDF.
+ */
+ public static JsonLoaderImpl createJsonLoader(ResultSetLoader rsLoader,
+ OptionManager options,
+ ClosingStreamIterator stream) {
+ JsonLoaderBuilder jsonLoaderBuilder = new JsonLoaderBuilder()
+ .resultSetLoader(rsLoader)
+ .standardOptions(options)
+ .fromStream(() -> stream);
+
+ return (JsonLoaderImpl) jsonLoaderBuilder.build();
+ }
+
+ /**
+ * Creates a {@link JsonLoaderImpl} for use in JSON conversion UDFs,
overriding the
+ * {@code allTextMode} and {@code readNumbersAsDouble} options with the
values supplied
+ * as function arguments. Remaining options are taken from the system JSON
options.
+ *
+ * @param rsLoader The {@link ResultSetLoader} used in the UDF
+ * @param options The {@link OptionManager} used in the UDF. This is used
to extract the global JSON options
+ * @param stream An input stream containing the input JSON data
+ * @param allTextMode Whether to read all scalars as text
+ * @param readNumbersAsDouble Whether to read all numbers as doubles
+ * @return A {@link JsonLoaderImpl} for use in the UDF.
+ */
+ public static JsonLoaderImpl createJsonLoader(ResultSetLoader rsLoader,
+ OptionManager options,
+ ClosingStreamIterator stream,
+ boolean allTextMode,
+ boolean readNumbersAsDouble) {
+ JsonLoaderOptions jsonOptions = new JsonLoaderOptions(options);
+ jsonOptions.allTextMode = allTextMode;
+ jsonOptions.readNumbersAsDouble = readNumbersAsDouble;
+
+ JsonLoaderBuilder jsonLoaderBuilder = new JsonLoaderBuilder()
+ .resultSetLoader(rsLoader)
+ .options(jsonOptions)
+ .fromStream(() -> stream);
+
+ return (JsonLoaderImpl) jsonLoaderBuilder.build();
+ }
+
+ /**
+ * Converts a single JSON value (one row of UDF input) into the result set
loader.
+ *
+ * <p>Exactly one row is always written to the loader
> Convert JSON UDF to EVF
> -----------------------
>
> Key: DRILL-8239
> URL: https://issues.apache.org/jira/browse/DRILL-8239
> Project: Apache Drill
> Issue Type: Improvement
> Components: Execution - Data Types
> Affects Versions: 1.20.1
> Reporter: Charles Givre
> Assignee: Charles Givre
> Priority: Minor
>
> In an effort to fully deprecate the old JsonReader, this PR converts the
> convert_from JSON UDF to EVF.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
