LuciferYang commented on PR #12954:
URL: https://github.com/apache/gluten/pull/12954#issuecomment-5511776015
Thanks. Fixed the first one; the second is real but I would rather not do it
here, and the reason is a cascade I did not expect.
**Error message.** Dropped the pointless `s` prefix. I left the wording
alone: the identical string appears three times, at
`VeloxSparkPlanExecApi.scala:1460` for `TimestampAdd` thirteen lines above this
one and at `CHSparkPlanExecApi.scala:1059`, so rewording only the copy my diff
happens to touch would make the pair inconsistent and rewording all three is
unrelated to this PR.
**`injectPostRuleFactory`.** You are right that nothing reads
`postRuleFactory` any more, and it is worth being precise about when that
started: `getExtendedColumnarPostRules` already returned `List()` in every shim
from 3.4 on, so `GlutenFormatFactory.getExtendedColumnarPostRule` has not been
called on any supported version since 3.3 left. Deleting the reader here
exposes that rather than causing it. The PR description says as much.
I tried removing the setter and both registrations, and the diff does not
stop there. `NativeWritePostRule` is then constructed by nobody, so it goes
too; that orphans the private `getNativeFormat`, which orphans
`BackendSettingsApi.skipNativeCtas` and `skipNativeInsertInto` and their
`VeloxBackend` overrides. That is the whole pre-planned-write 3.3 path, six
files, and it is one coherent removal rather than something to fold into a
shim-API change. Removing only the setter is worse than either end state: it
leaves `NativeWritePostRule` as a rule class nobody registers, which reads
exactly as misleadingly as what you flagged.
So it stays as is here, and the follow-up PR that clears the residual 3.3
version checks takes the path as a unit. It already owns the `if
(SparkShimLoader.getSparkVersion.startsWith("3.3"))` guard around the Velox
registration, which is the other half of the same thing.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
