bytea_output vs make installcheck
make installcheck currently fails against a server running
with bytea_output = escape.
Making it succeed is fairly easy, and I think it is worth doing.
Attached are two options for doing that. One overrides bytea_output
locally where-ever needed, and the other overrides it for the entire
'regression' database.
Cheers,
Jeff
Attachments:
installcheck_bytea_fix_2.patchapplication/octet-stream; name=installcheck_bytea_fix_2.patchDownload
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
new file mode 100644
index d4d00d9..28857ee
*** a/src/test/regress/pg_regress.c
--- b/src/test/regress/pg_regress.c
*************** create_database(const char *dbname)
*** 1927,1934 ****
"ALTER DATABASE \"%s\" SET lc_monetary TO 'C';"
"ALTER DATABASE \"%s\" SET lc_numeric TO 'C';"
"ALTER DATABASE \"%s\" SET lc_time TO 'C';"
"ALTER DATABASE \"%s\" SET timezone_abbreviations TO 'Default';",
! dbname, dbname, dbname, dbname, dbname);
/*
* Install any requested procedural languages. We use CREATE OR REPLACE
--- 1927,1935 ----
"ALTER DATABASE \"%s\" SET lc_monetary TO 'C';"
"ALTER DATABASE \"%s\" SET lc_numeric TO 'C';"
"ALTER DATABASE \"%s\" SET lc_time TO 'C';"
+ "ALTER DATABASE \"%s\" SET bytea_output TO 'hex';"
"ALTER DATABASE \"%s\" SET timezone_abbreviations TO 'Default';",
! dbname, dbname, dbname, dbname, dbname, dbname);
/*
* Install any requested procedural languages. We use CREATE OR REPLACE
installcheck_bytea_fix_1.patchapplication/octet-stream; name=installcheck_bytea_fix_1.patchDownload
diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out
new file mode 100644
index 0ff8062..91579f9
*** a/src/test/regress/expected/aggregates.out
--- b/src/test/regress/expected/aggregates.out
***************
*** 1,6 ****
--- 1,7 ----
--
-- AGGREGATES
--
+ SET bytea_output TO hex;
SELECT avg(four) AS avg_1 FROM onek;
avg_1
--------------------
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
new file mode 100644
index f66b443..38358b2
*** a/src/test/regress/expected/privileges.out
--- b/src/test/regress/expected/privileges.out
*************** GRANT SELECT, UPDATE ON LARGE OBJECT 99
*** 1110,1115 ****
--- 1110,1116 ----
ERROR: large object 999 does not exist
\c -
SET SESSION AUTHORIZATION regress_user2;
+ SET bytea_output to hex;
SELECT lo_create(2001);
lo_create
-----------
*************** SELECT oid, pg_get_userbyid(lomowner) ow
*** 1185,1190 ****
--- 1186,1192 ----
(6 rows)
SET SESSION AUTHORIZATION regress_user3;
+ SET bytea_output to hex;
SELECT loread(lo_open(1001, x'40000'::int), 32);
loread
------------
*************** SELECT lo_truncate(lo_open(2001, x'20000
*** 1211,1216 ****
--- 1213,1219 ----
\c -
SET lo_compat_privileges = false; -- default setting
SET SESSION AUTHORIZATION regress_user4;
+ SET bytea_output to hex;
SELECT loread(lo_open(1002, x'40000'::int), 32); -- to be denied
ERROR: permission denied for large object 1002
SELECT lowrite(lo_open(1002, x'20000'::int), 'abcd'); -- to be denied
*************** HINT: Anyone can use the client-side lo
*** 1225,1230 ****
--- 1228,1234 ----
\c -
SET lo_compat_privileges = true; -- compatibility mode
SET SESSION AUTHORIZATION regress_user4;
+ SET bytea_output to hex;
SELECT loread(lo_open(1002, x'40000'::int), 32);
loread
--------
diff --git a/src/test/regress/sql/aggregates.sql b/src/test/regress/sql/aggregates.sql
new file mode 100644
index 2eeb3ee..c384bf3
*** a/src/test/regress/sql/aggregates.sql
--- b/src/test/regress/sql/aggregates.sql
***************
*** 2,7 ****
--- 2,9 ----
-- AGGREGATES
--
+ SET bytea_output TO hex;
+
SELECT avg(four) AS avg_1 FROM onek;
SELECT avg(a) AS avg_32 FROM aggtest WHERE a < 100;
diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql
new file mode 100644
index 00dc7bd..879d663
*** a/src/test/regress/sql/privileges.sql
--- b/src/test/regress/sql/privileges.sql
*************** GRANT SELECT, UPDATE ON LARGE OBJECT 99
*** 705,710 ****
--- 705,711 ----
\c -
SET SESSION AUTHORIZATION regress_user2;
+ SET bytea_output to hex;
SELECT lo_create(2001);
SELECT lo_create(2002);
*************** SELECT lo_unlink(2002);
*** 732,737 ****
--- 733,739 ----
SELECT oid, pg_get_userbyid(lomowner) ownername, lomacl FROM pg_largeobject_metadata;
SET SESSION AUTHORIZATION regress_user3;
+ SET bytea_output to hex;
SELECT loread(lo_open(1001, x'40000'::int), 32);
SELECT loread(lo_open(1003, x'40000'::int), 32); -- to be denied
*************** SELECT lo_truncate(lo_open(2001, x'20000
*** 744,749 ****
--- 746,752 ----
\c -
SET lo_compat_privileges = false; -- default setting
SET SESSION AUTHORIZATION regress_user4;
+ SET bytea_output to hex;
SELECT loread(lo_open(1002, x'40000'::int), 32); -- to be denied
SELECT lowrite(lo_open(1002, x'20000'::int), 'abcd'); -- to be denied
*************** SELECT lo_export(1001, '/dev/null'); -
*** 754,759 ****
--- 757,763 ----
\c -
SET lo_compat_privileges = true; -- compatibility mode
SET SESSION AUTHORIZATION regress_user4;
+ SET bytea_output to hex;
SELECT loread(lo_open(1002, x'40000'::int), 32);
SELECT lowrite(lo_open(1002, x'20000'::int), 'abcd');
On Wed, Feb 15, 2017 at 8:50 AM, Jeff Janes <jeff.janes@gmail.com> wrote:
make installcheck currently fails against a server running
with bytea_output = escape.Making it succeed is fairly easy, and I think it is worth doing.
Attached are two options for doing that. One overrides bytea_output
locally where-ever needed, and the other overrides it for the entire
'regression' database.
The solution that overrides bytea_output locally looks appropriate. It may
not be required to change the format for entire database.
Had there been a way to convert bytea_output from 'hex' to 'escape'
internally, that could have simplified this customisation even more.
Regards,
Neha
Cheers,
Neha
On Wed, Feb 15, 2017 at 8:50 AM, Jeff Janes <jeff.janes@gmail.com> wrote:
Show quoted text
make installcheck currently fails against a server running
with bytea_output = escape.Making it succeed is fairly easy, and I think it is worth doing.
Attached are two options for doing that. One overrides bytea_output
locally where-ever needed, and the other overrides it for the entire
'regression' database.Cheers,
Jeff
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Feb 15, 2017 at 10:04 AM, neha khatri <nehakhatri5@gmail.com>
wrote:.
Attached are two options for doing that. One overrides bytea_output
locally where-ever needed, and the other overrides it for the entire
'regression' database.The solution that overrides bytea_output locally looks appropriate. It may
not be required to change the format for entire database.
Had there been a way to convert bytea_output from 'hex' to 'escape'
internally, that could have simplified this customization even more.
Well, the conversion from 'hex' to 'escape' is available using the function
encode().
So the queries that are failing due to the setting bytea_output = escape,
can be wrapped under encode(), to obtain the result in 'escape' format.
Here is another way to resolve the same problem. The patch is attached.
Regards,
Neha
Attachments:
intallcheck_bytea_output_escape.patchapplication/octet-stream; name=intallcheck_bytea_output_escape.patchDownload
diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out
index 45208a6..39b3001 100644
--- a/src/test/regress/expected/aggregates.out
+++ b/src/test/regress/expected/aggregates.out
@@ -1352,29 +1352,29 @@ select string_agg(v, '') from bytea_test_table;
(1 row)
insert into bytea_test_table values(decode('ff','hex'));
-select string_agg(v, '') from bytea_test_table;
- string_agg
-------------
- \xff
+select encode(string_agg(v, ''), 'escape') from bytea_test_table;
+ encode
+--------
+ \377
(1 row)
insert into bytea_test_table values(decode('aa','hex'));
-select string_agg(v, '') from bytea_test_table;
- string_agg
-------------
- \xffaa
+select encode(string_agg(v, ''), 'escape') from bytea_test_table;
+ encode
+----------
+ \377\252
(1 row)
-select string_agg(v, NULL) from bytea_test_table;
- string_agg
-------------
- \xffaa
+select encode(string_agg(v, NULL), 'escape') from bytea_test_table;
+ encode
+----------
+ \377\252
(1 row)
-select string_agg(v, decode('ee', 'hex')) from bytea_test_table;
- string_agg
-------------
- \xffeeaa
+select encode(string_agg(v, decode('ee', 'hex')), 'escape') from bytea_test_table;
+ encode
+--------------
+ \377\356\252
(1 row)
drop table bytea_test_table;
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index f66b443..22766a9 100644
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -1122,24 +1122,24 @@ SELECT lo_create(2002);
2002
(1 row)
-SELECT loread(lo_open(1001, x'40000'::int), 32);
- loread
+SELECT encode(loread(lo_open(1001, x'40000'::int), 32), 'escape');
+ encode
--------
- \x
+
(1 row)
SELECT loread(lo_open(1002, x'40000'::int), 32); -- to be denied
ERROR: permission denied for large object 1002
-SELECT loread(lo_open(1003, x'40000'::int), 32);
- loread
+SELECT encode(loread(lo_open(1003, x'40000'::int), 32), 'escape');
+ encode
--------
- \x
+
(1 row)
-SELECT loread(lo_open(1004, x'40000'::int), 32);
- loread
+SELECT encode(loread(lo_open(1004, x'40000'::int), 32), 'escape');
+ encode
--------
- \x
+
(1 row)
SELECT lowrite(lo_open(1001, x'20000'::int), 'abcd');
@@ -1185,18 +1185,18 @@ SELECT oid, pg_get_userbyid(lomowner) ownername, lomacl FROM pg_largeobject_meta
(6 rows)
SET SESSION AUTHORIZATION regress_user3;
-SELECT loread(lo_open(1001, x'40000'::int), 32);
- loread
-------------
- \x61626364
+SELECT encode(loread(lo_open(1001, x'40000'::int), 32), 'escape');
+ encode
+--------
+ abcd
(1 row)
SELECT loread(lo_open(1003, x'40000'::int), 32); -- to be denied
ERROR: permission denied for large object 1003
-SELECT loread(lo_open(1005, x'40000'::int), 32);
- loread
+SELECT encode(loread(lo_open(1005, x'40000'::int), 32), 'escape');
+ encode
--------
- \x
+
(1 row)
SELECT lo_truncate(lo_open(1005, x'20000'::int), 10); -- to be denied
@@ -1225,10 +1225,10 @@ HINT: Anyone can use the client-side lo_export() provided by libpq.
\c -
SET lo_compat_privileges = true; -- compatibility mode
SET SESSION AUTHORIZATION regress_user4;
-SELECT loread(lo_open(1002, x'40000'::int), 32);
- loread
+SELECT encode(loread(lo_open(1002, x'40000'::int), 32), 'escape');
+ encode
--------
- \x
+
(1 row)
SELECT lowrite(lo_open(1002, x'20000'::int), 'abcd');
diff --git a/src/test/regress/sql/aggregates.sql b/src/test/regress/sql/aggregates.sql
index 430ac49..755eee5 100644
--- a/src/test/regress/sql/aggregates.sql
+++ b/src/test/regress/sql/aggregates.sql
@@ -510,13 +510,13 @@ select string_agg(v, '') from bytea_test_table;
insert into bytea_test_table values(decode('ff','hex'));
-select string_agg(v, '') from bytea_test_table;
+select encode(string_agg(v, ''), 'escape') from bytea_test_table;
insert into bytea_test_table values(decode('aa','hex'));
-select string_agg(v, '') from bytea_test_table;
-select string_agg(v, NULL) from bytea_test_table;
-select string_agg(v, decode('ee', 'hex')) from bytea_test_table;
+select encode(string_agg(v, ''), 'escape') from bytea_test_table;
+select encode(string_agg(v, NULL), 'escape') from bytea_test_table;
+select encode(string_agg(v, decode('ee', 'hex')), 'escape') from bytea_test_table;
drop table bytea_test_table;
diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql
index 00dc7bd..8e6f47c 100644
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -709,10 +709,10 @@ SET SESSION AUTHORIZATION regress_user2;
SELECT lo_create(2001);
SELECT lo_create(2002);
-SELECT loread(lo_open(1001, x'40000'::int), 32);
+SELECT encode(loread(lo_open(1001, x'40000'::int), 32), 'escape');
SELECT loread(lo_open(1002, x'40000'::int), 32); -- to be denied
-SELECT loread(lo_open(1003, x'40000'::int), 32);
-SELECT loread(lo_open(1004, x'40000'::int), 32);
+SELECT encode(loread(lo_open(1003, x'40000'::int), 32), 'escape');
+SELECT encode(loread(lo_open(1004, x'40000'::int), 32), 'escape');
SELECT lowrite(lo_open(1001, x'20000'::int), 'abcd');
SELECT lowrite(lo_open(1002, x'20000'::int), 'abcd'); -- to be denied
@@ -733,9 +733,9 @@ SELECT oid, pg_get_userbyid(lomowner) ownername, lomacl FROM pg_largeobject_meta
SET SESSION AUTHORIZATION regress_user3;
-SELECT loread(lo_open(1001, x'40000'::int), 32);
+SELECT encode(loread(lo_open(1001, x'40000'::int), 32), 'escape');
SELECT loread(lo_open(1003, x'40000'::int), 32); -- to be denied
-SELECT loread(lo_open(1005, x'40000'::int), 32);
+SELECT encode(loread(lo_open(1005, x'40000'::int), 32), 'escape');
SELECT lo_truncate(lo_open(1005, x'20000'::int), 10); -- to be denied
SELECT lo_truncate(lo_open(2001, x'20000'::int), 10);
@@ -755,7 +755,7 @@ SELECT lo_export(1001, '/dev/null'); -- to be denied
SET lo_compat_privileges = true; -- compatibility mode
SET SESSION AUTHORIZATION regress_user4;
-SELECT loread(lo_open(1002, x'40000'::int), 32);
+SELECT encode(loread(lo_open(1002, x'40000'::int), 32), 'escape');
SELECT lowrite(lo_open(1002, x'20000'::int), 'abcd');
SELECT lo_truncate(lo_open(1002, x'20000'::int), 10);
SELECT lo_unlink(1002);
On February 14, 2017 9:02:14 PM PST, neha khatri <nehakhatri5@gmail.com> wrote:
On Wed, Feb 15, 2017 at 10:04 AM, neha khatri <nehakhatri5@gmail.com>
wrote:.Attached are two options for doing that. One overrides bytea_output
locally where-ever needed, and the other overrides it for the entire
'regression' database.The solution that overrides bytea_output locally looks appropriate.
It may
not be required to change the format for entire database.
Had there been a way to convert bytea_output from 'hex' to 'escape'
internally, that could have simplified this customization even more.Well, the conversion from 'hex' to 'escape' is available using the
function
encode().
So the queries that are failing due to the setting bytea_output =
escape,
can be wrapped under encode(), to obtain the result in 'escape' format.
Here is another way to resolve the same problem. The patch is attached.
I don't quite see the point of this - there's a lot of settings that cause spurious test failures. I don't see any point fixing random cases of that. And I don't think the continual cost of doing so overall is worth the minimal gain.
What's your reason to get this fixed?
Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Andres Freund <andres@anarazel.de> writes:
I don't quite see the point of this - there's a lot of settings that cause spurious test failures. I don't see any point fixing random cases of that. And I don't think the continual cost of doing so overall is worth the minimal gain.
What's your reason to get this fixed?
FWIW, I'm inclined to do something about Jeff's nearby complaint about
operator_precedence_warning, because the cause of that failure is pretty
obscure. I'm less excited about this one, because it should be obvious
what happened to anyone who looks at the regression diffs.
In general I think there's value in "make installcheck" passing when
it reasonably can, but you're quite right that there's a lot of setting
changes that would break it, and not all are going to be practical to
fix.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Feb 14, 2017 at 9:17 PM, Andres Freund <andres@anarazel.de> wrote:
On February 14, 2017 9:02:14 PM PST, neha khatri <nehakhatri5@gmail.com>
wrote:On Wed, Feb 15, 2017 at 10:04 AM, neha khatri <nehakhatri5@gmail.com>
wrote:.Attached are two options for doing that. One overrides bytea_output
locally where-ever needed, and the other overrides it for the entire
'regression' database.The solution that overrides bytea_output locally looks appropriate.
It may
not be required to change the format for entire database.
Had there been a way to convert bytea_output from 'hex' to 'escape'
internally, that could have simplified this customization even more.Well, the conversion from 'hex' to 'escape' is available using the
function
encode().
So the queries that are failing due to the setting bytea_output =
escape,
can be wrapped under encode(), to obtain the result in 'escape' format.
Here is another way to resolve the same problem. The patch is attached.I don't quite see the point of this - there's a lot of settings that cause
spurious test failures. I don't see any point fixing random cases of that.
And I don't think the continual cost of doing so overall is worth the
minimal gain.What's your reason to get this fixed?
More testing is better than less testing, and a good way to get less
testing is requiring the tester to memorize a list of false positives they
might encounter. I'd like to systematically clone my production system,
run it through pg_upgrade, and then do installcheck (plus my own
app-specific) on the result, and I'd like others to do that as well with
their own production systems.
I don't really see the cost here. It took me longer to satisfy myself that
this was not actually a real bug than it did to write the patch. Now much
of that time was just because there were 3 other problems as well, which
makes isolating and evaluating them exponentially harder--which itself is a
good reason for fixing the ones that are easy to fix, so we don't have to
get distracted by those while investigating the other ones. And if we go
with the option 2 patch, it is one line which seems pretty self-documenting
and easy to maintain. I'd rather light a candle than to curse the darkness,
at least when the candle is this easy to light.
Cheers,
Jeff
Agreed with Jeff, false alarms should be avoided, whenever it is easy to
put the avoiding mechanism in place.
Regards,
Neha
Hi,
On 2017-02-15 09:30:39 -0800, Jeff Janes wrote:
On Tue, Feb 14, 2017 at 9:17 PM, Andres Freund <andres@anarazel.de> wrote:
What's your reason to get this fixed?
More testing is better than less testing, and a good way to get less
testing is requiring the tester to memorize a list of false positives they
might encounter. I'd like to systematically clone my production system,
run it through pg_upgrade, and then do installcheck (plus my own
app-specific) on the result, and I'd like others to do that as well with
their own production systems.
I don't really see the cost here.
Because that means we essentially need to make sure that our tests pass
with a combination of about ~20-30 behaviour changing gucs, and ~5
different compilation settings that change output. Either we do that
systematically - which'd be a fair amount of effort - or we're not
getting anywhere, because the setting around the next corner breaks a
bunch of different tests.
Should tests pass with random client_min_messages settings? Random
enable_? Switched default_with_oids? statement_timeout? Could go on a
long while.
Having to think about all GUCs/compile time settings that could
potentially affect a test and manually set everything up so they don't
makes writing tests a lot harder, and we'll fail anyway unless it's
checked in an automated fashion. Unless that testing is done you're not
really benefiting, because you can't rely on test failures meaning much.
If we want to have a list of GUCs that we want tests to pass under, then
the proponents of that at least should bring up a buildfarm animal
afterwards that test that reasonable combinations of those pass and
continue to pass.
Alternatively we could ALTER DATABASE a bunch of settings on the
regression database at the start, but I'm not sure that's nice either,
because it makes ad-hoc tests with unusual settings harder.
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
Andres Freund <andres@anarazel.de> writes:
On 2017-02-15 09:30:39 -0800, Jeff Janes wrote:
I don't really see the cost here.
Because that means we essentially need to make sure that our tests pass
with a combination of about ~20-30 behaviour changing gucs, and ~5
different compilation settings that change output.
Yeah, the problem with addressing this non-systematically is that it'll
never stay fixed for long.
Alternatively we could ALTER DATABASE a bunch of settings on the
regression database at the start, but I'm not sure that's nice either,
because it makes ad-hoc tests with unusual settings harder.
I'd definitely be -1 on that.
I think that it is worth fixing cases where a parameter change leads to
surprising results, like the operator_precedence_warning case just now.
But people should not be surprised when, say, changes in planner
parameters lead to different EXPLAIN output or different row ordering.
If we tried to lock that down it'd be counterproductive for the reason
Andres mentions: sometimes you *want* to see what you get for other
settings.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2017-02-15 18:30:30 -0500, Tom Lane wrote:
If we tried to lock that down it'd be counterproductive for the reason
Andres mentions: sometimes you *want* to see what you get for other
settings.
We could kinda address that by doing it in a separate file early in the
schedule, which could just be commented out when doing something like
this. But I'm still unconvinced it's worth caring.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Andres Freund <andres@anarazel.de> writes:
On 2017-02-15 18:30:30 -0500, Tom Lane wrote:
If we tried to lock that down it'd be counterproductive for the reason
Andres mentions: sometimes you *want* to see what you get for other
settings.
We could kinda address that by doing it in a separate file early in the
schedule, which could just be commented out when doing something like
this. But I'm still unconvinced it's worth caring.
Actually, that idea might be worth pursuing. Right now pg_regress.c has a
hard-wired notion that it should ALTER DATABASE SET certain parameters on
the regression database. The best you can say for that is it's ugly as
sin. It'd definitely be nicer if we could move that into someplace where
it's more readily adjustable. Having done that, locking down most stuff
by default might become more practical.
However, I'm not sure that just dumping the responsibility into an initial
test script is very workable. We'd need another copy for each contrib
and PL test suite, the isolation tests, yadda yadda. And maintaining
that many copies would be a nightmare. I think we'd need some way to have
pg_regress pick up the desired settings from a central place.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2/14/17 16:50, Jeff Janes wrote:
make installcheck currently fails against a server running
with bytea_output = escape.Making it succeed is fairly easy, and I think it is worth doing.
Attached are two options for doing that. One overrides bytea_output
locally where-ever needed, and the other overrides it for the entire
'regression' database.
I would use option 2 here (ALTER DATABASE) and be done with it. Some
people didn't like using ALTER DATABASE, but it's consistent with
existing use. If someone wants to change that, that can be independent
of this issue.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Mar 10, 2017 at 6:14 AM, Peter Eisentraut <peter.eisentraut@
2ndquadrant.com> wrote:
On 2/14/17 16:50, Jeff Janes wrote:
make installcheck currently fails against a server running
with bytea_output = escape.Making it succeed is fairly easy, and I think it is worth doing.
Attached are two options for doing that. One overrides bytea_output
locally where-ever needed, and the other overrides it for the entire
'regression' database.I would use option 2 here (ALTER DATABASE) and be done with it. Some
people didn't like using ALTER DATABASE, but it's consistent with
existing use. If someone wants to change that, that can be independent
of this issue.
Sorry about the naive question, but if someone has set the GUC bytea_output
= 'escape', then the intention seem to be to obtain the output in 'escape'
format for bytea.
With this, if an installcheck is done, that might also have been done with
the expectation that the output will be in 'escape' format. In that case,
how much is it justified to hard code the format for regression database?
However, I agree that there are not many bytea outputs in the current
regression suite
Regards,
Neha
On Thu, Mar 9, 2017 at 4:45 PM, Neha Khatri <nehakhatri5@gmail.com> wrote:
On Fri, Mar 10, 2017 at 6:14 AM, Peter Eisentraut <peter.eisentraut@2
ndquadrant.com> wrote:On 2/14/17 16:50, Jeff Janes wrote:
make installcheck currently fails against a server running
with bytea_output = escape.Making it succeed is fairly easy, and I think it is worth doing.
Attached are two options for doing that. One overrides bytea_output
locally where-ever needed, and the other overrides it for the entire
'regression' database.I would use option 2 here (ALTER DATABASE) and be done with it. Some
people didn't like using ALTER DATABASE, but it's consistent with
existing use. If someone wants to change that, that can be independent
of this issue.Sorry about the naive question, but if someone has set the GUC
bytea_output = 'escape', then the intention seem to be to obtain the output
in 'escape' format for bytea.
With this, if an installcheck is done, that might also have been done with
the expectation that the output will be in 'escape' format. In that case,
how much is it justified to hard code the format for regression database?
However, I agree that there are not many bytea outputs in the current
regression suite
At a high level (which is all I know here) If we leave behind tests that
at least exercise bytea_output='escape' mode to ensure it is functioning
properly then I'd have no problem having the testing of other features
dependent upon bytea_output, but that are coded to compare against the
now-default output format, set that runtime configurable mode to that which
they require. If the choice of output mode is simply a byproduct we should
be free to set it to whatever we need for the currently executing test.
If a simple way of doing this involves fixing the default to what the suite
expects and one-off changing it when testing escape mode stuff that seems
like a reasonable position to take. Having to set bytea_output when it
isn't the item under test seems like its just going to add noise.
David J.
On Thu, Mar 9, 2017 at 6:45 PM, Neha Khatri <nehakhatri5@gmail.com> wrote:
Sorry about the naive question, but if someone has set the GUC bytea_output
= 'escape', then the intention seem to be to obtain the output in 'escape'
format for bytea.
With this, if an installcheck is done, that might also have been done with
the expectation that the output will be in 'escape' format. In that case,
how much is it justified to hard code the format for regression database?
However, I agree that there are not many bytea outputs in the current
regression suite
I don't understand this. People don't run the regression tests to get
the output. They run the regression tests to see whether they pass.
While it may not be possible to make them pass with arbitrarily-crazy
settings, that's not a reason not to patch up the cases we can handle
sanely.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Robert Haas <robertmhaas@gmail.com> writes:
On Thu, Mar 9, 2017 at 6:45 PM, Neha Khatri <nehakhatri5@gmail.com> wrote:
With this, if an installcheck is done, that might also have been done with
the expectation that the output will be in 'escape' format. In that case,
how much is it justified to hard code the format for regression database?
However, I agree that there are not many bytea outputs in the current
regression suite
I don't understand this. People don't run the regression tests to get
the output. They run the regression tests to see whether they pass.
While it may not be possible to make them pass with arbitrarily-crazy
settings, that's not a reason not to patch up the cases we can handle
sanely.
I think the question that has to be settled to move this forward is
whether we're content with hard-wiring something for bytea_output
(as in Jeff's installcheck_bytea_fix_2.patch, which I concur with
Peter is superior to installcheck_bytea_fix_1.patch), or whether
we want to hold out for a more flexible solution, probably about like
what I sketched in
/messages/by-id/30246.1487202663@sss.pgh.pa.us
I think the more flexible solution is definitely a desirable place to
get to, but somehow I doubt it's going to happen for v10. Meanwhile
the question is whether adding more hard-wired behavior in pg_regress
is desirable or not.
I tend to vote with Andres that it's not worth the trouble, but
considering that it's only a 2-line change, I won't stand in the
way if some other committer is convinced this is an improvement.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers