Hi all, I also prefer root cause over end effect, if it's known. The end effect should definitely be mentioned in the body. I even tend to edit summaries when I start with the end effect and find out the root cause later.
I also prefer a more impersonal tone. In our example I prefer this: > SubstitutionVisitor.unifyAggregates() doesn't handle conditions if Aggregate > has grouping sets Viliam On Wed, 12 Jan 2022 at 20:38, Vladimir Ozerov <ppoze...@gmail.com> wrote: > Hi Julian, > > In my opinion, both ways work well. People tend to think differently. Some > prefer symptoms, others - the root cause. I personally prefer the latter > for the following reason. If I face a problem, I first try to debug it on > my own. The result of the analysis is usually some questionable behavior in > a specific part of the code. Once you find the problematic place, you can > run a search in JIRA or Git log (class name, feature name, etc) and check > whether somebody else faced a similar issue. The description "Incorrect > plan ..." is less likely to help me than more concrete "In > SubstitutionVisitor ...". Especially, given that a single root cause may > manifest in several ways. But I would like to stress out - it is a matter > of personal habits and previous experience, not something that I > expect others to follow. > > In the past, I worked on the Apache Ignite project. We had a number of > contribution rules, such as "put a comma here", "set the proper component > there", "write the comment in that way", etc. I was the one who actively > enforced this for a may years, because it gave the feeling that everything > is "put in order". Eventually, I came to the conclusion that this does more > harm than good, because I regularly observed confusion and dissatisfaction > of the new contributors (and Apache Ignite community is far less diverse > and active than in Apache Calcite), as they were forced to change their > natural way of thinking or past habits to engage with the community. > > Regards, > Vladimir. > > ср, 12 янв. 2022 г. в 21:42, Julian Hyde <jhyde.apa...@gmail.com>: > > > Hi all, > > > > Can we discuss how we write summaries for Jira cases? In my opinion it’s > > really important, because summaries become commit messages, and commit > > messages become release notes, which is how most people figure out what > is > > in Calcite. I spend a lot of my time working with people to write good > > summaries. > > > > I’d like some feedback on whether this approach is useful. And to try to > > teach people how to do it for themselves. > > > > Consider this case https://issues.apache.org/jira/browse/CALCITE-4983 < > > https://issues.apache.org/jira/browse/CALCITE-4983>. (I chose a still > > current case because it doesn’t yet have an ‘answer’.) > > > > The current summary is > > > > > In SubstitutionVisitor's unifyAggregates, if Aggregate has > > > grouping sets, we need to handle the condition needs to pull up. > > > > It describes the cause but it doesn’t describe the problem (or the > > symptoms the user sees). > > > > If you take your car into your mechanic the cause is ‘Leaky gasket > results > > in oil dripping onto hot manifold’ but the problem is ‘Smoke comes from > > hood when engine gets hot’. Do you agree that the second description is > > much more useful? > > > > In this case, the author came up with an example: > > > > > Here is an example: > > > > > > sql: select empid, deptno from emps group by grouping sets ((empid, > > deptno),(empid)) > > > mv: select empid, count(distinct deptno) from emps where empid>100 > > > group by grouping sets ((empid, deptno), (empid)) > > > > > > the result plan is: > > > > > > LogicalCalc(expr#0..2=[{inputs}], deptno=[$t1], EXPR$1=[$t2]) > > > LogicalAggregate(group=[{0, 1}], groups=[[{0, 1}, {0}]], > > EXPR$1=[COUNT(DISTINCT $1)]) > > > EnumerableTableScan(table=[[hr, MV0]]) > > > > > > We can see that this plan doesn't handle the condition empid>100 > > > > I think it’s a great example. I especially like the last line, where the > > author pointed out what was wrong. I suggest the following summary: > > > > > Incorrect plan for query that has GROUPING SETS and WHERE > > > > Do you think the summary is more useful? Can it be improved? > > > > Julian > > > > > > > > > -- This message contains confidential information and is intended only for the individuals named. If you are not the named addressee you should not disseminate, distribute or copy this e-mail. Please notify the sender immediately by e-mail if you have received this e-mail by mistake and delete this e-mail from your system. E-mail transmission cannot be guaranteed to be secure or error-free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete, or contain viruses. The sender therefore does not accept liability for any errors or omissions in the contents of this message, which arise as a result of e-mail transmission. If verification is required, please request a hard-copy version. -Hazelcast
