Dear Vignesh,
Thanks for reviewing!
> 1) Comment inconsistency across create and alter subscription, better
> to keep it same:
A comment for CREATE SUBSCRIPTION became same as ALTER's one.
> 2) ereport inconsistency, braces around errcode is present in few
> places and not present in few places, it is better to keep it
> consistent by removing it:
Removed.
> 3) this include is not required, I could compile without it
Removed. Timestamp datatype is not used in subscriptioncmds.c.
> 4)
> 4.a)
> Should this be changed:
> /* Adds portion time (in ms) to the previous result. */
> to
> /* Adds portion time (in ms) to the previous result */
Changed.
> 4.b)
> Should this be changed:
> /* Detect whether the value of interval can cause an overflow. */
> to
> /* Detect whether the value of interval can cause an overflow */
Changed.
> 5) Can this "ALTER SUBSCRIPTION regress_testsub SET (min_apply_delay =
> '1d')" be combined along with "-- success -- 123 ms", that way few
> statements could be reduced
> 6) Can we do the interval testing along with alter subscription and
> combined with "-- success -- 123 ms" test, that way few statements
> could be reduced
To keep the code coverage, either of them must remain. 5) was cleanly removed and
6) was combined to you suggested. In addition, comments were updated to clarify
the testcase.
Please have a look at the latest patch v14 in [1].
[1]:
https://www.postgresql.org/message-id/TYAPR01MB5866D0527B1B8D589F1C2551F5FC9%40TYAPR01MB5866.jpnprd01.prod.outlook.com
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
