From: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
---|---|
To: | Surafel Temsgen <surafel3000(at)gmail(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: New CORRESPONDING clause design |
Date: | 2017-03-07 19:26:39 |
Message-ID: | CAFj8pRCCi--Zs4jOrnLXZkxOiJjF6Nn0VeEwBd=e7fovAb-z0g@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi
I am sending a review of this interesting feature.
I found following issues, questions:
1. unclosed tags <optional> in documentation
2. bad name "changeTargetEntry" - should be makeTargetEntry?
3. Why you removed lot of asserts in prepunion.c? These asserts should be
valid still
4. make_coresponding_target has wrong formatting
5. error "%s queries with a CORRESPONDING clause must have at least one
column with the same name" has wrong formatting, you can show position
6. previous issue is repeated - look on formatting ereport function,
please, you can use DETAIL and HINT fields
7. corresponding clause should to contain column list (I am looking to
ANSI/SQL 99) - you are using expr_list, what has not sense and probably it
has impact on all implementation.
8. typo orderCorrespondingLsit(List *targetlist)
9. I miss more tests for CORRESPONDING BY
10. if I understand to this feature, this query should to work
postgres=# SELECT 1 a, 2 b, 3 c UNION CORRESPONDING BY (c,b) SELECT 4
a, 5 b, 6 c, 8 d;
ERROR: each UNION query must have the same number of columns
LINE 1: ...1 a, 2 b, 3 c UNION CORRESPONDING BY (c,b) SELECT 4 a, 5 b, ...
postgres=# create table t1(a int, b int, c int);
CREATE TABLE
Time: 63,260 ms
postgres=# create table t2(a int, b int);
CREATE TABLE
Time: 57,120 ms
postgres=# select * from t1 union corresponding select * from t2;
ERROR: each UNION query must have the same number of columns
LINE 1: select * from t1 union corresponding select * from t2;
If it is your first patch to Postgres, then it is perfect work!
The @7 is probably most significant - I dislike a expression list there.
name_list should be better there.
Regards
Pavel
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2017-03-07 19:38:57 | Re: SQL/JSON in PostgreSQL |
Previous Message | Robert Haas | 2017-03-07 19:25:55 | Re: Bizarre choice of case for RELKIND_PARTITIONED_TABLE |