On Fri, Aug 2, 2019 at 1:49 PM Ibrar Ahmed <ibrar.ahmad@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
stagemust 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.
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.
Regards,
Paul