andygrove opened a new pull request, #5154: URL: https://github.com/apache/datafusion-comet/pull/5154
## Which issue does this PR close? Documentation-only follow-up to #5085. Does not close an issue; #5149 remains open and is now the tracker the string-cast note points at. ## Rationale for this change #5085 documented the then-open `correctness`+`bug` issues. Three of those entries have changed status since it merged, and none of the follow-up PRs updated the docs: - **#5065** was fixed by #5084 (`38c3f8bce`), but the `CometIntegralDivide.getCompatibleNotes` string still tells users that `Long.MinValue div -1` silently returns the wrapped value under ANSI. #5084 touched `arithmetic.scala` and left the note behind. - **#5066** was fixed by #5080 (`a1b022b52`), but `CometMakeDecimal.getCompatibleNotes` still says the native implementation ignores `nullOnOverflow`. - **#4959** was closed as superseded by **#5149**, not fixed. The `CAST(string AS boolean)` bullet in `compatibility/index.md` therefore links a closed issue, and the audit behind #5149 showed the bullet understates the bug substantially. The first two matter because the per-category compatibility pages (`math.md`, `misc.md`, ...) are generated from these strings at docs-build time and are not committed to the tree, so the stale text publishes on the next docs build rather than showing up in a diff. #5127 already set the precedent by removing its `null IN ()` bullet as part of the fix. ## What changes are included in this PR? ### Remove notes for fixed bugs Delete the `getCompatibleNotes` overrides on `CometIntegralDivide` (`arithmetic.scala`) and `CometMakeDecimal` (`decimalExpressions.scala`). ### Rescope the string-cast trim note (`compatibility/index.md`) Repoint the bullet at #5149 and widen it to what the audit actually found: | Old bullet | Reality per #5149 | |---|---| | `CAST(string AS boolean)` only | 7 of 8 supported targets diverge; only `CAST(string AS date)` matches Spark | | One direction: `NULL` in Comet where Spark parses a value | Both directions. Comet also **over**-trims non-ASCII Unicode whitespace Spark never trims, returning a value where Spark returns `NULL` and silently succeeding under ANSI where Spark throws | | One trim set | Two: `UTF8String.trimAll` (bytes `0x00`-`0x20` **and** `0x7F`) for boolean/integral/datetime, Java `String.trim` (`U+0000`-`U+0020`, no `0x7F`) for float/double/decimal | The over-trim direction is the more severe of the two and was absent from the old bullet entirely. Also drops the raw `0x01` bytes the old text embedded in its example string (`"<0x01>true<0x01>"`). They rendered as invisible characters on the published page, so the example demonstrated nothing. The rewrite names code points instead of embedding them. ### Deliberately unchanged Everything else from #5085 is still accurate and stays as-is: #5070, #5073, #4680, #4947, #5071, #5072, #4967, #5068, #5022, #4993, and the datetime-rebase note in `scans.md` — I re-checked the dead-code claim there, and `spark.comet.exceptionOnDatetimeRebase` is still only its `CometConf.scala` definition with no reader, so the note holds. #5107 did not touch rebase handling. The only `correctness`+`bug` issue opened since #5085 is #5149, covered above. #5148 was closed by #5114 and was never documented. A matching cleanup is needed on `branch-1.0`, which carries both fixes (#5145, #5146) and all three stale entries. That will be a separate backport. ## How are these changes tested? Documentation only — no behavior change, so no new tests. Verified locally: - `./mvnw package -Pspark-3.5 -DskipTests` compiles cleanly; scalastyle reports 0 errors. - Ran the `-Pgenerate-docs` step against a temp copy of the user guide for `spark-3.5` and confirmed the generated pages: `MakeDecimal` in `misc.md` now carries only its unsupported reason, `IntegralDivide` no longer emits a section at all (the removed note was its only content), and no generated page references #5065 or #5066. The still-valid notes for #5070, #5073, #4680, and #4947 are all still present. - `npx prettier "docs/source/user-guide/latest/compatibility/index.md" --check` passes. The first draft used a nested list, which prettier reformatted into a loose list and changed how the sibling bullets rendered; the committed version is a single tight bullet, so the surrounding list is untouched. - Confirmed no raw control bytes remain anywhere in `index.md`. -- 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]
