citusdata / citus

Distributed PostgreSQL as an extension
https://www.citusdata.com
GNU Affero General Public License v3.0
10.43k stars 662 forks source link

pg_regress_multi.pl additions in 23e43b6 need some cleanup #325

Open anarazel opened 8 years ago

anarazel commented 8 years ago

@onderkalaci @jasonmp85 : The pg_regress_multi.pl changes from 23e43b6 appear to be slightly unbaked - the perl code doesn't look right, and the way the the additional objects are handled seems to be rather inflexible, leading to pg_regress_multi.pl containing way too much knowledge.

%dataTypes = ('dummy_type', '(i integer)', 
               'order_side', ' ENUM (\'buy\', \'sell\')',
               'test_composite_type', '(i integer, i2 integer)',
               'bug_status', ' ENUM (\'new\', \'open\', \'closed\')');

this should, based on the usage, be an array containing the entire statement and it should be local 'my ..'.. And if not, the elements should at least be declared like 'dummy_type' => '(i integer)', to make this easier to read.

%dataTypes = ('dummy_type', '(i integer)', 
               'order_side', ' ENUM (\'buy\', \'sell\')',
               'test_composite_type', '(i integer, i2 integer)',
               'bug_status', ' ENUM (\'new\', \'open\', \'closed\')');

# define functions as signature->definition
%functions = ('fake_fdw_handler()', 'fdw_handler AS \'citusdb\' LANGUAGE C STRICT;');

#define fdws as name->handler name
%fdws = ('fake_fdw', 'fake_fdw_handler');

#define server_name->fdw
%fdwServers = ('fake_fdw_server', 'fake_fdw');

This seems to be too specific to be handled inside pg_regress_multi.pl - how about making it an 'citus_fdw_test' extension, that's then installed via the --load-extension mechanism?

onderkalaci commented 8 years ago

Do you think it should get into 5.0? Or 5.1 is good enough?