typo fix

Started by Amit Langoteabout 7 years ago6 messages

Langote_Amit_f8@lab.ntt.co.jp

about 7 years ago

1 attachment(s)

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

Attachments:

EquivalenceClass-typo.patchtext/plain; charset=UTF-8; name=EquivalenceClass-typo.patchDownload

diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h
index 6fd24203dd..a695012c7a 100644
--- a/src/include/nodes/relation.h
+++ b/src/include/nodes/relation.h
@@ -863,7 +863,7 @@ typedef struct StatisticExtInfo
 } StatisticExtInfo;
 
 /*
- * EquivalenceClasses
+ * EquivalenceClass
  *
  * Whenever we can determine that a mergejoinable equality clause A = B is
  * not delayed by any outer join, we create an EquivalenceClass containing
@@ -880,7 +880,7 @@ typedef struct StatisticExtInfo
  * that all or none of the input datatypes are collatable, so that a single
  * collation value is sufficient.)
  *
- * We also use EquivalenceClasses as the base structure for PathKeys, letting
+ * We also use EquivalenceClass as the base structure for PathKeys, letting
  * us represent knowledge about different sort orderings being equivalent.
  * Since every PathKey must reference an EquivalenceClass, we will end up
  * with single-member EquivalenceClasses whenever a sort key expression has

Michael Paquier

michael@paquier.xyz

about 7 years ago

In reply to: Amit Langote (#1)

Re: typo fix

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

Amit Langote

Langote_Amit_f8@lab.ntt.co.jp

about 7 years ago

In reply to: Michael Paquier (#2)

Re: typo fix

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

Tom Lane

tgl@sss.pgh.pa.us

about 7 years ago

In reply to: Michael Paquier (#2)

Re: typo fix

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

Michael Paquier

michael@paquier.xyz

about 7 years ago

In reply to: Tom Lane (#4)

Re: typo fix

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

Amit Langote

Langote_Amit_f8@lab.ntt.co.jp

about 7 years ago

In reply to: Tom Lane (#4)

1 attachment(s)

Re: typo fix

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

Attachments:

EquivalenceClass-typo-v2.patchtext/plain; charset=UTF-8; name=EquivalenceClass-typo-v2.patchDownload

diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h
index 6fd24203dd..ab71ba9674 100644
--- a/src/include/nodes/relation.h
+++ b/src/include/nodes/relation.h
@@ -863,7 +863,7 @@ typedef struct StatisticExtInfo
 } StatisticExtInfo;
 
 /*
- * EquivalenceClasses
+ * EquivalenceClass
  *
  * Whenever we can determine that a mergejoinable equality clause A = B is
  * not delayed by any outer join, we create an EquivalenceClass containing
@@ -880,7 +880,7 @@ typedef struct StatisticExtInfo
  * that all or none of the input datatypes are collatable, so that a single
  * collation value is sufficient.)
  *
- * We also use EquivalenceClasses as the base structure for PathKeys, letting
+ * We also use EquivalenceClass as the base structure for PathKey, letting
  * us represent knowledge about different sort orderings being equivalent.
  * Since every PathKey must reference an EquivalenceClass, we will end up
  * with single-member EquivalenceClasses whenever a sort key expression has