Improve correlation names in sanity tests

2022-02-08 Thread Peter Eisentraut
I had to do some analysis on the "sanity" tests in the regression test 
suite (opr_sanity, type_sanity) recently, and I found some of the 
queries very confusing.  One main stumbling block is that for some 
probably ancient reason many of the older queries are written with 
correlation names p1, p2, etc. independent of the name of the catalog. 
This one is a good example:


SELECT p1.oid, p1.oprname, p2.oid, p2.proname
FROM pg_operator AS p1, pg_proc AS p2  <-- HUH?!?
WHERE p1.oprcode = p2.oid AND
p1.oprkind = 'l' AND
(p2.pronargs != 1
 OR NOT binary_coercible(p2.prorettype, p1.oprresult)
 OR NOT binary_coercible(p1.oprright, p2.proargtypes[0])
 OR p1.oprleft != 0);

I think this is better written as

SELECT o1.oid, o1.oprname, p1.oid, p1.proname
FROM pg_operator AS o1, pg_proc AS p1
WHERE o1.oprcode = p1.oid AND
o1.oprkind = 'l' AND
(p1.pronargs != 1
 OR NOT binary_coercible(p1.prorettype, o1.oprresult)
 OR NOT binary_coercible(o1.oprright, p1.proargtypes[0])
 OR o1.oprleft != 0);

Attached is a patch that cleans up all the queries in this manner.

(As in the above case, I kept the digits like o1 and p1 even in cases 
where only one of each letter is used in a query.  This is mainly to 
keep the style consistent, but if people don't like that at all, it 
could be changed.)From a194456df5f56042b3dd2b6c5bd95b27a771761a Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Tue, 8 Feb 2022 14:37:56 +0100
Subject: [PATCH] Improve correlation names in sanity tests

---
 src/test/regress/expected/opr_sanity.out  | 374 -
 src/test/regress/expected/type_sanity.out | 466 +++---
 src/test/regress/sql/opr_sanity.sql   | 376 -
 src/test/regress/sql/type_sanity.sql  | 466 +++---
 4 files changed, 841 insertions(+), 841 deletions(-)

diff --git a/src/test/regress/expected/opr_sanity.out 
b/src/test/regress/expected/opr_sanity.out
index 562b586d8e..4ce6c039b4 100644
--- a/src/test/regress/expected/opr_sanity.out
+++ b/src/test/regress/expected/opr_sanity.out
@@ -991,9 +991,9 @@ WHERE c.castmethod = 'b' AND
 
 --  pg_conversion 
 -- Look for illegal values in pg_conversion fields.
-SELECT p1.oid, p1.conname
-FROM pg_conversion as p1
-WHERE p1.conproc = 0 OR
+SELECT c.oid, c.conname
+FROM pg_conversion as c
+WHERE c.conproc = 0 OR
 pg_encoding_to_char(conforencoding) = '' OR
 pg_encoding_to_char(contoencoding) = '';
  oid | conname 
@@ -1025,8 +1025,8 @@ WHERE p.oid = c.conproc AND
 -- them directly from SQL.  But there are no non-default built-in
 -- conversions anyway.
 -- (Similarly, this doesn't cope with any search path issues.)
-SELECT p1.oid, p1.conname
-FROM pg_conversion as p1
+SELECT c.oid, c.conname
+FROM pg_conversion as c
 WHERE condefault AND
 convert('ABC'::bytea, pg_encoding_to_char(conforencoding),
 pg_encoding_to_char(contoencoding)) != 'ABC';
@@ -1036,32 +1036,32 @@ WHERE condefault AND
 
 --  pg_operator 
 -- Look for illegal values in pg_operator fields.
-SELECT p1.oid, p1.oprname
-FROM pg_operator as p1
-WHERE (p1.oprkind != 'b' AND p1.oprkind != 'l') OR
-p1.oprresult = 0 OR p1.oprcode = 0;
+SELECT o1.oid, o1.oprname
+FROM pg_operator as o1
+WHERE (o1.oprkind != 'b' AND o1.oprkind != 'l') OR
+o1.oprresult = 0 OR o1.oprcode = 0;
  oid | oprname 
 -+-
 (0 rows)
 
 -- Look for missing or unwanted operand types
-SELECT p1.oid, p1.oprname
-FROM pg_operator as p1
-WHERE (p1.oprleft = 0 and p1.oprkind != 'l') OR
-(p1.oprleft != 0 and p1.oprkind = 'l') OR
-p1.oprright = 0;
+SELECT o1.oid, o1.oprname
+FROM pg_operator as o1
+WHERE (o1.oprleft = 0 and o1.oprkind != 'l') OR
+(o1.oprleft != 0 and o1.oprkind = 'l') OR
+o1.oprright = 0;
  oid | oprname 
 -+-
 (0 rows)
 
 -- Look for conflicting operator definitions (same names and input datatypes).
-SELECT p1.oid, p1.oprcode, p2.oid, p2.oprcode
-FROM pg_operator AS p1, pg_operator AS p2
-WHERE p1.oid != p2.oid AND
-p1.oprname = p2.oprname AND
-p1.oprkind = p2.oprkind AND
-p1.oprleft = p2.oprleft AND
-p1.oprright = p2.oprright;
+SELECT o1.oid, o1.oprcode, o2.oid, o2.oprcode
+FROM pg_operator AS o1, pg_operator AS o2
+WHERE o1.oid != o2.oid AND
+o1.oprname = o2.oprname AND
+o1.oprkind = o2.oprkind AND
+o1.oprleft = o2.oprleft AND
+o1.oprright = o2.oprright;
  oid | oprcode | oid | oprcode 
 -+-+-+-
 (0 rows)
@@ -1070,14 +1070,14 @@ WHERE p1.oid != p2.oid AND
 -- DEFINITIONAL NOTE: If A.oprcom = B, then x A y has the same result as y B x.
 -- We expect that B will always say that B.oprcom = A as well; that's not
 -- inherently essential, but it would be inefficient not to mark it so.

Re: Improve correlation names in sanity tests

2022-02-08 Thread Tom Lane
Peter Eisentraut  writes:
> I had to do some analysis on the "sanity" tests in the regression test 
> suite (opr_sanity, type_sanity) recently, and I found some of the 
> queries very confusing.  One main stumbling block is that for some 
> probably ancient reason many of the older queries are written with 
> correlation names p1, p2, etc. independent of the name of the catalog. 

I think that was at least partially my fault, and no there isn't
any very good reason for it.  Your proposal seems fine.

regards, tom lane