deemoliu commented on code in PR #12980: URL: https://github.com/apache/pinot/pull/12980#discussion_r1588511468
########## pinot-plugins/pinot-input-format/pinot-protobuf/src/test/java/org/apache/pinot/plugin/inputformat/protobuf/ProtoBufTestDataGenerator.java: ########## @@ -0,0 +1,145 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.plugin.inputformat.protobuf; + +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Sets; +import com.google.protobuf.ByteString; +import java.io.IOException; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import static java.nio.charset.StandardCharsets.UTF_8; + +public class ProtoBufTestDataGenerator { + + private ProtoBufTestDataGenerator() { + } + + public static final String STRING_FIELD = "string_field"; + public static final String INT_FIELD = "int_field"; + public static final String LONG_FIELD = "long_field"; + public static final String DOUBLE_FIELD = "double_field"; + public static final String FLOAT_FIELD = "float_field"; + public static final String BOOL_FIELD = "bool_field"; + public static final String BYTES_FIELD = "bytes_field"; + public static final String NESTED_MESSAGE = "nested_message"; + public static final String REPEATED_NESTED_MESSAGES = "repeated_nested_messages"; + public static final String COMPLEX_MAP = "complex_map"; + public static final String SIMPLE_MAP = "simple_map"; + public static final String ENUM_FIELD = "enum_field"; + public static final String NESTED_INT_FIELD = "nested_int_field"; + public static final String NESTED_STRING_FIELD = "nested_string_field"; + public static final String NULLABLE_STRING_FIELD = "nullable_string_field"; + public static final String NULLABLE_INT_FIELD = "nullable_int_field"; + public static final String NULLABLE_LONG_FIELD = "nullable_long_field"; + public static final String NULLABLE_DOUBLE_FIELD = "nullable_double_field"; + public static final String NULLABLE_FLOAT_FIELD = "nullable_float_field"; + public static final String NULLABLE_BOOL_FIELD = "nullable_bool_field"; + public static final String NULLABLE_BYTES_FIELD = "nullable_bytes_field"; Review Comment: what are the supported data types if pinot doesn't support the type natively. ########## pinot-plugins/pinot-input-format/pinot-protobuf/src/main/java/org/apache/pinot/plugin/inputformat/protobuf/ProtoBufUtils.java: ########## @@ -48,21 +51,156 @@ public static InputStream getDescriptorFileInputStream(String descriptorFilePath PinotFS pinotFS = PinotFSFactory.create(scheme); Path localTmpDir = Files.createTempDirectory(TMP_DIR_PREFIX + System.currentTimeMillis()); File protoDescriptorLocalFile = createLocalFile(descriptorFileURI, localTmpDir.toFile()); - LOGGER.info("Copying protocol buffer descriptor file from source: {} to dst: {}", descriptorFilePath, + LOGGER.info("Copying protocol buffer jar/descriptor file from source: {} to dst: {}", filePath, protoDescriptorLocalFile.getAbsolutePath()); pinotFS.copyToLocalFile(descriptorFileURI, protoDescriptorLocalFile); - return new FileInputStream(protoDescriptorLocalFile); + return protoDescriptorLocalFile; } else { throw new RuntimeException(String.format("Scheme: %s not supported in PinotFSFactory" - + " for protocol buffer descriptor file: %s.", scheme, descriptorFilePath)); + + " for protocol buffer jar/descriptor file: %s.", scheme, filePath)); } } + public static InputStream getDescriptorFileInputStream(String descriptorFilePath) + throws Exception { + return new FileInputStream(getFileCopiedToLocal(descriptorFilePath)); + } + public static File createLocalFile(URI srcURI, File dstDir) { String sourceURIPath = srcURI.getPath(); File dstFile = new File(dstDir, new File(sourceURIPath).getName()); LOGGER.debug("Created empty local temporary file {} to copy protocol " + "buffer descriptor {}", dstFile.getAbsolutePath(), srcURI); return dstFile; } + + // This is needed since the generated class name is not always the same as the proto file name. + // The descriptor that we get from the jar drops the first prefix of the proto class name. + // For example, insead of com.data.example.ExampleProto we get data.example.ExampleProto. + // Copied from Flink codebase. Review Comment: for copied code you can add comments like ``` /** * Copied from the presto TimeZoneKey. It basically caches the Joda Chronologies corresponding to each of the * timezones listed in the zone-index.properties * The zone-index.properties is kept in sync with the presto zone index properties. */ ``` -- 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: commits-unsubscr...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
