Horton

Thread

Commits

  1. Ensure acquire_inherited_sample_rows sets its output parameters.

  1. BUG #17880: Uninitialised value used when analyzing a table with an inheritance tree containing no children

    The Post Office <noreply@postgresql.org> — 2023-03-31T10:00:02Z

    The following bug has been logged on the website:

Bug reference:      17880
Logged by:          Alexander Lakhin
Email address:      exclusion@gmail.com
PostgreSQL version: 15.2
Operating system:   Ubuntu 22.04
Description:        

The following query:
CREATE TABLE t (a int)  PARTITION BY RANGE (a);
CREATE TABLE tp1 (a int);
ALTER TABLE t ATTACH PARTITION tp1 FOR VALUES FROM (100) to (200);
ALTER TABLE t DETACH PARTITION tp1;
ANALYZE (VERBOSE) t;

produces the error detected by valgrind:
2023-03-31 12:11:20.784 MSK|law|regression|6426a3b8.3543d1|LOG:  statement:
ANALYZE (VERBOSE) t;
2023-03-31 12:11:20.792 MSK|law|regression|6426a3b8.3543d1|INFO:  analyzing
"public.t" inheritance tree
2023-03-31 12:11:20.804 MSK|law|regression|6426a3b8.3543d1|INFO:  skipping
analyze of "public.t" inheritance tree --- this inheritance tree contains no
child tables
==00:00:00:03.670 3490769== Conditional jump or move depends on
uninitialised value(s)
==00:00:00:03.670 3490769==    at 0x3D52EE: vac_update_relstats
(vacuum.c:1354)
==00:00:00:03.670 3490769==    by 0x34B874: do_analyze_rel (analyze.c:675)
==00:00:00:03.670 3490769==    by 0x34BEB4: analyze_rel (analyze.c:269)
==00:00:00:03.670 3490769==    by 0x3D5EEC: vacuum (vacuum.c:493)
==00:00:00:03.670 3490769==    by 0x3D667A: ExecVacuum (vacuum.c:276)
==00:00:00:03.670 3490769==    by 0x596188: standard_ProcessUtility
(utility.c:866)
==00:00:00:03.670 3490769==    by 0x596672: ProcessUtility (utility.c:530)
==00:00:00:03.670 3490769==    by 0x593B42: PortalRunUtility
(pquery.c:1158)
==00:00:00:03.670 3490769==    by 0x593E19: PortalRunMulti (pquery.c:1315)
==00:00:00:03.670 3490769==    by 0x59419B: PortalRun (pquery.c:791)
==00:00:00:03.670 3490769==    by 0x59047D: exec_simple_query
(postgres.c:1250)
==00:00:00:03.670 3490769==    by 0x5922D3: PostgresMain (postgres.c:4593)
==00:00:00:03.670 3490769==
...

The similar query:
CREATE TABLE t (a int)  PARTITION BY RANGE (a);
CREATE TABLE tp1 (a int) PARTITION BY RANGE (a);
ALTER TABLE t ATTACH PARTITION tp1 FOR VALUES FROM (100) to (200);
ANALYZE (VERBOSE) t;

produces the same error but with a slightly different INFO message:
2023-03-31 12:13:24.070 MSK|law|regression|6426a433.3549e7|LOG:  statement:
ANALYZE (VERBOSE) t;
2023-03-31 12:13:24.093 MSK|law|regression|6426a433.3549e7|INFO:  analyzing
"public.t" inheritance tree
2023-03-31 12:13:24.107 MSK|law|regression|6426a433.3549e7|INFO:  skipping
analyze of "public.t" inheritance tree --- this inheritance tree contains no
analyzable child tables
==00:00:00:03.621 3492327== Conditional jump or move depends on
uninitialised value(s)
==00:00:00:03.621 3492327==    at 0x3D52EE: vac_update_relstats
(vacuum.c:1354)
==00:00:00:03.621 3492327==    by 0x34B874: do_analyze_rel (analyze.c:675)
==00:00:00:03.621 3492327==    by 0x34BEB4: analyze_rel (analyze.c:26
...

As I can see, this error is explained by the fact that
acquire_inherited_sample_rows() can exit before setting
    *totalrows = 0;
    *totaldeadrows = 0;

so variables declared in do_analyze_rel() as
	double		totalrows,
				totaldeadrows;
stay uninitialised and totalrows passed to vac_update_relstats() where it
triggers the valgrind complaint.

On a quick glance, the peer functions acquire_sample_rows(),
postgresAcquireSampleRowsFunc(), file_acquire_sample_rows() can't exit
without setting totalrows/totaldeadrows, so maybe
acquire_inherited_sample_rows() deserves fixing (or maybe those variables
can be initialized in do_analyze_rel()).

  2. Re: BUG #17880: Uninitialised value used when analyzing a table with an inheritance tree containing no children

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-03-31T13:40:35Z

    PG Bug reporting form <noreply@postgresql.org> writes:
> As I can see, this error is explained by the fact that
> acquire_inherited_sample_rows() can exit before setting
>     *totalrows = 0;
>     *totaldeadrows = 0;
> so variables declared in do_analyze_rel() as
> 	double		totalrows,
> 				totaldeadrows;
> stay uninitialised and totalrows passed to vac_update_relstats() where it
> triggers the valgrind complaint.

Good catch!

> On a quick glance, the peer functions acquire_sample_rows(),
> postgresAcquireSampleRowsFunc(), file_acquire_sample_rows() can't exit
> without setting totalrows/totaldeadrows, so maybe
> acquire_inherited_sample_rows() deserves fixing (or maybe those variables
> can be initialized in do_analyze_rel()).

Yeah, I'd put the blame on acquire_inherited_sample_rows.  Its head
comment says it has the same API as acquire_sample_rows, and that
one's comment says that totalrows/totaldeadrows are always set.
Will fix, thanks.

			regards, tom lane