Horton

Thread

Commits

  1. Check column list length in XMLTABLE/JSON_TABLE alias

  1. BUG #17480: Assertion failure in parse_relation.c

    The Post Office <noreply@postgresql.org> — 2022-05-10T08:21:02Z

    The following bug has been logged on the website:

Bug reference:      17480
Logged by:          Wang Ke
Email address:      krking@zju.edu.cn
PostgreSQL version: 14.2
Operating system:   Ubuntu 20.04.4 LTS x86_64
Description:        

Hello developers, I found an assertion failure in both postgresql 14.2 and
HEAD version. The detail is as follows:

Reported by:
Wang Ke of Zhejiang University

OS version and name:
Linux ubuntu 5.13.0-40-generic #45~20.04.1-Ubuntu SMP Mon Apr 4 09:38:31
UTC
2022 x86_64 x86_64 x86_64 GNU/Linux

Testcase:
```
CREATE TABLE v0 ( v3 INT , v2 VARCHAR , v1 INT , UNIQUE ( v1 , v3 ) ) ;
 VALUES ( - - -128 , - - - - 127 , NULL ) ;
SELECT FROM LATERAL XMLTABLE ( ROW ( ) PASSING NULL BY VALUE COLUMNS v1
TIMESTAMP WITHOUT TIME ZONE ARRAY ) AS DELETE ( v3 , v3 , v2 , v3 , v1 ,
FIRST , v1 , FLOAT , INT , v2 , v3 , VERSION , v2 , FLOAT , INT , v1 , NO ,
ROW , v3 , v2 ) ORDER BY v2 , v2 ;
```

Position:
(HEAD version) src/backend/parser/parse_relation.c:1310
1309        /* colnames must have the same number of entries as the nsitem
*/
1310        Assert(maxattrs == list_length(rte->eref->colnames));


Crash log:
TRAP: FailedAssertion("maxattrs == list_length(rte->eref->colnames)", File:
"/postgres/bld/../src/backend/parser/parse_relation.c", Line: 1310, PID:
284194)
/lib/x86_64-linux-gnu/libasan.so.5(+0x6cd40)[0x7f05b592ad40]
postgres: krking test [local]
SELECT(ExceptionalCondition+0x147)[0x5626dd114b23]
postgres: krking test [local] SELECT(+0x739303)[0x5626dc729303]
postgres: krking test [local]
SELECT(addRangeTableEntryForTableFunc+0x536)[0x5626dc72fcf9]
postgres: krking test [local] SELECT(+0x6f5d42)[0x5626dc6e5d42]
postgres: krking test [local]
SELECT(transformFromClause+0x184)[0x5626dc6e9b71]
postgres: krking test [local] SELECT(transformStmt+0x3eef)[0x5626dc6a0c54]
postgres: krking test [local] SELECT(+0x6b7f4c)[0x5626dc6a7f4c]
postgres: krking test [local]
SELECT(transformTopLevelStmt+0x31)[0x5626dc6a81c2]
postgres: krking test [local]
SELECT(parse_analyze_fixedparams+0x8d)[0x5626dc6a830a]
postgres: krking test [local]
SELECT(pg_analyze_and_rewrite_fixedparams+0x64)[0x5626dcdd9385]
postgres: krking test [local] SELECT(+0xdea58b)[0x5626dcdda58b]
postgres: krking test [local] SELECT(PostgresMain+0x14ad)[0x5626dcddbdf1]
postgres: krking test [local] SELECT(+0xc422c4)[0x5626dcc322c4]
postgres: krking test [local]
SELECT(PostmasterMain+0x21aa)[0x5626dcc34d88]
postgres: krking test [local] SELECT(main+0x605)[0x5626dca26578]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf3)[0x7f05b55500b3]
postgres: krking test [local] SELECT(_start+0x2e)[0x5626dc39f64e]
2022-05-10 16:11:44.220 CST [284180] LOG:  server process (PID 284194) was
terminated by signal 6: aborted
2022-05-10 16:11:44.220 CST [284180] DETAIL:  Failed process was running:
SELECT FROM LATERAL XMLTABLE ( ROW ( ) PASSING NULL BY VALUE COLUMNS v1
TIMESTAMP WITHOUT TIME ZONE ARRAY ) AS DELETE ( v3 , v3 , v2 , v3 , v1 ,
FIRST , v1 , FLOAT , INT , v2 , v3 , VERSION , v2 , FLOAT , INT , v1 , NO ,
ROW , v3 , v2 ) ORDER BY v2 , v2 ;
2022-05-10 16:11:44.220 CST [284180] LOG:  terminating any other active
server processes
2022-05-10 16:11:44.221 CST [284195] FATAL:  the database system is in
recovery mode
2022-05-10 16:11:44.225 CST [284180] LOG:  all server processes terminated;
reinitializing
2022-05-10 16:11:44.235 CST [284196] LOG:  database system was interrupted;
last known up at 2022-05-10 16:11:00 CST
2022-05-10 16:11:44.261 CST [284196] LOG:  database system was not properly
shut down; automatic recovery in progress
2022-05-10 16:11:44.263 CST [284196] LOG:  redo starts at 0/1498B68
2022-05-10 16:11:44.279 CST [284196] LOG:  invalid record length at
0/1C6B348: wanted 24, got 0
2022-05-10 16:11:44.279 CST [284196] LOG:  redo done at 0/1C6ADD0 system
usage: CPU: user: 0.00 s, system: 0.01 s, elapsed: 0.01 s
2022-05-10 16:11:44.303 CST [284197] LOG:  checkpoint starting:
end-of-recovery immediate wait
2022-05-10 16:11:44.339 CST [284197] LOG:  checkpoint complete: wrote 1480
buffers (9.0%); 0 WAL file(s) added, 0 removed, 0 recycled; write=0.034 s,
sync=0.002 s, total=0.037 s; sync files=350, longest=0.001 s, average=0.001
s; distance=8010 kB, estimate=8010 kB
2022-05-10 16:11:44.346 CST [284180] LOG:  database system is ready to
accept connections

  2. Re: BUG #17480: Assertion failure in parse_relation.c

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2022-05-10T10:50:57Z

    On 2022-May-10, PG Bug reporting form wrote:

