Re: reprise: pretty print viewdefs - Mailing list pgsql-hackers

On Tue, Dec 27, 2011 at 8:02 AM, Andrew Dunstan <andrew@dunslane.net> wrote:
>
>
> Updated, with docs and tests. Since the docs mark the versions of
> pg_get_viewdef() that take text as the first param as deprecated, I removed
> that variant of the new flavor. I left adding extra psql support to another
> day - I think this already does a good job of cleaning it up without any
> extra adjustments.
>
> I'll add this to the upcoming commitfest.

I've looked at (actually tested with folks in reviewfest, so not so in
detail :P) the patch. It applies with some hunks and compiles cleanly,
documentation and tests are prepared. make install passed without
fails.

$ patch -p1 < ~/Downloads/viewdef.patch
patching file doc/src/sgml/func.sgml
Hunk #1 succeeded at 13736 (offset -22 lines).
Hunk #2 succeeded at 13747 (offset -22 lines).
patching file src/backend/utils/adt/ruleutils.c
patching file src/include/catalog/pg_proc.h
Hunk #1 succeeded at 3672 (offset -43 lines).
patching file src/include/utils/builtins.h
Hunk #1 succeeded at 604 (offset -20 lines).
patching file src/test/regress/expected/polymorphism.out
Hunk #1 succeeded at 1360 (offset -21 lines).
patching file src/test/regress/expected/rules.out
patching file src/test/regress/expected/with.out
patching file src/test/regress/sql/rules.sql

The change of the code is in only ruleutiles.c, which looks sane to
me, with some trailing white spaces. I wonder if it's worth working
more than on target list, namely like FROM clause, expressions.

For example, pg_get_viewdef('pg_stat_activity', true).

# select pg_get_viewdef('pg_stat_activity'::regclass, true);
                                  pg_get_viewdef

----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
SELECTs.datid, d.datname, s.procpid, s.usesysid, u.rolname AS
 
usename,
                 +    s.application_name, s.client_addr, s.client_hostname,
s.client_port,
                       +    s.backend_start, s.xact_start, s.query_start, s.waiting,
s.current_query
                    +   FROM pg_database d, pg_stat_get_activity(NULL::integer) s(datid,
procpid, usesysid, application_name, current_query, waiting,
xact_start, query_start, backend_start, client_addr, client_hostname,
client_port), pg_authid u+  WHERE s.datid = d.oid AND s.usesysid = u.oid;
(1 row)

It doesn't help much although the SELECT list is wrapped within 80
characters. For expressions, I mean:

=# select pg_get_viewdef('v', true);                                                pg_get_viewdef
----------------------------------------------------------------------------------------------------- SELECT a.a, a.a -
1AS b, a.a - 2 AS c, a.a - 3 AS d,                            +    a.a / 1 AS long_long_name, a.a * 1000 AS bcd,
                   +        CASE                            +            WHEN a.a = 1 THEN 1
+           WHEN (a.a + a.a + a.a + a.a + a.a + a.a + a.a + a.a + a.a
 
+ a.a + a.a + a.a) = 2 THEN 2+            ELSE 10                            +        END AS what_about_case_expression
                          +   FROM generate_series(1, 100) a(a);
 
(1 row)

So my conclusion is it's better than nothing, but we could do better
job here. From timeline perspective, it'd be ok to apply this patch
and improve more later in 9.3+.

Thanks,
-- 
Hitoshi Harada