BUG #14549: pl/pgsql parser

Started by Stefan Stefanovabout 9 years ago10 messagesbugs
Jump to latest
#1Stefan Stefanov
stefanov.sm@abv.bg

The following bug has been logged on the website:

Bug reference: 14549
Logged by: Stefan Stefanov
Email address: stefanov.sm@abv.bg
PostgreSQL version: 9.5.3
Operating system: Red Hat, 64 bit
Description:

Hi all,
I found (the hard way) that in pl/pgsql SELECT INTO statement a syntax error
may remain unnoticed.
This simple example works as expected and produces '1, 2, 3' notice.

DO language plpgsql
$$
DECLARE
vara integer;
varb integer;
varc integer;
BEGIN
SELECT 1, 2, 3 INTO vara, varb, varc;
RAISE NOTICE '% % %', vara, varb, varc;
END;
$$;

However if you omit a comma (or even replace the comma with AS) between varb
and varc in the INTO list then no syntax error is produced and the resulting
notice is '1 2 <NULL>'.

DO language plpgsql
$$
DECLARE
vara integer;
varb integer;
varc integer;
BEGIN
SELECT 1, 2, 3 INTO vara, varb AS varc;
RAISE NOTICE '% % %', vara, varb, varc;
END;
$$;

A few more clearly erratic combinations of SELECT expressions and the INTO
list also 'work' and issue misleading results.
Same in functions. For me it produced a bug that was difficult to see and
track.

Best,
Stefan

--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