> Testcase:
> ```
> CREATE TABLE v0 ( v3 INT , v2 VARCHAR , v1 INT , UNIQUE ( v1 , v3 ) ) ;
>  VALUES ( - - -128 , - - - - 127 , NULL ) ;
> SELECT FROM LATERAL XMLTABLE ( ROW ( ) PASSING NULL BY VALUE COLUMNS v1
> TIMESTAMP WITHOUT TIME ZONE ARRAY ) AS DELETE ( v3 , v3 , v2 , v3 , v1 ,
> FIRST , v1 , FLOAT , INT , v2 , v3 , VERSION , v2 , FLOAT , INT , v1 , NO ,
> ROW , v3 , v2 ) ORDER BY v2 , v2 ;
> ```

Hmm, were you using a fuzzer to generate this query?  It would sure be
useful to have it process some more and see what other crashes it
discovers.

Your example can be reduced to this, which already crashes the server:

SELECT * FROM XMLTABLE (ROW () passing null COLUMNS v1 TIMESTAMP, v2 int) AS f (v1, v2, v3);

and with this it is clear that the problem is that the alias list is too
long for the COLUMNS specification of the XMLTABLE function.  This can
be avoided by throwing an error:

diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c
index 7465919044..6c817a9c55 100644
--- a/src/backend/parser/parse_relation.c
+++ b/src/backend/parser/parse_relation.c
@@ -2001,6 +2001,12 @@ addRangeTableEntryForTableFunc(ParseState *pstate,
 		eref->colnames = list_concat(eref->colnames,
 									 list_copy_tail(tf->colnames, numaliases));
 
+	if (numaliases > list_length(tf->colnames))
+		ereport(ERROR,
+				(errcode(ERRCODE_SYNTAX_ERROR),
+				 errmsg("%s function has %d columns available but %d columns specified",
+						"XMLTABLE", list_length(tf->colnames), numaliases)));
+
 	rte->eref = eref;
 
 	/*

(Obviously, no regression test changes output after this patch.)

This mirrors what happens when you give too many aliases to tables or
other things.  (In pg15 and up, it needs to be aware of JSON_TABLE too.
I made this on pg14.)

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"Uno puede defenderse de los ataques; contra los elogios se esta indefenso"

  3. Re: BUG #17480: Assertion failure in parse_relation.c

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-05-11T00:04:15Z

    Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> and with this it is clear that the problem is that the alias list is too
> long for the COLUMNS specification of the XMLTABLE function.

Check.

> This can be avoided by throwing an error:

+1, but you should match the ERRCODE thrown by buildRelationAliases,
that is ERRCODE_INVALID_COLUMN_REFERENCE.

Looking at the other addRangeTableEntry* functions, 
addRangeTableEntryForJoin seems to similarly lack a defense
against too many aliases.  Testing shows that the case is covered
elsewhere:

regression=# select * from (int8_tbl i cross join int4_tbl j) ss(a,b,c,d) limit 2;
ERROR:  column alias list for "ss" has too many entries
regression=# \errverbose 
ERROR:  42601: column alias list for "ss" has too many entries
LOCATION:  transformFromClauseItem, parse_clause.c:1458

but this is randomly different both in the message wording and in the
choice of ERRCODE.  I wonder if we shouldn't harmonize that, and maybe
move detection of the case to addRangeTableEntryForJoin for consistency
with the other cases.

			regards, tom lane

  4. Re: BUG #17480: Assertion failure in parse_relation.c

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2022-05-15T10:42:10Z

    On 2022-May-10, Tom Lane wrote:

> Looking at the other addRangeTableEntry* functions, 
> addRangeTableEntryForJoin seems to similarly lack a defense
> against too many aliases.  Testing shows that the case is covered
> elsewhere:
> 
> regression=# select * from (int8_tbl i cross join int4_tbl j) ss(a,b,c,d) limit 2;
> ERROR:  column alias list for "ss" has too many entries
> regression=# \errverbose 
> ERROR:  42601: column alias list for "ss" has too many entries
> LOCATION:  transformFromClauseItem, parse_clause.c:1458
> 
> but this is randomly different both in the message wording and in the
> choice of ERRCODE.  I wonder if we shouldn't harmonize that, and maybe
> move detection of the case to addRangeTableEntryForJoin for consistency
> with the other cases.

That makes sense; I ended up with the attached patch, in which I also
attempt to add regression test cases to cover all the cases.

I find that the check in addRangeTableEntryForValues is dead code,
because both callers pass a NULL Alias.  I find no way to give VALUES an
alias directly.  This has annoyed me in the past (not a big deal tbh),
but that makes me refrain from suggesting to removing the code.

We do get the correct error with this query,
  select * from (values (1)) as f (a, b);
but that is coming from addRangeTableEntryForSubquery.

This is backpatchable (sans the JSON_TABLE bits).  I'll attempt to get
it pushed later today.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"People get annoyed when you try to debug them."  (Larry Wall)

  5. Re: BUG #17480: Assertion failure in parse_relation.c

    Richard Guo <guofenglinux@gmail.com> — 2022-05-16T06:39:15Z

    On Sun, May 15, 2022 at 7:29 PM Alvaro Herrera <alvherre@alvh.no-ip.org>
wrote:

> I find that the check in addRangeTableEntryForValues is dead code,
> because both callers pass a NULL Alias.  I find no way to give VALUES an
> alias directly.  This has annoyed me in the past (not a big deal tbh),
> but that makes me refrain from suggesting to removing the code.
>

I have the same finding. For SelectStmt, the VALUES clause always
appears inside RangeSubselect, and you can give alias only to the
subquery, not the VALUES clause. For InsertStmt, seems you cannot
specify an alias for the VALUES clause.

Since the only two callers to addRangeTableEntryForValues both pass NULL
as Alias, I think we can remove the related codes inside that function.

Thanks
Richard