alexvanboxel commented on issue #10413: [BEAM-9035] Typed options for Row Schema and Field URL: https://github.com/apache/beam/pull/10413#issuecomment-598193823 @reuvenlax did most of your fixup's. Except: 1) Kept remove, as this is the only way to remove an option when you copy options from one schema into another. 2) getValue/Type should return null, as it's different then a Row (a row has a schema, and you would expect a field to be available). An option you first (in general) inspect for availability before doing something. Here is an example with the current code base: ``` dataSchema .getFields() .forEach( f -> { Integer pkSequence = f.getOptions().getValue("vptech.data.contract.v1.primary_key"); if (pkSequence != null) { primaryKeyMap.put(pkSequence, dataSchema.indexOf(f.getName())); } }); ``` if it would throw an exception... or you try catch everything (no no no!) or you use the hasOption method like this: ``` dataSchema .getFields() .forEach( f -> { if (f.getOptions().hasOption("vptech.data.contract.v1.primary_key")) { primaryKeyMap.put(f.getOptions().getValue("vptech.data.contract.v1.primary_key", Integer.class), dataSchema.indexOf(f.getName())); } }); ``` I and my colleagues prefer the simpler null check.
---------------------------------------------------------------- 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: [email protected] With regards, Apache Git Services
