Re: old_snapshot: add test for coverage
On Mon, Aug 8, 2022 at 2:37 PM Tom Lane wrote: > > Dong Wook Lee writes: > > I wrote a test of the old_snapshot extension for coverage. > > Hmm, does this really provide any meaningful coverage? The test > sure looks like it's not doing much. Previously written tests were simply test codes to increase coverage. Therefore, we can make a better test. I'll think about it by this week. If this work exceeds my ability, I will let you know by reply. It's okay that the issue should be closed unless I write a meaningful test. --- Regards, DongWook Lee.
Re: pg_basebackup: add test about zstd compress option
> The only thing that I can think of would be if $decompress_program > --test were failing, but actually trying to decompress succeeded. I > would be inclined to dismiss that particular scenario as not important > enough to be worth the additional CPU cycles. When I wrote this test, it was just to increase coverage for pg_basebackup. As I checked again, it already does that in the pg_verifybackup 008_untar.pl, 009_extrack.pl test you mentioned. Therefore, I agree with your opinion. --- Regards, DongWook Lee.
Re: Improve tab completion for ALTER FUNCTION/PROCEDURE/ROUTINE
On Fri, Oct 28, 2022 at 12:08 AM vignesh C wrote: > > Hi, > > Tab completion for ALTER FUNCTION/PROCEDURE/ROUTINE action was > missing, this patch adds the tab completion for the same. > > Regards, > Vignesh Hi, I applied your patch and did some tests. Is it okay not to consider SET and RESET commands? (e.g ALTER FUNCTION) --- Regards, DongWook Lee.
Re: pgstattuple: add test for coverage
> Which indeed is the case, e.g. on 32bit systems it fails: > > https://cirrus-ci.com/task/4619535222308864?logs=test_world_32#L253 > > https://api.cirrus-ci.com/v1/artifact/task/4619535222308864/testrun/build-32/testrun/pgstattuple/regress/regression.diffs > > table_len | tuple_count | tuple_len | tuple_percent | dead_tuple_count | > dead_tuple_len | dead_tuple_percent | free_space | free_percent > > ---+-+---+---+--++++-- > - 1171456 |5000 |56 | 47.8 | 5000 | > 56 | 47.8 | 7452 | 0.64 > + 1138688 |5000 |54 | 47.42 | 5000 | > 54 | 47.42 | 14796 | 1.3 > (1 row) > > ... > > > You definitely can't rely on such details not to change across platforms. Thank you for letting me know I'll fix it and check if there's any problem.
Re: add test to psql for coverage with --help, \e option, encoding option
I confirmed that I missed the patch file. And the current code is different from when I wrote the patch, so I don't think my patch will be meaningful anymore. On Sat, Sep 3, 2022 at 6:08 PM Dong Wook Lee wrote: > > Hi hackers, > I try to add to psql test about --help, \e, and the encoding option. > > --- > Regards, > DongWook Lee. v1_add_test_to_psql.patch Description: Binary data
add test to psql for coverage with --help, \e option, encoding option
Hi hackers, I try to add to psql test about --help, \e, and the encoding option. --- Regards, DongWook Lee.
vacuumlo: add test to vacuumlo for test coverage
Hi hackers, I write a tiny patch about vacuumlo to improve test coverage. I hope my work is meaningful. --- Regards, DongWook Lee. v1_add_test_to_vacuumlo.patch Description: Binary data
Re: add test: pg_rowlocks extension
On Fri, Sep 2, 2022 at 4:07 AM Tom Lane wrote: > Pushed with some revisions. Notably, I didn't see any point in > repeating each test case four times, so I trimmed it down to once > per case. I checked it. Thank you for correcting it in a better way.
pg_checksum: add test for coverage
Hi, I add a tiny test to pg_checksum for coverage. I checked it improve test coverage 77.9% -> 87.7%. --- Regards, DongWook Lee. v1_add_test_to_pg_checksum.patch Description: Binary data
Re: pg_basebackup: add test about zstd compress option
On Tue, Aug 23, 2022 at 11:37 AM Michael Paquier wrote:
> It seems to me that checking that the contents generated are valid is
> equally necessary. We do that with zlib with gzip --test, and you
> could use ${ZSTD} in the context of this test.
Thank you for the good points.
I supplemented the test according to your suggestion.
However, there was a problem.
Even though I did export ZSTD on the Makefile , the test runner can't
find ZSTD when it actually tests.
```
my $zstd = $ENV{ZSTD};
skip "program zstd is not found in your system", 1
if (!defined $zstd
|| $zstd eq '');
```
log: regress_log_010_pg_basebackup
```
ok 183 # skip program zstd is not found in your system.
```
Could you check if I missed anything?
v2_add_test_pg_basebackup.patch
Description: Binary data
pg_basebackup: add test about zstd compress option
Hi hackers, I checked the test code not to test the zstd option, then added it. I hope my patch will help us to ensure safety of the test. --- Regards, DongWook Lee. v1_add_test_pg_basebackup.patch Description: Binary data
pg_waldump: add test for coverage
Hi Hackers, I wrote a test for coverage. Unfortunately, it seems to take quite a while to run the test. I want to improve these execution times, but I don't know exactly what to do. Therefore, I want to hear feedback from many people. --- Regards, Dong Wook Lee v1_add_test_pg_waldump.patch Description: Binary data
xml2: add test for coverage
Hi, hackers I made a small patch for xml2 to improve test coverage. However, there was a problem using the functions below. - xpath_number - xpath_bool - xpath_nodeset - xpath_list Do you have any advice on how to use this function correctly? It would also be good to add an example of using the function to the document. --- Regards, DongWook Lee. v1_add_test_xml2.patch Description: Binary data
old_snapshot: add test for coverage
Hi hackers, I wrote a test of the old_snapshot extension for coverage. I hope that this is written correctly. before: 0% after: 100% --- regards, Lee Dong Wook. v1_add_test_oldsnapshot.patch Description: Binary data
Re: pgstattuple: add test for coverage
Tom Lane writes: > I do not think it's a great idea to create random dependencies > between modules like the pgstattuple -> bloom dependency you > casually added here. I agree with your option. Is there no problem with selecting all the columns during SELECT statements? I thought there might be a problem where the test results could change easily. --- regards Lee Dong Wook. v2_add_test_pgstattuple.patch Description: Binary data
Re: add test: pg_rowlocks extension
I modified my previous patch by reflecting the feedback. and I wrote most of the queries for the test after looking at the file below. - ref: (https://github.com/postgres/postgres/blob/master/src/test/isolation/specs/tuplelock-conflict.spec) The coverage of the test is approximately 81.5%. If there is any problem, I would appreciate it if you let me know anytime. Thank you always for your kind reply. v2_add_test_pgrowlocks.patch Description: Binary data
Re: add test: pg_rowlocks extension
2022년 7월 31일 (일) 오전 6:32, Tom Lane 님이 작성: > > Dong Wook Lee writes: > > I just wrote test about pg_rowlocks extension. > > I added sql and spec test for locking state. > > I think this could be cut down quite a bit. Do we really need > both a SQL test and an isolation test? Seems like you could > easily do everything in the isolation test. I agree with your optionion. > Also, it is not a good idea to go creating superusers in a contrib > test: we support "make installcheck" for these tests, but people don't > especially like new superusers cropping up in their installations. > I doubt that we need *any* of the permissions-ish tests that you > propose adding here; those are not part of the module's own > functionality, and we don't generally have similar tests in other > contrib modules. I agree it's right to remove that part. > If you do keep any of it, remember to drop the roles you create --- > leaving global objects behind is not OK. (For one thing, it > breaks doing repeat "make installcheck"s.) > > Another thing that's bad style is the "drop table if exists". > This should be running in an empty database, and if somehow it's > not, destroying pre-existing objects would be pretty unfriendly. > Better to fail at the CREATE. Thank you for the good explanation. It will be very helpful to write a test in the future. > See also my comments about your pg_buffercache patch, which > largely apply here too. OK. I will add the `.gitignore` file. I will revise my patch and submit it again as soon as possible.
pgstattuple: add test for coverage
Hi, hackers I added some SQL statements to improve test coverage. As data was inserted, the expected file changed. So should I change all `select *` for a stable expected result? And it's the coverage change as I add 50.6% -> 78.7% --- regards, Lee Dong Wook v1_add_test_pgstattuple.patch Description: Binary data
Re: pg_buffercache: add sql test
I've been missing what I have to add to the .gitignore file when I write the test. I will refer to it when I write the test code from now on. Thank you. 2022년 7월 31일 (일) 오전 4:39, Tom Lane 님이 작성: > > Dong Wook Lee writes: > > I just wrote a test code for the `pg_buffercache` extension which > > doesn't not have test code. > > Pushed with minor adjustments. Some notes: > > * A .gitignore file is needed so that "git status" won't whine after > running the test. This tends to be pretty much boilerplate; I copied > it from another contrib directory. > > * Pay attention to "git diff --check" formatting warnings. In this > case it bleated about an extra blank line at the end of the .sql file. > > * I didn't care for the direct use of pg_show_all_settings(). The > official API there is the pg_settings view, and there's no need for > this test to get friendly with the view's internals. > > Thanks for the patch! > > regards, tom lane
Re: Add test of pg_prewarm extenion
2022년 8월 1일 (월) 오후 11:33, Dong Wook Lee 님이 작성: > > Thank you for letting me know. > I edited my patch with `wait_for_log()`. > > 2022년 8월 1일 (월) 오후 6:55, Julien Rouhaud 님이 작성: > > > > On Mon, Aug 1, 2022 at 5:16 PM Dong Wook Lee wrote: > > > > > > > Keep in mind also that the logfile accumulates over stops and > > > > restarts. As you've coded this test, you don't know which DB start > > > > emitted the matching line, so the test proves a lot less than it > > > > ought to. > > > > > > > > I wonder also about race conditions. On fast machines, or those > > > > with weird schedulers, the test script might reach slurp_file > > > > before autoprewarm has had a chance to emit the log entry you want. > > > > > > I have no idea how to deal with race conditions. > > > Does anybody know how to deal with this? > > > > Couldn't you use $node->wait_for_log() instead? Please forgive my carelessness. After trimming the code a little more, I sent the patch again. v4_add_test_pg_prewarm.patch Description: Binary data
Re: Add test of pg_prewarm extenion
Thank you for letting me know. I edited my patch with `wait_for_log()`. 2022년 8월 1일 (월) 오후 6:55, Julien Rouhaud 님이 작성: > > On Mon, Aug 1, 2022 at 5:16 PM Dong Wook Lee wrote: > > > > > Keep in mind also that the logfile accumulates over stops and > > > restarts. As you've coded this test, you don't know which DB start > > > emitted the matching line, so the test proves a lot less than it > > > ought to. > > > > > > I wonder also about race conditions. On fast machines, or those > > > with weird schedulers, the test script might reach slurp_file > > > before autoprewarm has had a chance to emit the log entry you want. > > > > I have no idea how to deal with race conditions. > > Does anybody know how to deal with this? > > Couldn't you use $node->wait_for_log() instead? v3_add_test_pg_prewarm.patch Description: Binary data
pg_waldump got an error with waldump file generated by initdb
Hi, hackers I don't know if this is an error. when I do ./initdb -D ../data and execute pg_waldump like this, In the last part I got an error. ``` ./pg_waldump ../data/pg_wal/00010001 ``` pg_waldump: error: error in WAL record at 0/140: invalid record length at 0/1499A08: wanted 24, got 0 my environment is `16devel on x86_64-pc-linux-gnu, compiled by gcc (Ubuntu 9.4.0-1ubuntu1~20.04.1) 9.4.0, 64-bit` Is this normal?
Re: doc: pg_prewarm add configuration example
On 22/06/29 03:57오후, Jacob Champion wrote: > On 6/18/22 01:55, Dong Wook Lee wrote: > > Hi hackers, > > > > I thought it would be nice to have an configuration example of the > > pg_prewarm extension. > > Therefore, I have written an example of a basic configuration. > > [offlist] > > Hi Dong Wook, I saw a commitfest entry registered for some of your other > patches, but not for this one. Quick reminder to register it in the > Documentation section if that was your intent. Thank you very much for letting me know. The patch is so trivial that I thought about whether to post it on commitfest, but I think it would be better to post it. > > Thanks, > --Jacob > > [NOTE] This is part of a mass communication prior to the July > commitfest, which begins in two days. If I've made a mistake -- for > example, if the patch has already been registered, withdrawn, or > committed -- just let me know, and sorry for the noise.
add test: pg_rowlocks extension
Hi Hackers,
I just wrote test about pg_rowlocks extension.
I added sql and spec test for locking state.
---
Regards
DongWook Lee
diff --git a/contrib/pgrowlocks/Makefile b/contrib/pgrowlocks/Makefile
index 294c05dd0f..128f345b7e 100644
--- a/contrib/pgrowlocks/Makefile
+++ b/contrib/pgrowlocks/Makefile
@@ -9,6 +9,9 @@ EXTENSION = pgrowlocks
DATA = pgrowlocks--1.2.sql pgrowlocks--1.1--1.2.sql pgrowlocks--1.0--1.1.sql
PGFILEDESC = "pgrowlocks - display row locking information"
+REGRESS = pgrowlocks
+ISOLATION = show_rowlocks_info
+
ifdef USE_PGXS
PG_CONFIG = pg_config
PGXS := $(shell $(PG_CONFIG) --pgxs)
diff --git a/contrib/pgrowlocks/expected/pgrowlocks.out b/contrib/pgrowlocks/expected/pgrowlocks.out
new file mode 100644
index 00..9daf600e50
--- /dev/null
+++ b/contrib/pgrowlocks/expected/pgrowlocks.out
@@ -0,0 +1,28 @@
+--
+-- Test pgrowlocks extension
+--
+CREATE EXTENSION pgrowlocks;
+-- set up
+CREATE TABLE test(c int);
+-- check permission to use pgrowlocks extension;
+CREATE ROLE regress_priv_user1 superuser login;
+CREATE ROLE regress_priv_user2;
+CREATE ROLE regress_priv_user3;
+SET SESSION AUTHORIZATION regress_priv_user1;
+SELECT locked_row, locker, multi, xids, modes FROM pgrowlocks('test'); -- ok
+ locked_row | locker | multi | xids | modes
+++---+--+---
+(0 rows)
+
+SET SESSION AUTHORIZATION regress_priv_user2;
+SELECT locked_row, locker, multi, xids, modes FROM pgrowlocks('test'); -- fail
+ERROR: permission denied for table test
+-- switch to superuser
+\c -
+GRANT SELECT ON test TO regress_priv_user3;
+SET SESSION AUTHORIZATION regress_priv_user3;
+SELECT locked_row, locker, multi, xids, modes FROM pgrowlocks('test'); -- ok
+ locked_row | locker | multi | xids | modes
+++---+--+---
+(0 rows)
+
diff --git a/contrib/pgrowlocks/expected/show_rowlocks_info.out b/contrib/pgrowlocks/expected/show_rowlocks_info.out
new file mode 100644
index 00..7f06f55785
--- /dev/null
+++ b/contrib/pgrowlocks/expected/show_rowlocks_info.out
@@ -0,0 +1,17 @@
+Parsed test spec with 2 sessions
+
+starting permutation: s1b s1u1 s2_rowlocks s1c s2_rowlocks
+step s1b: BEGIN;
+step s1u1: UPDATE t1 SET c1 = 2 WHERE c1 = 1;
+step s2_rowlocks: select locked_row, multi, modes from pgrowlocks('t1');
+locked_row|multi|modes
+--+-+-
+(0,1) |f|{"No Key Update"}
+(1 row)
+
+step s1c: COMMIT;
+step s2_rowlocks: select locked_row, multi, modes from pgrowlocks('t1');
+locked_row|multi|modes
+--+-+-
+(0 rows)
+
diff --git a/contrib/pgrowlocks/output_iso/results/show_rowlocks_info.out b/contrib/pgrowlocks/output_iso/results/show_rowlocks_info.out
new file mode 100644
index 00..7f06f55785
--- /dev/null
+++ b/contrib/pgrowlocks/output_iso/results/show_rowlocks_info.out
@@ -0,0 +1,17 @@
+Parsed test spec with 2 sessions
+
+starting permutation: s1b s1u1 s2_rowlocks s1c s2_rowlocks
+step s1b: BEGIN;
+step s1u1: UPDATE t1 SET c1 = 2 WHERE c1 = 1;
+step s2_rowlocks: select locked_row, multi, modes from pgrowlocks('t1');
+locked_row|multi|modes
+--+-+-
+(0,1) |f|{"No Key Update"}
+(1 row)
+
+step s1c: COMMIT;
+step s2_rowlocks: select locked_row, multi, modes from pgrowlocks('t1');
+locked_row|multi|modes
+--+-+-
+(0 rows)
+
diff --git a/contrib/pgrowlocks/results/pgrowlocks.out b/contrib/pgrowlocks/results/pgrowlocks.out
new file mode 100644
index 00..9daf600e50
--- /dev/null
+++ b/contrib/pgrowlocks/results/pgrowlocks.out
@@ -0,0 +1,28 @@
+--
+-- Test pgrowlocks extension
+--
+CREATE EXTENSION pgrowlocks;
+-- set up
+CREATE TABLE test(c int);
+-- check permission to use pgrowlocks extension;
+CREATE ROLE regress_priv_user1 superuser login;
+CREATE ROLE regress_priv_user2;
+CREATE ROLE regress_priv_user3;
+SET SESSION AUTHORIZATION regress_priv_user1;
+SELECT locked_row, locker, multi, xids, modes FROM pgrowlocks('test'); -- ok
+ locked_row | locker | multi | xids | modes
+++---+--+---
+(0 rows)
+
+SET SESSION AUTHORIZATION regress_priv_user2;
+SELECT locked_row, locker, multi, xids, modes FROM pgrowlocks('test'); -- fail
+ERROR: permission denied for table test
+-- switch to superuser
+\c -
+GRANT SELECT ON test TO regress_priv_user3;
+SET SESSION AUTHORIZATION regress_priv_user3;
+SELECT locked_row, locker, multi, xids, modes FROM pgrowlocks('test'); -- ok
+ locked_row | locker | multi | xids | modes
+++---+--+---
+(0 rows)
+
diff --git a/contrib/pgrowlocks/specs/show_rowlocks_info.spec b/contrib/pgrowlocks/specs/show_rowlocks_info.spec
new file mode 100644
index 00..0d5d629702
--- /dev/null
+++ b/contrib/pgrowlocks/specs/show_rowlocks_info.spec
@@ -0,0 +1,27 @@
+setup
+{ CREATE EXTENSION pgrowlocks;
+
+DROP TABLE IF EXISTS t1;
+CREATE TABLE t1(c1 integer);
+
+INSERT INTO t1 (c1) VALUES (1);
+}
+
+teardown
+{
+
Add test of pg_prewarm extenion
Hi hackers,
I wrote a test for pg_prewarm extension. and I wrote it with the aim of
improving test coverage, and feedback is always welcome.
---
Regards
DongWook Lee
diff --git a/contrib/pg_prewarm/Makefile b/contrib/pg_prewarm/Makefile
index b13ac3c813..617ac8e09b 100644
--- a/contrib/pg_prewarm/Makefile
+++ b/contrib/pg_prewarm/Makefile
@@ -10,6 +10,10 @@ EXTENSION = pg_prewarm
DATA = pg_prewarm--1.1--1.2.sql pg_prewarm--1.1.sql pg_prewarm--1.0--1.1.sql
PGFILEDESC = "pg_prewarm - preload relation data into system buffer cache"
+REGRESS = pg_prewarm
+
+TAP_TESTS = 1
+
ifdef USE_PGXS
PG_CONFIG = pg_config
PGXS := $(shell $(PG_CONFIG) --pgxs)
diff --git a/contrib/pg_prewarm/expected/pg_prewarm.out b/contrib/pg_prewarm/expected/pg_prewarm.out
new file mode 100644
index 00..325e545cee
--- /dev/null
+++ b/contrib/pg_prewarm/expected/pg_prewarm.out
@@ -0,0 +1,35 @@
+--
+-- Test pg_prewarm extension
+--
+CREATE EXTENSION pg_prewarm;
+CREATE TABLE t1 (c1 integer);
+INSERT INTO t1 VALUES (1);
+INSERT INTO t1 VALUES (2);
+INSERT INTO t1 VALUES (3);
+--
+-- prefetch mode
+--
+SELECT pg_prewarm('t1', 'prefetch');
+ pg_prewarm
+
+ 1
+(1 row)
+
+--
+-- read mode
+--
+SELECT pg_prewarm('t1', 'read');
+ pg_prewarm
+
+ 1
+(1 row)
+
+--
+-- buffer_mode
+--
+SELECT pg_prewarm('t1', 'buffer');
+ pg_prewarm
+
+ 1
+(1 row)
+
diff --git a/contrib/pg_prewarm/sql/pg_prewarm.sql b/contrib/pg_prewarm/sql/pg_prewarm.sql
new file mode 100644
index 00..29646f5835
--- /dev/null
+++ b/contrib/pg_prewarm/sql/pg_prewarm.sql
@@ -0,0 +1,28 @@
+--
+-- Test pg_prewarm extension
+--
+
+CREATE EXTENSION pg_prewarm;
+
+CREATE TABLE t1 (c1 integer);
+
+INSERT INTO t1 VALUES (1);
+INSERT INTO t1 VALUES (2);
+INSERT INTO t1 VALUES (3);
+
+
+--
+-- prefetch mode
+--
+SELECT pg_prewarm('t1', 'prefetch');
+
+--
+-- read mode
+--
+SELECT pg_prewarm('t1', 'read');
+
+--
+-- buffer_mode
+--
+SELECT pg_prewarm('t1', 'buffer');
+
diff --git a/contrib/pg_prewarm/t/001_basic.pl b/contrib/pg_prewarm/t/001_basic.pl
new file mode 100644
index 00..3fcb438d07
--- /dev/null
+++ b/contrib/pg_prewarm/t/001_basic.pl
@@ -0,0 +1,34 @@
+
+# Copyright (c) 2021-2022, PostgreSQL Global Development Group
+
+use strict;
+use warnings;
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+
+my $node = PostgreSQL::Test::Cluster->new('main');
+
+$node->init;
+$node->append_conf('postgresql.conf',
+qq{shared_preload_libraries = 'pg_prewarm'
+pg_prewarm.autoprewarm = true
+pg_prewarm.autoprewarm_interval = 0});
+$node->start;
+$node->stop;
+$node->append_conf('postgresql.conf',
+qq{shared_preload_libraries = 'pg_prewarm'
+pg_prewarm.autoprewarm = true
+pg_prewarm.autoprewarm_interval = 1s});
+
+$node->restart;
+
+my $logfile = PostgreSQL::Test::Utils::slurp_file($node->logfile);
+
+ok ($logfile =~
+qr/autoprewarm successfully prewarmed 0 of 1 previously-loaded blocks/);
+
+done_testing();
+
Re: Add TAP test for auth_delay extension
On 22/06/18 12:07오후, Michael Paquier wrote:
> On Sat, Jun 18, 2022 at 11:06:02AM +0900, Dong Wook Lee wrote:
> > I have written a test for the auth_delay extension before,
> > but if it is okay, can you review it?
>
> +# check enter wrong password
> +my $t0 = [gettimeofday];
> +test_login($node, 'user_role', "wrongpass", 2);
> +my $elapsed = tv_interval($t0, [gettimeofday]);
> +ok($elapsed >= $delay_milliseconds / 1000, "auth_delay $elapsed seconds");
> +
> +# check enter correct password
> +my $t0 = [gettimeofday];
> +test_login($node, 'user_role', "pass", 0);
> +my $elapsed = tv_interval($t0, [gettimeofday]);
> +ok($elapsed < $delay_milliseconds / 1000, "auth_delay $elapsed seconds");
>
> On a slow machine, I suspect that the second test is going to be
> unstable as it would fail if the login attempt (that succeeds) takes
> more than $delay_milliseconds. You could increase more
> delay_milliseconds to leverage that, but it would make the first test
> slower for nothing on faster machines in the case where the
> authentication attempt has failed. I guess that you could leverage
> that by using a large value for delay_milliseconds in the second test,
> because we are never going to wait. For the first test, you could on
> the contrary use a much lower value, still on slow machines it may not
> test what the code path of auth_delay you are willing to test.
>
Thank you for your valuable advice I didn't think about the slow system.
Therefore, in the case of the second test, the time was extended a little.
> As a whole, I am not sure that this is really worth spending cycles on
> when running check-world or similar, and the code of the extension is
> trivial.
Even though it is trivial, I think it would be better if there was a test.
> --
> Michael
diff --git a/contrib/auth_delay/Makefile b/contrib/auth_delay/Makefile
index 4b86ec37f0..b65097789a 100644
--- a/contrib/auth_delay/Makefile
+++ b/contrib/auth_delay/Makefile
@@ -3,6 +3,8 @@
MODULES = auth_delay
PGFILEDESC = "auth_delay - delay authentication failure reports"
+TAP_TESTS = 1
+
ifdef USE_PGXS
PG_CONFIG = pg_config
PGXS := $(shell $(PG_CONFIG) --pgxs)
diff --git a/contrib/auth_delay/t/001_auth_delay.pl b/contrib/auth_delay/t/001_auth_delay.pl
new file mode 100644
index 00..644026e4f2
--- /dev/null
+++ b/contrib/auth_delay/t/001_auth_delay.pl
@@ -0,0 +1,87 @@
+
+# Copyright (c) 2021-2022, PostgreSQL Global Development Group
+
+use strict;
+use warnings;
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+use Time::HiRes qw(gettimeofday tv_interval);
+
+# Delete pg_hba.conf from the given node, add a new entry to it
+# and then execute a reload to refresh it.
+sub reset_pg_hba
+{
+my $node = shift;
+my $hba_method = shift;
+
+unlink($node->data_dir . '/pg_hba.conf');
+# just for testing purposes, use a continuation line
+$node->append_conf('pg_hba.conf', "local all all $hba_method");
+$node->reload;
+return;
+}
+
+# Test access for a single role, with password
+sub test_login
+{
+local $Test::Builder::Level = $Test::Builder::Level + 1;
+
+my $node = shift;
+my $role = shift;
+my $password = shift;
+my $expected_res = shift;
+my $status_string = 'failed';
+
+$status_string = 'success' if ($expected_res eq 0);
+
+my $connstr = "user=$role";
+my $testname =
+"authentication $status_string for role $role with password $password";
+
+$ENV{"PGPASSWORD"} = $password;
+
+if ($expected_res eq 0)
+{
+$node->connect_ok($connstr, $testname);
+}
+else
+{
+# No checks of the error message, only the status code.
+$node->connect_fails($connstr, $testname);
+}
+}
+
+note "setting up PostgreSQL instance";
+
+my $delay_milliseconds = 500;
+
+my $node = PostgreSQL::Test::Cluster->new('main');
+$node->init;
+$node->append_conf('postgresql.conf',
+qq{shared_preload_libraries = 'auth_delay'
+ auth_delay.milliseconds = '$delay_milliseconds'});
+$node->start;
+
+$node->safe_psql(
+'postgres',
+"CREATE ROLE user_role LOGIN PASSWORD 'pass';");
+reset_pg_hba($node, 'password');
+
+note "running tests";
+
+# check enter wrong password
+my $t0 = [gettimeofday];
+test_login($node, 'user_role', "wrongpass", 2);
+my $elapsed = tv_interval($t0, [gettimeofday]);
+ok($elapsed >= $delay_milliseconds / 1000, "auth_delay $elapsed seconds");
+
+# check enter correct password
+my $t0 = [gettimeofday];
+test_login($node, 'user_role', "pass", 0);
+my $elapsed = tv_interval($t0, [gettimeofday]);
+ok($elapsed < $delay_milliseconds / 1000, "auth_delay $elapsed seconds");
+
+done_testing();
+
doc: pg_prewarm add configuration example
Hi hackers, I thought it would be nice to have an configuration example of the pg_prewarm extension. Therefore, I have written an example of a basic configuration. --- Regards, DongWook Lee diff --git a/doc/src/sgml/pgprewarm.sgml b/doc/src/sgml/pgprewarm.sgml index 4e68812250..754a21a076 100644 --- a/doc/src/sgml/pgprewarm.sgml +++ b/doc/src/sgml/pgprewarm.sgml @@ -121,6 +121,19 @@ autoprewarm_dump_now() RETURNS int8 + + These parameters set in postgresql.conf. + Typical usage might be: + + + +# postgresql.conf +shared_preload_libraries = 'pg_prewarm' + +pg_prewarm.autoprewarm = true +pg_prewarm.autoprewarm_interval = 300s + +
Re: Add TAP test for auth_delay extension
2022년 6월 7일 (화) 오후 6:32, Dong Wook Lee 님이 작성: > > Hi Hackers, > I just wrote a test for `auth_delay` extension. > It's a test which confirms whether there is a delay for a second when > you enter the wrong password. > I sent an email using mutt, but I have a problem and sent it again. > > --- > Regards, > Dong Wook Lee. Hi, I have written a test for the auth_delay extension before, but if it is okay, can you review it? --- Regards, DongWook Lee.
Re: Add TAP test for auth_delay extension
Hi Hackers, I just wrote a test for `auth_delay` extension. It's a test which confirms whether there is a delay for a second when you enter the wrong password. I sent an email using mutt, but I have a problem and sent it again. --- Regards, Dong Wook Lee.
Re: pg_buffercache: add sql test
I removed it on your advice. Thanks. 2022년 6월 7일 (화) 오전 2:04, Stephen Frost 님이 작성: > > Greetings, > > * Daniel Gustafsson (dan...@yesql.se) wrote: > > > On 6 Jun 2022, at 15:30, Dong Wook Lee wrote: > > > > > I just wrote a test code for the `pg_buffercache` extension which > > > doesn't not have test code. > > > > Please add this patch to the next commitfest to make sure it's not lost > > before > > then. > > > > https://commitfest.postgresql.org/38/ > > Seems to be there now, at least: > > https://commitfest.postgresql.org/38/3674/ > > However, I don't think we should have a 'target version' set for this > (and in particular it shouldn't be 15). I'd suggest removing that. > > Thanks, > > Stephen
pg_buffercache: add sql test
Hi, hackers I just wrote a test code for the `pg_buffercache` extension which doesn't not have test code. I wrote the sql query to ensure that the buffer cache results are the same when `make installcheck` is performed. --- regards Lee Dong Wook. 0001_add_test_pg_buffercache.patch Description: Binary data
Re: Add pg_freespacemap extension sql test
2022년 3월 20일 (일) 03:13, Fabrízio de Royes Mello 님이 작성: > > > On Sat, Mar 19, 2022 at 1:18 PM Dong Wook Lee wrote: > > > > > Well, my guess is that you basically just care about being able to > > > detect if there is free space in the map or not, which goes down to > > > detecting if pg_freespace() returns 0 or a number strictly higher than > > > 0, so wouldn't it be enough to stick some > 0 in your test queries? > > > > I edited the previous patch file. > > Am I correct in understanding that? > > > > I think what Michael meant is something like attached. > > Regards, > > -- > Fabrízio de Royes Mello > I think you’re right, thank you for sending it instead of me. >
Re: Add pg_freespacemap extension sql test
> Well, my guess is that you basically just care about being able to > detect if there is free space in the map or not, which goes down to > detecting if pg_freespace() returns 0 or a number strictly higher than > 0, so wouldn't it be enough to stick some > 0 in your test queries? I edited the previous patch file. Am I correct in understanding that? 0002_add_test_pg_fsm.patch Description: Binary data
Re: Add pg_freespacemap extension sql test
2022년 3월 11일 (금) 오후 2:51, Michael Paquier 님이 작성: > > On Wed, Mar 09, 2022 at 08:13:15PM +0900, Dong Wook Lee wrote: > > I agree with you, but I have no good idea how to deal with it. > > Well, my guess is that you basically just care about being able to > detect if there is free space in the map or not, which goes down to > detecting if pg_freespace() returns 0 or a number strictly higher than > 0, so wouldn't it be enough to stick some > 0 in your test queries? > Btw, if you want to test 32-bit builds, gcc allows that by passing > down -m32. > > > Can the Perl TAP test be a good way? > > That does not seem necessary here. > -- > Michael so, you mean it's not necessary to add cases for negative numbers or beyond the range? I just wrote down testable cases, and if it doesn't have a big advantage, I don't mind not adding that case.
Re: Add pg_freespacemap extension sql test
2022년 3월 9일 (수) 오전 1:19, Tom Lane 님이 작성: > Dong Wook Lee writes: > > [ 0001_add_test_pg_fsm.patch ] > > I think having some coverage here would be great, but I'm concerned that > this patch doesn't look very portable. Aren't the numbers liable to > change on 32-bit machines, in particular? > > regards, tom lane > I agree with you, but I have no good idea how to deal with it. Can the Perl TAP test be a good way? Thought?
Re: Add pg_freespacemap extension sql test
That's right, so I attached the correct file again. 2022년 3월 8일 (화) 오후 11:45, Justin Pryzby 님이 작성: > On Tue, Mar 08, 2022 at 11:39:08PM +0900, Dong Wook Lee wrote: > > Hi, > > I just added some tests for the pg_freespacemap extension because the > test > > coverage was 0 percent. > > But I don't know if I did it correctly. > > The patch only touches doc/*.sgml. > I suppose you forgot to use "git add". > > -- > Justin >
Re: Add pg_freespacemap extension sql test
I'm sorry for attaching the wrong patch file. 2022년 3월 8일 (화) 오후 11:39, Dong Wook Lee 님이 작성: > Hi, > I just added some tests for the pg_freespacemap extension because the test > coverage was 0 percent. > But I don't know if I did it correctly. > > --- > Regards > Lee Dong Wook > 0001_add_test_pg_fsm.patch Description: Binary data
Add pg_freespacemap extension sql test
Hi, I just added some tests for the pg_freespacemap extension because the test coverage was 0 percent. But I don't know if I did it correctly. --- Regards Lee Dong Wook 0001_add_link_fsm.patch Description: Binary data
pg_stat_statements: remove redundant function call in pg_stat_statements_internal
Hi, I found some redundant function calls in pg_stat_statements.c/pg_stat_statements_internal(), There is no need to call GetUserId() again because the value was previously obtained. so I propose a patch to fix it. --- a/contrib/pg_stat_statements/pg_stat_statements.c +++ b/contrib/pg_stat_statements/pg_stat_statements.c @@ -1508,7 +1508,7 @@ pg_stat_statements_internal(FunctionCallInfo fcinfo, pgssEntry *entry; /* Superusers or members of pg_read_all_stats members are allowed */ - is_allowed_role = is_member_of_role(GetUserId(), ROLE_PG_READ_ALL_STATS); + is_allowed_role = is_member_of_role(userid, ROLE_PG_READ_ALL_STATS); /* hash table must exist already */ if (!pgss || !pgss_hash) Regards, Lee Dong Wook. v1_tiny_improvement_pg_stat_statements.patch Description: Binary data
Re: Print warning when I execute my own extension function
now I found a bug in this code. I found the code of this part below is wrong ``` char *buffer = palloc(7 * sizeof(char)); >> unsigned int offset = sizeof(buffer); // wrong buffer[--offset] = '\0'; /* here */ ``` Thank you for replying 2022년 2월 20일 (일) 오후 11:51, Dong Wook Lee 님이 작성: > > I found a source code line that prints warning in the source code > (src/backend/utils/mmgr/aset.c, line 1496) > /* > │ 1492 * Check for overwrite of > padding space in an allocated chunk. > │ 1493 */ > │ 1494if (chunk->aset == (void *) > set && dsize < chsize && > │ 1495!sentinel_ok(chunk, > ALLOC_CHUNKHDRSZ + dsize)) > │B+>1496elog(WARNING, "problem > in alloc set %s: detected write past chunk end in block %p, chunk %p", > │ 1497 name, block, chunk); > > In my extension c code. > I assigned value at the allocated memory address > and I found this assigned statement occurs warning > should I don't use it like this? > ``` > char *buffer = palloc(7 * sizeof(char)); > unsigned int offset = sizeof(buffer); > > buffer[--offset] = '\0'; /* here */ > ``` > > > > 2022년 2월 20일 (일) 오후 7:29, Julien Rouhaud 님이 작성: > > > > Hi, > > > > On Sun, Feb 20, 2022 at 07:23:56PM +0900, Dong Wook Lee wrote: > > > Hi hackers, > > > I've read in this blog ( > > > http://big-elephants.com/2015-10/writing-postgres-extensions-part-i/) > > > and I wrote an extension about base36_encode with c code > > > but when I executed a query like this below I got a warning below. > > > > > > ``` > > > postgres=# SELECT base36_encode(123); > > > WARNING: problem in alloc set ExprContext: detected write past chunk end > > > in block 0x55fb75334d40, chunk 0x55fb75334d68 > > > WARNING: problem in alloc set ExprContext: detected write past chunk end > > > in block 0x55fb75334d40, chunk 0x55fb75334d68 > > > base36_encode > > > --- > > > 3f > > > (1 row) > > > ``` > > > > > > I don't know what this warning means and how I can fix it. > > > > It means that you have some problem in your memory allocation. You can > > refer > > to src/backend/utils/mmgr/aset.c for more details on those safety checks.
Re: Print warning when I execute my own extension function
I found a source code line that prints warning in the source code (src/backend/utils/mmgr/aset.c, line 1496) /* │ 1492 * Check for overwrite of padding space in an allocated chunk. │ 1493 */ │ 1494if (chunk->aset == (void *) set && dsize < chsize && │ 1495!sentinel_ok(chunk, ALLOC_CHUNKHDRSZ + dsize)) │B+>1496elog(WARNING, "problem in alloc set %s: detected write past chunk end in block %p, chunk %p", │ 1497 name, block, chunk); In my extension c code. I assigned value at the allocated memory address and I found this assigned statement occurs warning should I don't use it like this? ``` char *buffer = palloc(7 * sizeof(char)); unsigned int offset = sizeof(buffer); > buffer[--offset] = '\0'; /* here */ ``` 2022년 2월 20일 (일) 오후 7:29, Julien Rouhaud 님이 작성: > > Hi, > > On Sun, Feb 20, 2022 at 07:23:56PM +0900, Dong Wook Lee wrote: > > Hi hackers, > > I've read in this blog ( > > http://big-elephants.com/2015-10/writing-postgres-extensions-part-i/) > > and I wrote an extension about base36_encode with c code > > but when I executed a query like this below I got a warning below. > > > > ``` > > postgres=# SELECT base36_encode(123); > > WARNING: problem in alloc set ExprContext: detected write past chunk end > > in block 0x55fb75334d40, chunk 0x55fb75334d68 > > WARNING: problem in alloc set ExprContext: detected write past chunk end > > in block 0x55fb75334d40, chunk 0x55fb75334d68 > > base36_encode > > --- > > 3f > > (1 row) > > ``` > > > > I don't know what this warning means and how I can fix it. > > It means that you have some problem in your memory allocation. You can refer > to src/backend/utils/mmgr/aset.c for more details on those safety checks.
Print warning when I execute my own extension function
Hi hackers, I've read in this blog ( http://big-elephants.com/2015-10/writing-postgres-extensions-part-i/) and I wrote an extension about base36_encode with c code but when I executed a query like this below I got a warning below. ``` postgres=# SELECT base36_encode(123); WARNING: problem in alloc set ExprContext: detected write past chunk end in block 0x55fb75334d40, chunk 0x55fb75334d68 WARNING: problem in alloc set ExprContext: detected write past chunk end in block 0x55fb75334d40, chunk 0x55fb75334d68 base36_encode --- 3f (1 row) ``` I don't know what this warning means and how I can fix it. Thanks Dong Wook Lee.
Re: [PATCH] Improve function toast_delete_external (src/backend/access/table/toast_helper.c)
Yes, now I understand it. Thank you for letting me know about that. Thanks. 2022년 2월 10일 (목) 00:39, Ranier Vilela 님이 작성: > > Em qua., 9 de fev. de 2022 às 11:34, Dong Wook Lee > escreveu: > >> Hi, >> >> I've applied your patch. I think it's reasonable. >> but IMHO It looks more complicated to read because of many conditions >> in if statement. >> so what about just moving up if-statement? >> > > No. > Your version is worse than HEAD, please don't. > > 1. Do not declare variables after statement. > Although Postgres accepts C99, this is not acceptable, for a number of > problems, already discussed. > > 2. Still slow unnecessarily, because still test TupleDescAttr(tupleDesc, > i)->attlen, > which is much more onerous than test isnull. > > 3. We don't write Baby C code. > C has short break expressions, use-it! > > My version is the right thing, here. > > regards, > Ranier Vilela >
Re: [PATCH] Improve function toast_delete_external (src/backend/access/table/toast_helper.c)
Oops, I realized that I made a mistake in my patch and now I have corrected it. 2022년 2월 9일 (수) 오후 11:33, Dong Wook Lee 님이 작성: > > Hi, > > I've applied your patch. I think it's reasonable. > but IMHO It looks more complicated to read because of many conditions > in if statement. > so what about just moving up if-statement? > > Thanks. > Dong Wook Lee > > 2022년 2월 9일 (수) 오후 7:56, Ranier Vilela 님이 작성: > > > > Hi, > > > > I think this change can improve this particular function by avoiding > > touching value if not needed. > > Test if not isnull is cheaper than test TupleDescAttr is -1. > > > > best regards, > > > > Ranier Vilela v3_tiny_improvemnt_toast_helper.patch Description: Binary data
Re: [PATCH] Improve function toast_delete_external (src/backend/access/table/toast_helper.c)
Hi, I've applied your patch. I think it's reasonable. but IMHO It looks more complicated to read because of many conditions in if statement. so what about just moving up if-statement? Thanks. Dong Wook Lee 2022년 2월 9일 (수) 오후 7:56, Ranier Vilela 님이 작성: > > Hi, > > I think this change can improve this particular function by avoiding touching > value if not needed. > Test if not isnull is cheaper than test TupleDescAttr is -1. > > best regards, > > Ranier Vilela v2_tiny_improvemnt_toast_helper.patch Description: Binary data
Re: Add tap test for --extra-float-digits option
> That's more of an habit to look around, find similar patterns and the > check if these are covered. > > I have applied your patch, and you may want to be careful about a > couple of things: > - Please avoid top-posting on the mailing lists: > https://en.wikipedia.org/wiki/Posting_style#Top-posting > Top-posting breaks the logic of a thread. > - Your patch format is good. When sending a new version of the patch, > it may be better to send things as a complete diff on the master > branch (or the branch you are working on), instead of just sending one > patch that applies on top of something you sent previously. Here for > example your patch 0002 applied on top of 0001 that was sent at the > top of the thread. We have also guidelines about patch submission: > https://wiki.postgresql.org/wiki/Submitting_a_Patch > > Thanks! > -- > Michael Hi Michael First of all, thank you for merging my patch. And I'm sorry, I should have been more careful about it. Next time I will follow format. And there is something I will tell you Would you mind if I ask you specify my author info with --author on the git commit? The new contributor can get involved in the PostgreSQL project. When they sent a patch and it was merged to the main repository, it'd be better to keep the author info on the git commit, IMHO. Because many opensource hackers who interested in PostgreSQL project can want to keep a record of author info on commits they wrote. Otherwise, contribution records can not be found by 'git shortlog -sn' and GitHub and OpenHub cannot track their opensource contribution records... So what about using --author for PostgreSQL contributors when merging their patches? like the Linux Kernel project https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8a16c09edc58982d56c49ab577fdcdf830fbc3a5 If so, many contributors would be highly encouraged. Thanks, Dong Wook
Re: Add tap test for --extra-float-digits option
Oh, now I understand. and I added a test of --row-per-insert option. I'd better find more options missing test 2020년 6월 12일 (금) 오후 4:04, Michael Paquier 님이 작성: > On Fri, Jun 12, 2020 at 02:30:35PM +0900, Dong Wook Lee wrote: > > Thank you for your response > > Do you mean to move it under the test of --compression option? > > You could move the test where you see is best, and I would have done > that. My point is that we could have a test also for > --rows-per-insert as it deals with the same problem. > -- > Michael > 0002-pg_dump-Add-test-for-rows-per-insert-option.patch Description: Binary data
Add tap test for --extra-float-digits option
Hi hackers! I added tap test code for pg_dump --extra-float-digits option because it hadn't tested it. There was no problem when writing test code and running TAP tests. 0001-Add-tap-test-for-extra-float-digits-option.patch Description: Binary data
TAP tests not enabled in pg_dump
I want to execute some testing code but I conflict against error. Please tell me how to fix it :) Environment: = OSX: 10.14.6 postgresql: version 13 current development In postgresql/src/bin/pg_dump dir I command below $ make check PROVE_TESTS='t/001_basic.pi' Error: /Library/Developer/CommandLineTools/usr/bin/make -C ../../../src/backend generated-headers /Library/Developer/CommandLineTools/usr/bin/make -C catalog distprep generated-header-symlinks make[2]: Nothing to be done for `distprep'. make[2]: Nothing to be done for `generated-header-symlinks'. /Library/Developer/CommandLineTools/usr/bin/make -C utils distprep generated-header-symlinks make[2]: Nothing to be done for `distprep'. make[2]: Nothing to be done for `generated-header-symlinks'. rm -rf '/Users/admin/Documents/Github/postgresql'/tmp_install /bin/sh ../../../config/install-sh -c -d '/Users/admin/Documents/Github/postgresql'/tmp_install/log /Library/Developer/CommandLineTools/usr/bin/make -C '../../..' DESTDIR='/Users/admin/Documents/Github/postgresql'/tmp_install install >'/Users/admin/Documents/Github/postgresql'/tmp_install/log/install.log 2>&1 /Library/Developer/CommandLineTools/usr/bin/make -j1 checkprep >>'/Users/admin/Documents/Github/postgresql'/tmp_install/log/install.log 2>&1 TAP tests not enabled
