Horton

Re: [Proposal] Global temporary tables

Robert Haas <robertmhaas@gmail.com>

View thread
From: Robert Haas <robertmhaas@gmail.com>
To: wenjing zeng <wjzeng2012@gmail.com>
Cc: Pavel Stehule <pavel.stehule@gmail.com>, Prabhat Sahu <prabhat.sahu@enterprisedb.com>, tushar <tushar.ahuja@enterprisedb.com>, Tomas Vondra <tomas.vondra@2ndquadrant.com>, Konstantin Knizhnik <k.knizhnik@postgrespro.ru>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2020-07-30T20:57:42Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. PageAddItemExtended(): Add LP_UNUSED assertion.

  2. Remove temporary files after backend crash

  3. Fix comment in indexing.c

  4. Fix failure to ignore leftover temp tables after a server crash.

On Thu, Jul 30, 2020 at 8:09 AM wenjing zeng <wjzeng2012@gmail.com> wrote:
> Please continue to review the code.

This patch is pretty light on comments. Many of the new functions have
no header comments, for example. There are comments here and there in
the body of the new functions that are added, and in places where
existing code is changed there are comments here and there, but
overall it's not a whole lot. There's no documentation and no README,
either. Since this adds a new feature and a bunch of new SQL-callable
functions that interact with that feature, the feature itself should
be documented, along with its limitations and the new SQL-callable
functions that interact with it. I think there should be either a
lengthy comment in some suitable file, or maybe various comments in
various files, or else a README file, that clearly sets out the major
design principles behind the patch, and explaining also what that
means in terms of features and limitations. Without that, it's really
hard for anyone to jump into reviewing this code, and it will be hard
for people who have to maintain it in the future to understand it,
either. Or for users, for that matter.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company