Thread: [HACKERS] New CORRESPONDING clause design

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

ERROR:  each UNION query must have the same number of columns
LINE 1: ...elect 10 as a, 20 as b union corresponding select 20 as a, 3...                            

Regards

Pavel

 

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.stehule@gmail.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

2017-03-09 13:18 GMT+01:00 Surafel Temesgen <surafel3000@gmail.com>:

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

I don't understand. 

The following statement should to work

postgres=# select 10 as a, 20 as b union corresponding select 20 as a, 30 as b, 40 as c;

ERROR:  each UNION query must have the same number of columns
LINE 1: ...elect 10 as a, 20 as b union corresponding select 20 as a, 3...                            

Corresponding clause should to work like projection filter. 

Regards

Pavel

 

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.stehule@gmail.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

Yes, you are correct it should to work on CORRESPONDING clause case. SQL 20nn standard draft only said each query to be of the same degree in a case of set operation without corresponding clause. The attached patch is corrected as such .I add those new test case to regression test too

Regards

Surafel  

On Thu, Mar 9, 2017 at 9:49 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:

hi

2017-03-09 17:19 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:

2017-03-09 13:18 GMT+01:00 Surafel Temesgen <surafel3000@gmail.com>:

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

I don't understand. 

The following statement should to work

postgres=# select 10 as a, 20 as b union corresponding select 20 as a, 30 as b, 40 as c;

ERROR:  each UNION query must have the same number of columns
LINE 1: ...elect 10 as a, 20 as b union corresponding select 20 as a, 3...                            

Corresponding clause should to work like projection filter. 

I found a link to postgresql mailing list related to some older try to this feature implementation

 https://www.postgresql.org/message-id/CAJZSWkX7C6Wmfo9Py4BaF8vHz_Ofko3AFSOsJPsb17rGmgBuDQ@mail.gmail.com

Regards

Pavel

 

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.stehule@gmail.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

2017-03-10 10:13 GMT+01:00 Surafel Temesgen <surafel3000@gmail.com>:

Yes, you are correct it should to work on CORRESPONDING clause case. SQL 20nn standard draft only said each query to be of the same degree in a case of set operation without corresponding clause. The attached patch is corrected as such .I add those new test case to regression test too

Thank you - I will recheck it.

Hi

2017-03-10 12:55 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:

2017-03-10 10:13 GMT+01:00 Surafel Temesgen <surafel3000@gmail.com>:

Yes, you are correct it should to work on CORRESPONDING clause case. SQL 20nn standard draft only said each query to be of the same degree in a case of set operation without corresponding clause. The attached patch is corrected as such .I add those new test case to regression test too

Thank you - I will recheck it.

 Fast check - it looks well

Regards

Pavel

I am sending minor update - cleaning formatting and white spaces, error messages + few more tests

Maybe correspondingClause needs own node type with attached location. Then context can be much better positioned.

On Sat, Mar 11, 2017 at 9:01 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:

I am sending minor update - cleaning formatting and white spaces, error messages + few more tests

 

Thank you very much for your help

  

Maybe correspondingClause needs own node type with attached location. Then context can be much better positioned.

 

I think we can solve it by using your option or using expr_list for corresponding column and check the syntax manually.

In my opinion, the last option eliminate the introduction of new node for only the sake of error position.

What did you think about the second option?

Regards 

Surafel

Some errors are related to just CORRESPONDING without any columns. So using expr doesn't help here. So parse node CORRESPONDING can solve both issues.   

hi

Some errors are related to just CORRESPONDING without any columns. So using expr doesn't help here. So parse node CORRESPONDING can solve both issues.   

 In current implementation pointing to a node means pointing to a node’s first element so I don’t think we can be able to point to CORRESPONDING without any columns

I find out that there is already a node prepare for the case called A_Const.

The attached patch use that node 

Regards 

Surafel

Pavel Stehule <pavel.stehule@gmail.com> writes:
> I have not any objection - I'll mark this patch as ready for commiter

I took a quick look through this and noted that it fails to touch
ruleutils.c, which means that dumping of views containing CORRESPONDING
certainly doesn't work.

