Dear Horiguchi-san,

Thanks for making the patch!

> > 4. I think a test case to hit the error in the check hook in
> > 003_logical_slots.pl will help a lot here - not only covers the code
> > but also helps demonstrate how one can reach the error.
> 
> Yeah, of course. I was planning to add tests once the direction of the
> discussion became clear. I will add them in the next version.

I tried to make the part. Feel free to include it if not yet. We can check the
server log, but I think it may be overkill.

Also, I have one comment.

```
+bool
+check_max_slot_wal_keep_size(int *newval, void **extra, GucSource source)
+{
+    if (IsBinaryUpgrade && *newval != -1)
+    {
+        GUC_check_errdetail("\"max_slot_wal_keep_size\" must be set to -1 
during binary upgrade mode.");
+        return false;
+    }
+    return true;
+}
```

Just to confirm - should we check the GucSource? Based on ur requirement, it 
might
be enough we avoid overwriting while starting the server.
Personally current code is OK because it is simpler, but I want to hear your 
opinion.

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

Attachment: add_test.patch
Description: add_test.patch

Reply via email to