andygrove commented on issue #5327:
URL: 
https://github.com/apache/datafusion-comet/issues/5327#issuecomment-5637109913

   ## Proposed priorities for 1.1.0: correctness first
   
   I went through the open backlog to work out what we should resolve before 
cutting 1.1.0, focusing on correctness bugs and real performance regressions 
rather than new features. The milestone currently has 61 open issues, but most 
of the critical correctness bugs are not on it: of the 15 open issues labelled 
`priority:critical`, only 5 carry the 1.1.0 milestone. The ranking below 
ignores the milestone and orders by user impact and how close a fix is.
   
   ### Tier 1: silent wrong results with an approved fix
   
   These just need to land.
   
   | Issue | Problem | Fix PR | Status |
   |---|---|---|---|
   | #5824 | Semi/anti joins and `explode_outer` share a reused exchange and 
return duplicated rows | #5828 | Approved, CI green |
   | #5783 | Native scan returns N times the rows for a struct with duplicate 
field names | #5786 | Approved, needs rebase |
   | #5701 | `array_distinct` / `array_union` diverge on `-0.0` on Spark 
versions without SPARK-54918 | #5750 | Approved, CI green |
   | #5366 | `to_time` rejects inputs Spark accepts (already on the 1.0.1 
milestone) | #5738 | Approved, one failing check |
   
   ### Tier 2: silent wrong results, fix needs review or does not exist yet
   
   This is where I would put reviewer time before anything else in the 
milestone.
   
   - **AVG returns NULL from empty native partials** (#5418, #5419). A plain 
`avg(x) ... WHERE ...` with a Spark final aggregate can return NULL. PRs #5420 
and #5421 are open. #5509 is the decimal AVG follow-on under 
`ObjectHashAggregateExec`.
   - **ABFS object store cache key drops the container** (#4993). Two Azure 
containers in one storage account share a store instance. PR #5053 has no 
reviewer yet. Cross-container data mixing is the worst outcome on this list.
   - **Four serdes give wrong answers for a nondeterministic child** (#5781). 
`size`, `arrays_zip`, `map_from_arrays` and `array_append` serialize the child 
twice. No PR and unassigned, but #5766 already fixed the same shape for 
`element_at`, so it should be a small change.
   - **Nested Parquet schema evolution silently NULLs or parses strings** 
(#5671). Conversions Spark rejects at top level are performed inside structs. 
PR #5681 is open.
   - **Wide date-to-timestamp casts wrap or panic** (#5456). PR #5457 is open.
   - **`array_min` / `array_max` signed-zero ties** (#5401). PR #5403 is open.
   - **`get_json_object` duplicate keys** (#4947). PR #4971 is open.
   - **Nested float ordering and rank** (#5507). Assigned, no PR yet.
   - **`CometHashAggregateExec` and `AQEPropagateEmptyRelation`** (#4412). 
Known to cause wrong results. On the 1.0.1 milestone, no PR.
   - **Map null-key rejection and `mapKeyDedupPolicy`** (#4680, #5857). PRs 
#5854 and #5846 are open.
   
   ### Tier 3: crashes and panics
   
   - #5013 negative-scale decimal panic. PR #5050 is open.
   - #5633 `timestamp_trunc` panics on DST transitions. Assigned.
   - #5743 `slice` crashes on non-nullable element arrays. Assigned.
   - #5239 `collect_set(struct)` aborts on spill. Root cause is 
apache/datafusion#24069; worth checking whether the DataFusion 55 bump already 
fixed it.
   - #5058 and #5059 `CalendarInterval` null literal and hashing. PRs #5133 and 
#5135 have been open since July.
   - #5605 native shuffle accepts duplicate struct field names then fails on 
import. Same family as #5783 and #5801.
   - #5023 sporadic SIGSEGV in the macOS scans CI job. Unassigned; a real 
segfault that should at least get a root cause before release.
   - #5719 `revertToSpark` drops the write node. Only affects the opt-in native 
write paths, so lower priority.
   
   ### Memory and performance regressions
   
   Only two items look like genuine regressions rather than long-standing gaps.
   
   - **Fair pool caps a task at `pool_size / num_consumers`** (#5212, finding 
1). Users hit OOM well below their configured off-heap size. PRs #5466, #5847 
and #5613 target it and all need review.
   - **DataFusion 55 TPC-H off-heap bump from 2g to 4g** (#5704). If this is 
real it is a user-visible memory regression that belongs in the changelog. If 
it is an accounting artifact of the `peak_mem_used` change, the bump should be 
reverted. Cheap to settle either way.
   
   Not proposed as release blockers: #4988 only matters when row-filter 
pushdown is opted in, #3873 and #4614 are long-standing, #5112 predates 1.0.0, 
and #5834 and #4949 are Spark 4.2 fallbacks. #5795 (`element_at` on maps slower 
than Spark) already has PR #5806.
   
   ### Milestone housekeeping
   
   1. I have added the critical issues that had no milestone to 1.1.0: #5824, 
#5783, #5553, #5456, #5419, #5418, #5401, #5295.
   2. I have moved the sort-aware Iceberg read work (#5323 and #5337 through 
#5343) plus #4871 and #3432 to the 1.2.0 milestone, since they are enhancements 
rather than fixes. Process items such as #4844, #4329, #4420 and #4419 are 
still on 1.1.0 and could also move if nobody objects.
   
   Happy to adjust the ordering if others see different priorities.
   


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