|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|
|Views:||Raw Message | Whole Thread | Download mbox|
> 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.
|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|