deepakpanda93 opened a new pull request, #19509: URL: https://github.com/apache/hudi/pull/19509
### Describe the issue this Pull Request addresses Closes #16423 / [HUDI-7526](https://issues.apache.org/jira/browse/HUDI-7526). A partitioner named through `hoodie.bulkinsert.user.defined.partitioner.class` is instantiated by reflection: ```java // DataSourceUtils#createUserDefinedBulkInsertPartitioner Option.of((BulkInsertPartitioner) ReflectionUtils.loadClass(bulkInsertPartitionerClass, config)); ``` `ReflectionUtils.loadClass(clazz, Object...)` infers the constructor argument types from the instances it is handed, so it resolves `getConstructor(HoodieWriteConfig.class)`. A class is therefore usable as a user defined partitioner only if it exposes a public constructor taking exactly one `HoodieWriteConfig`. Twelve partitioners back an out of the box `BulkInsertSortMode`. Eight of them did not have such a constructor, and failed with `Unable to instantiate class ...`: | Sort mode | Spark RDD | Spark row writer | Java client | | --- | --- | --- | --- | | `NONE` | `NonSortPartitioner` (fixed) | `NonSortPartitionerWithRows` (fixed) | `JavaNonSortPartitioner` (fixed) | | `GLOBAL_SORT` | `GlobalSortPartitioner` | `GlobalSortPartitionerWithRows` | `JavaGlobalSortPartitioner` (fixed) | | `PARTITION_SORT` | `RDDPartitionSortPartitioner` | `PartitionSortPartitionerWithRows` | not supported | | `PARTITION_PATH_REPARTITION` | `PartitionPathRepartitionPartitioner` (fixed) | `PartitionPathRepartitionPartitionerWithRows` (fixed) | not supported | | `PARTITION_PATH_REPARTITION_AND_SORT` | `PartitionPathRepartitionAndSortPartitioner` (fixed) | `PartitionPathRepartitionAndSortPartitionerWithRows` (fixed) | not supported | `NonSortPartitioner`, backing `BulkInsertSortMode.NONE`, is the example named in the ticket. ### Summary and Changelog Every partitioner that an out of the box bulk insert sort mode maps to can now be named through `hoodie.bulkinsert.user.defined.partitioner.class`. - Adds a public `(HoodieWriteConfig)` constructor to the eight partitioners marked above. - `NonSortPartitioner`, `NonSortPartitionerWithRows`, `JavaNonSortPartitioner` and `JavaGlobalSortPartitioner` have nothing configurable, so the new constructor behaves as the default one does. The value used for `enforceNumOutputPartitions` is `false`, matching what `BulkInsertInternalPartitionerFactory.get(table, config)` and `BulkInsertInternalPartitionerWithRowsFactory.get(config, isTablePartitioned)` pass on the bulk insert write path. - The two java client partitioners relied on the implicit no-arg constructor. Declaring any constructor removes it, and `JavaBulkInsertInternalPartitionerFactory` calls it, so the no-arg constructor is now declared explicitly alongside the new one. - The partition path repartition partitioners take the table partitioned flag from the `HoodieTable` when built by the factory, which reflection cannot supply. The new constructor derives it from the configured partition path field, added as `BulkInsertPartitioner.isTablePartitioned`. Those partitioners branch on whether records carry a non-empty partition path, and the write side partition path field is what governs that. - Purely additive. No existing constructor is changed or removed, so no existing caller is affected. Left alone, because no sort mode maps to them: - `RDDSpatialCurveSortPartitioner` and `RowSpatialCurveSortPartitioner` are clustering layout partitioners, not sort mode partitioners. The RDD one also needs a `HoodieSparkEngineContext` that cannot be derived from a write config. - `JavaCustomColumnsSortPartitioner` is not selected by any sort mode. Tests: - `TestDataSourceUtils` gains a parameterized test over all thirteen constructible spark partitioners, asserting each loads through both `createUserDefinedBulkInsertPartitioner` and `createUserDefinedBulkInsertPartitionerWithRows`. It covers the ones that already worked, so the whole contract is pinned rather than only the classes being fixed. It also gains a test pinning `isTablePartitioned` for a configured partition path field, an explicitly empty one, and an unset one. - `TestJavaBulkInsertInternalPartitionerFactory` gains a test that both java client sort mode partitioners load from a write config, and a test that both still expose a no-arg constructor, which guards the implicit constructor removal described above. Reverting only the new constructors makes those tests fail for exactly the classes concerned and nothing else. ### Impact None for existing users. The change only adds constructors, so every current call site resolves as before, and the factories are unaffected. What changes is that eight built in partitioners can now be named as a user defined partitioner where that previously failed at instantiation. ### Risk Level low Additive only, with no signature changed or removed. The one piece of new logic, `isTablePartitioned`, is used solely by the new constructors; the existing `(boolean, HoodieWriteConfig)` constructors still take the flag from the caller. Verified end to end by running a bulk insert with each fixed partitioner named as the user defined partitioner, over both the RDD and row writer paths, and on both a partitioned and a non partitioned table so both branches of the derived flag are exercised. ### Documentation Update None. No new config, and no change to an existing config's meaning or default. ### Contributor's checklist - [x] Read through [contributor's guide](https://hudi.apache.org/contribute/how-to-contribute) - [x] Change Logs and Impact were stated clearly - [x] Adequate tests were added if applicable - [x] CI passed -- 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]
