On Tue, Nov 3, 2026 at 12:16 PM Pavel Stehule <[email protected]> wrote:
> Hi > > po 2. 11. 2026 v 15:21 odesílatel Jeevan Chalke < > [email protected]> napsal: > >> >> >> On Mon, Nov 2, 2026 at 5:23 PM Pavel Stehule <[email protected]> >> wrote: >> >>> >>> >>> po 2. 11. 2026 v 12:35 odesílatel Jeevan Chalke < >>> [email protected]> napsal: >>> >>>> >>>> >>>> On Fri, Oct 30, 2026 at 6:57 PM Pavel Stehule <[email protected]> >>>> wrote: >>>> >>>>> Hi >>>>> >>>>> pá 30. 10. 2026 v 12:43 odesílatel Jeevan Chalke < >>>>> [email protected]> napsal: >>>>> >>>>>> Hi, >>>>>> >>>>>> The documentation for plpgsql.extra_warnings / plpgsql.extra_errors >>>>>> says that the too_many_rows check will "*check if a given query >>>>>> returns more than one row when an INTO clause is used*" *[1]*. Only >>>>>> static SQL statements implement this (exec_stmt_execsql()). >>>>>> exec_stmt_dynexecute() only looks at STRICT, so EXECUTE ... INTO >>>>>> silently uses the first row and nothing is reported: >>>>>> >>>>>> set plpgsql.extra_warnings = 'too_many_rows'; >>>>>> do $$ declare x int; >>>>>> begin >>>>>> execute 'select g from generate_series(1,2) g' into x; >>>>>> end $$; >>>>>> DO >>>>>> >>>>>> The same statement written as static SQL does warn (or fail, with >>>>>> extra_errors). >>>>>> >>>>>> The attached patch computes too_many_rows_level in >>>>>> exec_stmt_dynexecute() the same way exec_stmt_execsql() does, and >>>>>> reports at that level when more than one row comes back and STRICT >>>>>> was not specified. With it: >>>>>> >>>>>> WARNING: query returned more than one row >>>>>> HINT: Make sure the query returns a single row, or use LIMIT 1. >>>>>> DO >>>>>> >>>>>> and with plpgsql.extra_errors the same message is raised as an ERROR. >>>>>> Checking costs nothing extra here, since EXECUTE already fetches all >>>>>> the rows (no row limit is passed to SPI). >>>>>> >>>>>> While there, I added the "Make sure the query returns a single row, >>>>>> or use LIMIT 1." hint to the error raised for EXECUTE ... INTO STRICT, >>>>>> so that it matches the error for static SQL. This changes the expected >>>>>> output of three existing STRICT tests, which now show the HINT line. >>>>>> >>>>>> The new tests are in the "runtime extra checks" section of plpgsql.sql >>>>>> and cover EXECUTE ... INTO with both extra_warnings and extra_errors. >>>>>> >>>>>> This is a behavior change: someone who has too_many_rows in >>>>>> extra_errors may start getting errors from EXECUTE ... INTO >>>>>> statements that used to pass. So I think this should go to master only >>>>>> (or >>>>>> v19, as it is yet to release), but I'd like to hear what others think. >>>>>> >>>>>> >>>>> Yes, this is my omission. >>>>> >>>>> The dynamic SQL is more problematic - it allows multiquery >>>>> >>>>> (2026-10-30 14:24:06) postgres=# do $$ >>>>> declare r record; >>>>> begin >>>>> execute 'select 1 union select 2; select 2' into strict r; >>>>> raise notice '%', r; >>>>> end; >>>>> $$; >>>>> NOTICE: (2) >>>>> DO >>>>> >>>>> Maybe extra errors should handle this case too. >>>>> >>>> >>>> Hi Pavel, >>>> >>>> Thanks for looking at this. >>>> >>>> I confirmed the multi-statement case. With INTO, only the result of >>>> the last statement is used, so even *STRICT* passes for: >>>> >>>> do $$ >>>> declare r record; >>>> begin >>>> execute 'select 1 union select 2; select 2' into strict r; >>>> raise notice '%', r; >>>> end; >>>> $$; >>>> NOTICE: (2) >>>> >>>> That behavior is independent of this patch, which counts the rows of >>>> the statement whose result is actually used, so I'd prefer to keep this >>>> patch scoped to that. >>>> >>>> Seems like a separate feature altogether. >>>> >>>> Detecting multiple statements would need a different check, since >>>> exec_stmt_dynexecute() currently hands the query string straight to >>>> SPI_execute_extended() and has no way to see how many statements it >>>> contained. >>>> >>> >>> Yes, it is hard to say what is expected behaviour in the mentioned case. >>> >>> Maybe it should be documented, so this case is not covered by checks. >>> >> >> Hi Pavel, >> >> Thanks for the follow-up. >> >> I agree that it is hard to say what the expected behavior is for that >> case, so documenting it seems like the right thing to do for now. >> >> Attached is v2, which is v1 plus a short paragraph in the EXECUTE >> section of plpgsql.sgml saying that when the command string contains >> more than one SQL command, only the result of the last one is assigned to >> the INTO variable(s), and that STRICT and the too_many_rows extra check >> likewise look only at that result. >> >> There are no code changes compared to v1. >> > > +1 > > I tested this patch, and I don't see any issue > > all tests passed > > I'll mark this patch as ready for committer > Thanks for the review, Pavel, and for marking it Ready for Committer. > > Regards > > Pavel > > >> >> Thanks >> >> >>> >>> Regards >>> >>> Pavel >>> >>> >>>> Thanks >>>> >>>> >>>>> >>>>> regards >>>>> >>>>> Pavel >>>>> >>>>> >>>>> >>>>> >>>>>> Thanks >>>>>> >>>>>> *[1] >>>>>> https://www.postgresql.org/docs/current/plpgsql-development-tips.html#PLPGSQL-EXTRA-CHECKS >>>>>> <https://www.postgresql.org/docs/current/plpgsql-development-tips.html#PLPGSQL-EXTRA-CHECKS>* >>>>>> >>>>>> -- >>>>>> *Jeevan Chalke* >>>>>> *Senior Principal Engineer, Engineering Manager* >>>>>> *Product Development* >>>>>> >>>>>> enterprisedb.com <https://www.enterprisedb.com> >>>>>> >>>>> >>>> >>>> -- >>>> *Jeevan Chalke* >>>> *Senior Principal Engineer, Engineering Manager* >>>> *Product Development* >>>> >>>> enterprisedb.com <https://www.enterprisedb.com> >>>> >>> >> >> -- >> *Jeevan Chalke* >> *Senior Principal Engineer, Engineering Manager* >> *Product Development* >> >> enterprisedb.com <https://www.enterprisedb.com> >> > -- *Jeevan Chalke* *Senior Principal Engineer, Engineering Manager* *Product Development* enterprisedb.com <https://www.enterprisedb.com>
