Обсуждение: typo fix

Hi,

It seems to me that EquivalenceClass, the struct/type name, has been
misspelled as 'EquivalenceClasses' a couple of times in the comment above
its definition.

Attached fixes that.

Thanks,
Amit

On Tue, Nov 20, 2018 at 02:00:39PM +0900, Amit Langote wrote:
> It seems to me that EquivalenceClass, the struct/type name, has been
> misspelled as 'EquivalenceClasses' a couple of times in the comment above
> its definition.

EquivalenceClasses stands for the plural of EquivalenceClass.  So
thinking like that...

> - * EquivalenceClasses
> + * EquivalenceClass

...  This is fine.

> - * We also use EquivalenceClasses as the base structure for PathKeys, letting
> + * We also use EquivalenceClass as the base structure for PathKeys, letting

...  But not that.
--
Michael

Thank you for looking.

On 2018/11/20 14:13, Michael Paquier wrote:
> On Tue, Nov 20, 2018 at 02:00:39PM +0900, Amit Langote wrote:
>> It seems to me that EquivalenceClass, the struct/type name, has been
>> misspelled as 'EquivalenceClasses' a couple of times in the comment above
>> its definition.
> 
> EquivalenceClasses stands for the plural of EquivalenceClass.  So
> thinking like that...
> 
>> - * EquivalenceClasses
>> + * EquivalenceClass
> 
> ...  This is fine.
> 
>> - * We also use EquivalenceClasses as the base structure for PathKeys, letting
>> + * We also use EquivalenceClass as the base structure for PathKeys, letting
> 
> ...  But not that.

Hmm, I classified this one as a typo too, because the sentence calls
EquivalenceClasses "the base structure for ...", whereas I think
'EquivalenceClass' is the base structure of PathKey.  That said, I don't
mind to using EquivalanceClasses when speaking of *instances* of
EquivalenceClass, of which I see many in the source code:

$ git grep EquivalenceClasses
postgres_fdw.c: *       Determine which EquivalenceClasses might be
postgres_fdw.c: /* Get the list of interesting EquivalenceClasses. */
copyfuncs.c:   /* EquivalenceClasses are never moved, so just shallow-copy
copyfuncs.c:   /* EquivalenceClasses are never copied, so shallow-copy the
copyfuncs.c:   /* EquivalenceClasses are never copied, so shallow-copy the
optimizer/README:EquivalenceClasses
optimizer/README:merging two existing EquivalenceClasses.  At the end of
<so on>

But maybe I'm being overly nit-picky. :)

Thanks,
Amit

Michael Paquier <michael@paquier.xyz> writes:
> On Tue, Nov 20, 2018 at 02:00:39PM +0900, Amit Langote wrote:
>> - * We also use EquivalenceClasses as the base structure for PathKeys, letting
>> + * We also use EquivalenceClass as the base structure for PathKeys, letting

> ...  But not that.

The reason that's not good is that it creates a singular-plural mismatch.
If you'd also changed "PathKeys" to "PathKey", it would still read OK,
though I don't think it's an improvement particularly.

(Hm ... though arguably, "structure" should be "structures" if we're
going to let it stand as plural.)

            regards, tom lane

On Tue, Nov 20, 2018 at 01:58:22AM -0500, Tom Lane wrote:
> The reason that's not good is that it creates a singular-plural mismatch.
> If you'd also changed "PathKeys" to "PathKey", it would still read OK,
> though I don't think it's an improvement particularly.
>
> (Hm ... though arguably, "structure" should be "structures" if we're
> going to let it stand as plural.)

Indeed, missed that.  This first sentence mentions "orderings" for those
PathKeys, which refers to multiple PathKeys, so actually Amit's patch
seems to be fine, no?
--
Michael

On 2018/11/20 15:58, Tom Lane wrote:
> Michael Paquier <michael@paquier.xyz> writes:
>> On Tue, Nov 20, 2018 at 02:00:39PM +0900, Amit Langote wrote:
>>> - * We also use EquivalenceClasses as the base structure for PathKeys, letting
>>> + * We also use EquivalenceClass as the base structure for PathKeys, letting
> 
>> ...  But not that.
> 
> The reason that's not good is that it creates a singular-plural mismatch.

Hmm, yeah.

> If you'd also changed "PathKeys" to "PathKey", it would still read OK,
> though I don't think it's an improvement particularly.

So,

- * We also use EquivalenceClasses as the base structure for PathKeys,
+ * We also use EquivalenceClass as the base structure for PathKey,

> (Hm ... though arguably, "structure" should be "structures" if we're
> going to let it stand as plural.)

vs.

- * We also use EquivalenceClasses as the base structure for PathKeys,
+ * We also use EquivalenceClasses as the base structures for PathKeys,

If I'm understanding this right, aren't different orderings represented by
different PathKey nodes considered equivalent if they share the base
EquivalenceClass?  If that's the case, I think the former reads better.

Thanks,
Amit