And yes all those Table implementations should check for null and raise (probably IllegalArgumentException) with a good error message.
Kenn On Tue, Jul 21, 2020 at 3:48 PM Rui Wang <[email protected]> wrote: > I think so. NULL (or call it unknown) is used to distinguish set/not set a > field. > > > -Rui > > On Tue, Jul 21, 2020 at 3:29 PM Brian Hulette <[email protected]> wrote: > >> I think properties should be @Nullable as well then? It's also optional >> in the parser. >> >> On Tue, Jul 21, 2020 at 2:21 PM Rui Wang <[email protected]> wrote: >> >>> Hi Jayendra, >>> >>> The "location" of a table is where the table data stores. So it could be >>> null because that field in parser is optional [1] >>> >>> >>> [1]: >>> https://github.com/apache/beam/blob/master/sdks/java/extensions/sql/src/main/codegen/includes/parserImpls.ftl#L177 >>> >>> -Rui >>> >>> On Tue, Jul 21, 2020 at 9:40 AM Jayendra Parmar < >>> [email protected]> wrote: >>> >>>> Hi I am working on https://issues.apache.org/jira/browse/BEAM-10497 >>>> >>>> The location string defined here[1] be Nullable ? If yes we should >>>> throw some exceptions while that location field is needed and not present >>>> like here[2] or [3] or [4] and there are yet some other places. >>>> >>>> Or should I make that NonNullable ? >>>> >>>> As I am still not acquainted to the code and new contributor, I am not >>>> aware of repercussions in either of the cases. >>>> >>>> [1] >>>> https://github.com/apache/beam/blob/v2.23.0-RC1/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/meta/Table.java#L40 >>>> [2] >>>> https://github.com/apache/beam/blob/v2.23.0-RC1/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/mongodb/MongoDbTable.java#L91 >>>> [3] >>>> https://github.com/apache/beam/blob/v2.23.0-RC1/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/text/TextTableProvider.java#L88 >>>> [4] >>>> https://github.com/apache/beam/blob/v2.23.0-RC1/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/avro/AvroTableProvider.java#L50 >>>> >>>> Regards >>>> Jayendra >>>> >>>
