public inbox for [email protected]  
help / color / mirror / Atom feed
From: Rémi Lapeyre <[email protected]>
To: Surafel Temesgen <[email protected]>
Cc: [email protected]
Cc: David Steele <[email protected]>
Cc: Vik Fearing <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: WIP: System Versioned Temporal Table
Date: Sat, 18 Jul 2020 18:05:00 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <CALAY4q9=RTfMu=O9=2Ovzx+iVhU+a4M5ZzxmQ2bvod77dKAg3g@mail.gmail.com>
References: <CALAY4q-cXCD0r4OybD=w7Hr7F026ZUY6=LMsVPUe6yw_PJpTKQ@mail.gmail.com>
	<[email protected]>
	<CALAY4q9u4r_J-=v_MXJTpEx6WA=YhT3A3n9sw6P2XxK0rWk5bg@mail.gmail.com>
	<[email protected]>
	<CALAY4q8xpuU7Qz7qy+5F8bh=GPAwR-pyscorAzTpyTQGyZbqtQ@mail.gmail.com>
	<[email protected]>
	<CALAY4q8ETRoFLBR8qz1P=SNskBBW5z-Kr90t_XGrm9mc9zCEbQ@mail.gmail.com>
	<[email protected]>
	<CALAY4q_voAROyjHL0fidvfY7=GGERjqSf0r=uGwrJ-T_9-VBKg@mail.gmail.com>
	<[email protected]>
	<CALAY4q88-1P1CWCJKEGHy1S3azG5g8cP_z_sEh+cd0G14YfmgA@mail.gmail.com>
	<[email protected]>
	<CALAY4q9B=D2EBEVMj6RG5tQPVw-rYw8mvKjDLYF8Bxzjr7APng@mail.gmail.com>
	<CAHHrWDP2txk9FQGqTj39iubHE=eTnoLPqVsBxdDJ-1FS9xZAaQ@mail.gmail.com>
	<CALAY4q9=RTfMu=O9=2Ovzx+iVhU+a4M5ZzxmQ2bvod77dKAg3g@mail.gmail.com>

Hi, thanks for working on this. I had planned to work on it and I’m looking forward to this natively in Postgres.

The patch builds with the following warnings:

plancat.c:2368:18: warning: variable 'name' is used uninitialized whenever 'for' loop exits because its condition is false [-Wsometimes-uninitialized]
        for (int i = 0; i < natts; i++)
                        ^~~~~~~~~
plancat.c:2379:9: note: uninitialized use occurs here
        return name;
               ^~~~
plancat.c:2368:18: note: remove the condition if it is always true
        for (int i = 0; i < natts; i++)
                        ^~~~~~~~~
plancat.c:2363:15: note: initialize the variable 'name' to silence this warning
        char       *name;
                        ^
                         = NULL
plancat.c:2396:18: warning: variable 'name' is used uninitialized whenever 'for' loop exits because its condition is false [-Wsometimes-uninitialized]
        for (int i = 0; i < natts; i++)
                        ^~~~~~~~~
plancat.c:2407:9: note: uninitialized use occurs here
        return name;
               ^~~~
plancat.c:2396:18: note: remove the condition if it is always true
        for (int i = 0; i < natts; i++)
                        ^~~~~~~~~
plancat.c:2391:15: note: initialize the variable 'name' to silence this warning
        char       *name;
                        ^
                         = NULL
2 warnings generated.


make check pass without issues, but make check-world fails for postgres_fdw, the diff is attached.


Before going further in the review, I’m a bit surprised by the quantity of code needed here. In https://github.com/xocolatl/periods there is far less code and I would have expected the same here. For example, are the changes to copy necessary or would it be possible to have a first patch the only implement the minimal changes required for this feature?



Thanks a lot!

Rémi









Attachments:

  [application/octet-stream] regression.diffs (17.7K, ../[email protected]/2-regression.diffs)
  download

view thread (97+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: WIP: System Versioned Temporal Table
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox