damccorm commented on code in PR #34394:
URL: https://github.com/apache/beam/pull/34394#discussion_r2018765916
##########
sdks/java/io/jdbc/src/main/java/org/apache/beam/sdk/io/jdbc/JdbcSchemaIOProvider.java:
##########
@@ -72,6 +72,8 @@ public Schema configurationSchema() {
// readQuery
.addNullableField("partitionColumn", FieldType.STRING)
.addNullableField("partitions", FieldType.INT16)
+ .addNullableField("longLowerBound", FieldType.INT64)
Review Comment:
Sorry for the delay, thought I'd chimed in but looks like I didn't finish
the review.
Thoughts:
1) I think like Kenn mentioned below, omitting the `long` from these params
seems reasonable. If we need to support more non-default types later, we could
do so with postfixes, but it seems like making long the default is fine.
2) An alternative option for supporting this in x-lang is to make these a
row/schema'd object which takes different types as fields (e.g.
`lowerBound=Row('longLowerBound': 123, 'dateLowerBound': null)`). This might be
easier/cleaner to evolve, though it is harder to map to yaml. I don't love this
option.
3) Another alternative would be something like: `lowerBound='123',
'upperBound'='456', 'boundType'='long')` where we take the boundaries as
strings and convert them to the right type. Since we only accept `long` as a
bound type for now, we could omit this param. I kinda like this as a way to
evolve this over time since it is pretty x-lang/yaml friendly and lets us
evolve it over time. Plus we can still provide good typing in the python
wrapper. `boundType` might not even be necessary in the long term if we can
auto-infer the type (we could at least do this if we only evolve to long/date)
Overall, I think we should have a plan on how we plan to evolve this if we
need to support date columns or abitrary other column types, I don't think it
is safe to assume we will never need this. (3) is an option, but I'm open to
other paths here if folks have ideas.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
