The following review has been posted through the commitfest application:
make installcheck-world: tested, failed
Implements feature: not tested
Spec compliant: not tested
Documentation: not tested
Hello
Thank you for the patch and the effort to enhance \d+ 's output on partitioned tables that contain sub-partitions.
However,the patch does not apply and I notice that this patch is generated as a differ file from 2 files, describe.c
anddescribe_change.c. You should use git diff to generate a patch rather than maintaining 2 files yourself. Also I
noticedthat you include a "create_object.sql" file to illustrate the feature, which is not necessary. Instead, you
shouldadd them as a regression test cases in the existing regression test suite under "src/test/regress", so these will
getrun as tests to illustrate the feature. This patch changes the output of \d+ and it could potentially break other
testcases so you should fix them in the patch in addition to providing the feature
Now, regarding the feature, I see that you intent to print the sub partitions' partitions in the output, which is okay
inmy opinion. However, a sub-partition can also contain another sub-partition, which contains another sub-partition and
soon. So it is possible that sub-partitions can span very, very deep. Your example assumes only 1 level of
sub-partitions.Are you going to print all of them out in \d+? If so, it would definitely cluster the output so much
thatit starts to become annoying. Are you planning to set a limit on how many levels of sub-partitions to print or just
letit print as many as it needs?
thank you
Cary Huang
-----------------------
Highgo Software Canada
www.highgo.ca