You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@beam.apache.org by GitBox <gi...@apache.org> on 2020/03/12 17:51:50 UTC

[GitHub] [beam] reuvenlax commented on issue #10413: [BEAM-9035] Typed options for Row Schema and Field

reuvenlax commented on issue #10413: [BEAM-9035] Typed options for Row Schema and Field
URL: https://github.com/apache/beam/pull/10413#issuecomment-598334091
 
 
   @alexvanboxel 
   1. I'm not entirely sure I understand the use case. Why do options need to be copied, while other constructs (such as schemas) don't?
   
   2. I disagree for a couple of reasons:
      * A FieldType can be nullable, and there's no reason not to support null FieldTypes in options. By returning null for non-existent fields, you make it hard to distinguish between a null value and an invalid get call (i.e. passing an option that is not part of the schema). This feels like the sort of behavior that seems simpler, but actually makes things more complex (e.g. the fact that Java Maps return null for non-existent values is often considered a mistaken design, for this very reason. It's hard to distinguish between a missing value and an explicit null stored in a map).
   
      * Options are no different than schemas - there is a static list of options for each field, so I don't see why this is so different.
   
   I also want to understand better why we can't just use Row here (revisiting Brian's question). You mentioned using dots in field names, but AFAIK protobuf also prohibits dots. It's true that dots are often used in option specifications in proto, but that's just to set individual fields of a message. In Beam, we really should model those as a single row-valued option, not as multiple individual options. I.e. (from the proto docs):
   
       option (my_method_option).foo = 567;
       option (my_method_option).bar = "Some string";
   
   should translate to a single Beam option named "my_method_option" not to two separate options.
   
   dots are also used to represent package names of course. Is this the main reason you need to support dots?

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


With regards,
Apache Git Services