On Tue, Aug 11, 2026 at 3:01 PM Corey Huinker <[email protected]> wrote: >> Overall, the changes look good to me, and I don't have any further comments. > Same here, I'd set it to Ready For Committer status if it weren't already set.
I have mixed feelings about this patch. I agree that the way that the batch size is capped based on the supplied number of parameters could be confusing to someone. I also think that the message that are emitted here are not necessarily too verbose. I think it would be unacceptable to emit a WARNING for every statement, but this only emits a WARNING at object definition time and a DEBUG message at usage time, which might be OK. All of that basically speaks in favor of the patch. On the flip side, this isn't the only thing in postgresGetForeignModifyBatchSize that could trip somebody up. The batch size can be taken from the table level option or the server level option initially, and then it can be reset to 1 for various different reasons, and then it can be capped by the limit at issue here. So I feel like it's fair to wonder how many people would really be helped by this patch. They'd have to have a confusion about the batch size, and then they'd have to have the specific confusion that corresponds to what the patch can tell you about, and then (unless they have a value so large that it triggers the definition-time warning) would have to think of setting log_min_messages to a low enough level to let them see the DEBUG message. I'm not sure how many people are going to think of that. I think my instinct if I were having a batch_size problem would be to check the documentation, which already does mention the fact that the batch size can be clamped to avoid passing too many libpq parameters. Interestingly, that's the only thing it mentions: the restrictions on RETURNING, BEFORE/AFTER ROW INSERT TRIGGERS, and WITH CHECK OPTION constraints are not documented. If I didn't find something talking about this in the documentation, personally, my next move would be to check the source code. I'd probably only find the proposed DEBUG message after I'd already found the code that explains the behavior I was seeing. And all that assumes that there's really a problem here in the first place. Large batch sizes are obviously nicer, but a batch size of 10 rather than 1 by definition eliminates 90% of the overhead that can theoretically be eliminated. Is limiting the batch size to a value that is likely to still be over 1000 really going to be a problem for people? And if is, to Corey's point upthread, what are they going to do about it? It's not like the libpq limit can feasibly be raised, so you just have to live with the batch size that the system can manage. You might want to understand what's happening, but the existing documentation seems more useful for that than the proposed messages. So overall I'm inclined to feel like this isn't really worth it. -- Robert Haas Databricks
