Re: [HACKERS] Create a separate test file for exercising system views

2017-01-29 Thread Ashutosh Bapat
On Mon, Jan 30, 2017 at 2:32 AM, Tom Lane  wrote:
> In connection with the "pg_hba_file_settings view patch" thread, I was
> wondering where we could logically insert a regression test case for that
> view.  I realized that there is no natural home for it among the existing
> regression tests, because it's not really connected to any SQL language
> feature.  The same is true for a number of other built-in views, and
> unsurprisingly, most of them are not exercised anywhere :-(.
>
> Accordingly, I propose creating a new regression test file whose charter
> is to exercise the SRFs underlying system views, as per attached.
> I don't think we desperately need new tests for views that expand to
> simple SQL, but these test cases correspond directly to code coverage
> for C functions, so they seem worthwhile.
>
> I did not do anything about testing the various pg_stat_xxx views.
> Those could be added later, or maybe they deserve their own home.
> (In many cases, those would need something smarter than the basic
> count(*) technique used here, because the C functions are invoked
> in the view's SELECT list not in FROM, so the planner would throw
> away those calls.)
>
> Comments/objections?
>

I think this is good in the given infrastructure. Thanks for working on it.
-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company


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


Re: [HACKERS] Create a separate test file for exercising system views

2017-01-29 Thread Michael Paquier
On Mon, Jan 30, 2017 at 6:02 AM, Tom Lane  wrote:
> In connection with the "pg_hba_file_settings view patch" thread, I was
> wondering where we could logically insert a regression test case for that
> view.  I realized that there is no natural home for it among the existing
> regression tests, because it's not really connected to any SQL language
> feature.  The same is true for a number of other built-in views, and
> unsurprisingly, most of them are not exercised anywhere :-(.
>
> Accordingly, I propose creating a new regression test file whose charter
> is to exercise the SRFs underlying system views, as per attached.
> I don't think we desperately need new tests for views that expand to
> simple SQL, but these test cases correspond directly to code coverage
> for C functions, so they seem worthwhile.
>
> I did not do anything about testing the various pg_stat_xxx views.
> Those could be added later, or maybe they deserve their own home.
> (In many cases, those would need something smarter than the basic
> count(*) technique used here, because the C functions are invoked
> in the view's SELECT list not in FROM, so the planner would throw
> away those calls.)
>
> Comments/objections?

Nice idea to group those tests in the same file. I am not noticing any
issues with the patch proposed.
-- 
Michael


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


Re: [HACKERS] Create a separate test file for exercising system views

2017-01-29 Thread Andres Freund
Hi,

On 2017-01-29 16:02:21 -0500, Tom Lane wrote:
> I did not do anything about testing the various pg_stat_xxx views.
> Those could be added later, or maybe they deserve their own home.
> (In many cases, those would need something smarter than the basic
> count(*) technique used here, because the C functions are invoked
> in the view's SELECT list not in FROM, so the planner would throw
> away those calls.)

I've previously wished there were a portable equivalent of \o /dev/null
that'd not be perfect, but it'd still exercise more than what we
currently have.  Alternatively casting the entire row to text should
allow to use count(*) trickery in some of the cases at least.

> Comments/objections?

Sounds like a good idea here.

Greetings,

Andres Freund


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


[HACKERS] Create a separate test file for exercising system views

2017-01-29 Thread Tom Lane
In connection with the "pg_hba_file_settings view patch" thread, I was
wondering where we could logically insert a regression test case for that
view.  I realized that there is no natural home for it among the existing
regression tests, because it's not really connected to any SQL language
feature.  The same is true for a number of other built-in views, and
unsurprisingly, most of them are not exercised anywhere :-(.

Accordingly, I propose creating a new regression test file whose charter
is to exercise the SRFs underlying system views, as per attached.
I don't think we desperately need new tests for views that expand to
simple SQL, but these test cases correspond directly to code coverage
for C functions, so they seem worthwhile.

I did not do anything about testing the various pg_stat_xxx views.
Those could be added later, or maybe they deserve their own home.
(In many cases, those would need something smarter than the basic
count(*) technique used here, because the C functions are invoked
in the view's SELECT list not in FROM, so the planner would throw
away those calls.)

Comments/objections?

regards, tom lane

diff --git a/src/test/regress/expected/rangefuncs.out b/src/test/regress/expected/rangefuncs.out
index 56481de..526a4ae 100644
*** a/src/test/regress/expected/rangefuncs.out
--- b/src/test/regress/expected/rangefuncs.out
***
*** 1,19 
- SELECT name, setting FROM pg_settings WHERE name LIKE 'enable%';
-  name | setting 
- --+-
-  enable_bitmapscan| on
-  enable_hashagg   | on
-  enable_hashjoin  | on
-  enable_indexonlyscan | on
-  enable_indexscan | on
-  enable_material  | on
-  enable_mergejoin | on
-  enable_nestloop  | on
-  enable_seqscan   | on
-  enable_sort  | on
-  enable_tidscan   | on
- (11 rows)
- 
  CREATE TABLE foo2(fooid int, f2 int);
  INSERT INTO foo2 VALUES(1, 11);
  INSERT INTO foo2 VALUES(2, 22);
--- 1,3 
diff --git a/src/test/regress/expected/sysviews.out b/src/test/regress/expected/sysviews.out
index ...852a7c3 .
*** a/src/test/regress/expected/sysviews.out
--- b/src/test/regress/expected/sysviews.out
***
*** 0 
--- 1,113 
+ --
+ -- Test assorted system views
+ --
+ -- This test is mainly meant to provide some code coverage for the
+ -- set-returning functions that underlie certain system views.
+ -- The output of most of these functions is very environment-dependent,
+ -- so our ability to test with fixed expected output is pretty limited;
+ -- but even a trivial check of count(*) will exercise the normal code path
+ -- through the SRF.
+ select count(*) >= 0 as ok from pg_available_extension_versions;
+  ok 
+ 
+  t
+ (1 row)
+ 
+ select count(*) >= 0 as ok from pg_available_extensions;
+  ok 
+ 
+  t
+ (1 row)
+ 
+ -- At introduction, pg_config had 23 entries; it may grow
+ select count(*) > 20 as ok from pg_config;
+  ok 
+ 
+  t
+ (1 row)
+ 
+ -- We expect no cursors in this test; see also portals.sql
+ select count(*) = 0 as ok from pg_cursors;
+  ok 
+ 
+  t
+ (1 row)
+ 
+ select count(*) >= 0 as ok from pg_file_settings;
+  ok 
+ 
+  t
+ (1 row)
+ 
+ -- There will surely be at least one active lock
+ select count(*) > 0 as ok from pg_locks;
+  ok 
+ 
+  t
+ (1 row)
+ 
+ -- We expect no prepared statements in this test; see also prepare.sql
+ select count(*) = 0 as ok from pg_prepared_statements;
+  ok 
+ 
+  t
+ (1 row)
+ 
+ -- See also prepared_xacts.sql
+ select count(*) >= 0 as ok from pg_prepared_xacts;
+  ok 
+ 
+  t
+ (1 row)
+ 
+ -- This is to record the prevailing planner enable_foo settings during
+ -- a regression test run.
+ select name, setting from pg_settings where name like 'enable%';
+  name | setting 
+ --+-
+  enable_bitmapscan| on
+  enable_hashagg   | on
+  enable_hashjoin  | on
+  enable_indexonlyscan | on
+  enable_indexscan | on
+  enable_material  | on
+  enable_mergejoin | on
+  enable_nestloop  | on
+  enable_seqscan   | on
+  enable_sort  | on
+  enable_tidscan   | on
+ (11 rows)
+ 
+ -- Test that the pg_timezone_names and pg_timezone_abbrevs views are
+ -- more-or-less working.  We can't test their contents in any great detail
+ -- without the outputs changing anytime IANA updates the underlying data,
+ -- but it seems reasonable to expect at least one entry per major meridian.
+ -- (At the time of writing, the actual counts are around 38 because of
+ -- zones using fractional GMT offsets, so this is a pretty loose test.)
+ select count(distinct utc_offset) >= 24 as ok from pg_timezone_names;
+  ok 
+ 
+  t
+ (1 row)
+ 
+ select count(distinct utc_offset) >= 24 as ok from pg_timezone_abbrevs;
+  ok 
+ 
+  t
+ (1 row)
+ 
+ -- Let's check the non-default timezone abbreviation sets, too
+ set timezone_abbreviations = 'Australia';
+ select count(distinct