I used the term "slop" because I was seeing a load of machine-generated content coming at me at high velocity. The name "Frank Chen" was attached to each of the 5 Jiras and 4 PRs. I presumed that a human called Frank Chen was behind these, but I wasn't sure whether Frank was logging the cases or how to contact him.
Adding "-1 slop" to the PRs was the best way I could find, in that moment, to say STOP! Stop logging these PRs, and Calcite committers, please don't merge any until we figure out what's going on. I noticed an alignment problem in these PRs. These PRs are very focused on fixing one particular Druid issue. They claim to improve performance, but they cite a benchmark based on just one Druid query. As a Calcite committer, I need to ascertain whether a change is good for Calcite as a whole. Not whether they are good for Druid, and not whether they are good for one query and bad for others. For example, RexShuttle and SqlShuttle are widely used utilities but your PR [1] only measured the impact of the change in one scenario. Submitting a change to an open source project is a conversation between the contributor and the committer. The sheer volume and velocity of these changes made it very difficult to have that conversation. We're having it now. I briefly asked myself whether Calcite should be moving to an automated review process for Jiras and PRs. Then I realized how ridiculous that would be. By letting the machines talk to each other directly, we would be losing control without any guarantee of improved results. Frank, please do the following exercise. First, decide how important this change is. (If we ranked all changes to Calcite over the last year, from the most significant to the least significant, which percentile would it be in?) Second, imagine that you need to describe to an engineering manager in person. How much time would you allocate for the meeting, and what would you say? I would characterize this as a moderate performance improvement. Not trivial, worth having, but not a performance bug like the quadratic bug you fixed last week. I would take one minute to describe it to an engineering manager, in the following two sentences: > After CALCITE-7287 fixed a quadratic-time bug in the ARRAY > constructor, we did some further profiling and discovered a few > places in Calcite (types, expression operand lists, and expression > digests) where sometimes objects are allocated and immediately > discovered to be unnecessary copies. Short-term memory allocations > can sometimes be optimized away, but sometimes add to GC > pressure, so these patches are worth further investigation. In contrast, the text you submitted is as verbose as if you had discovered a cure for cancer. Human attention is the scarcest resource in 2026. You need to find ways to make better use of that resource. Julian [1] https://github.com/apache/calcite/pull/5283 On Wed, Sep 23, 2026 at 8:16 AM Frank Chen <[email protected]> wrote: > > Hi Vladimir, > > Thanks for the concrete suggestions. > > I agree that no PRs should be merged without sufficient human review. > At the same time, controlling what is merged is part of the project’s review > and merge process. > > As I decide to push this IN filter performance problem to be fully addressed > and touch the code in the Calcite, > my responsibility is to make each proposal independently reviewable, provide > sufficient tests, share the improvement with data and evidence, and address > technical feedback. > > Your concrete suggestions will be adopted and applied accordingly, > The existence of several open proposals should not itself create a risk of > unreviewed changes being merged. > > Thanks and regards. > > On Wed, Sep 23, 2026 at 1:40 PM Vladimir Sitnikov > <[email protected]> wrote: >> >> Frank, thanks for raising this. >> >> I'm with Julian regarding "I am afraid someone could merge the changes >> unreviewed". >> >> "10 PRs created per minute" does not bother me, and I do not think we >> should block contributions based on the rate alone. >> I can easily understand if Frank spent noticeable time trying various >> approaches to fix Druid/Calcite, and then created all findings at >> once. >> It would be the right approach for performance-related changes. >> >> I did check a few PRs. >> From my point of view the changes themselves look reasonable, except two >> things: >> 1) The tests are either missing or incomplete. I expect the PR to >> cover its changes (functional, performance). >> It is good you have a Druid-level benchmark, however, we need a test >> in Calcite. Otherwise the next innocent change in Calcite >> could introduce the same regression. >> 2) Commit messages seem to be empty. The commit message is empty, and >> the PR description is like a story to tell. >> This creates a burden on a committer that would have to come up with a >> commit message at the merge time. >> >> With LLMs, I suggest we invest more time into testing things. >> I do not suggest creating more tests for the sake of tests. >> I mean use old-school test design techniques like equivalence >> partitioning, boundary value analysis, and so on. >> >> Let us take an example: https://github.com/apache/calcite/pull/5281 >> >> The change adds a fast path for "same collation and charset". Nice catch. >> It properly uses "==" for collation, however, it does not clarify the >> reason .equals is wrong there. >> The PR adds no tests to distinguish collation== vs collation.equals in >> the newly added code. >> >> Even though the PR code is valid, it might easily become invalid >> should someone "use .equals instead of identity comparison". >> >> Calcite does have junit for functional tests, and jmh for performance tests. >> I expect contributions to update those. >> >> PS. Frank, I suggest you add the following to your >> ~/.../AGENTS.md,CLAUDE.md: "When preparing text for GitHub or JIRA (PR >> or issue description, comment, discussion), don't hard-wrap — keep >> each paragraph and list item on one line; GitHub, Gitlab, JIRA re-flow >> it" >> >> PPS. I'm not fond of creating JIRA for every small change. Frank's >> umbrella JIRA approach looked good enough to me. >> If the community wants "jira for every individual PR" I suggest we add >> that to AGENTS.md (that should better be discussed separately) >> >> Vladimir >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: [email protected] >> For additional commands, e-mail: [email protected] >> --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
