andygrove opened a new pull request, #5350:
URL: https://github.com/apache/datafusion-comet/pull/5350

   ## Which issue does this PR close?
   
   N/A. This records assessment results in the audit log. The assessments 
themselves are tracked by the
   issues they produced, #5347 and #5349.
   
   ## Rationale for this change
   
   Split out of #5348 in response to [@comphead's review 
comment](https://github.com/apache/datafusion-comet/pull/5348#discussion_r_) 
asking whether the audit-log lines belonged in that PR. They did not: #5348 
adds the
   `suggest-native-expression` skill, and these two lines are output the skill 
produced when it was run.
   Keeping them separate means the skill can be reviewed on its own merits and 
these lines can be
   reviewed on theirs.
   
   Draft because the lines are only worth landing if reviewers agree the 
assessments they record are
   sound. The issues they point at (#5347, #5349) carry the full reasoning.
   
   ## What changes are included in this PR?
   
   The first two `Native candidate (assessed ...)` lines in the per-expression 
audit logs:
   
   - **`string_funcs.md`, `unbase64`**: Recommended (#5347). Compatibility 
High, upside High. 5 JVM heap
     allocations per non-null row, and a note that upstream 
`datafusion-spark::SparkUnBase64` is not
     usable because it lowers to a strict base64 engine that cannot round-trip 
Comet's own CRLF-chunked
     `base64` output.
   - **`array_funcs.md`, `sequence`**: Recommended for integral element types 
only (#5349).
     Compatibility Medium, upside High. 3 heap allocations per non-null row and 
three passes over every
     element, measured from the emitted dispatcher kernel.
   
   The point of these lines is to make the assessment cumulative. A later run 
of the skill greps for
   `Native candidate (assessed` and skips anything already covered, so a 
candidate that was examined and
   rejected does not get re-proposed. That is why the lines record the blocker 
and the ratings rather
   than just linking the issue.
   
   Both expressions were re-verified against the Spark versions pinned in 
`pom.xml` (3.4.3, 3.5.9,
   4.0.4, 4.1.3, 4.2.0). The `UnBase64` and `Sequence` class bodies are 
byte-identical across all five,
   so neither line needs a version caveat.
   
   The new line kind is documented in `expression-audits/index.md`, which is 
part of #5348.
   
   ## How are these changes tested?
   
   Documentation only. `npx prettier` was run over the changed files.
   


-- 
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]

Reply via email to