Hi Paul, On Mon, Aug 5, 2019 at 3:11 AM Paul A Jungwirth <p...@illuminatedcomputing.com> wrote:
> On Fri, Aug 2, 2019 at 1:49 PM Ibrar Ahmed <ibrar.ah...@gmail.com> wrote: > > I did some clean-up on this patch. I have also refactored a small > portion of the code > > to reduce the footprint of the patch. For simplicity, I have divided the > patch into 6 > > patches, now it is easy to review and debug. > > Please follow the PostgreSQL coding guidelines. I have found places > where you missed that, secondly code even in WIP stage must not have > WARNING because it looks ugly. > > Thank you for the cleanup Ibrar! I'll try to stick to the coding > standards more closely going forward. If you have any review comments > I would certainly appreciate them, especially about the overall > approach. I know that the implementation in its current form is not > very tasteful, but I wanted to get some feedback on the ideas. > > I have reviewed the main design, and in my opinion, it is a good start. - Why we are not allowing any other datatype other than ranges in the primary key. Without that there is no purpose of a primary key. - Thinking about some special token to differentiate between normal primary key and temporal primary key Also just to reiterate: this patch depends on my other CF entry > (range_agg), whose scope has expanded considerably. Right now I'm > focusing on that. And if you're trying to make this code work, it's > important to apply the range_agg patch first, since the temporal > foreign key implementation calls that function. > > Also: since this patch raises the question of how to conform to > SQL:2011 while still supporting Postgres range types, I wrote an > article that surveys SQL:2011 temporal features in MariaDB, DB2, > Oracle, and MS SQL Server: > > https://illuminatedcomputing.com/posts/2019/08/sql2011-survey/ > > A few highlights are: > > - Everyone lets you define PERIODs, but what you can do with them is > still *very* limited. > - No one treats PERIODs as first-class types or expressions; they are > more like table metadata. > > - Oracle PERIODs do permit NULL start/end values, and it interprets > them as "unbounded". That goes against the standard but since it's > what Postgres does with ranges, it suggests to me that maybe we should > follow their lead. Anyway I think a NULL is nicer than a sentinel for > this purpose. > That is an open debate, that we want to strictly follow the standard or map that to PostgreSQL range type which allows NULL. But how you will define a primary key on that? > > Regards, > Paul > -- Ibrar Ahmed
