andygrove commented on PR #5348: URL: https://github.com/apache/datafusion-comet/pull/5348#issuecomment-5286271819
Second run of the skill, added to this PR as another worked example: **[#5349: Implement `sequence` natively for integral types instead of JVM codegen dispatch](https://github.com/apache/datafusion-comet/issues/5349)**. This one exercises parts of the skill that the `unbase64` run did not: - **The exclusion check did its job.** `unbase64` was skipped because the audit log already records it as assessed, so the run went to the next-ranked candidate instead of re-deriving the same verdict. - **A `Medium` rating triggered the required empirical calibration.** Rather than reasoning from Spark's source, the run dumped the actual dispatcher kernel through `CometBatchKernelCodegen.generateSource` and counted allocation sites in the emitted Java: `new long[n]`, plus the internal `new long[...]` and `new UnsafeArrayData()` inside `UnsafeArrayData.fromPrimitiveArray`, with two of the three scaling with sequence length, and three separate passes over every element (fill loop, `copyMemory`, output-side `getLong`/`setSafe`). Roughly 48 MB of transient `long[]` per 8192-row batch for a 365-element date spine. - **A hypothesis got tested and came back negative.** The generated error branch calls `Map<String,String>.put(String, long)`, which looked like it might fail the Janino compile and therefore mean `sequence` silently falls back to Spark today (which would have raised the upside further). Calling `CometBatchKernelCodegen.compile` directly showed it compiles fine, so the issue says there is no hidden fallback rather than claiming one. - **A `Medium` compatibility rating produced a scoped recommendation instead of a blanket one.** The timezone hazard is real but confined to the temporal element types, and `Sequence.impl` selects on `dataType.elementType`, so the guard is knowable at plan time. The proposal is integral types natively, date and timestamp staying on `CodegenDispatchFallback`. The scratch test used for the kernel dump was deleted after the run, per the skill. -- 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]
