check_function_bodies not doing much

Started by Marcelo Lacerdaover 7 years ago9 messagesgeneral
Jump to latest
#1Marcelo Lacerda
marceloslacerda@gmail.com

I was trying to get postgres to warn me that I'm referencing a table that
it doesn't exists inside a function so I was told on the IRC to check the
setting "check_function_bodies", however when I use it in a plpgsql
function it doesn't actually check if the tables in the body exist. Is this
the correct behavior?

Example:
http://paste.debian.net/1037080/

#2Pavel Stehule
pavel.stehule@gmail.com
In reply to: Marcelo Lacerda (#1)
Re: check_function_bodies not doing much

Hi

2018-08-07 21:17 GMT+02:00 Marcelo Lacerda <marceloslacerda@gmail.com>:

I was trying to get postgres to warn me that I'm referencing a table that
it doesn't exists inside a function so I was told on the IRC to check the
setting "check_function_bodies", however when I use it in a plpgsql
function it doesn't actually check if the tables in the body exist. Is this
the correct behavior?

Example:
http://paste.debian.net/1037080/

It is expected behave. PL/pgSQL checks immediately only syntax of embedded
SQL. With this design plpgsql functions are not too sensitive on objects'
dependency. You can use reference on temporary tables what usually doesn't
exists in plpgsql validation time.

For deeper check you can use plpgsql_check
https://github.com/okbob/plpgsql_check

It does almost all possible static checks.

Regards

Pavel

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Marcelo Lacerda (#1)
Re: check_function_bodies not doing much

Marcelo Lacerda <marceloslacerda@gmail.com> writes:

I was trying to get postgres to warn me that I'm referencing a table that
it doesn't exists inside a function so I was told on the IRC to check the
setting "check_function_bodies", however when I use it in a plpgsql
function it doesn't actually check if the tables in the body exist. Is this
the correct behavior?

Yes. It's supposed to be a syntax check, not a check that the function
would work when executed. (Depending on the particular PL you're using,
which you didn't mention, it might be a pretty weak syntax check too.)

An example of why a thorough check would be inadvisable is that a trigger
function might contain references to OLD and NEW that are in code paths
protected by checks on the trigger event type. That could be perfectly
OK, but a static check couldn't tell.

I believe there are some external tools floating around that check things
more aggressively, and hence with a higher rate of false positives.

regards, tom lane

#4David G. Johnston
david.g.johnston@gmail.com
In reply to: Tom Lane (#3)
Re: check_function_bodies not doing much

On Tue, Aug 7, 2018 at 12:31 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Marcelo Lacerda <marceloslacerda@gmail.com> writes:

I was trying to get postgres to warn me that I'm referencing a table that
it doesn't exists inside a function so I was told on the IRC to check the
setting "check_function_bodies", however when I use it in a plpgsql
function it doesn't actually check if the tables in the body exist. Is

this

the correct behavior?

Yes. It's supposed to be a syntax check, not a check that the function
would work when executed. (Depending on the particular PL you're using,
which you didn't mention, it might be a pretty weak syntax check too.)

The quoted text includes "however when I use it in a plpgsql function" so
we're good there.

Might be worth updating the docs for the GUC (or a note in the languages
themselves) to mention what the check covers for each of them. At least
distinguishing between syntax and semantics for each.
David J.

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: David G. Johnston (#4)
Re: check_function_bodies not doing much

"David G. Johnston" <david.g.johnston@gmail.com> writes:

On Tue, Aug 7, 2018 at 12:31 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Yes. It's supposed to be a syntax check, not a check that the function
would work when executed. (Depending on the particular PL you're using,
which you didn't mention, it might be a pretty weak syntax check too.)

The quoted text includes "however when I use it in a plpgsql function" so
we're good there.

Ah, you're right, my apologies for reading too hastily.

regards, tom lane

#6Merlin Moncure
mmoncure@gmail.com
In reply to: Tom Lane (#3)
Re: check_function_bodies not doing much

On Tue, Aug 7, 2018 at 2:31 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Marcelo Lacerda <marceloslacerda@gmail.com> writes:

I was trying to get postgres to warn me that I'm referencing a table that
it doesn't exists inside a function so I was told on the IRC to check the
setting "check_function_bodies", however when I use it in a plpgsql
function it doesn't actually check if the tables in the body exist. Is this
the correct behavior?

Yes. It's supposed to be a syntax check, not a check that the function
would work when executed. (Depending on the particular PL you're using,
which you didn't mention, it might be a pretty weak syntax check too.)

An example of why a thorough check would be inadvisable is that a trigger
function might contain references to OLD and NEW that are in code paths
protected by checks on the trigger event type. That could be perfectly
OK, but a static check couldn't tell.

I believe there are some external tools floating around that check things
more aggressively, and hence with a higher rate of false positives.

The only valid use of this GUC that I can think of is to work around
this problem;
postgres=# create or replace function f() returns void as
$$
create temp table x(id int);
delete from x;
$$ language sql;
ERROR: relation "x" does not exist

...I've since given up on writing plain sql functions except for
inline cases though so I don't use it anymore. Static resolution of
tables is not very useful since the state of the database as the time
of function creation is different than what it might be when the
function is run (as opposed to compiled languages obviously).

merlin

#7Marcelo Lacerda
marceloslacerda@gmail.com
In reply to: Merlin Moncure (#6)
Re: check_function_bodies not doing much

That's a whole different nightmare that I'm expecting.

"Yep I double-checked all my functions to see if any would break if I
change this field mytable.a into 2 fields mytable.a1 and mytable.a2 and
everything is ok."

*1 month later*

"Why is this error log for this application that I wrote one year ago so
big? I haven't changed anything!"

Error table mytable has no column a
Error table mytable has no column a
Error table mytable has no column a
...

It's frustrating that the references that a function make to the tables and
fields it access aren't taken in account for the validation of whether a
change to the structure of the database breaks the APIs that the database
exposes.

On Tue, Aug 7, 2018 at 6:44 PM Merlin Moncure <mmoncure@gmail.com> wrote:

Show quoted text

On Tue, Aug 7, 2018 at 2:31 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Marcelo Lacerda <marceloslacerda@gmail.com> writes:

I was trying to get postgres to warn me that I'm referencing a table

that

it doesn't exists inside a function so I was told on the IRC to check

the

setting "check_function_bodies", however when I use it in a plpgsql
function it doesn't actually check if the tables in the body exist. Is

this

the correct behavior?

Yes. It's supposed to be a syntax check, not a check that the function
would work when executed. (Depending on the particular PL you're using,
which you didn't mention, it might be a pretty weak syntax check too.)

An example of why a thorough check would be inadvisable is that a trigger
function might contain references to OLD and NEW that are in code paths
protected by checks on the trigger event type. That could be perfectly
OK, but a static check couldn't tell.

I believe there are some external tools floating around that check things
more aggressively, and hence with a higher rate of false positives.

The only valid use of this GUC that I can think of is to work around
this problem;
postgres=# create or replace function f() returns void as
$$
create temp table x(id int);
delete from x;
$$ language sql;
ERROR: relation "x" does not exist

...I've since given up on writing plain sql functions except for
inline cases though so I don't use it anymore. Static resolution of
tables is not very useful since the state of the database as the time
of function creation is different than what it might be when the
function is run (as opposed to compiled languages obviously).

merlin

#8Pavel Stehule
pavel.stehule@gmail.com
In reply to: Marcelo Lacerda (#7)
Re: check_function_bodies not doing much

2018-08-08 0:02 GMT+02:00 Marcelo Lacerda <marceloslacerda@gmail.com>:

That's a whole different nightmare that I'm expecting.

"Yep I double-checked all my functions to see if any would break if I
change this field mytable.a into 2 fields mytable.a1 and mytable.a2 and
everything is ok."

*1 month later*

"Why is this error log for this application that I wrote one year ago so
big? I haven't changed anything!"

Error table mytable has no column a
Error table mytable has no column a
Error table mytable has no column a
...

It's frustrating that the references that a function make to the tables
and fields it access aren't taken in account for the validation of whether
a change to the structure of the database breaks the APIs that the database
exposes.

This cannot be done due possible dynamic SQL. And this issue solve
plpgsql_check really well.

Regards

Pavel

Show quoted text

On Tue, Aug 7, 2018 at 6:44 PM Merlin Moncure <mmoncure@gmail.com> wrote:

On Tue, Aug 7, 2018 at 2:31 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Marcelo Lacerda <marceloslacerda@gmail.com> writes:

I was trying to get postgres to warn me that I'm referencing a table

that

it doesn't exists inside a function so I was told on the IRC to check

the

setting "check_function_bodies", however when I use it in a plpgsql
function it doesn't actually check if the tables in the body exist.

Is this

the correct behavior?

Yes. It's supposed to be a syntax check, not a check that the function
would work when executed. (Depending on the particular PL you're using,
which you didn't mention, it might be a pretty weak syntax check too.)

An example of why a thorough check would be inadvisable is that a

trigger

function might contain references to OLD and NEW that are in code paths
protected by checks on the trigger event type. That could be perfectly
OK, but a static check couldn't tell.

I believe there are some external tools floating around that check

things

more aggressively, and hence with a higher rate of false positives.

The only valid use of this GUC that I can think of is to work around
this problem;
postgres=# create or replace function f() returns void as
$$
create temp table x(id int);
delete from x;
$$ language sql;
ERROR: relation "x" does not exist

...I've since given up on writing plain sql functions except for
inline cases though so I don't use it anymore. Static resolution of
tables is not very useful since the state of the database as the time
of function creation is different than what it might be when the
function is run (as opposed to compiled languages obviously).

merlin

#9Marcelo Lacerda
marceloslacerda@gmail.com
In reply to: Pavel Stehule (#8)
Re: check_function_bodies not doing much

I'll take a look at it. Thanks for the recommendation.

On Tue, Aug 7, 2018 at 7:22 PM Pavel Stehule <pavel.stehule@gmail.com>
wrote:

Show quoted text

2018-08-08 0:02 GMT+02:00 Marcelo Lacerda <marceloslacerda@gmail.com>:

That's a whole different nightmare that I'm expecting.

"Yep I double-checked all my functions to see if any would break if I
change this field mytable.a into 2 fields mytable.a1 and mytable.a2 and
everything is ok."

*1 month later*

"Why is this error log for this application that I wrote one year ago so
big? I haven't changed anything!"

Error table mytable has no column a
Error table mytable has no column a
Error table mytable has no column a
...

It's frustrating that the references that a function make to the tables
and fields it access aren't taken in account for the validation of whether
a change to the structure of the database breaks the APIs that the database
exposes.

This cannot be done due possible dynamic SQL. And this issue solve
plpgsql_check really well.

Regards

Pavel

On Tue, Aug 7, 2018 at 6:44 PM Merlin Moncure <mmoncure@gmail.com> wrote:

On Tue, Aug 7, 2018 at 2:31 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Marcelo Lacerda <marceloslacerda@gmail.com> writes:

I was trying to get postgres to warn me that I'm referencing a table

that

it doesn't exists inside a function so I was told on the IRC to

check the

setting "check_function_bodies", however when I use it in a plpgsql
function it doesn't actually check if the tables in the body exist.

Is this

the correct behavior?

Yes. It's supposed to be a syntax check, not a check that the function
would work when executed. (Depending on the particular PL you're

using,

which you didn't mention, it might be a pretty weak syntax check too.)

An example of why a thorough check would be inadvisable is that a

trigger

function might contain references to OLD and NEW that are in code paths
protected by checks on the trigger event type. That could be perfectly
OK, but a static check couldn't tell.

I believe there are some external tools floating around that check

things

more aggressively, and hence with a higher rate of false positives.

The only valid use of this GUC that I can think of is to work around
this problem;
postgres=# create or replace function f() returns void as
$$
create temp table x(id int);
delete from x;
$$ language sql;
ERROR: relation "x" does not exist

...I've since given up on writing plain sql functions except for
inline cases though so I don't use it anymore. Static resolution of
tables is not very useful since the state of the database as the time
of function creation is different than what it might be when the
function is run (as opposed to compiled languages obviously).

merlin