Warnings "unrecognized node type" for some DDLs with log_statement = 'ddl'

Started by Michael Paquierover 8 years ago4 messages

michael.paquier@gmail.com

over 8 years ago

1 attachment(s)

Hi all,

While reviewing another patch, I have bumped into a couple of failures
when running installcheck if log_statement = 'ddl'. This pops
regression failures for 4 tests: object_address, alter_generic,
alter_operator and stats_ext involving commands CREATE STATISTICS and
ALTER OPERATOR.

You can as well reproduce the failures using simply that:
=# create table aa (a int, b int);
CREATE TABLE
=# CREATE STATISTICS aa_stat ON a, b FROM aa;
WARNING: 01000: unrecognized node type: 332
LOCATION: GetCommandLogLevel, utility.c:3357
ERROR: 42P17: extended statistics require at least 2 columns
LOCATION: CreateStatistics, statscmds.c:220
=# ALTER OPERATOR = (boolean, boolean) SET (RESTRICT = NONE);
WARNING: 01000: unrecognized node type: 294
LOCATION: GetCommandLogLevel, utility.c:3357
ALTER OPERATOR

Attached is a patch to fix all the failures I have spotted. As CREATE
STATISTICS is new in PG10, I am adding an open item as things come
from 7b504eb2. The problems of ALTER OPERATOR are introduced by 9.6.
Still I would suggest to fix everything at the same time. The problem
comes from GetCommandLogLevel() which forgot to add T_CreateStatsStmt
and T_AlterOperatorStmt. I have noticed as well that
T_AlterCollationStmt was missing.

Thanks,
--
Michael

Attachments:

log-statement-ddl-fix.patchapplication/octet-stream; name=log-statement-ddl-fix.patchDownload

diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 775477c6cf..5c69ecf0f7 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -3007,6 +3007,10 @@ GetCommandLogLevel(Node *parsetree)
 			lev = LOGSTMT_DDL;
 			break;
 
+		case T_AlterOperatorStmt:
+			lev = LOGSTMT_DDL;
+			break;
+
 		case T_AlterTableMoveAllStmt:
 		case T_AlterTableStmt:
 			lev = LOGSTMT_DDL;
@@ -3291,6 +3295,14 @@ GetCommandLogLevel(Node *parsetree)
 			lev = LOGSTMT_DDL;
 			break;
 
+		case T_CreateStatsStmt:
+			lev = LOGSTMT_DDL;
+			break;
+
+		case T_AlterCollationStmt:
+			lev = LOGSTMT_DDL;
+			break;
+
 			/* already-planned queries */
 		case T_PlannedStmt:
 			{

Ashutosh Sharma

ashu.coek88@gmail.com

over 8 years ago

In reply to: Michael Paquier (#1)

Re: Warnings "unrecognized node type" for some DDLs with log_statement = 'ddl'

On Thu, Sep 14, 2017 at 9:24 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:

Hi all,

While reviewing another patch, I have bumped into a couple of failures
when running installcheck if log_statement = 'ddl'. This pops
regression failures for 4 tests: object_address, alter_generic,
alter_operator and stats_ext involving commands CREATE STATISTICS and
ALTER OPERATOR.

You can as well reproduce the failures using simply that:
=# create table aa (a int, b int);
CREATE TABLE
=# CREATE STATISTICS aa_stat ON a, b FROM aa;
WARNING: 01000: unrecognized node type: 332
LOCATION: GetCommandLogLevel, utility.c:3357
ERROR: 42P17: extended statistics require at least 2 columns
LOCATION: CreateStatistics, statscmds.c:220
=# ALTER OPERATOR = (boolean, boolean) SET (RESTRICT = NONE);
WARNING: 01000: unrecognized node type: 294
LOCATION: GetCommandLogLevel, utility.c:3357
ALTER OPERATOR

Attached is a patch to fix all the failures I have spotted. As CREATE
STATISTICS is new in PG10, I am adding an open item as things come
from 7b504eb2. The problems of ALTER OPERATOR are introduced by 9.6.
Still I would suggest to fix everything at the same time. The problem
comes from GetCommandLogLevel() which forgot to add T_CreateStatsStmt
and T_AlterOperatorStmt. I have noticed as well that
T_AlterCollationStmt was missing.

Hmm...I am also able to reproduce the failures reported by you. Your
patch does solve the problem. Just to confirm if we are still missing
TAGS for any other Statement nodes, I had a quick look into the list
given in 'nodes.h' file but couldn't find any missing nodes. So, yes,
your patch does solve the problem completely and looks good to me.
Thanks.

--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com

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

Robert Haas

robertmhaas@gmail.com

over 8 years ago

In reply to: Ashutosh Sharma (#2)

Re: Warnings "unrecognized node type" for some DDLs with log_statement = 'ddl'

On Thu, Sep 14, 2017 at 6:02 AM, Ashutosh Sharma <ashu.coek88@gmail.com> wrote:

Hmm...I am also able to reproduce the failures reported by you. Your
patch does solve the problem. Just to confirm if we are still missing
TAGS for any other Statement nodes, I had a quick look into the list
given in 'nodes.h' file but couldn't find any missing nodes. So, yes,
your patch does solve the problem completely and looks good to me.
Thanks.

Committed and back-patched.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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

Michael Paquier

michael.paquier@gmail.com

over 8 years ago

In reply to: Robert Haas (#3)

Re: Warnings "unrecognized node type" for some DDLs with log_statement = 'ddl'

On Fri, Sep 15, 2017 at 6:19 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Thu, Sep 14, 2017 at 6:02 AM, Ashutosh Sharma <ashu.coek88@gmail.com> wrote:

Hmm...I am also able to reproduce the failures reported by you. Your
patch does solve the problem. Just to confirm if we are still missing
TAGS for any other Statement nodes, I had a quick look into the list
given in 'nodes.h' file but couldn't find any missing nodes. So, yes,
your patch does solve the problem completely and looks good to me.
Thanks.

Committed and back-patched.

Thanks Ashutosh and Robert.
--
Michael

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