Also, the changes in parser/analyze.c seem rather massive and
correspondingly hard to review.  Is it possible to rearrange the
patch to reduce the size of that diff?  If you can avoid moving
or reindenting existing code, that'd help.

The code in that area seems rather confused, too.  For instance,
I'm not sure exactly what orderCorrespondingList() is good for,
but it certainly doesn't look to be doing anything that either its
name or its header comment (or the comments at the call sites) would
suggest.  Its input and output tlists are always in the same order.

I'm a little disturbed by the fact that determineMatchingColumns()
is called twice, and more disturbed by the fact that it looks to be
O(N^2).  This could be really slow with a lot of columns, couldn't it?

I also think there should be some comments about exactly what matching
semantics we're enforcing.   The SQL standard says

            a) If CORRESPONDING is specified, then:
              i) Within the columns of T1, equivalent <column name>s shall
                 not be specified more than once and within the columns of
                 T2, equivalent <column name>s shall not be specified more
                 than once.

That seems unreasonably stringent to me; it ought to be sufficient to
forbid duplicates of the names listed in CORRESPONDING, or the common
column names if there's no BY list.  But whichever restriction you prefer,
this code seems to be failing to check it --- I certainly don't see any
new error message about "column name "foo" appears more than once".

I don't think you want to be leaving behind debugging cruft like this:
+                       elog(DEBUG4, "%s", ltle->resname);

I'm not impressed by using A_Const for the members of the CORRESPONDING
name list.  That's not a clever solution, that's a confusing kluge,
because it's a complete violation of the meaning of A_Const.  Elsewhere
we just use lists of String for name lists, and that seems sufficient
here.  Personally I'd just use the existing columnList production rather
than rolling your own.

The comments in parsenodes.h about the new fields seem rather confused,
in fact I think they're probably backwards.

Also, I thought the test cases were excessively uninspired and repetitive.
This, for example, seems like about two tests too many:

+SELECT 1 a, 2 b, 3 c UNION CORRESPONDING BY(a) SELECT 4 a, 5 b, 6 c;
+ a
+---
+ 1
+ 4
+(2 rows)
+
+SELECT 1 a, 2 b, 3 c UNION CORRESPONDING BY(b) SELECT 4 a, 5 b, 6 c;
+ b
+---
+ 2
+ 5
+(2 rows)
+
+SELECT 1 a, 2 b, 3 c UNION CORRESPONDING BY(c) SELECT 4 a, 5 b, 6 c;
+ c
+---
+ 3
+ 6
+(2 rows)

without even considering the fact that these are pretty duplicative of
tests around them.  And some of the added tests seem to be just testing
basic UNION functionality, which we already have tests for.  I fail to
see the point of adding any of these:

+SELECT 1 AS two UNION CORRESPONDING SELECT 2 two;
+ two
+-----
+   1
+   2
+(2 rows)
+
+SELECT 1 AS one UNION CORRESPONDING SELECT 1 one;
+ one
+-----
+   1
+(1 row)
+
+SELECT 1 AS two UNION ALL CORRESPONDING SELECT 2 two;
+ two
+-----
+   1
+   2
+(2 rows)
+
+SELECT 1 AS two UNION ALL CORRESPONDING SELECT 1 two;
+ two
+-----
+   1
+   1
+(2 rows)

What I think actually is needed is some targeted testing showing
non-interference with nearby features.  As an example, CORRESPONDING
can't safely be implemented by just replacing the tlists of the
input sub-selects with shortened versions, because that would break
situations such as DISTINCT in the sub-selects.  I think it'd be a
good idea to have a test along the lines of

        SELECT DISTINCT x, y FROM ...
        UNION ALL CORRESPONDING
        SELECT DISTINCT x, z FROM ...

with values chosen to show that the DISTINCT operators did operate
on x/y and x/z, not just x alone.

                        regards, tom lane

Pavel Stehule <pavel.stehule@gmail.com> writes:
> 2017-03-18 17:50 GMT+01:00 Tom Lane <tgl@sss.pgh.pa.us>:
>> I'm not impressed by using A_Const for the members of the CORRESPONDING
>> name list.  That's not a clever solution, that's a confusing kluge,
>> because it's a complete violation of the meaning of A_Const.  Elsewhere
>> we just use lists of String for name lists, and that seems sufficient
>> here.  Personally I'd just use the existing columnList production rather
>> than rolling your own.

