FrankChen021 commented on PR #5283: URL: https://github.com/apache/calcite/pull/5283#issuecomment-5806483084
> > It may be long, but it's clear. > > From my point of view, it is hard to answer "why do we need the change at all?" question. As a reviewer and Calcite maintainer, I don't want to dig into details until I understand why is that needed at all. > > Frank, I wonder if you could try this skill: https://github.com/Netcracker/qubership-ai-packages/tree/main/agent-packages/change-description-authoring > > See https://github.com/Netcracker/qubership-ai-packages/blob/main/agent-packages/change-description-authoring/.apm/skills/change-description-authoring/SKILL.md#pull-request-title > > From my point of view, it produces decent commit messages: #5278, #5230, #5213 That's a good suggestion. Let's talk about this case here, the "why" question I think is stated in the JIRA as: https://issues.apache.org/jira/browse/CALCITE-7809 > RexShuttle and SqlShuttle visit every child and return either the original child or a replacement. They currently copy operand collections before knowing whether any child changes, then discard those copies when the traversal is read-only. > > Make these operand collections copy-on-write. Allocate only when the first changed child is encountered, while preserving RexShuttle's immutable-result contract and SqlShuttle's explicit alwaysCopy behavior. > > The isolated Druid benchmark reduced allocation by 2.72% at 100,000 string literals and 1.55% at 1,000,000 string literals. Do you think the description is clear to answer the why ? If not, then I can improve the description, if yes, I think the problem here is that we have PR and JIRA separated, next time I can attach these description in PR, and to focus on why first, I can fold the details by default in the description. does it sound good to you? -- 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]
