Thread
Commits
-
Mop-up for letting VOID-returning SQL functions end with a SELECT.
- 877cdf11eaa9 11.0 landed
-
segmentation fault in pg head with SQL function.
Prabhat Sahu <prabhat.sahu@enterprisedb.com> — 2018-03-16T06:05:13Z
Hi, I found a segmentation fault on pg master Head with below steps and stacktrace. postgres=# CREATE OR REPLACE FUNCTION func1() RETURNS VOID LANGUAGE SQL AS $$ select 10; $$; CREATE FUNCTION postgres=# select func1(); server closed the connection unexpectedly This probably means the server terminated abnormally before or while processing the request. The connection to the server was lost. Attempting reset: Failed. !> \q -- stack trace: [edb@localhost bin]$ gdb -q -c data/core.31498 postgres Reading symbols from /home/edb/PG/PGsrcNew/postgresql/inst/bin/postgres...done. [New LWP 31498] [Thread debugging using libthread_db enabled] Using host libthread_db library "/usr/lib64/libthread_db.so.1". Core was generated by `postgres: edb postgres [local] SELECT '. Program terminated with signal 6, Aborted. #0 0x00007fb376a001f7 in raise () from /usr/lib64/libc.so.6 Missing separate debuginfos, use: debuginfo-install glibc-2.17-196.el7.x86_64 keyutils-libs-1.5.8-3.el7.x86_64 krb5-libs-1.15.1-8.el7.x86_64 libcom_err-1.42.9-10.el7.x86_64 libselinux-2.5-11.el7.x86_64 openssl-libs-1.0.2k-8.el7.x86_64 pcre-8.32-17.el7.x86_64 zlib-1.2.7-17.el7.x86_64 (gdb) bt #0 0x00007fb376a001f7 in raise () from /usr/lib64/libc.so.6 #1 0x00007fb376a018e8 in abort () from /usr/lib64/libc.so.6 #2 0x0000000000a01368 in ExceptionalCondition (conditionName=0xbea4a0 "!(exprType(newexpr) == result_type)", errorType=0xbe98c6 "FailedAssertion", fileName=0xbe9916 "clauses.c", lineNumber=4627) at assert.c:54 #3 0x00000000007d1611 in inline_function (funcid=16384, result_type=2278, result_collid=0, input_collid=0, args=0x0, funcvariadic=0 '\000', func_tuple=0x7fb377ebc918, context=0x7fff01e5fca0) at clauses.c:4627 #4 0x00000000007d0781 in simplify_function (funcid=16384, result_type=2278, result_typmod=-1, result_collid=0, input_collid=0, args_p=0x7fff01e5eb70, funcvariadic=0 '\000', process_args=1 '\001', allow_non_const=1 '\001', context=0x7fff01e5fca0) at clauses.c:4095 #5 0x00000000007ce15f in eval_const_expressions_mutator (node=0x2a88cf8, context=0x7fff01e5fca0) at clauses.c:2676 #6 0x000000000073727b in expression_tree_mutator (node=0x2a88d88, mutator=0x7cdc5c <eval_const_expressions_mutator>, context=0x7fff01e5fca0) at nodeFuncs.c:2854 #7 0x00000000007d007c in eval_const_expressions_mutator (node=0x2a88d88, context=0x7fff01e5fca0) at clauses.c:3671 #8 0x0000000000737464 in expression_tree_mutator (node=0x2a88d50, mutator=0x7cdc5c <eval_const_expressions_mutator>, context=0x7fff01e5fca0) at nodeFuncs.c:2903 #9 0x00000000007d007c in eval_const_expressions_mutator (node=0x2a88d50, context=0x7fff01e5fca0) at clauses.c:3671 #10 0x00000000007cdc09 in eval_const_expressions (root=0x2a88f00, node=0x2a88d50) at clauses.c:2474 #11 0x00000000007ae7cb in preprocess_expression (root=0x2a88f00, expr=0x2a88d50, kind=1) at planner.c:996 #12 0x00000000007adf40 in subquery_planner (glob=0x2a88b68, parse=0x2a888e0, parent_root=0x0, hasRecursion=0 '\000', tuple_fraction=0) at planner.c:687 #13 0x00000000007ad541 in standard_planner (parse=0x2a888e0, cursorOptions=256, boundParams=0x0) at planner.c:385 #14 0x00000000007ad2d2 in planner (parse=0x2a888e0, cursorOptions=256, boundParams=0x0) at planner.c:243 #15 0x000000000089980c in pg_plan_query (querytree=0x2a888e0, cursorOptions=256, boundParams=0x0) at postgres.c:807 #16 0x0000000000899939 in pg_plan_queries (querytrees=0x2a88ec8, cursorOptions=256, boundParams=0x0) at postgres.c:873 #17 0x0000000000899c08 in exec_simple_query (query_string=0x2a87a28 "select func1();") at postgres.c:1048 #18 0x000000000089dfb0 in PostgresMain (argc=1, argv=0x2ab3550, dbname=0x2ab33b0 "postgres", username=0x2a84428 "edb") at postgres.c:4144 #19 0x00000000007feed4 in BackendRun (port=0x2aab390) at postmaster.c:4409 #20 0x00000000007fe64d in BackendStartup (port=0x2aab390) at postmaster.c:4081 #21 0x00000000007fab64 in ServerLoop () at postmaster.c:1754 #22 0x00000000007fa19d in PostmasterMain (argc=5, argv=0x2a82330) at postmaster.c:1362 #23 0x0000000000731fb8 in main (argc=5, argv=0x2a82330) at main.c:228 (gdb) -- With Regards, Prabhat Kumar Sahu Skype ID: prabhat.sahu1984 EnterpriseDB Corporation The Postgres Database Company -
Re: segmentation fault in pg head with SQL function.
Michael Paquier <michael@paquier.xyz> — 2018-03-16T07:07:46Z
On Fri, Mar 16, 2018 at 11:35:13AM +0530, Prabhat Sahu wrote: > I found a segmentation fault on pg master Head with below steps and > stacktrace. > > postgres=# CREATE OR REPLACE FUNCTION func1() RETURNS VOID > LANGUAGE SQL > AS $$ > select 10; > $$; > CREATE FUNCTION > > postgres=# select func1(); > server closed the connection unexpectedly > This probably means the server terminated abnormally > before or while processing the request. > The connection to the server was lost. Attempting reset: Failed. > !> \q (Adding Peter and Tom in CC) Problem reproducible here, and the bug has been introduced by fd1a421f. It seems to me that the function should not be authorized to be created to begin with, as it returns an integer in its last query, where I think that check_sql_fn_retval is doing it wrong when called in inline_function() as we know that it handles a function, and not a procedure thanks to the first sanity checks at the top of the function. Here is what happens before this commit: =# CREATE OR REPLACE FUNCTION func1() RETURNS VOID LANGUAGE SQL AS $$ select 10; $; ERROR: 42P13: return type mismatch in function declared to return void DETAIL: Actual return type is integer. CONTEXT: SQL function "func1" LOCATION: check_sql_fn_retval, functions.c:1655 As far as I can see, here is the faulty code: - if (!rettype) + /* + * If it's declared to return VOID, we don't care what's in the function. + * (This takes care of the procedure case, as well.) + */ + if (rettype == VOIDOID) return false; While this can be bypassed for a procedure, we should fall down and fail for a SQL function returning void depending on the tlist generated by the last SQL query. @@ -1624,8 +1622,7 @@ check_sql_fn_retval(Oid func_id, Oid rettype, List *queryTreeList, if (fn_typtype == TYPTYPE_BASE || fn_typtype == TYPTYPE_DOMAIN || fn_typtype == TYPTYPE_ENUM || - fn_typtype == TYPTYPE_RANGE || - rettype == VOIDOID) + fn_typtype == TYPTYPE_RANGE This also causes the error message generated to change. Peter, what's the intention here? Shouldn't void still be treated as a scalar type? -- Michael -
Re: segmentation fault in pg head with SQL function.
Tom Lane <tgl@sss.pgh.pa.us> — 2018-03-16T15:40:21Z
Michael Paquier <michael@paquier.xyz> writes: > On Fri, Mar 16, 2018 at 11:35:13AM +0530, Prabhat Sahu wrote: >> postgres=# CREATE OR REPLACE FUNCTION func1() RETURNS VOID >> LANGUAGE SQL >> AS $$ >> select 10; >> $$; > Problem reproducible here, and the bug has been introduced by fd1a421f. > It seems to me that the function should not be authorized to be created > to begin with, as it returns an integer in its last query, where I think > that check_sql_fn_retval is doing it wrong when called in > inline_function() as we know that it handles a function, and not a > procedure thanks to the first sanity checks at the top of the function. Hm. Actually, I think this is my fault. It is true that previous PG versions would have rejected this function definition, but my intention while revising Peter's prokind patch was that we'd start allowing a VOID-returning SQL function to contain anything, and just ignore whatever the last statement within it might be. The documentation doesn't say much about VOID-returning SQL functions, but I certainly don't see anything saying that they can't end with a SELECT, so arguably the old behavior is a POLA violation. In any case, this is the behavior we need for VOID-returning procedures, and there seems little reason not to make functions act similarly. So apparently I missed something with that. Will look more closely. regards, tom lane
-
Re: segmentation fault in pg head with SQL function.
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2018-04-11T13:21:07Z
On 3/16/18 11:40, Tom Lane wrote: > Michael Paquier <michael@paquier.xyz> writes: >> On Fri, Mar 16, 2018 at 11:35:13AM +0530, Prabhat Sahu wrote: >>> postgres=# CREATE OR REPLACE FUNCTION func1() RETURNS VOID >>> LANGUAGE SQL >>> AS $$ >>> select 10; >>> $$; > >> Problem reproducible here, and the bug has been introduced by fd1a421f. >> It seems to me that the function should not be authorized to be created >> to begin with, as it returns an integer in its last query, where I think >> that check_sql_fn_retval is doing it wrong when called in >> inline_function() as we know that it handles a function, and not a >> procedure thanks to the first sanity checks at the top of the function. > > Hm. Actually, I think this is my fault. It is true that previous PG > versions would have rejected this function definition, but my intention > while revising Peter's prokind patch was that we'd start allowing a > VOID-returning SQL function to contain anything, and just ignore whatever > the last statement within it might be. The documentation doesn't say > much about VOID-returning SQL functions, but I certainly don't see > anything saying that they can't end with a SELECT, so arguably the old > behavior is a POLA violation. In any case, this is the behavior we > need for VOID-returning procedures, and there seems little reason not > to make functions act similarly. > > So apparently I missed something with that. Will look more closely. This was listed as an open item, but it was already fixed by 877cdf11eaa9cabcb9b1e3c1bef0760fe08efdc3, so I'll remove it. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services