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 and describe_change.c. You should use git diff to generate a patch rather than maintaining 2 files yourself. Also I noticed that you include a "create_object.sql" file to illustrate the feature, which is not necessary. Instead, you should add them as a regression test cases in the existing regression test suite under "src/test/regress", so these will get run as tests to illustrate the feature. This patch changes the output of \d+ and it could potentially break other test cases 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 in my opinion. However, a sub-partition can also contain another sub-partition, which contains another sub-partition and so on. 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 that it starts to become annoying. Are you planning to set a limit on how many levels of sub-partitions to print or just let it print as many as it needs? thank you Cary Huang ----------------------- Highgo Software Canada www.highgo.ca
