> > If it's > > to stay, it *must* get a line-by-line review from some committer-level > > person; and I think there are other more important things for us to be > > doing for 9.5. > > > > Honestly, I am very surprised by this.
Tom's partial review found quite a crop of unvarnished bugs:
1. sample node can give different tuples across rescans within an executor run 2. missing dependency machinery to restrict dropping a sampling extension 3. missing "pg_dump --binary-upgrade" treatment 4. "potential core dumps due to dereferencing values that could be null" 5. factually incorrect comments 6. null argument checks in strict functions 7. failure to check for constisnull 8. "failure to sanity-check" ntuples 9. arithmetic errors in random_relative_prime()
(That's after sifting out design counterproposals, feature requests, and other topics of regular disagreement. I erred on the side of leaving things off that list.) Finding one or two like that during a complete post-commit review would be business as usual. Finding nine in a partial review diagnoses a critical shortfall in pre-commit review vigilance. Fixing the bugs found to date will not cure that shortfall. A qualified re-review could cure it.
Thank you for the summary of points. I agree with that list.
I will work on the re-review as you suggest.
1 and 4 relate to the sample API exposed, which needs some rework. We'll see how big that is; at this time I presume not that hard, but I will wait for Petr's opinion also when he returns on Friday.
--
Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services