Don't print extra parens around expressions in extended stats

tvondra · tvondra · commit 13380e147649 · 2021-09-01T00:43:22.000+02:00
The code printing expressions for extended statistics doubled the parens, producing results like ((a+1)), which is unnecessary and not consistent with how we print expressions elsewhere. Fixed by tweaking the code to produce just a single set of parens. Reported by Mark Dilger, fix by me. Backpatch to 14, where support for extended statistics on expressions was added. Reported-by: Mark Dilger Discussion: https://postgr.es/m/20210122040101.GF27167%40telsasoft.com
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
@@ -1712,7 +1712,7 @@ pg_get_statisticsobj_worker(Oid statextid, bool columns_only, bool missing_ok)
 	{
 		Node	   *expr = (Node *) lfirst(lc);
 		char	   *str;
-		int			prettyFlags = PRETTYFLAG_INDENT;
+		int			prettyFlags = PRETTYFLAG_PAREN;
 
 		str = deparse_expression_pretty(expr, context, false, false,
 										prettyFlags, 0);
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -2811,7 +2811,7 @@
 		create_sql   => 'CREATE STATISTICS dump_test.test_ext_stats_expr
 							ON (2 * col1) FROM dump_test.test_fifth_table',
 		regexp => qr/^
-			\QCREATE STATISTICS dump_test.test_ext_stats_expr ON ((2 * col1)) FROM dump_test.test_fifth_table;\E
+			\QCREATE STATISTICS dump_test.test_ext_stats_expr ON (2 * col1) FROM dump_test.test_fifth_table;\E
 		    /xms,
 		like =>
 		  { %full_runs, %dump_test_schema_runs, section_post_data => 1, },
diff --git a/src/test/regress/expected/create_table_like.out b/src/test/regress/expected/create_table_like.out
@@ -417,7 +417,7 @@ Check constraints:
     "ctlt1_a_check" CHECK (length(a) > 2)
 Statistics objects:
     "public.ctlt_all_a_b_stat" ON a, b FROM ctlt_all
-    "public.ctlt_all_expr_stat" ON ((a || b)) FROM ctlt_all
+    "public.ctlt_all_expr_stat" ON (a || b) FROM ctlt_all
 
 SELECT c.relname, objsubid, description FROM pg_description, pg_index i, pg_class c WHERE classoid = 'pg_class'::regclass AND objoid = i.indexrelid AND c.oid = i.indexrelid AND i.indrelid = 'ctlt_all'::regclass ORDER BY c.relname, objsubid;
     relname     | objsubid | description 
@@ -457,7 +457,7 @@ Check constraints:
     "ctlt1_a_check" CHECK (length(a) > 2)
 Statistics objects:
     "public.pg_attrdef_a_b_stat" ON a, b FROM public.pg_attrdef
-    "public.pg_attrdef_expr_stat" ON ((a || b)) FROM public.pg_attrdef
+    "public.pg_attrdef_expr_stat" ON (a || b) FROM public.pg_attrdef
 
 DROP TABLE public.pg_attrdef;
 -- Check that LIKE isn't confused when new table masks the old, either
@@ -479,7 +479,7 @@ Check constraints:
     "ctlt1_a_check" CHECK (length(a) > 2)
 Statistics objects:
     "ctl_schema.ctlt1_a_b_stat" ON a, b FROM ctlt1
-    "ctl_schema.ctlt1_expr_stat" ON ((a || b)) FROM ctlt1
+    "ctl_schema.ctlt1_expr_stat" ON (a || b) FROM ctlt1
 
 ROLLBACK;
 DROP TABLE ctlt1, ctlt2, ctlt3, ctlt4, ctlt12_storage, ctlt12_comments, ctlt1_inh, ctlt13_inh, ctlt13_like, ctlt_all, ctla, ctlb CASCADE;
diff --git a/src/test/regress/expected/stats_ext.out b/src/test/regress/expected/stats_ext.out
@@ -3004,21 +3004,21 @@ insert into stts_t1 select i,i from generate_series(1,100) i;
 analyze stts_t1;
 set search_path to public, stts_s1, stts_s2, tststats;
 \dX
-                                                           List of extended statistics
-  Schema  |          Name          |                               Definition                               | Ndistinct | Dependencies |   MCV   
-----------+------------------------+------------------------------------------------------------------------+-----------+--------------+---------
- public   | func_deps_stat         | ((a * 2)), upper(b), ((c + (1)::numeric)) FROM functional_dependencies |           | defined      | 
- public   | mcv_lists_arrays_stats | a, b, c FROM mcv_lists_arrays                                          |           |              | defined
- public   | mcv_lists_bool_stats   | a, b, c FROM mcv_lists_bool                                            |           |              | defined
- public   | mcv_lists_stats        | a, b, d FROM mcv_lists                                                 |           |              | defined
- public   | stts_1                 | a, b FROM stts_t1                                                      | defined   |              | 
- public   | stts_2                 | a, b FROM stts_t1                                                      | defined   | defined      | 
- public   | stts_3                 | a, b FROM stts_t1                                                      | defined   | defined      | defined
- public   | stts_4                 | b, c FROM stts_t2                                                      | defined   | defined      | defined
- public   | stts_hoge              | col1, col2, col3 FROM stts_t3                                          | defined   | defined      | defined
- stts_s1  | stts_foo               | col1, col2 FROM stts_t3                                                | defined   | defined      | defined
- stts_s2  | stts_yama              | col1, col3 FROM stts_t3                                                |           | defined      | defined
- tststats | priv_test_stats        | a, b FROM priv_test_tbl                                                |           |              | defined
+                                                        List of extended statistics
+  Schema  |          Name          |                            Definition                            | Ndistinct | Dependencies |   MCV   
+----------+------------------------+------------------------------------------------------------------+-----------+--------------+---------
+ public   | func_deps_stat         | (a * 2), upper(b), (c + 1::numeric) FROM functional_dependencies |           | defined      | 
+ public   | mcv_lists_arrays_stats | a, b, c FROM mcv_lists_arrays                                    |           |              | defined
+ public   | mcv_lists_bool_stats   | a, b, c FROM mcv_lists_bool                                      |           |              | defined
+ public   | mcv_lists_stats        | a, b, d FROM mcv_lists                                           |           |              | defined
+ public   | stts_1                 | a, b FROM stts_t1                                                | defined   |              | 
+ public   | stts_2                 | a, b FROM stts_t1                                                | defined   | defined      | 
+ public   | stts_3                 | a, b FROM stts_t1                                                | defined   | defined      | defined
+ public   | stts_4                 | b, c FROM stts_t2                                                | defined   | defined      | defined
+ public   | stts_hoge              | col1, col2, col3 FROM stts_t3                                    | defined   | defined      | defined
+ stts_s1  | stts_foo               | col1, col2 FROM stts_t3                                          | defined   | defined      | defined
+ stts_s2  | stts_yama              | col1, col3 FROM stts_t3                                          |           | defined      | defined
+ tststats | priv_test_stats        | a, b FROM priv_test_tbl                                          |           |              | defined
 (12 rows)
 
 \dX stts_?
@@ -3039,21 +3039,21 @@ set search_path to public, stts_s1, stts_s2, tststats;
 (1 row)
 
 \dX+
-                                                           List of extended statistics
-  Schema  |          Name          |                               Definition                               | Ndistinct | Dependencies |   MCV   
-----------+------------------------+------------------------------------------------------------------------+-----------+--------------+---------
- public   | func_deps_stat         | ((a * 2)), upper(b), ((c + (1)::numeric)) FROM functional_dependencies |           | defined      | 
- public   | mcv_lists_arrays_stats | a, b, c FROM mcv_lists_arrays                                          |           |              | defined
- public   | mcv_lists_bool_stats   | a, b, c FROM mcv_lists_bool                                            |           |              | defined
- public   | mcv_lists_stats        | a, b, d FROM mcv_lists                                                 |           |              | defined
- public   | stts_1                 | a, b FROM stts_t1                                                      | defined   |              | 
- public   | stts_2                 | a, b FROM stts_t1                                                      | defined   | defined      | 
- public   | stts_3                 | a, b FROM stts_t1                                                      | defined   | defined      | defined
- public   | stts_4                 | b, c FROM stts_t2                                                      | defined   | defined      | defined
- public   | stts_hoge              | col1, col2, col3 FROM stts_t3                                          | defined   | defined      | defined
- stts_s1  | stts_foo               | col1, col2 FROM stts_t3                                                | defined   | defined      | defined
- stts_s2  | stts_yama              | col1, col3 FROM stts_t3                                                |           | defined      | defined
- tststats | priv_test_stats        | a, b FROM priv_test_tbl                                                |           |              | defined
+                                                        List of extended statistics
+  Schema  |          Name          |                            Definition                            | Ndistinct | Dependencies |   MCV   
+----------+------------------------+------------------------------------------------------------------+-----------+--------------+---------
+ public   | func_deps_stat         | (a * 2), upper(b), (c + 1::numeric) FROM functional_dependencies |           | defined      | 
+ public   | mcv_lists_arrays_stats | a, b, c FROM mcv_lists_arrays                                    |           |              | defined
+ public   | mcv_lists_bool_stats   | a, b, c FROM mcv_lists_bool                                      |           |              | defined
+ public   | mcv_lists_stats        | a, b, d FROM mcv_lists                                           |           |              | defined
+ public   | stts_1                 | a, b FROM stts_t1                                                | defined   |              | 
+ public   | stts_2                 | a, b FROM stts_t1                                                | defined   | defined      | 
+ public   | stts_3                 | a, b FROM stts_t1                                                | defined   | defined      | defined
+ public   | stts_4                 | b, c FROM stts_t2                                                | defined   | defined      | defined
+ public   | stts_hoge              | col1, col2, col3 FROM stts_t3                                    | defined   | defined      | defined
+ stts_s1  | stts_foo               | col1, col2 FROM stts_t3                                          | defined   | defined      | defined
+ stts_s2  | stts_yama              | col1, col3 FROM stts_t3                                          |           | defined      | defined
+ tststats | priv_test_stats        | a, b FROM priv_test_tbl                                          |           |              | defined
 (12 rows)
 
 \dX+ stts_?
@@ -3082,36 +3082,36 @@ set search_path to public, stts_s1, stts_s2, tststats;
 
 set search_path to public, stts_s1;
 \dX
-                                                          List of extended statistics
- Schema  |          Name          |                               Definition                               | Ndistinct | Dependencies |   MCV   
----------+------------------------+------------------------------------------------------------------------+-----------+--------------+---------
- public  | func_deps_stat         | ((a * 2)), upper(b), ((c + (1)::numeric)) FROM functional_dependencies |           | defined      | 
- public  | mcv_lists_arrays_stats | a, b, c FROM mcv_lists_arrays                                          |           |              | defined
- public  | mcv_lists_bool_stats   | a, b, c FROM mcv_lists_bool                                            |           |              | defined
- public  | mcv_lists_stats        | a, b, d FROM mcv_lists                                                 |           |              | defined
- public  | stts_1                 | a, b FROM stts_t1                                                      | defined   |              | 
- public  | stts_2                 | a, b FROM stts_t1                                                      | defined   | defined      | 
- public  | stts_3                 | a, b FROM stts_t1                                                      | defined   | defined      | defined
- public  | stts_4                 | b, c FROM stts_t2                                                      | defined   | defined      | defined
- public  | stts_hoge              | col1, col2, col3 FROM stts_t3                                          | defined   | defined      | defined
- stts_s1 | stts_foo               | col1, col2 FROM stts_t3                                                | defined   | defined      | defined
+                                                       List of extended statistics
+ Schema  |          Name          |                            Definition                            | Ndistinct | Dependencies |   MCV   
+---------+------------------------+------------------------------------------------------------------+-----------+--------------+---------
+ public  | func_deps_stat         | (a * 2), upper(b), (c + 1::numeric) FROM functional_dependencies |           | defined      | 
+ public  | mcv_lists_arrays_stats | a, b, c FROM mcv_lists_arrays                                    |           |              | defined
+ public  | mcv_lists_bool_stats   | a, b, c FROM mcv_lists_bool                                      |           |              | defined
+ public  | mcv_lists_stats        | a, b, d FROM mcv_lists                                           |           |              | defined
+ public  | stts_1                 | a, b FROM stts_t1                                                | defined   |              | 
+ public  | stts_2                 | a, b FROM stts_t1                                                | defined   | defined      | 
+ public  | stts_3                 | a, b FROM stts_t1                                                | defined   | defined      | defined
+ public  | stts_4                 | b, c FROM stts_t2                                                | defined   | defined      | defined
+ public  | stts_hoge              | col1, col2, col3 FROM stts_t3                                    | defined   | defined      | defined
+ stts_s1 | stts_foo               | col1, col2 FROM stts_t3                                          | defined   | defined      | defined
 (10 rows)
 
 create role regress_stats_ext nosuperuser;
 set role regress_stats_ext;
 \dX
-                                                          List of extended statistics
- Schema |          Name          |                               Definition                               | Ndistinct | Dependencies |   MCV   
---------+------------------------+------------------------------------------------------------------------+-----------+--------------+---------
- public | func_deps_stat         | ((a * 2)), upper(b), ((c + (1)::numeric)) FROM functional_dependencies |           | defined      | 
- public | mcv_lists_arrays_stats | a, b, c FROM mcv_lists_arrays                                          |           |              | defined
- public | mcv_lists_bool_stats   | a, b, c FROM mcv_lists_bool                                            |           |              | defined
- public | mcv_lists_stats        | a, b, d FROM mcv_lists                                                 |           |              | defined
- public | stts_1                 | a, b FROM stts_t1                                                      | defined   |              | 
- public | stts_2                 | a, b FROM stts_t1                                                      | defined   | defined      | 
- public | stts_3                 | a, b FROM stts_t1                                                      | defined   | defined      | defined
- public | stts_4                 | b, c FROM stts_t2                                                      | defined   | defined      | defined
- public | stts_hoge              | col1, col2, col3 FROM stts_t3                                          | defined   | defined      | defined
+                                                       List of extended statistics
+ Schema |          Name          |                            Definition                            | Ndistinct | Dependencies |   MCV   
+--------+------------------------+------------------------------------------------------------------+-----------+--------------+---------
+ public | func_deps_stat         | (a * 2), upper(b), (c + 1::numeric) FROM functional_dependencies |           | defined      | 
+ public | mcv_lists_arrays_stats | a, b, c FROM mcv_lists_arrays                                    |           |              | defined
+ public | mcv_lists_bool_stats   | a, b, c FROM mcv_lists_bool                                      |           |              | defined
+ public | mcv_lists_stats        | a, b, d FROM mcv_lists                                           |           |              | defined
+ public | stts_1                 | a, b FROM stts_t1                                                | defined   |              | 
+ public | stts_2                 | a, b FROM stts_t1                                                | defined   | defined      | 
+ public | stts_3                 | a, b FROM stts_t1                                                | defined   | defined      | defined
+ public | stts_4                 | b, c FROM stts_t2                                                | defined   | defined      | defined
+ public | stts_hoge              | col1, col2, col3 FROM stts_t3                                    | defined   | defined      | defined
 (9 rows)
 
 reset role;

Original file line number	Diff line number	Diff line change
`@@ -1712,7 +1712,7 @@ pg_get_statisticsobj_worker(Oid statextid, bool columns_only, bool missing_ok)`
`1712`	`1712`	`{`
`1713`	`1713`	`Node expr = (Node ) lfirst(lc);`
`1714`	`1714`	`char *str;`
`1715`		`- int prettyFlags = PRETTYFLAG_INDENT;`
	`1715`	`+ int prettyFlags = PRETTYFLAG_PAREN;`
`1716`	`1716`
`1717`	`1717`	`str = deparse_expression_pretty(expr, context, false, false,`
`1718`	`1718`	`prettyFlags, 0);`