On 04.09.20 08:52, Peter Eisentraut wrote:
>
> For the remaining patch I have a couple of concerns:
This patch should not be changing the formatting choices for these queries, just the addition of a JOIN clause and modification of the WHERE clause. Specifically, SELECT is left-aligned while all subsequent clauses indent under it. Forced alignment by adding extra spaces isn't done here either. I have not altered those in the attached.
Did some word-smithing on the first paragraph. The part about the cross-join was hurt by "in some way" and "may be" is not needed.
Pointing out that values from both tables doesn't seem like an improvement when the second item covers that and it is more specific in noting that the city name that is joined on appears twice - once from each table.
ON expression is more precise and the reader should be ok with the term.
Removal of the exercise is good. Not the time to discuss cross join anyway. Given that "ON true" works the cross join form isn't even required.
In the FROM clause form I would not add table prefixes to the column names. They are not part of the form changing. If discussion about table prefixing is desired it should be done explicitly and by itself. They are used later on, I didn't check to see whether that was covered or might be confusing.
I suggested a wording for why to use join syntax that doesn't involve legacy and points out its merit compared to sticking a join expression into the where clause.
The original patch missed having the syntax for the first left outer join conform to the multi-line query writing standard you introduced. I did not change.
The "AND" ON clause should just go with (not changed):
ON (w1.temp_lo < w2.temp_lo
AND w1.temp_hi > w2.temp_high);
Attaching my suggestions made on top of the attached original 0002-query.patch
David J.
