Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation:tested, passed All three patches applied nivcely. Code fits standart, comments are relevant. The new status of this patch is: Ready for Committer
Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: not tested Spec compliant: not tested Documentation:not tested Sorry, tests passed when applying all patches. I planned to check without optimisation first. The new status of this patch is: Needs review
Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
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
Hi,
I have failing tap test after patches apply:
ok 201 + partition_merge 2635 ms
not ok 202 + partition_split 5719 ms
@@ -805,6 +805,7 @@
(PARTITION salesmans2_3 FOR VALUES FROM (2) TO (3),
PARTITION salesmans3_4 FOR VALUES FROM (3) TO (4),
PARTITION salesmans4_5 FOR VALUES FROM (4) TO (5));
+ERROR: no owned sequence found
INSERT INTO salesmans (salesman_name) VALUES ('May');
INSERT INTO salesmans (salesman_name) VALUES ('Ford');
SELECT * FROM salesmans1_2;
@@ -814,23 +815,17 @@
(1 row)
SELECT * FROM salesmans2_3;
- salesman_id | salesman_name
--+---
- 2 | Ivanov
-(1 row)
-
+ERROR: relation "salesmans2_3" does not exist
+LINE 1: SELECT * FROM salesmans2_3;
The new status of this patch is: Waiting on Author
Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation:tested, passed It is just a rebase I check with make and meson run manual split and merge on list and range partition Doc fits The new status of this patch is: Ready for Committer
Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
The following review has been posted through the commitfest application: make installcheck-world: not tested Implements feature: not tested Spec compliant: not tested Documentation:not tested Only documentation patch applied on 4e465aac36ce9a9533c68dbdc83e67579880e628 Checked with v18 The new status of this patch is: Waiting on Author
Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation:tested, failed Hi, Just a minor warning with documentation patch git apply ../v16-0003-Documentation-for-ALTER-TABLE-SPLIT-PARTITION-ME.patch ../v16-0003-Documentation-for-ALTER-TABLE-SPLIT-PARTITION-ME.patch:54: trailing whitespace. One of the new partitions partition_name1, warning: 1 ligne a ajouté des erreurs d'espace. (perhaps due to my Ubuntu 22.04.2 french install) Everything else is ok. Thanks a lot for your work Stéphane
Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Hi, Patch v15-0001-ALTER-TABLE-MERGE-PARTITIONS-command.patch Apply nicely. One warning on meson compile (configure -Dssl=openssl -Dldap=enabled -Dauto_features=enabled -DPG_TEST_EXTRA='ssl,ldap,kerberos' -Dbsd_auth=disabled -Dbonjour=disabled -Dpam=disabled -Dpltcl=disabled -Dsystemd=disabled -Dzstd=disabled -Db_coverage=true) ../../src/pgmergesplit/src/test/modules/test_ddl_deparse/test_ddl_deparse.c: In function ‘get_altertable_subcmdinfo’: ../../src/pgmergesplit/src/test/modules/test_ddl_deparse/test_ddl_deparse.c:112:17: warning: enumeration value ‘AT_MergePartitions’ not handled in switch [-Wswitch] 112 | switch (subcmd->subtype) | ^~ Should be the same with 0002... meson test perfect, patch coverage is very good. Patch v15-0002-ALTER-TABLE-SPLIT-PARTITION-command.patch Doesn't apply on 326a33a289c7ba2dbf45f17e610b7be98dc11f67 Patch v15-0003-Documentation-for-ALTER-TABLE-SPLIT-PARTITION-ME.patch Apply with one warning 1 line add space error (translate from french "warning: 1 ligne a ajouté des erreurs d'espace"). v15-0003-Documentation-for-ALTER-TABLE-SPLIT-PARTITION-ME.patch:54: trailing whitespace. One of the new partitions partition_name1, Comment are ok for me. A non native english speaker. Perhaps you could add some remarks in ddl.html and alter-ddl.html Stéphane The new status of this patch is: Waiting on Author
Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation:tested, failed Feature is clearly missing with partition handling in PostgreSQL, so, this patch is very welcome (as are futur steps) Code presents good, comments are explicit Patch v14 apply nicely on 4f46f870fa56fa73d6678273f1bd059fdd93d5e6 Compilation ok with meson compile LCOV after meson test shows good new code coverage. Documentation is missing in v14.
Re: [Proposal] Allow pg_dump to include all child tables with the root table
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation:tested, passed V5 is ok (aside for LLVM 14 deprecated warnings, but that's another problem) with meson compile and meson test on Ubuntu 20.04.2. Code fits well and looks standart, --help explain what it does clearly, and documentation is ok (but as a Français, I'm not an expert in English).
