Bug 1292064 - engine-setup should set PG check_function_bodies flag to ON
engine-setup should set PG check_function_bodies flag to ON
Status: CLOSED WORKSFORME
Product: ovirt-engine
Classification: oVirt
Component: Setup.Engine (Show other bugs)
3.6.0
Unspecified Unspecified
medium Severity medium (vote)
: ovirt-4.0.0-rc
: ---
Assigned To: Yedidyah Bar David
Pavel Stehlik
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2015-12-16 06:31 EST by Eli Mesika
Modified: 2016-05-25 08:43 EDT (History)
8 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2016-05-25 08:43:21 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: Integration
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
ylavi: ovirt‑4.0.0?
emesika: planning_ack?
sbonazzo: devel_ack+
pstehlik: testing_ack+


Attachments (Terms of Use)

  None (edit)
Description Eli Mesika 2015-12-16 06:31:51 EST
Description of problem:

PG check_function_bodies flag is OFF by default.
That makes PG compile a FUNCTION only in the 1st time it is used.
That was done in order to support creating recursive functions. 
Since recursive functions are slow and we currently do not use them, we would like to turn the check_function_bodies flag to ON to prevent cases when the function has an error in it and we will know it only when we get the exception on the first function call

Version-Release number of selected component (if applicable):


How reproducible:

Always


Steps to Reproduce:
1.With check_function_bodies OFF in /var/lib/pgsql/data/postgresql.conf and PG 8.4 create the following function 

CREATE OR REPLACE FUNCTION foo()
RETURNS VOID
AS $function$
BEGIN
    select concat('a','b');
END;$function$
LANGUAGE plpgsql;

2.Call the function 
3.You will get :

select foo();
ERROR:  function concat(unknown, unknown) does not exist
LINE 1: select concat('a','b')             ^
HINT:  No function matches the given name and argument types. You might need to add explicit type casts.
QUERY:  select concat('a','b')
CONTEXT:  PL/pgSQL function "foo" line 2 at SQL statement

Actual results:

Function is created and error will be displayed only on 1st call 

Expected results:

Function is not created with an invalid syntax 

Additional info:

engine-setup should set  check_function_bodies flag to ON in /var/lib/pgsql/data/postgresql.conf

Flag value is effective after PG is restarted
Comment 1 Yedidyah Bar David 2015-12-16 07:56:36 EST
Eli:

1. Do we want to change that also on upgrade? Or during initial setup is enough?

2. Do we want to also enforce this with a remote db - so that setup will abort if a remote db has it off?

If it's mainly so that we know that our code is good, I guess 'initial setup' and 'local db' are enough.
Comment 2 Eli Mesika 2015-12-17 04:25:49 EST
(In reply to Yedidyah Bar David from comment #1)

> If it's mainly so that we know that our code is good, I guess 'initial
> setup' and 'local db' are enough.

+1
Comment 3 Sandro Bonazzola 2016-05-02 05:47:51 EDT
Moving from 4.0 alpha to 4.0 beta since 4.0 alpha has been already released and bug is not ON_QA.
Comment 5 Yaniv Lavi 2016-05-23 09:12:50 EDT
oVirt 4.0 beta has been released, moving to RC milestone.
Comment 6 Yedidyah Bar David 2016-05-24 10:40:23 EDT
Now checked on 8.4 (rhel6) and 9.2 (fedora 23) and both have check_function_bodies set to "on" by default. Why/where do you think it defaults to off?

With both on and off, it does not fail with your above example.

I tried searching a bit and found some relevant stuff, but none that seems to do exactly what you ask for. We might have to use some other means to do that.

https://github.com/okbob/plpgsql_check
https://github.com/okbob/plpgsql_lint
https://github.com/markdrago/pgsanity
http://pgtap.org/
http://stackoverflow.com/questions/8271606/postgresql-syntax-check-without-running-the-query
http://stackoverflow.com/questions/26343778/postgresql-vs-oracle-compile-time-checking-of-pl-pgsql
Comment 7 Eli Mesika 2016-05-25 05:18:53 EDT
(In reply to Yedidyah Bar David from comment #6)
> Now checked on 8.4 (rhel6) and 9.2 (fedora 23) and both have
> check_function_bodies set to "on" by default. Why/where do you think it
> defaults to off?

Strange, AFAIR it was OFF when I checked it in 8.4 


> 
> With both on and off, it does not fail with your above example.

Did you restarted PG after changing this value ?

> I tried searching a bit and found some relevant stuff, but none that seems to >  do exactly what you ask for. We might have to use some other means to do that.

The main reason for this requirement came from an issue raised in DWH , actually it was a call to SP that failed in run-time , so , the SP compiled and stored without any errors or warnings and we found out that it has a syntax error only when it was already part of the version that was delivered to the customer.
That's really bad and I looked for a solution for that and found that the check_function_bodies set to ON should do the magic....
Comment 8 Yedidyah Bar David 2016-05-25 08:43:21 EDT
Now verified that the example in comment 0 behaves the same with check_function_bodies both off and on, and also that if I comment it out in the conf it defaults to on.

This conf option affects *syntax* errors. E.g., something like:

CREATE OR REPLACE FUNCTION foo()
RETURNS VOID
AS $function$
BEGIN
    a
END;$function$
LANGUAGE plpgsql;

With check_function_bodies on, the default, this will fail. With off, it will not fail, and calling foo() will fail with the same error.

Closing for now.

We might want to have some more thorough checking of our plpgsql code, see comment 6, but it will require heavier means, and those I currently found do not fully cover all relevant cases.

Note You need to log in before you can comment on or make changes to this bug.