Hi Fujii-san, Thank you for the review.
On Tue, Nov 3, 2026 at 2:09 PM Fujii Masao <[email protected]> wrote: > On Tue, Nov 3, 2026 at 4:27 PM Jeevan Chalke > <[email protected]> wrote: > > Thanks for the review, Pavel, and for marking it Ready for Committer. > > Since the patch was marked Ready for Committer, I've just started reading > it. > > + If the command string contains more than one SQL command, only the > + result of the last one is assigned to the <literal>INTO</literal> > + variable(s); the results of the earlier commands are discarded. > > This does not seem to be always true. If the last command is an INSERT with > an unconditional DO INSTEAD NOTHING rule, the result of the preceding > SELECT is used instead. Is that right? > Yes, you are right. I tried it, and with such a rule execute 'select 42; insert into u values (1)' into x; assigns 42 to x, so the paragraph I added was not accurate in general. (If the last command returns no data at all and there is no such rule, we instead get "INTO used with a command that cannot return data".) > > Also, since this describes existing behavior, isn't it better to leave this > paragraph out of the patch and address it separately? > Agreed. I have removed it from 0001, and 0001 is now the same as v1 (code and tests). I have put a reworded version in a separate patch, 0003, in case you and Pavel still think it is worth documenting. It now says that the results of all but the last command are discarded and only the last command's result is assigned to the INTO variable(s), and it mentions your DO INSTEAD NOTHING case. If you think this is not needed, please feel free to ignore 0003. > > > + errhint("Make sure the query returns a single > row, or use LIMIT 1."))); > > LIMIT 1 is not applicable to every command, such as UPDATE ... RETURNING. > So,perhaps it's better to remove "or use LIMIT 1" from the hint? But, since > the same issue seems to exist for static SQL, that might need to be > addressed > in a separate patch anyway. > Right, the hint here is the same text static SQL uses, so I kept them consistent in 0001. Since the issue exists in both places, I have done what you suggested as a separate patch: 0002 removes "or use LIMIT 1" from the hint in both exec_stmt_execsql() and exec_stmt_dynexecute(), and updates the expected output accordingly. Attached is v3 with three patches: 0001: the too_many_rows check for EXECUTE ... INTO (same as v1) 0002: the hint change 0003: the documentation change One more thing: I proposed this for master only, since it changes behavior for users who have too_many_rows in extra_errors, and I'd appreciate your view on that as well. Thanks > > Regards, > > -- > Fujii Masao > -- *Jeevan Chalke* *Senior Principal Engineer, Engineering Manager* *Product Development* enterprisedb.com <https://www.enterprisedb.com>
v3-0002-Remove-or-use-LIMIT-1-from-PL-pgSQL-more-than-one.patch
Description: Binary data
v3-0001-Apply-the-too_many_rows-extra-check-to-EXECUTE-.-.patch
Description: Binary data
v3-0003-doc-Describe-EXECUTE-.-INTO-with-multiple-command.patch
Description: Binary data
