pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Mark Dilger <hornschnorter@gmail.com>
To: PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Missing test of SPI copy functionality
Date: Wed, 6 Nov 2019 16:16:14 -0800
Message-ID: <a6e9b130-7fd5-387b-4ec5-89bda24373ab@gmail.com> (raw)

Hackers,

While working on cleaning up the SPI interface, I found that one of the 
SPI error codes, SPI_ERROR_COPY, is never encountered in any test case 
when running `make check-world`.  This case is certainly reachable by a 
user, as is shown in the attached patch.  Is this tested from some other 
infrastructure?

To verify that SPI_ERROR_COPY is not tested, before and after applying 
the patch, try this modification, and notice before the patch that the 
fatal error is never encountered:

inline patch 1: src/backend/executor/spi.c (+0/-0 lines) download inline-1.patchdiff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c index 2c0ae395ba..ced38abbf6 100644 --- a/src/backend/executor/spi.c +++ b/src/backend/executor/spi.c @@ -2245,6 +2245,7 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo
paramLI, if (cstmt->filename == NULL) { + elog(FATAL, "SPI_ERROR_COPY tested"); my_res = SPI_ERROR_COPY; goto fail; } I am submitting this patch separately from other patches related to SPI, since (a) it does not touch any of the SPI code, (b) it fixes missing test coverage to do with COPY and PL/pgSQL, only indirectly to do with SPI, and (c) it should be possible to commit this patch even if other SPI patches are rejected. -- Mark Dilger Patches and attachments: download all (2 files, .tar.gz)
v1-0001-Testing-COPY-inside-PL-pgSQL.patch (text/x-patch, 4.3K) download From 019a37280145ef437acd26b4cd2c0ce58aa7709a Mon Sep 17 00:00:00 2001 From: Mark Dilger <hornschnorter@gmail.com> Date: Wed, 6 Nov 2019 15:37:50 -0800 Subject: [PATCH v1] Testing COPY inside PL/pgSQL Adding a test for COPY being run inside PL/pgSQL code, as part of verifying that COPY works correctly from inside SPI (which plpgsql uses internally). --- src/pl/plpgsql/src/Makefile | 2 +- src/pl/plpgsql/src/data/copy1.data | 3 ++ src/pl/plpgsql/src/expected/.gitignore | 1 + src/pl/plpgsql/src/input/plpgsql_copy.source | 41 +++++++++++++++ src/pl/plpgsql/src/output/plpgsql_copy.source | 50 +++++++++++++++++++ src/pl/plpgsql/src/sql/.gitignore | 1 + 6 files changed, 97 insertions(+), 1 deletion(-) create mode 100644 src/pl/plpgsql/src/data/copy1.data create mode 100644 src/pl/plpgsql/src/expected/.gitignore create mode 100644 src/pl/plpgsql/src/input/plpgsql_copy.source create mode 100644 src/pl/plpgsql/src/output/plpgsql_copy.source create mode 100644 src/pl/plpgsql/src/sql/.gitignore diff --git a/src/pl/plpgsql/src/Makefile b/src/pl/plpgsql/src/Makefile index 4d57bf5f1e..d5a5d6d449 100644 --- a/src/pl/plpgsql/src/Makefile +++ b/src/pl/plpgsql/src/Makefile @@ -32,7 +32,7 @@ DATA = plpgsql.control plpgsql--1.0.sql plpgsql--unpackaged--1.0.sql REGRESS_OPTS = --dbname=$(PL_TESTDB) -REGRESS = plpgsql_call plpgsql_control plpgsql_domain plpgsql_record \ +REGRESS = plpgsql_call plpgsql_control plpgsql_copy plpgsql_domain plpgsql_record \ plpgsql_cache plpgsql_transaction plpgsql_trap \ plpgsql_trigger plpgsql_varprops diff --git a/src/pl/plpgsql/src/data/copy1.data b/src/pl/plpgsql/src/data/copy1.data new file mode 100644 index 0000000000..5d8478facb --- /dev/null +++ b/src/pl/plpgsql/src/data/copy1.data @@ -0,0 +1,3 @@ +1 1.1 +2 2.2 +3 3.3 diff --git a/src/pl/plpgsql/src/expected/.gitignore b/src/pl/plpgsql/src/expected/.gitignore new file mode 100644 index 0000000000..13e5918721 --- /dev/null +++ b/src/pl/plpgsql/src/expected/.gitignore @@ -0,0 +1 @@ +/plpgsql_copy.out diff --git a/src/pl/plpgsql/src/input/plpgsql_copy.source b/src/pl/plpgsql/src/input/plpgsql_copy.source new file mode 100644 index 0000000000..c549243088 --- /dev/null +++ b/src/pl/plpgsql/src/input/plpgsql_copy.source @@ -0,0 +1,41 @@ +CREATE TABLE copy1 (a int, b float); + +-- error cases +DO LANGUAGE plpgsql $$ +BEGIN + COPY copy1 TO stdout; +END; +$$; + +DO LANGUAGE plpgsql $$ +BEGIN + COPY copy1 FROM stdin; +END; +$$; + +-- valid data copying +DO LANGUAGE plpgsql $$ +BEGIN + COPY copy1 FROM '@abs_srcdir@/data/copy1.data'; +END; +$$; + +SELECT * FROM copy1; + +DO LANGUAGE plpgsql $$ +BEGIN + COPY copy1 TO '@abs_builddir@/results/copy1.data'; +END; +$$; + +DELETE FROM copy1; + +DO LANGUAGE plpgsql $$ +BEGIN + COPY copy1 FROM '@abs_builddir@/results/copy1.data'; +END; +$$; + +SELECT * FROM copy1; + +DROP TABLE copy1; diff --git a/src/pl/plpgsql/src/output/plpgsql_copy.source b/src/pl/plpgsql/src/output/plpgsql_copy.source new file mode 100644 index 0000000000..ee9a3d793d --- /dev/null +++ b/src/pl/plpgsql/src/output/plpgsql_copy.source @@ -0,0 +1,50 @@ +CREATE TABLE copy1 (a int, b float); +-- error cases +DO LANGUAGE plpgsql $$ +BEGIN + COPY copy1 TO stdout; +END; +$$; +ERROR: cannot COPY to/from client in PL/pgSQL +CONTEXT: PL/pgSQL function inline_code_block line 3 at SQL statement +DO LANGUAGE plpgsql $$ +BEGIN + COPY copy1 FROM stdin; +END; +$$; +ERROR: cannot COPY to/from client in PL/pgSQL +CONTEXT: PL/pgSQL function inline_code_block line 3 at SQL statement +-- valid data copying +DO LANGUAGE plpgsql $$ +BEGIN + COPY copy1 FROM '@abs_srcdir@/data/copy1.data'; +END; +$$; +SELECT * FROM copy1; + a | b +---+----- + 1 | 1.1 + 2 | 2.2 + 3 | 3.3 +(3 rows) + +DO LANGUAGE plpgsql $$ +BEGIN + COPY copy1 TO '@abs_builddir@/results/copy1.data'; +END; +$$; +DELETE FROM copy1; +DO LANGUAGE plpgsql $$ +BEGIN + COPY copy1 FROM '@abs_builddir@/results/copy1.data'; +END; +$$; +SELECT * FROM copy1; + a | b +---+----- + 1 | 1.1 + 2 | 2.2 + 3 | 3.3 +(3 rows) + +DROP TABLE copy1; diff --git a/src/pl/plpgsql/src/sql/.gitignore b/src/pl/plpgsql/src/sql/.gitignore new file mode 100644 index 0000000000..210bee188e --- /dev/null +++ b/src/pl/plpgsql/src/sql/.gitignore @@ -0,0 +1 @@ +/plpgsql_copy.sql base-commit: 6e3e6cc0e884a6091e1094dff29db430af08fb93 -- 2.20.1

view thread (4+ messages)  latest in thread

Message-ID: <a6e9b130-7fd5-387b-4ec5-89bda24373ab@gmail.com>
Permalink:  ../a6e9b130-7fd5-387b-4ec5-89bda24373ab@gmail.com/
Also on:    postgresql.org/message-id/a6e9b130-7fd5-387b-4ec5-89bda24373ab@gmail.com

 ·  · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: hornschnorter@gmail.com
  Subject: Re: Missing test of SPI copy functionality
  In-Reply-To: <a6e9b130-7fd5-387b-4ec5-89bda24373ab@gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox