SparkExtension.COLLECTION — the standard extension collection merged with spark.yml — is public API with no consumer in spark/src/main at all. DialectGenerator was the last one, and #1133 rewires it to the collections the runtime converters actually bind against. The only remaining use in the repo is SubstraitPlanTestBase.scala:76, which passes it to ProtoRelConverter.
Worth deciding what it is for before it drifts further. The interesting part is that the merged collection is the right thing for ProtoRelConverter (a plan can legitimately reference spark.yml functions) and the wrong thing for anything that has to bind a call, since toAggregateFunction and toWindowFunction are built from the standard collection alone — the asymmetry #1133 exists to fix. So COLLECTION is not simply dead: it is the collection a reader wants, used only from test code, sitting next to per-kind function sequences that are what writers want.
Options, roughly: keep it and document which side it serves; move it to the test source set if nothing outside tests should be reading plans this way; or expose it from wherever the plan-reading entry point ends up living. Also worth checking whether downstream consumers reference it, since removing or moving it is a breaking change to a public Scala object.
Follow-up from review of #1133.
SparkExtension.COLLECTION— the standard extension collection merged withspark.yml— is public API with no consumer inspark/src/mainat all.DialectGeneratorwas the last one, and #1133 rewires it to the collections the runtime converters actually bind against. The only remaining use in the repo isSubstraitPlanTestBase.scala:76, which passes it toProtoRelConverter.Worth deciding what it is for before it drifts further. The interesting part is that the merged collection is the right thing for
ProtoRelConverter(a plan can legitimately referencespark.ymlfunctions) and the wrong thing for anything that has to bind a call, sincetoAggregateFunctionandtoWindowFunctionare built from the standard collection alone — the asymmetry #1133 exists to fix. SoCOLLECTIONis not simply dead: it is the collection a reader wants, used only from test code, sitting next to per-kind function sequences that are what writers want.Options, roughly: keep it and document which side it serves; move it to the test source set if nothing outside tests should be reading plans this way; or expose it from wherever the plan-reading entry point ends up living. Also worth checking whether downstream consumers reference it, since removing or moving it is a breaking change to a public Scala object.
Follow-up from review of #1133.