A small tweak to some comments for PartitionKeyData

Started by David Rowleyalmost 8 years ago8 messageshackers
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

never appliedsuccessCI history

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t39564
psql -h localhost -U postgres

Built from patchset v1 (message #1), July 28, 2026 at 01:51 PM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t39564_1 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t39564_1 && git checkout t39564_1

Patchset v1 (message #1) is on t39564_1

Jump to latest
#1David Rowley
dgrowleyml@gmail.com

While doing a bit of work on a partitioning patch I noticed that it's
not really that obvious that there's meant to be exactly 1 item in the
partexprs List for each zero-valued partattrs element. Some incorrect
code using these fields was the cause of CVE-2018-1052, so I think
it's worthwhile to mention how they should be used in the comments.

Patch attached.

--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Attachments:

t39564_1
reword_comments_in_partitionkeydata_struct.patchapplication/octet-stream; name=reword_comments_in_partitionkeydata_struct.patchDownload+2-2
#2Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: David Rowley (#1)
Re: A small tweak to some comments for PartitionKeyData

On 2018/10/25 12:43, David Rowley wrote:

While doing a bit of work on a partitioning patch I noticed that it's
not really that obvious that there's meant to be exactly 1 item in the
partexprs List for each zero-valued partattrs element. Some incorrect
code using these fields was the cause of CVE-2018-1052, so I think
it's worthwhile to mention how they should be used in the comments.

Patch attached.

Good idea.

+ * key, one for each zero-valued partattrs */

How about: for each zero-valued member of partattrs?

Regards,
Amit

#3David Rowley
dgrowleyml@gmail.com
In reply to: Amit Langote (#2)
Re: A small tweak to some comments for PartitionKeyData

On 25 October 2018 at 16:46, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:

+ * key, one for each zero-valued partattrs */

How about: for each zero-valued member of partattrs?

Aren't arrays made up of elements? I did have "element" on the end,
but I didn't think it was worth having the extra line caused by the 80
line length limit, so I deleted it.

Seems "element" is mentioned 73 times in [1]https://en.wikipedia.org/wiki/Array_data_structure, but "member" does not
get a mention.

[1]: https://en.wikipedia.org/wiki/Array_data_structure

--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

#4Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: David Rowley (#3)
Re: A small tweak to some comments for PartitionKeyData

On 2018/10/25 12:54, David Rowley wrote:

On 25 October 2018 at 16:46, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:

+ * key, one for each zero-valued partattrs */

How about: for each zero-valued member of partattrs?

Aren't arrays made up of elements? I did have "element" on the end,
but I didn't think it was worth having the extra line caused by the 80
line length limit, so I deleted it.

Seems "element" is mentioned 73 times in [1], but "member" does not
get a mention.

[1] https://en.wikipedia.org/wiki/Array_data_structure

I thought they can be used interchangeably, but perhaps not.

Anyway, it's just that "each zero-valued partattrs" sounds a bit odd to
me, especially because it you seem to be referring to the previous array
field 'partattrs'. It would've sounded better with "each zero-valued
partition attribute", for example, but again that's too long for the line too.

Thanks,
Amit

#5David Rowley
dgrowleyml@gmail.com
In reply to: Amit Langote (#4)
Re: A small tweak to some comments for PartitionKeyData

On 25 October 2018 at 17:05, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:

On 2018/10/25 12:54, David Rowley wrote:

On 25 October 2018 at 16:46, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:

+ * key, one for each zero-valued partattrs */

How about: for each zero-valued member of partattrs?

Aren't arrays made up of elements? I did have "element" on the end,
but I didn't think it was worth having the extra line caused by the 80
line length limit, so I deleted it.

Seems "element" is mentioned 73 times in [1], but "member" does not
get a mention.

[1] https://en.wikipedia.org/wiki/Array_data_structure

I thought they can be used interchangeably, but perhaps not.

A struct or a class has members.

--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

#6Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: David Rowley (#5)
Re: A small tweak to some comments for PartitionKeyData

On 2018/10/25 13:13, David Rowley wrote:

On 25 October 2018 at 17:05, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:

On 2018/10/25 12:54, David Rowley wrote:

On 25 October 2018 at 16:46, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:

+ * key, one for each zero-valued partattrs */

How about: for each zero-valued member of partattrs?

Aren't arrays made up of elements? I did have "element" on the end,
but I didn't think it was worth having the extra line caused by the 80
line length limit, so I deleted it.

Seems "element" is mentioned 73 times in [1], but "member" does not
get a mention.

[1] https://en.wikipedia.org/wiki/Array_data_structure

I thought they can be used interchangeably, but perhaps not.

A struct or a class has members.

You're right.

Thanks,
Amit

#7Peter Eisentraut
peter_e@gmx.net
In reply to: David Rowley (#1)
Re: A small tweak to some comments for PartitionKeyData

On 25/10/2018 05:43, David Rowley wrote:

While doing a bit of work on a partitioning patch I noticed that it's
not really that obvious that there's meant to be exactly 1 item in the
partexprs List for each zero-valued partattrs element. Some incorrect
code using these fields was the cause of CVE-2018-1052, so I think
it's worthwhile to mention how they should be used in the comments.

Patch attached.

committed

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#8David Rowley
dgrowleyml@gmail.com
In reply to: Peter Eisentraut (#7)
Re: A small tweak to some comments for PartitionKeyData

On Fri, 16 Nov 2018 at 11:23, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

committed

Thank you.

--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services