Re: New CORRESPONDING clause design

From: Surafel Temesgen <surafel3000(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: New CORRESPONDING clause design
Date: 2017-03-25 12:41:07
Views: Raw Message | Whole Thread | Download mbox
Lists: pgsql-hackers

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

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

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.


Attachment Content-Type Size
corresponding_clause_v7.patch application/octet-stream 59.0 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Arseny Sher 2017-03-25 12:47:44 Re: [GSoC] Push-based query executor discussion
Previous Message Rushabh Lathia 2017-03-25 12:09:55 Re: crashes due to setting max_parallel_workers=0