> The reason was attach a location to name for more descriptive error
> message.

[ shrug... ] The patch fails to actually use the location anywhere.
If it had, you might have noticed that it's attaching the wrong location
to all elements except the first :-(.  So I'm not very excited about that.
I definitely don't see a reason for CORRESPONDING to track locations of
name list elements when no other name list productions do.  It might be
worth proposing a followon patch to change all of them (perhaps by adding
a location field to struct "Value") and then make use of the locations in
error messages more widely.

                        regards, tom lane

Pavel Stehule <pavel.stehule@gmail.com> writes:
> 2017-03-18 18:32 GMT+01:00 Tom Lane <tgl@sss.pgh.pa.us>:
>> I definitely don't see a reason for CORRESPONDING to track locations of
>> name list elements when no other name list productions do.  It might be
>> worth proposing a followon patch to change all of them (perhaps by adding
>> a location field to struct "Value") and then make use of the locations in
>> error messages more widely.

> I had a idea use own node for  CORRESPONDING with location - and using this
> location in related error messages.

I think using a private node type for CORRESPONDING is exactly the wrong
thing.  It's a columnList and it should be like other columnLists.  If
there's an argument for providing a location for "no such column" errors
for CORRESPONDING, then surely there's also an argument for providing
a location for "no such column" errors for FOREIGN KEY and the other
places where we have lists of column names.

                        regards, tom lane

Pavel Stehule <pavel.stehule@gmail.com> writes:
> I have not any objection - I'll mark this patch as ready for commiter


I'm a little disturbed by the fact that determineMatchingColumns()
is called twice, and more disturbed by the fact that it looks to be
O(N^2).  This could be really slow with a lot of columns, couldn't it?

I also think there should be some comments about exactly what matching
semantics we're enforcing.   The SQL standard says

            a) If CORRESPONDING is specified, then:
              i) Within the columns of T1, equivalent <column name>s shall
                 not be specified more than once and within the columns of
                 T2, equivalent <column name>s shall not be specified more
                 than once.

That seems unreasonably stringent to me; it ought to be sufficient to
forbid duplicates of the names listed in CORRESPONDING, or the common
column names if there's no BY list.  But whichever restriction you prefer,
this code seems to be failing to check it --- I certainly don't see any
new error message about "column name "foo" appears more than once".

What I think actually is needed is some targeted testing showing
non-interference with nearby features.  As an example, CORRESPONDING
can't safely be implemented by just replacing the tlists of the
input sub-selects with shortened versions, because that would break
situations such as DISTINCT in the sub-selects.  I think it'd be a
good idea to have a test along the lines of

        SELECT DISTINCT x, y FROM ...
        UNION ALL CORRESPONDING
        SELECT DISTINCT x, z FROM ...

with values chosen to show that the DISTINCT operators did operate
on x/y and x/z, not just x alone.

I took a quick look through this and noted that it fails to touch
ruleutils.c, which means that dumping of views containing CORRESPONDING
certainly doesn't work.

Also, the changes in parser/analyze.c seem rather massive and
correspondingly hard to review.  Is it possible to rearrange the
patch to reduce the size of that diff?  If you can avoid moving
or reindenting existing code, that'd help.

The code in that area seems rather confused, too.  For instance,
I'm not sure exactly what orderCorrespondingList() is good for,
but it certainly doesn't look to be doing anything that either its
name or its header comment (or the comments at the call sites) would
suggest.  Its input and output tlists are always in the same order.

I also think there should be some comments about exactly what matching
semantics we're enforcing.   The SQL standard says

            a) If CORRESPONDING is specified, then:
              i) Within the columns of T1, equivalent <column name>s shall
                 not be specified more than once and within the columns of
                 T2, equivalent <column name>s shall not be specified more
                 than once.