#2Pavel Stehule
pavel.stehule@gmail.com
In reply to: Stefan Stefanov (#1)
Re: BUG #14549: pl/pgsql parser

Hi

2017-02-17 8:58 GMT+01:00 <stefanov.sm@abv.bg>:

The following bug has been logged on the website:

Bug reference: 14549
Logged by: Stefan Stefanov
Email address: stefanov.sm@abv.bg
PostgreSQL version: 9.5.3
Operating system: Red Hat, 64 bit
Description:

Hi all,
I found (the hard way) that in pl/pgsql SELECT INTO statement a syntax
error
may remain unnoticed.
This simple example works as expected and produces '1, 2, 3' notice.

DO language plpgsql
$$
DECLARE
vara integer;
varb integer;
varc integer;
BEGIN
SELECT 1, 2, 3 INTO vara, varb, varc;
RAISE NOTICE '% % %', vara, varb, varc;
END;
$$;

However if you omit a comma (or even replace the comma with AS) between
varb
and varc in the INTO list then no syntax error is produced and the
resulting
notice is '1 2 <NULL>'.

DO language plpgsql
$$
DECLARE
vara integer;
varb integer;
varc integer;
BEGIN
SELECT 1, 2, 3 INTO vara, varb AS varc;
RAISE NOTICE '% % %', vara, varb, varc;
END;
$$;

A few more clearly erratic combinations of SELECT expressions and the INTO
list also 'work' and issue misleading results.
Same in functions. For me it produced a bug that was difficult to see and
track.

Best,
Stefan

It is not a bug - plpgsql is designed be tolerant to different columns and
data types in left and right part of assignment.

You can use some tools for easy detecting these issues:

1. plpgsql_check https://github.com/okbob/plpgsql_check - it is available
in community repository
2. prepared extra_checks https://commitfest.postgresql.org/13/962/

Regards

Pavel

Show quoted text

--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

#3Stefan Stefanov
stefanov.sm@abv.bg
In reply to: Pavel Stehule (#2)
Re: BUG #14549: pl/pgsql parser

Thanks Pavel.
Different columns and data types tolerance is ok, however what about wrong and meaningless syntax?
Best,
Stefan

-------- Оригинално писмо --------

От: Pavel Stehule pavel.stehule@gmail.com

Относно: Re: [BUGS] BUG #14549: pl/pgsql parser

До: stefanov.sm@abv.bg

Изпратено на: 17.02.2017 11:19

Hi

2017-02-17 8:58 GMT+01:00
stefanov.sm@abv.bg > :

The following bug has been logged on the website:

Bug reference:

14549

Logged by:

Stefan Stefanov

Email address:

stefanov.sm@abv.bg

PostgreSQL version: 9.5.3

Operating system:

Red Hat, 64 bit

Description:

Hi all,

I found (the hard way) that in pl/pgsql SELECT INTO statement a syntax error

may remain unnoticed.

This simple example works as expected and produces '1, 2, 3' notice.

DO language plpgsql

$$

DECLARE

vara integer;

varb integer;

varc integer;

BEGIN

SELECT 1, 2, 3 INTO vara, varb, varc;

RAISE NOTICE '% % %', vara, varb, varc;

END;

$$;

However if you omit a comma (or even replace the comma with AS) between varb

and varc in the INTO list then no syntax error is produced and the resulting

notice is '1 2 '.

DO language plpgsql

$$

DECLARE

vara integer;

varb integer;

varc integer;

BEGIN

SELECT 1, 2, 3 INTO vara, varb AS varc;

RAISE NOTICE '% % %', vara, varb, varc;

END;

$$;

A few more clearly erratic combinations of SELECT expressions and the INTO

list also 'work' and issue misleading results.

Same in functions. For me it produced a bug that was difficult to see and

track.

Best,

Stefan

It is not a bug - plpgsql is designed be tolerant to different columns and data types in left and right part of assignment.

You can use some tools for easy detecting these issues:

1. plpgsql_check
https://github.com/okbob/plpgsql_check - it is available in community repository

2. prepared extra_checks
https://commitfest.postgresql.org/13/962/

Regards

Pavel

--
Sent via pgsql-bugs mailing list ( pgsql-bugs@postgresql.org )
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

#4Wèi Cōngruì
crvv.mail@gmail.com
In reply to: Pavel Stehule (#2)
Re: BUG #14549: pl/pgsql parser

Hello,

In the document,
https://www.postgresql.org/docs/9.6/static/plpgsql-statements.html#PLPGSQL-STATEMENTS-SQL-ONEROW

"If a row or a variable list is used as target, the query's result
columns must exactly match the structure of the target as to
number and data types, or else a run-time error occurs. When
a record variable is the target, it automatically configures itself
to the row type of the query result columns."

I think this is a bug according to the document.

Regards,
Wei Congrui

2017-02-17 17:19 GMT+08:00 Pavel Stehule <pavel.stehule@gmail.com>:

Show quoted text

Hi

2017-02-17 8:58 GMT+01:00 <stefanov.sm@abv.bg>:

The following bug has been logged on the website:

Bug reference: 14549
Logged by: Stefan Stefanov
Email address: stefanov.sm@abv.bg
PostgreSQL version: 9.5.3
Operating system: Red Hat, 64 bit
Description:

Hi all,
I found (the hard way) that in pl/pgsql SELECT INTO statement a syntax
error
may remain unnoticed.
This simple example works as expected and produces '1, 2, 3' notice.

DO language plpgsql
$$
DECLARE
vara integer;
varb integer;
varc integer;
BEGIN
SELECT 1, 2, 3 INTO vara, varb, varc;
RAISE NOTICE '% % %', vara, varb, varc;
END;
$$;

However if you omit a comma (or even replace the comma with AS) between
varb
and varc in the INTO list then no syntax error is produced and the
resulting
notice is '1 2 <NULL>'.

DO language plpgsql
$$
DECLARE
vara integer;
varb integer;
varc integer;
BEGIN
SELECT 1, 2, 3 INTO vara, varb AS varc;
RAISE NOTICE '% % %', vara, varb, varc;
END;
$$;

A few more clearly erratic combinations of SELECT expressions and the INTO
list also 'work' and issue misleading results.
Same in functions. For me it produced a bug that was difficult to see and
track.

Best,
Stefan

It is not a bug - plpgsql is designed be tolerant to different columns and
data types in left and right part of assignment.

You can use some tools for easy detecting these issues:

1. plpgsql_check https://github.com/okbob/plpgsql_check - it is available
in community repository
2. prepared extra_checks https://commitfest.postgresql.org/13/962/

Regards

Pavel

--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

#5Pavel Stehule
pavel.stehule@gmail.com
In reply to: Stefan Stefanov (#3)
Re: BUG #14549: pl/pgsql parser

2017-02-17 10:43 GMT+01:00 Stefan Stefanov <stefanov.sm@abv.bg>:

Thanks Pavel.
Different columns and data types tolerance is ok, however what about wrong
and meaningless syntax?

There was a discussion about more restrictivity or about different syntax.

More restrict behave can breaks compatibility - now we have good enough
tools, so compatibility break is not necessary.

Regards

Pavel

Show quoted text

Best,
Stefan

-------- Оригинално писмо --------
От: Pavel Stehule pavel.stehule@gmail.com
Относно: Re: [BUGS] BUG #14549: pl/pgsql parser
До: stefanov.sm@abv.bg
Изпратено на: 17.02.2017 11:19

Hi

2017-02-17 8:58 GMT+01:00 <stefanov.sm@abv.bg>:

The following bug has been logged on the website:

Bug reference: 14549
Logged by: Stefan Stefanov
Email address: stefanov.sm@abv.bg
PostgreSQL version: 9.5.3
Operating system: Red Hat, 64 bit
Description:

Hi all,
I found (the hard way) that in pl/pgsql SELECT INTO statement a syntax
error
may remain unnoticed.
This simple example works as expected and produces '1, 2, 3' notice.

DO language plpgsql
$$
DECLARE
vara integer;
varb integer;
varc integer;
BEGIN
SELECT 1, 2, 3 INTO vara, varb, varc;
RAISE NOTICE '% % %', vara, varb, varc;
END;
$$;

However if you omit a comma (or even replace the comma with AS) between
varb
and varc in the INTO list then no syntax error is produced and the
resulting
notice is '1 2 <NULL>'.

DO language plpgsql
$$
DECLARE
vara integer;
varb integer;
varc integer;
BEGIN
SELECT 1, 2, 3 INTO vara, varb AS varc;
RAISE NOTICE '% % %', vara, varb, varc;
END;
$$;

A few more clearly erratic combinations of SELECT expressions and the INTO
list also 'work' and issue misleading results.
Same in functions. For me it produced a bug that was difficult to see and
track.

Best,
Stefan

It is not a bug - plpgsql is designed be tolerant to different columns and
data types in left and right part of assignment.

You can use some tools for easy detecting these issues:

1. plpgsql_check https://github.com/okbob/plpgsql_check - it is available
in community repository
2. prepared extra_checks https://commitfest.postgresql.org/13/962/

Regards

Pavel

--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

#6Pavel Stehule
pavel.stehule@gmail.com
In reply to: Wèi Cōngruì (#4)
Re: BUG #14549: pl/pgsql parser

2017-02-17 10:44 GMT+01:00 Wei Congrui <crvv.mail@gmail.com>:

Hello,

In the document, https://www.postgresql.org/docs/9.6/
static/plpgsql-statements.html#PLPGSQL-STATEMENTS-SQL-ONEROW

"If a row or a variable list is used as target, the query's result
columns must exactly match the structure of the target as to
number and data types, or else a run-time error occurs. When
a record variable is the target, it automatically configures itself
to the row type of the query result columns."

I think this is a bug according to the document.

yes, it is not valid

Pavel

Show quoted text

Regards,
Wei Congrui

2017-02-17 17:19 GMT+08:00 Pavel Stehule <pavel.stehule@gmail.com>:

Hi

2017-02-17 8:58 GMT+01:00 <stefanov.sm@abv.bg>:

The following bug has been logged on the website:

Bug reference: 14549
Logged by: Stefan Stefanov
Email address: stefanov.sm@abv.bg
PostgreSQL version: 9.5.3
Operating system: Red Hat, 64 bit
Description:

Hi all,
I found (the hard way) that in pl/pgsql SELECT INTO statement a syntax
error
may remain unnoticed.
This simple example works as expected and produces '1, 2, 3' notice.

DO language plpgsql
$$
DECLARE
vara integer;
varb integer;
varc integer;
BEGIN
SELECT 1, 2, 3 INTO vara, varb, varc;
RAISE NOTICE '% % %', vara, varb, varc;
END;
$$;

However if you omit a comma (or even replace the comma with AS) between
varb
and varc in the INTO list then no syntax error is produced and the
resulting
notice is '1 2 <NULL>'.

DO language plpgsql
$$
DECLARE
vara integer;
varb integer;
varc integer;
BEGIN
SELECT 1, 2, 3 INTO vara, varb AS varc;
RAISE NOTICE '% % %', vara, varb, varc;
END;
$$;

A few more clearly erratic combinations of SELECT expressions and the
INTO
list also 'work' and issue misleading results.
Same in functions. For me it produced a bug that was difficult to see and
track.

Best,
Stefan

It is not a bug - plpgsql is designed be tolerant to different columns
and data types in left and right part of assignment.

You can use some tools for easy detecting these issues:

1. plpgsql_check https://github.com/okbob/plpgsql_check - it is
available in community repository
2. prepared extra_checks https://commitfest.postgresql.org/13/962/

Regards

Pavel

--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Wèi Cōngruì (#4)
Re: BUG #14549: pl/pgsql parser

Wei Congrui <crvv.mail@gmail.com> writes:

In the document,
https://www.postgresql.org/docs/9.6/static/plpgsql-statements.html#PLPGSQL-STATEMENTS-SQL-ONEROW
"If a row or a variable list is used as target, the query's result
columns must exactly match the structure of the target as to
number and data types, or else a run-time error occurs. When
a record variable is the target, it automatically configures itself
to the row type of the query result columns."

I think this is a bug according to the document.

I don't think that's the relevant point. What is relevant is the
next paragraph:

"The INTO clause can appear almost anywhere in the SQL
command. Customarily it is written either just before or just after the
list of select_expressions in a SELECT command, or at the end of the
command for other command types. It is recommended that you follow this
convention in case the PL/pgSQL parser becomes stricter in future
versions."

What's happening in Stefan's example

SELECT 1, 2, 3 INTO vara, varb AS varc;

is that "INTO vara, varb" is pulled out as being the INTO clause, and
what's left is

SELECT 1, 2, 3 AS varc;

which is a perfectly legal SQL statement so no error is reported.

To make this throw an error, we'd need to become stricter about the
placement of INTO (as the manual hints), or become stricter about the
number of SELECT output columns matching the number of INTO target
variables, or possibly both. Any such change would doubtless draw
complaints from people whose code worked fine before. It might be
a good idea anyway, but selling backwards-compatibility breakage
to the Postgres community is usually a hard sell.

regards, tom lane

--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

#8Wèi Cōngruì
crvv.mail@gmail.com
In reply to: Tom Lane (#7)
Re: BUG #14549: pl/pgsql parser

I think there are two potential problems in the origin SQL
"SELECT 1, 2, 3 INTO vara, varb AS varc".

1. like "SELECT 1, 2, 3 INTO a, b"

Query result is "1, 2, 3", target is "a, b". The problem is that the query's
result columns don't match the structure of the target.

I think there is a difference between behavior and document at this point.

2. like "SELECT 1, 2, 3 INTO a, b, c AS x"

This SQL is equivalent to "SELECT 1, 2, 3 AS x INTO a, b, c".
There are other equivalent SQLs sush as
"SELECT INTO a, b, c 1, 2, 3 AS x",
"SELECT 1, 2, INTO a, b, c 3 AS x" and
"SELECT 1, INTO a, b, c 2, 3 AS x".

This is in conformity with document.

Thanks,
Wei Congrui

2017-02-18 0:54 GMT+08:00 Tom Lane <tgl@sss.pgh.pa.us>:

Show quoted text

Wei Congrui <crvv.mail@gmail.com> writes:

In the document,
https://www.postgresql.org/docs/9.6/static/plpgsql-

statements.html#PLPGSQL-STATEMENTS-SQL-ONEROW

"If a row or a variable list is used as target, the query's result
columns must exactly match the structure of the target as to
number and data types, or else a run-time error occurs. When
a record variable is the target, it automatically configures itself
to the row type of the query result columns."

I think this is a bug according to the document.

I don't think that's the relevant point. What is relevant is the
next paragraph:

"The INTO clause can appear almost anywhere in the SQL
command. Customarily it is written either just before or just after the
list of select_expressions in a SELECT command, or at the end of the
command for other command types. It is recommended that you follow this
convention in case the PL/pgSQL parser becomes stricter in future
versions."

What's happening in Stefan's example

SELECT 1, 2, 3 INTO vara, varb AS varc;

is that "INTO vara, varb" is pulled out as being the INTO clause, and
what's left is

SELECT 1, 2, 3 AS varc;

which is a perfectly legal SQL statement so no error is reported.

To make this throw an error, we'd need to become stricter about the
placement of INTO (as the manual hints), or become stricter about the
number of SELECT output columns matching the number of INTO target
variables, or possibly both. Any such change would doubtless draw
complaints from people whose code worked fine before. It might be
a good idea anyway, but selling backwards-compatibility breakage
to the Postgres community is usually a hard sell.

regards, tom lane

#9Stefan Stefanov
stefanov.sm@abv.bg
In reply to: Tom Lane (#7)
Re: BUG #14549: pl/pgsql parser

The documentation keeps the door open. Two compatible suggestions:
SET plpgsql_syntax TO strict; -- with loose as default The parser becomes strict about the placement of INTO (as the manual hints) and about the

number and type of SELECT output columns matching the number of INTO target variables.
A warning and a hint rather then an exception to keep backwards compatibility
Sincerely,

Stefan

-------- Оригинално писмо --------

От: Tom Lane tgl@sss.pgh.pa.us

Относно: Re: [BUGS] BUG #14549: pl/pgsql parser

До: Wei Congrui

Изпратено на: 17.02.2017 18:54

Wei Congrui crvv.mail@gmail.com > writes:

In the document,

https://www.postgresql.org/docs/9.6/static/plpgsql-statements.html#PLPGSQL-STATEMENTS-SQL-ONEROW

"If a row or a variable list is used as target, the query's result

columns must exactly match the structure of the target as to

number and data types, or else a run-time error occurs. When

a record variable is the target, it automatically configures itself

to the row type of the query result columns."

I think this is a bug according to the document.

I don't think that's the relevant point. What is relevant is the

next paragraph:

"The INTO clause can appear almost anywhere in the SQL

command. Customarily it is written either just before or just after the

list of select_expressions in a SELECT command, or at the end of the

command for other command types. It is recommended that you follow this

convention in case the PL/pgSQL parser becomes stricter in future

versions."

What's happening in Stefan's example

SELECT 1, 2, 3 INTO vara, varb AS varc;

is that "INTO vara, varb" is pulled out as being the INTO clause, and

what's left is

SELECT 1, 2, 3 AS varc;

which is a perfectly legal SQL statement so no error is reported.

To make this throw an error, we'd need to become stricter about the

placement of INTO (as the manual hints), or become stricter about the

number of SELECT output columns matching the number of INTO target

variables, or possibly both. Any such change would doubtless draw

complaints from people whose code worked fine before. It might be

a good idea anyway, but selling backwards-compatibility breakage

to the Postgres community is usually a hard sell.

regards, tom lane

#10Pavel Stehule
pavel.stehule@gmail.com
In reply to: Stefan Stefanov (#9)
Re: BUG #14549: pl/pgsql parser

2017-02-19 17:41 GMT+01:00 Stefan Stefanov <stefanov.sm@abv.bg>:

The documentation keeps the door open. Two compatible suggestions:

- SET plpgsql_syntax TO strict; -- with loose as default

The parser becomes strict about the placement of INTO (as the manual
hints) and about the
number and type of SELECT output columns matching the number of INTO
target variables.

We talked about changing behave by GUC, and this is usually disallowed.

But new extra check can raise warning, so this behave should not be a issue.

Regards

Pacel

Show quoted text

- A warning and a hint rather then an exception to keep backwards
compatibility

Sincerely, Stefan

-------- Оригинално писмо --------
От: Tom Lane tgl@sss.pgh.pa.us
Относно: Re: [BUGS] BUG #14549: pl/pgsql parser
До: Wei Congrui <crvv.mail@gmail.com>
Изпратено на: 17.02.2017 18:54

Wei Congrui <crvv.mail@gmail.com> writes:

In the document,
https://www.postgresql.org/docs/9.6/static/plpgsql-

statements.html#PLPGSQL-STATEMENTS-SQL-ONEROW

"If a row or a variable list is used as target, the query's result
columns must exactly match the structure of the target as to
number and data types, or else a run-time error occurs. When
a record variable is the target, it automatically configures itself
to the row type of the query result columns."

I think this is a bug according to the document.

I don't think that's the relevant point. What is relevant is the
next paragraph:

"The INTO clause can appear almost anywhere in the SQL
command. Customarily it is written either just before or just after the
list of select_expressions in a SELECT command, or at the end of the
command for other command types. It is recommended that you follow this
convention in case the PL/pgSQL parser becomes stricter in future
versions."

What's happening in Stefan's example

SELECT 1, 2, 3 INTO vara, varb AS varc;

is that "INTO vara, varb" is pulled out as being the INTO clause, and
what's left is

SELECT 1, 2, 3 AS varc;

which is a perfectly legal SQL statement so no error is reported.

To make this throw an error, we'd need to become stricter about the
placement of INTO (as the manual hints), or become stricter about the
number of SELECT output columns matching the number of INTO target
variables, or possibly both. Any such change would doubtless draw
complaints from people whose code worked fine before. It might be
a good idea anyway, but selling backwards-compatibility breakage
to the Postgres community is usually a hard sell.

regards, tom lane