Re: New CORRESPONDING clause design

From: Surafel Temesgen <surafel3000(at)gmail(dot)com>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: New CORRESPONDING clause design
Date: 2017-03-09 12:18:37
Message-ID: CALAY4q-rJ2_Fy+-G29L4Ao2kMwRPN8LcwSZ8wTo+JfP3nT0kCw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi ,

Here is a patch corrected as your feedback except missed tests case because
corresponding by clause is implemented on the top of set operation and you
can’t do that to set operation without corresponding by clause too

Eg

postgres=# SELECT 1 a, 2 b, 3 c UNION SELECT 4 a, 5 b, 6 c, 8 d;

ERROR: each UNION query must have the same number of columns

LINE 1: SELECT 1 a, 2 b, 3 c UNION SELECT 4 a, 5 b, 6 c, 8 d;

^

postgres=# create table t1(a int, b int, c int);

CREATE TABLE

postgres=# create table t2(a int, b int);

CREATE TABLE

postgres=# select * from t1 union select * from t2;

ERROR: each UNION query must have the same number of columns

LINE 1: select * from t1 union select * from t2;

Thanks

Surafel

On Tue, Mar 7, 2017 at 10:26 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
wrote:

> 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
>
>

Attachment Content-Type Size
corresponding_clause_v2.patch application/octet-stream 56.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Naytro Naytro 2017-03-09 12:39:40 Performance issue after upgrading from 9.4 to 9.6
Previous Message Alexander Korotkov 2017-03-09 12:00:57 Re: [PATCH] kNN for btree