deepakpanda93 commented on code in PR #19509:
URL: https://github.com/apache/hudi/pull/19509#discussion_r3882336339
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/execution/bulkinsert/NonSortPartitioner.java:
##########
@@ -47,6 +48,18 @@ public NonSortPartitioner() {
this(false);
}
+ /**
Review Comment:
Applied — and it was worse than the comment suggests: the block was in 8
files, not six.
The duplicated part is the rationale, and this PR already gives that a
single home in `BulkInsertPartitioner.isTablePartitioned`:
> A partitioner named through
`HoodieWriteConfig.BULKINSERT_USER_DEFINED_PARTITIONER_CLASS_NAME` is
instantiated by reflection with only the write config, so an implementation
that otherwise takes the flag from the `HoodieTable` has nothing else to derive
it from.
So the copies were repeating something already documented. Each constructor
now carries a pointer instead:
```java
/**
* Constructor for reflection-based instantiation via
* {@code
HoodieWriteConfig.BULKINSERT_USER_DEFINED_PARTITIONER_CLASS_NAME}.
*
* @param config Write config, unused.
*/
```
I kept the one distinction that carries information rather than making all
eight byte-identical: the four that ignore the config say `Write config,
unused.`, the four `PartitionPathRepartition*` ones that read the partition
path field from it say `Write config.`
One occurrence of the phrase remains, in `TestDataSourceUtils`, but that one
documents what the test asserts rather than a constructor contract, so it is
not a copy of the same explanation:
```java
/**
* Every out of the box bulk insert partitioner has to be usable as a user
defined partitioner.
* One is instantiated by reflection with only the write config, so each has
to expose a
* constructor taking only a {@link HoodieWriteConfig}. See HUDI-7526.
*/
```
Rebased onto latest master (106 commits, no conflicts) and re-verified:
checkstyle clean on `hudi-client-common`, `hudi-java-client` and
`hudi-spark-client`; `TestJavaBulkInsertInternalPartitionerFactory` 7/7;
`TestDataSourceUtils` 24/24, which is the test that reflectively constructs all
of these.
--
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]
