On 2017/09/12 20:17, Ashutosh Bapat wrote:
> On Tue, Sep 12, 2017 at 2:27 PM, Amit Langote
> <langote_amit...@lab.ntt.co.jp> wrote:
>> Thanks Ashutosh for taking a look at this.
>>
>> On 2017/09/05 21:16, Ashutosh Bapat wrote:
>>> The patch needs a rebase.
>>
>> Attached rebased patch.
>
> Thanks for rebased patch.
Thanks for the review.
> We could annotate each ERROR with an explanation as to why that's an
> error, but then this file doesn't do that for other commands, so may
> be the patch is just fine.
Agreed. Note that this patch is just about adding the tests, not
modifying foreigncmds.c to change error handling around HANDLER functions.
> Also, I am wondering whether we should create the new handler function
> in foreign.c similar to postgresql_fdw_validator(). The prologue has a
> caution
>
> 606 * Caution: this function is deprecated, and is now meant only for testing
> 607 * purposes, because the list of options it knows about doesn't
> necessarily
> 608 * square with those known to whichever libpq instance you might be using.
> 609 * Inquire of libpq itself, instead.
>
> So, may be we don't want to add it there. But adding the handler
> function in create_function_1 doesn't seem good. If that's the correct
> place, then at least it should be moved before " -- Things that
> shouldn't work:"; it doesn't belong to functions that don't work.
In the attached updated patch, I created separate .source files in
src/test/regress/input and output directories called fdw_handler.source
and put the test_fdw_handler function definition there. When I had
originally thought of it back when I wrote the patch, it seemed to be an
overkill, because we're just normally defining a single C function there
to be used in the newly added foreign_data tests. In any case, we need to
go the .source file way, because that's the only way to refer to paths to
.so library when defining C language functions.
Thanks,
Amit
From 510987531bfdf22df0bc8eef27f232e580d415b1 Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Wed, 10 May 2017 10:37:42 +0900
Subject: [PATCH] Add some FDW HANDLER DDL tests
---
src/test/regress/expected/foreign_data.out | 28 ++++++++++++++++++++++------
src/test/regress/input/fdw_handler.source | 5 +++++
src/test/regress/output/fdw_handler.source | 5 +++++
src/test/regress/parallel_schedule | 2 +-
src/test/regress/regress.c | 7 +++++++
src/test/regress/serial_schedule | 1 +
src/test/regress/sql/.gitignore | 1 +
src/test/regress/sql/foreign_data.sql | 13 +++++++++++++
8 files changed, 55 insertions(+), 7 deletions(-)
create mode 100644 src/test/regress/input/fdw_handler.source
create mode 100644 src/test/regress/output/fdw_handler.source
diff --git a/src/test/regress/expected/foreign_data.out
b/src/test/regress/expected/foreign_data.out
index c6e558b07f..331f7a911f 100644
--- a/src/test/regress/expected/foreign_data.out
+++ b/src/test/regress/expected/foreign_data.out
@@ -89,6 +89,14 @@ CREATE FOREIGN DATA WRAPPER foo VALIDATOR
postgresql_fdw_validator;
postgresql | regress_foreign_data_user | - | postgresql_fdw_validator |
| |
(3 rows)
+-- HANDLER related checks
+CREATE FUNCTION invalid_fdw_handler() RETURNS int LANGUAGE SQL AS 'SELECT 1;';
+CREATE FOREIGN DATA WRAPPER test_fdw HANDLER invalid_fdw_handler; -- ERROR
+ERROR: function invalid_fdw_handler must return type fdw_handler
+CREATE FOREIGN DATA WRAPPER test_fdw HANDLER test_fdw_handler HANDLER
invalid_fdw_handler; -- ERROR
+ERROR: conflicting or redundant options
+CREATE FOREIGN DATA WRAPPER test_fdw HANDLER test_fdw_handler;
+DROP FOREIGN DATA WRAPPER test_fdw;
-- ALTER FOREIGN DATA WRAPPER
ALTER FOREIGN DATA WRAPPER foo; -- ERROR
ERROR: syntax error at or near ";"
@@ -188,18 +196,26 @@ ALTER FOREIGN DATA WRAPPER foo RENAME TO foo1;
(3 rows)
ALTER FOREIGN DATA WRAPPER foo1 RENAME TO foo;
+-- HANDLER related checks
+ALTER FOREIGN DATA WRAPPER foo HANDLER invalid_fdw_handler; -- ERROR
+ERROR: function invalid_fdw_handler must return type fdw_handler
+ALTER FOREIGN DATA WRAPPER foo HANDLER test_fdw_handler HANDLER anything; --
ERROR
+ERROR: conflicting or redundant options
+ALTER FOREIGN DATA WRAPPER foo HANDLER test_fdw_handler;
+WARNING: changing the foreign-data wrapper handler can change behavior of
existing foreign tables
+DROP FUNCTION invalid_fdw_handler();
-- DROP FOREIGN DATA WRAPPER
DROP FOREIGN DATA WRAPPER nonexistent; -- ERROR
ERROR: foreign-data wrapper "nonexistent" does not exist
DROP FOREIGN DATA WRAPPER IF EXISTS nonexistent;
NOTICE: foreign-data wrapper "nonexistent" does not exist, skipping
\dew+
- List of foreign-data
wrappers
- Name | Owner | Handler | Validator |
Access privileges | FDW options | Description
-------------+---------------------------+---------+--------------------------+-------------------+------------------------------+-------------
- dummy | regress_foreign_data_user | - | - |
| | useless
- foo | regress_test_role_super | - | - |
| (b '3', c '4', a '2', d '5') |
- postgresql | regress_foreign_data_user | - | postgresql_fdw_validator |
| |
+ List of
foreign-data wrappers
+ Name | Owner | Handler | Validator
| Access privileges | FDW options | Description
+------------+---------------------------+------------------+--------------------------+-------------------+------------------------------+-------------
+ dummy | regress_foreign_data_user | - | -
| | | useless
+ foo | regress_test_role_super | test_fdw_handler | -
| | (b '3', c '4', a '2', d '5') |
+ postgresql | regress_foreign_data_user | - |
postgresql_fdw_validator | | |
(3 rows)
DROP ROLE regress_test_role_super; -- ERROR
diff --git a/src/test/regress/input/fdw_handler.source
b/src/test/regress/input/fdw_handler.source
new file mode 100644
index 0000000000..103ae6e8b9
--- /dev/null
+++ b/src/test/regress/input/fdw_handler.source
@@ -0,0 +1,5 @@
+--
+-- For FDW handler tests in sql/foreign_data.sql
+--
+CREATE FUNCTION test_fdw_handler () RETURNS fdw_handler LANGUAGE C
+ AS '@libdir@/regress@DLSUFFIX@', 'test_fdw_handler';
diff --git a/src/test/regress/output/fdw_handler.source
b/src/test/regress/output/fdw_handler.source
new file mode 100644
index 0000000000..c9d0f7de6f
--- /dev/null
+++ b/src/test/regress/output/fdw_handler.source
@@ -0,0 +1,5 @@
+--
+-- For FDW handler tests in sql/foreign_data.sql
+--
+CREATE FUNCTION test_fdw_handler () RETURNS fdw_handler LANGUAGE C
+ AS '/home/amit/pg/mygit/src/test/regress/regress.so', 'test_fdw_handler';
diff --git a/src/test/regress/parallel_schedule
b/src/test/regress/parallel_schedule
index 2fd3f2b1b1..f8361c49ce 100644
--- a/src/test/regress/parallel_schedule
+++ b/src/test/regress/parallel_schedule
@@ -89,7 +89,7 @@ test: brin gin gist spgist privileges init_privs
security_label collate matview
# ----------
# Another group of parallel tests
# ----------
-test: alter_generic alter_operator misc psql async dbsize misc_functions
sysviews tsrf tidscan stats_ext
+test: alter_generic alter_operator misc psql async dbsize misc_functions
sysviews tsrf tidscan stats_ext fdw_handler
# rules cannot run concurrently with any test that creates a view
test: rules psql_crosstab amutils
diff --git a/src/test/regress/regress.c b/src/test/regress/regress.c
index 734947cc98..0a123f2b39 100644
--- a/src/test/regress/regress.c
+++ b/src/test/regress/regress.c
@@ -1096,3 +1096,10 @@ test_atomic_ops(PG_FUNCTION_ARGS)
PG_RETURN_BOOL(true);
}
+
+PG_FUNCTION_INFO_V1(test_fdw_handler);
+Datum
+test_fdw_handler(PG_FUNCTION_ARGS)
+{
+ PG_RETURN_NULL();
+}
diff --git a/src/test/regress/serial_schedule b/src/test/regress/serial_schedule
index 76b0de30a7..b4bb866616 100644
--- a/src/test/regress/serial_schedule
+++ b/src/test/regress/serial_schedule
@@ -147,6 +147,7 @@ test: bitmapops
test: combocid
test: tsearch
test: tsdicts
+test: fdw_handler
test: foreign_data
test: window
test: xmlmap
diff --git a/src/test/regress/sql/.gitignore b/src/test/regress/sql/.gitignore
index 46c8112094..21b5a62dd5 100644
--- a/src/test/regress/sql/.gitignore
+++ b/src/test/regress/sql/.gitignore
@@ -2,6 +2,7 @@
/copy.sql
/create_function_1.sql
/create_function_2.sql
+/fdw_handler.sql
/largeobject.sql
/misc.sql
/security_label.sql
diff --git a/src/test/regress/sql/foreign_data.sql
b/src/test/regress/sql/foreign_data.sql
index ebe8ffbffe..1af7258718 100644
--- a/src/test/regress/sql/foreign_data.sql
+++ b/src/test/regress/sql/foreign_data.sql
@@ -51,6 +51,13 @@ RESET ROLE;
CREATE FOREIGN DATA WRAPPER foo VALIDATOR postgresql_fdw_validator;
\dew+
+-- HANDLER related checks
+CREATE FUNCTION invalid_fdw_handler() RETURNS int LANGUAGE SQL AS 'SELECT 1;';
+CREATE FOREIGN DATA WRAPPER test_fdw HANDLER invalid_fdw_handler; -- ERROR
+CREATE FOREIGN DATA WRAPPER test_fdw HANDLER test_fdw_handler HANDLER
invalid_fdw_handler; -- ERROR
+CREATE FOREIGN DATA WRAPPER test_fdw HANDLER test_fdw_handler;
+DROP FOREIGN DATA WRAPPER test_fdw;
+
-- ALTER FOREIGN DATA WRAPPER
ALTER FOREIGN DATA WRAPPER foo; -- ERROR
ALTER FOREIGN DATA WRAPPER foo VALIDATOR bar; -- ERROR
@@ -88,6 +95,12 @@ ALTER FOREIGN DATA WRAPPER foo RENAME TO foo1;
\dew+
ALTER FOREIGN DATA WRAPPER foo1 RENAME TO foo;
+-- HANDLER related checks
+ALTER FOREIGN DATA WRAPPER foo HANDLER invalid_fdw_handler; -- ERROR
+ALTER FOREIGN DATA WRAPPER foo HANDLER test_fdw_handler HANDLER anything; --
ERROR
+ALTER FOREIGN DATA WRAPPER foo HANDLER test_fdw_handler;
+DROP FUNCTION invalid_fdw_handler();
+
-- DROP FOREIGN DATA WRAPPER
DROP FOREIGN DATA WRAPPER nonexistent; -- ERROR
DROP FOREIGN DATA WRAPPER IF EXISTS nonexistent;
--
2.11.0
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
