pabloem commented on a change in pull request #15848:
URL: https://github.com/apache/beam/pull/15848#discussion_r798090588
##########
File path:
sdks/java/io/jdbc/src/main/java/org/apache/beam/sdk/io/jdbc/JdbcIO.java
##########
@@ -325,17 +325,24 @@ public static ReadRows readRows() {
*
* @param <T> Type of the data to be read.
*/
- public static <T> ReadWithPartitions<T> readWithPartitions() {
- return new AutoValue_JdbcIO_ReadWithPartitions.Builder<T>()
+ public static <T, PartitionColumnT> ReadWithPartitions<T, PartitionColumnT>
readWithPartitions(
Review comment:
you're right. Indeed, I wanted to avoid breaking as much as possible -
so this change does not break the existing integration tests (see JdbcIOIT),
while adding support for any type partitioning - and the transform is fairly
new, so I think we may be able to pull this off : ) - WDYT?
##########
File path:
sdks/java/io/jdbc/src/main/java/org/apache/beam/sdk/io/jdbc/JdbcIO.java
##########
@@ -999,59 +1015,84 @@ public void populateDisplayData(DisplayData.Builder
builder) {
/** Implementation of {@link #readWithPartitions}. */
@AutoValue
- public abstract static class ReadWithPartitions<T> extends
PTransform<PBegin, PCollection<T>> {
+ public abstract static class ReadWithPartitions<T, PartitionColumnT>
+ extends PTransform<PBegin, PCollection<T>> {
abstract @Nullable SerializableFunction<Void, DataSource>
getDataSourceProviderFn();
abstract @Nullable RowMapper<T> getRowMapper();
abstract @Nullable Coder<T> getCoder();
- abstract Integer getNumPartitions();
+ abstract @Nullable Integer getNumPartitions();
abstract @Nullable String getPartitionColumn();
- abstract @Nullable Long getLowerBound();
+ abstract @javax.annotation.Nullable @Nullable PartitionColumnT
getLowerBound();
- abstract @Nullable Long getUpperBound();
+ abstract @javax.annotation.Nullable @Nullable PartitionColumnT
getUpperBound();
abstract @Nullable String getTable();
- abstract Builder<T> toBuilder();
+ abstract TypeDescriptor<PartitionColumnT> getPartitionColumnType();
+
+ abstract Builder<T, PartitionColumnT> toBuilder();
+
+ /**
+ * A helper for {@link ReadWithPartitions} that handles range calculations.
+ *
+ * @param <PartitionT>
+ */
+ public interface JdbcReadWithPartitionsHelper<PartitionT>
Review comment:
I think this makes sense - we can leave this internal for now, and in
the future consider any changes to the types that we use, and externalize for
users if we want to. Done,
--
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]
