Re: fix for BUG #3720: wrong results at using ltree

From: Benjie Gillam <benjie(at)jemjie(dot)com>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Cc: Filip Rembiałkowski <filip(dot)rembialkowski(at)gmail(dot)com>
Subject: Re: fix for BUG #3720: wrong results at using ltree
Date: 2019-11-19 10:27:55
Message-ID: 157415927562.29912.5497068377906958040.pgcf@coridan.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: not tested
Documentation: tested, failed

This is my first PostgreSQL commitfest and review, guidance welcome.

This patch is straightforward, it applies cleanly, and it includes tests (I've also tested the feature manually).

The (existing) documentation states "The length of a label path must be less than 65kB," I believe that the 65kB mentioned here should instead be 64kB - perhaps the patch could be updated with this single-character fix? At first I thought the 65kB limit would be applied to the label path string (e.g. 'Top.Countries.Europe.Russia' would be 27 bytes), but it seems the limit applies to the number of labels in the path - perhaps `kB` is not the right measurement here and it should explicitly state 65536?

It is not stated in the documentation what should happen if the label path length is greater than 65535, so raising an error makes sense (but may be a breaking change).

The new status of this patch is: Waiting on Author

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro Horiguchi 2019-11-19 10:48:10 Re: Protect syscache from bloating with negative cache entries
Previous Message Rushabh Lathia 2019-11-19 10:00:17 Re: backup manifests