[GitHub] [arrow] westonpace commented on a change in pull request #10289: PARQUET-1798: [C++] Review logic around automatic assignment of field_id's

[GitBox <gi...@apache.org>]

westonpace commented on a change in pull request #10289:
URL: https://github.com/apache/arrow/pull/10289#discussion_r640045424



##########
File path: cpp/src/parquet/schema.h
##########
@@ -268,7 +268,7 @@ class PARQUET_EXPORT GroupNode : public Node {
  public:
   // The field_id here is the default to use if it is not set in the SchemaElement
   static std::unique_ptr<Node> FromParquet(const void* opaque_element,
-                                           NodeVector fields = {}, int field_id = -1);
+                                           NodeVector fields = {});

Review comment:
       I don't believe so.  There is no reason they can't create the node and then modify the `field_id` after the fact.  The purpose of `FromParquet` is to take the information in the `opaque_element` and put it into the `Node`.  Why allow the `field_id` to be overridden in this way but not allow it for `name` or `repetition`?




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org