That seems unreasonably stringent to me; it ought to be sufficient to
forbid duplicates of the names listed in CORRESPONDING, or the common
column names if there's no BY list.  But whichever restriction you prefer,
this code seems to be failing to check it --- I certainly don't see any
new error message about "column name "foo" appears more than once".

I'm not impressed by using A_Const for the members of the CORRESPONDING
name list.  That's not a clever solution, that's a confusing kluge,
because it's a complete violation of the meaning of A_Const.  Elsewhere
we just use lists of String for name lists, and that seems sufficient
here.  Personally I'd just use the existing columnList production rather
than rolling your own.

I took a quick look through this and noted that it fails to touch
ruleutils.c, which means that dumping of views containing CORRESPONDING
certainly doesn't work.

fixed  

Also, the changes in parser/analyze.c seem rather massive and
correspondingly hard to review.  Is it possible to rearrange the
patch to reduce the size of that diff?  If you can avoid moving
or reindenting existing code, that'd help.

Part of transformSetOperationTree that make union data type of 

set operation target list became makeUnionDatatype inorder to 

easy using it multiple time and avoid very long transformSetOperationTree 

function 

 

The code in that area seems rather confused, too.  For instance,
I'm not sure exactly what orderCorrespondingList() is good for,
but it certainly doesn't look to be doing anything that either its
name or its header comment (or the comments at the call sites) would
suggest.  Its input and output tlists are always in the same order.

It give corresponding target list a sequential resnos

Inorder to avoid touching generate_append_tlist I change 

the comment and function name as such 

I also think there should be some comments about exactly what matching
semantics we're enforcing.   The SQL standard says

            a) If CORRESPONDING is specified, then:
              i) Within the columns of T1, equivalent <column name>s shall
                 not be specified more than once and within the columns of
                 T2, equivalent <column name>s shall not be specified more
                 than once.

That seems unreasonably stringent to me; it ought to be sufficient to
forbid duplicates of the names listed in CORRESPONDING, or the common
column names if there's no BY list.  But whichever restriction you prefer,
this code seems to be failing to check it --- I certainly don't see any
new error message about "column name "foo" appears more than once".

fixed  

I'm not impressed by using A_Const for the members of the CORRESPONDING
name list.  That's not a clever solution, that's a confusing kluge,
because it's a complete violation of the meaning of A_Const.  Elsewhere
we just use lists of String for name lists, and that seems sufficient
here.  Personally I'd just use the existing columnList production rather
than rolling your own.

fixed  

can you help with fixing it Pavel?

On Mon, Mar 27, 2017 at 11:48 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:

Hi

fresh update - I enhanced Value node by location field as Tom proposal.

Few more regress tests.

But I found significant issue, that needs bigger fix - Surafel, please, can you fix it.

It crash on 

SELECT 0 AS x1, 1 AS a, 0 AS x2, 2 AS b, 0 AS x3, -1 AS x3 

UNION ALL CORRESPONDING SELECT 4 AS b, 0 AS x4, 3 AS a, 0 AS x6, -1 AS x6 

UNION ALL CORRESPONDING SELECT 0 AS x8, 6 AS b, -100 AS x9;

I'll mark this patch as waiting on author

Regards

Pavel 

2017-03-28 13:58 GMT+02:00 Surafel Temesgen <surafel3000@gmail.com>:

can you help with fixing it Pavel?

There must be some new preanalyze stage - you have to know result columns before you are starting a analyze

Regards

Pavel

On Mon, Mar 27, 2017 at 11:48 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:

Hi

fresh update - I enhanced Value node by location field as Tom proposal.

Few more regress tests.

But I found significant issue, that needs bigger fix - Surafel, please, can you fix it.

It crash on 

SELECT 0 AS x1, 1 AS a, 0 AS x2, 2 AS b, 0 AS x3, -1 AS x3 

UNION ALL CORRESPONDING SELECT 4 AS b, 0 AS x4, 3 AS a, 0 AS x6, -1 AS x6 

UNION ALL CORRESPONDING SELECT 0 AS x8, 6 AS b, -100 AS x9;

I'll mark this patch as waiting on author

Regards

Pavel 

hi

Thank you very much for your help .

here is the patch fix that issue as you suggest

 

Regards

 

