Re: Boolean partitions syntax
От | Amit Langote |
---|---|
Тема | Re: Boolean partitions syntax |
Дата | |
Msg-id | 7ac6b44e-4638-3320-1512-f6c03a28d0f7@lab.ntt.co.jp обсуждение исходный текст |
Ответ на | Re: Boolean partitions syntax (Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>) |
Ответы |
Re: Boolean partitions syntax
(Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>)
Re: Boolean partitions syntax (Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>) |
Список | pgsql-hackers |
Horiguchi-san, Thank you for updating the patch. On 2018/04/16 16:17, Kyotaro HORIGUCHI wrote: > the attached v6 patch differs only in gram.y since v5. Patch fails to compile, because it adds get_partition_col_collation to rel.h instead of partcache.h: src/include/utils/rel.h: In function ‘get_partition_col_collation’: src/include/utils/rel.h:594:12: error: dereferencing pointer to incomplete type ‘struct PartitionKeyData’ PartitionKeyData definition was recently moved to partcache.h as part of the big partition code reorganization. > At Fri, 13 Apr 2018 18:55:30 +0900, Amit Langote wrote: >> Looking at the gram.y changes in the latest patch, I think there needs to >> be some explanatory comments about about the new productions -- u_expr, >> b0_expr, and c0_expr. > > I think I did that. And refactord the rules. > > It was a bother that some rules used c_expr directly but I > managed to replace all of them with a_expr by lowering precedence > of some ordinary keywords (PASSING, BY, COLUMNS and ROW). c_expr > is no loger used elsewhere so we can just remove columnref from > c_expr. Finally [abc]0_expr was eliminated and we have only > a_expr, b_expr, u_expr and c_expr. This seems simple enough. > > The relationship among the rules after this change is as follows. > > a_expr --+-- columnref > +-- u_expr ---+-- c_expr -- (all old c_expr stuff except columnref) > +-- (all old a_expr stuff) > > b_expr --+-- columnref > +-- c_expr -- (ditto) > +-- (all old b_expr stuff) > > On the way fixing this, I noticed that the precedence of some > keywords (PRESERVE, STRIP_P) that was more than necessary and > corrected it. Thank you for simplifying gram.y changes. I think I was able to understand them. Having said that, I think your proposed patch to gram.y could be rewritten such that they do not *appear* to impact the syntax for other features like XML, etc. For example, allowing a_expr in places where previously only c_expr was allowed seems to me a dangerous, although I don't have any examples to show for it. It seems that we would like to use a_expr syntax but without columnref for partition_bound expressions. No columnrefs because they cause conflicts with unreserved keywords MINVALUE and MAXVALUE that are used in range bound productions. I think that can be implemented with minimal changes to expression syntax productions, as shown in the attached patch. About the changes in transformPartitionBoundValue() to check for collation conflict, I think that seems unnecessary. We're not going to use the partition bound expression's collation anywhere, so trying to validate it seems unnecessary. I wondered if we should issue a WARNING to warn the user that COLLATE specified in a partition bound expression is ignored, but not sure after seeing that we issue none when a user specifies COLLATION in the expression for a column's DEFAULT value. We do still store the COLLATION expression in pg_attrdef, but it doesn't seem to be used anywhere. Please find attached an updated version of your patch. I think we'll need to make some documentation changes and think about a way to back-patch this to PG10. Thanks, Amit
Вложения
В списке pgsql-hackers по дате отправления:
Предыдущее
От: Craig RingerДата:
Сообщение: Re: PostgreSQL's handling of fsync() errors is unsafe and risks data loss at least on XFS
Следующее
От: Ashutosh BapatДата:
Сообщение: de-deduplicate code in DML execution hooks in postgres_fdw