address comments

jinchengchenghh · Jun 3, 2024 · 1360221 · 1360221
1 parent 04f2145
commit 1360221
Show file tree

Hide file tree

Showing 8 changed files with 20 additions and 9 deletions.
diff --git a/cpp/src/arrow/dataset/file_csv.h b/cpp/src/arrow/dataset/file_csv.h
@@ -84,7 +84,8 @@ class ARROW_DS_EXPORT CsvFileFormat : public FileFormat {
 /// \brief Per-scan options for CSV fragments
 struct ARROW_DS_EXPORT CsvFragmentScanOptions : public FragmentScanOptions {
   std::string type_name() const override { return kCsvTypeName; }
-
+
+  /// \brief Construct FragmentScanOptions from config map
   static Result<std::shared_ptr<FragmentScanOptions>> From(
       const std::unordered_map<std::string, std::string>& configs);
 

diff --git a/cpp/src/arrow/engine/substrait/serde.h b/cpp/src/arrow/engine/substrait/serde.h
@@ -184,6 +184,10 @@ ARROW_ENGINE_EXPORT Result<BoundExpressions> DeserializeExpressions(
     const ConversionOptions& conversion_options = {},
     ExtensionSet* ext_set_out = NULLPTR);
 
+/// \brief Deserialize a Substrait Literal message to the map
+///
+/// \param[in] buf a buffer containing the protobuf serialization of a Substrait Literal
+/// \param[out] out deserialize to this map.
 ARROW_ENGINE_EXPORT Status
 DeserializeMap(const Buffer& buf, std::unordered_map<std::string, std::string>& out);
 

diff --git a/java/dataset/src/main/cpp/jni_wrapper.cc b/java/dataset/src/main/cpp/jni_wrapper.cc
@@ -163,7 +163,7 @@ GetFragmentScanOptions(jint file_format_id,
       return arrow::dataset::CsvFragmentScanOptions::From(configs);
 #endif
     default:
-      return arrow::Status::Invalid("Illegal file format id: " ,file_format_id);
+      return arrow::Status::Invalid("Illegal file format id: ", file_format_id);
   }
 }
 

diff --git a/java/dataset/src/main/java/org/apache/arrow/dataset/file/JniWrapper.java b/java/dataset/src/main/java/org/apache/arrow/dataset/file/JniWrapper.java
@@ -41,7 +41,8 @@ private JniWrapper() {
    * intermediate shared_ptr of the factory instance.
    *
    * @param uri file uri to read, either a file or a directory
-   * @param fileFormat file format ID
+   * @param fileFormat file format ID.
+   * @param serializedFragmentScanOptions serialized FragmentScanOptions.
    * @return the native pointer of the arrow::dataset::FileSystemDatasetFactory instance.
    * @see FileFormat
    */
@@ -53,7 +54,8 @@ public native long makeFileSystemDatasetFactory(String uri, int fileFormat,
    * intermediate shared_ptr of the factory instance.
    *
    * @param uris List of file uris to read, each path pointing to an individual file
-   * @param fileFormat file format ID
+   * @param fileFormat file format ID.
+   * @param serializedFragmentScanOptions serialized FragmentScanOptions.
    * @return the native pointer of the arrow::dataset::FileSystemDatasetFactory instance.
    * @see FileFormat
    */

diff --git a/java/dataset/src/main/java/org/apache/arrow/dataset/jni/JniWrapper.java b/java/dataset/src/main/java/org/apache/arrow/dataset/jni/JniWrapper.java
@@ -76,6 +76,8 @@ private JniWrapper() {
    * @param substraitProjection substrait extended expression to evaluate for project new columns
    * @param substraitFilter substrait extended expression to evaluate for apply filter
    * @param batchSize batch size of scanned record batches.
+   * @param fileFormat file format ID.
+   * @param serializedFragmentScanOptions serialized FragmentScanOptions.
    * @param memoryPool identifier of memory pool used in the native scanner.
    * @return the native pointer of the arrow::dataset::Scanner instance.
    */

diff --git a/java/dataset/src/main/java/org/apache/arrow/dataset/scanner/FragmentScanOptions.java b/java/dataset/src/main/java/org/apache/arrow/dataset/scanner/FragmentScanOptions.java
@@ -32,10 +32,10 @@ public interface FragmentScanOptions {
   ByteBuffer serialize();
 
   /**
-   * serialize the map.
+   * Serialize the map.
    *
    * @param config config map
-   * @return bufer to jni call argument, should be DirectByteBuffer
+   * @return buffer to jni call argument, should be DirectByteBuffer
    */
   default ByteBuffer serializeMap(Map<String, String> config) {
     if (config.isEmpty()) {

diff --git a/java/dataset/src/main/java/org/apache/arrow/dataset/scanner/ScanOptions.java b/java/dataset/src/main/java/org/apache/arrow/dataset/scanner/ScanOptions.java
@@ -147,7 +147,7 @@ public Builder substraitFilter(ByteBuffer substraitFilter) {
     /**
      * Set the FragmentScanOptions.
      *
-     * @param fragmentScanOptions scan options
+     * @param fragmentScanOptions fragment scan options
      * @return the ScanOptions configured.
      */
     public Builder fragmentScanOptions(FragmentScanOptions fragmentScanOptions) {

diff --git a/java/dataset/src/main/java/org/apache/arrow/dataset/substrait/util/ConvertUtil.java b/java/dataset/src/main/java/org/apache/arrow/dataset/substrait/util/ConvertUtil.java
@@ -24,10 +24,12 @@
 public class ConvertUtil {
 
   /**
-   * Convert map to substrait Expression.
+   * Convert map to substrait Expression Literal.
+   * @param values config map
    *
-   * @return Substrait Expression
+   * @return substrait Expression Literal
    */
+
   public static Expression.Literal mapToExpressionLiteral(Map<String, String> values) {
     Expression.Literal.Builder literalBuilder = Expression.Literal.newBuilder();
     Expression.Literal.Map.KeyValue.Builder keyValueBuilder =