zanmato1984 commented on PR #50423: URL: https://github.com/apache/arrow/pull/50423#issuecomment-4996338927
中文逐行说明，帮助理解这次改动；这不是要求修改的 review，只是对 diff 的阅读注释。 <details> <summary><code>cpp/src/arrow/acero/hash_aggregate_test.cc</code></summary> - `+4822`: 新增 `PivotNonMonotonicGroupId` 的 GroupBy 测试，覆盖 GH-48679 的核心场景。 - `+4823`: 说明这个测试是针对 GH-48679 写死的数据，因为 `FastGrouperImpl` 可能生成非单调的 group id。 - `+4824`: 继续说明 `pivot_wider` 必须能处理这种非单调 group id。 - `+4825`: 空行，用来分隔问题背景和后面的测试数据说明。 - `+4826`: 提醒这些 key 是为了触发当前实现细节而挑选的。 - `+4827`: 继续说明触发条件依赖 `FastGrouperImpl` 的内部实现。 - `+4828`: 说明如果 grouper 内部变化，这个测试可能不再覆盖原问题。 - `+4829`: 设置 pivot key 的类型为 UTF-8 字符串。 - `+4830`: 设置 pivot value 的类型为 `float32`。 - `+4831`: 开始构造输入表的 JSON batch 列表。 - `+4832`: 开始第一个输入 batch。 - `+4833`: 输入一行：group key 为 `1`，pivot key 为 `k`，值为 `10.5`。 - `+4834`: 输入一行：group key 为 `2`，pivot key 为 `l`，值为 `11.5`。 - `+4835`: 结束第一个输入 batch。 - `+4836`: 开始第二个输入 batch。 - `+4837`: 输入一行：group key 为 `2`，pivot key 为 `m`，值为 `12.5`。 - `+4838`: 结束第二个输入 batch。 - `+4839`: 开始第三个输入 batch。 - `+4840`: 输入一行：group key 为 `3`，pivot key 为 `k`，值为 `13.5`。 - `+4841`: 输入一行：group key 为 `1`，pivot key 为 `n`，值为 `14.5`。 - `+4842`: 输入一行：group key 为 `1`，pivot key 为 `o`，值为 `15.5`。 - `+4843`: 结束第三个 batch，并结束 `table_json` 输入集合。 - `+4844`: 开始声明期望输出 JSON。 - `+4845`: 期望 group `1` 的 pivot 结果包含 `k/n/o` 三个字段和值。 - `+4846`: 期望 group `2` 的 pivot 结果包含 `l/m` 两个字段和值。 - `+4847`: 期望 group `3` 的 pivot 结果只包含 `k`。 - `+4848`: 结束期望输出 JSON。 - `+4849`: 开始遍历两种 unexpected key 行为。 - `+4850`: 分别测试 `kIgnore` 和 `kRaise`，确保这两种模式下修复都有效。 - `+4851`: 创建 `PivotWiderOptions`，指定合法 key 顺序为 `k/l/m/n/o`。 - `+4852`: 把当前循环中的 unexpected key 行为传给 options。 - `+4853`: 调用 `TestPivot`，用上面的输入和期望结果验证 pivot 输出。 - `+4854`: 结束 unexpected key 行为循环。 - `+4855`: 结束第一个新增测试。 - `+4856`: 空行，用来分隔两个测试用例。 - `+4857`: 新增第二个 GroupBy 测试，覆盖 pivot key 是 scalar 的情况。 - `+4858`: 说明这个测试和前一个类似，但 pivot key 输入形态变成 scalar。 - `+4859`: 创建 `BatchesWithSchema`，后面手动填 batch 和 schema。 - `+4860`: 定义三列类型：group key 为 `int32`，pivot key 为 `utf8`，value 为 `float32`。 - `+4861`: 定义三列形态：group key 是数组，pivot key 是 scalar，value 是数组。 - `+4862`: 开始填充输入 batches。 - `+4863`: 开始第一个 `ExecBatchFromJSON`。 - `+4864`: 第一行 group `1` 使用 scalar pivot key `m`，值为 `10.5`。 - `+4865`: 第二行 group `2` 也使用 scalar pivot key `m`，值为 `11.5`。 - `+4866`: 结束第一个 batch。 - `+4867`: 开始第二个 batch。 - `+4868`: group `2` 使用 scalar pivot key `o`，值为 `12.5`。 - `+4869`: 结束第二个 batch。 - `+4870`: 开始第三个 batch。 - `+4871`: group `3` 使用 scalar pivot key `n`，值为 `13.5`。 - `+4872`: group `1` 使用 scalar pivot key `n`，值为 `14.5`。 - `+4873`: 结束第三个 batch。 - `+4874`: 结束输入 batch 列表。 - `+4875`: 开始声明输入 schema，包含 `group_key` 和 `pivot_key`。 - `+4876`: 继续声明 schema 的 `pivot_value` 字段并闭合 schema。 - `+4877`: 开始构造期望输出 `Datum`。 - `+4878`: 期望输出的顶层 struct 先包含 `group_key` 字段。 - `+4879`: 期望输出的 `pivoted` 字段是嵌套 struct，先声明 `m` 和 `n`。 - `+4880`: 继续声明 `o` 字段并闭合嵌套 struct 类型。 - `+4881`: 开始写期望输出的 JSON 数据。 - `+4882`: 期望 group `1` 的 pivot 结果有 `m=10.5`、`n=14.5`。 - `+4883`: 期望 group `2` 的 pivot 结果有 `m=11.5`、`o=12.5`。 - `+4884`: 期望 group `3` 的 pivot 结果有 `n=13.5`。 - `+4885`: 结束期望输出 JSON。 - `+4886`: 开始创建共享的 `PivotWiderOptions`。 - `+4887`: 指定 pivot 输出字段顺序为 `m/n/o`。 - `+4888`: 定义 `hash_pivot_wider` 聚合。 - `+4889`: 指定聚合输入字段为 `pivot_key/pivot_value`，输出字段名为 `pivoted`。 - `+4890`: 同时测试串行执行和并行合并执行。 - `+4891`: 给当前循环加 trace，方便失败时看出是串行还是并行路径。 - `+4892`: 执行 GroupBy，并捕获实际输出。 - `+4893`: 传入 group key、聚合定义和是否使用线程的参数。 - `+4894`: 先做通用输出校验。 - `+4895`: 再把实际输出和期望输出做近似比较。 - `+4896`: 结束串行/并行循环。 - `+4897`: 结束第二个新增测试。 - `+4898`: 空行，用来和后面的已有测试分隔。 </details> <details> <summary><code>cpp/src/arrow/compute/kernels/aggregate_test.cc</code></summary> - `+4765`: 新增 pivot kernel 层的非单调 group id 测试。 - `+4766`: 说明测试目标仍然是 GH-48679 里 `FastGrouperImpl` 生成非单调 group id 的情况。 - `+4767`: 继续说明 `pivot_wider` 需要处理这个情况。 - `+4768`: 空行，用来分隔背景说明。 - `+4769`: 提醒触发该问题的 key 依赖当前实现细节。 - `+4770`: 继续说明依赖 `FastGrouperImpl` 的内部行为。 - `+4771`: 说明内部实现变化后，这个测试可能不再触发同样的问题。 - `+4772`: 指向 `hash_aggregate_test.cc` 中类似的高层测试。 - `+4773`: 空行，用来分隔注释和测试主体。 - `+4774`: 设置 key 类型为 UTF-8 字符串。 - `+4775`: 设置 value 类型为 `int16`。 - `+4776`: 构造 key 数组 `m/n/o`。 - `+4777`: 构造对应 values 数组 `10/11/12`。 - `+4778`: 开始构造期望输出 scalar。 - `+4779`: 期望输出是包含 `m/n/o` 三个字段的 struct。 - `+4780`: 期望输出的值按 `m/n/o` 顺序是 `10/11/12`。 - `+4781`: 调用 `AssertPivot` 验证数组 key 路径下输出顺序正确。 - `+4782`: 结束第一个 kernel 层测试。 - `+4783`: 空行，用来分隔两个 kernel 测试。 - `+4784`: 新增 scalar key 版本的 kernel 层测试。 - `+4785`: 说明这个测试和前一个类似，但 key 是 scalar。 - `+4786`: 说明即使数据中只有一个 key，`key_names` 里有多个 key 也可能触发问题。 - `+4787`: 继续说明 scalar key 仍然需要覆盖。 - `+4788`: 设置 key 类型为 UTF-8 字符串。 - `+4789`: 设置 value 类型为 `int16`。 - `+4790`: 空行，用来分隔类型定义和数据构造。 - `+4791`: 构造 scalar key `o`。 - `+4792`: 构造 values 数组，其中只有中间位置有 `11`。 - `+4793`: 开始构造期望输出 scalar。 - `+4794`: 期望输出 struct 的字段顺序仍然是 `m/n/o`。 - `+4795`: 期望值为 `null/null/11`，也就是输入 key `o` 必须落到第三个字段。 - `+4796`: 调用 `AssertPivot` 验证 scalar key 路径下映射正确。 - `+4797`: 结束 scalar key 测试。 - `+4798`: 空行，用来和后面的已有测试分隔。 </details> <details> <summary><code>cpp/src/arrow/compute/kernels/pivot_internal.cc</code></summary> - `+26`: 引入 array 工具函数，后面需要用 `InversePermutation`。 - `+27`: 引入 vector compute API，后面需要用 `Take`。 - `+32`: 引入 `arrow/result.h`，支持新增逻辑里使用的 Arrow result/assignment 相关类型。 - `+51`: 保存 `ExecContext*`，因为构造函数之后的 `MapKeys` 里还要调用 compute kernel。 - `-64`: 删除旧注释；旧逻辑只说明把 key 放进 grouper。 - `+68`: 新注释说明现在不只是 populate，还要拿到 key index 到 group id 的映射。 - `-66`: 删除旧的 `Populate` 调用；它只返回状态，拿不到 group id 映射。 - `+70`: 改用 `Consume`，同时填充 grouper 并取得 `key_indices_to_group_ids`。 - `+82`: 开始说明 GH-48679 的根因：fast grouper 可能产生非单调 group id。 - `+83`: 举例说明 group id 可能是 `[0,1,2,4,3]`，而不是自然顺序。 - `+84`: 说明修复思路是反过来构造 group id 到原始 key index 的映射。 - `+85`: 取出 `key_indices_to_group_ids` 的底层 array 数据，供后面求逆排列。 - `+86`: 说明 `InversePermutation` 不接受 unsigned integer，所以需要临时改成 signed 类型。 - `+87`: 断言当前 mapping 的类型确实是 `UINT32`。 - `+88`: 临时把类型标记改成 `int32`，让 `InversePermutation` 能处理。 - `+89`: 开始计算 `group_ids_to_key_indices_` 这个逆映射。 - `+90`: 把临时改成 signed 的 mapping 传给 `InversePermutation`。 - `+91`: 使用默认选项和当前执行上下文完成逆排列计算。 - `+92`: 计算完成后把结果类型改回 `uint32`，匹配后续索引用法。 - `+93`: 断言逆映射长度等于 grouper 的 group 数量。 - `+94`: 断言逆映射没有 null，确保每个 group id 都能映射回 key index。 - `-137`: 删除旧返回值；旧逻辑直接返回 grouper 产生的 group id，会把非单调 group id 当成 key index 用。 - `+154`: 新注释说明返回前要把 group id 映射回原始 `key_names` 的 index。 - `+155`: 说明可以有另一种实现：不在这里 materialize `Take` 的结果。 - `+156`: 继续说明也可以把 `group_ids_to_key_indices_` 暴露给调用方。 - `+157`: 说明由调用方按需应用映射可以少一次内存分配。 - `+158`: 使用 `Take` 按 `result` 里的 group id 去查 `group_ids_to_key_indices_`。 - `+159`: 使用 `NoBoundsCheck` 和保存的 `ctx_` 执行 `Take`。 - `+160`: 断言 `Take` 的结果是 array。 - `+161`: 断言映射后的结果类型是 `UINT32`。 - `+162`: 返回映射后的 array；这时值已经是原始 key index，而不是原始 group id。 - `+167`: 新增成员 `ctx_`，保存执行上下文供 `MapKeys` 中的 `Take` 使用。 - `+169`: 新增成员 `group_ids_to_key_indices_`，保存 group id 到 key index 的逆映射。 </details> -- 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]
