rahil-c commented on code in PR #17475:
URL: https://github.com/apache/hudi/pull/17475#discussion_r2607312734
##########
hudi-spark-datasource/hudi-spark4-common/src/main/scala/org/apache/spark/sql/adapter/BaseSpark4Adapter.scala:
##########
@@ -73,6 +73,8 @@ abstract class BaseSpark4Adapter extends SparkAdapter with
Logging {
override def getAvroSchemaConverters: HoodieAvroSchemaConverters =
HoodieSparkAvroSchemaConverters
+ override def getHoodieSchemaConverters: HoodieSchemaConverters =
HoodieSparkSchemaConverters
Review Comment:
I actually think its not as straightforward as I thought refactoring this to
remove the spark adapter pattern, here is the reasoning.
To keep the existing pattern for `AvroConversionUtils` we craeted the
`HoodieSchemaConversionUtils`. Both these classes sit in the
`hudi-spark-client` module. If we want to remove the
`getHoodieSchemaConverters`, and directly use the `HoodieSparkSchemaConverters`
this is not possible as it sits within the `hudi-spark-common` modudle. We want
to keep this conveter in `hudi-spark-common` and we also want to keep this
private[sql] in order to be able to access certain spark data types see comment
here https://github.com/apache/hudi/pull/17475#discussion_r2606937790
Ultimately there is no clean way to access this
`HoodieSparkSchemaConverters`, hence likely why this spark adapter pattern
existed in the first place, as `SparkAdapter` sits in the `hudi-spark-client`
and is directly accessible to then leverage methods such as
`getAvroSchemaConverters` or `getHoodieSchemaConverters`
For now @yihua my opinion is we should keep the pattern in this pr so we can
make progress and maybe file a follow up future task on just trying to better
clean up the spark adapter pattern itself.
--
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]