Surafel 

On Tue, Mar 28, 2017 at 5:44 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:

2017-03-28 14:18 GMT+02:00 Pavel Stehule <pavel.stehule@gmail.com>:

2017-03-28 13:58 GMT+02:00 Surafel Temesgen <surafel3000@gmail.com>:

can you help with fixing it Pavel?

There must be some new preanalyze stage - you have to know result columns before you are starting a analyze

maybe some recheck after analyze stage to remove invalid columns can be good enough.

Regards

Pavel

 

Regards

Pavel

On Mon, Mar 27, 2017 at 11:48 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:

Hi

fresh update - I enhanced Value node by location field as Tom proposal.

Few more regress tests.

But I found significant issue, that needs bigger fix - Surafel, please, can you fix it.

It crash on 

SELECT 0 AS x1, 1 AS a, 0 AS x2, 2 AS b, 0 AS x3, -1 AS x3 

UNION ALL CORRESPONDING SELECT 4 AS b, 0 AS x4, 3 AS a, 0 AS x6, -1 AS x6 

UNION ALL CORRESPONDING SELECT 0 AS x8, 6 AS b, -100 AS x9;

I'll mark this patch as waiting on author

Regards

Pavel 

Pavel Stehule <pavel.stehule@gmail.com> writes:
> Is following use case defined in standard?

> postgres=# SELECT 0 AS x1, 1 AS a, 0 AS x2, 2 AS b, 0 AS x3, -1 AS x3
>            UNION ALL CORRESPONDING BY(a,b) SELECT 4 AS b, 0 AS x4, 3 AS a,
> 0 AS x6, -1 AS x6
>            UNION ALL CORRESPONDING SELECT 0 AS x8, 6 AS a, -100 AS aa;
> ┌───┐
> │ a │
> ╞═══╡
> │ 1 │
> │ 3 │
> │ 6 │
> └───┘
> (3 rows)

> It depends on order of implementation

> if we do (T1 U T2) U T3 ---> then result is correct,
> but if we do T1 U (T2 U T3) ---> than it should to fail

UNION ALL should associate left-to-right, just like most other binary
operators, so this looks fine to me.  Did you check that you get an
error if you put in parens to force the other order?

                        regards, tom lane

2017-03-30 21:43 GMT+02:00 Tom Lane <tgl@sss.pgh.pa.us>:

Pavel Stehule <pavel.stehule@gmail.com> writes:
> Is following use case defined in standard?

> postgres=# SELECT 0 AS x1, 1 AS a, 0 AS x2, 2 AS b, 0 AS x3, -1 AS x3
>            UNION ALL CORRESPONDING BY(a,b) SELECT 4 AS b, 0 AS x4, 3 AS a,
> 0 AS x6, -1 AS x6
>            UNION ALL CORRESPONDING SELECT 0 AS x8, 6 AS a, -100 AS aa;
> ┌───┐
> │ a │
> ╞═══╡
> │ 1 │
> │ 3 │
> │ 6 │
> └───┘
> (3 rows)

> It depends on order of implementation

> if we do (T1 U T2) U T3 ---> then result is correct,
> but if we do T1 U (T2 U T3) ---> than it should to fail

UNION ALL should associate left-to-right, just like most other binary
operators, so this looks fine to me.  Did you check that you get an
error if you put in parens to force the other order?

yes - it fails

 postgres=# SELECT 0 AS x1, 1 AS a, 0 AS x2, 2 AS b, 0 AS x3, -1 AS x3 UNION ALL CORRESPONDING BY(a,b) (SELECT 4 AS b, 0 AS x4, 3 AS a, 0 AS x6, -1 AS x6 UNION ALL CORRESPONDING SELECT 0 AS x8, 6 AS a, -100 AS aa);

ERROR:  column name "b" can not be used in CORRESPONDING BY list

LINE 1: ...b, 0 AS x3, -1 AS x3 UNION ALL CORRESPONDING BY(a,b) (SELECT...

                                                             ^

HINT:  UNION queries with a CORRESPONDING BY clause must contain column names from both tables.

Time: 1,135 ms

Regards

Pavel

                        regards, tom lane