I agree with Vladimir Ozerov, When I was a newcomer to Calcite, I just used its core part rather than framework, so I don't know some concepts. I don't know what JDBC adapter really is, such as CALCITE-4901, CALCITE-4740 I reported before. I don't know why the title needs to add 'JDBC adapter '. Another example is CALCITE-4683, I don't know how I could describe the symptom, because it's just an online bad case, I can't summarize it well before I found a more simple and representative case. Frankly, as newcomers will be more, I think the community needs patience and a friendly guide to make the summaries friendly to the most contributors. I appreciate Julian Hyde's work very much, He found the summary problem and tried to resolve it which gave me lots of trouble for a long time.
xiong duan <nobigo...@gmail.com> 于2022年1月13日周四 19:56写道： > Hi all, > I prefer to describe the symptoms the user sees. Because For developers > and users both can know what happens when Calcite has a wrong result or > unexpected exception. If the user encounters a problem, I guess he wants to > find the related symptoms summary in the git log, not the root cause. The > root cause requires the user have to do some debug or know related > implement in Calcite. I think this is not very friendly to describe > the root cause. When we release a new Calcite version with use the > description of the symptoms, I think the commit log is very friendly for > users and followers to know what has been fixed or improved. > But for CALCITE-4983, I think `Incorrect plan for query that has > GROUPING SETS and WHERE` is not enough, Because only MV rewriting have > wrong according to the description, not all queries. So I suggest `MV > rewriting generates the incorrect plan for query that has GROUPING SETS and > WHERE` > In addition, I admit this forced me to change my natural way of thinking > and not every issue I can describe correctly. But I think this is good for > me. > > Viliam Durina <vil...@hazelcast.com> 于2022年1月13日周四 16:11写道： > > > 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 > > >
