On 08/01/2016 11:45 AM, Aleksander Alekseev wrote:
Hello.
Thanks everyone for great comments!
Well, jokes aside, that's a pretty lousy excuse for not writing any
docs
I think maybe I put it in a wrong way. There are currently a lot of
comments in a code, more then enough to understand how this feature
works. What I meant is that this is not a final version of a patch and
a few paragraphs are yet to be written. At least it's how I see it. If
you believe that some parts of the code are currently hard to understand
and some comments could be improved, please name it and I will be happy
to fix it.
I don't think there's "a lot of comments in the code", not even
remotely. At least not in the files I looked into - heapam, indexam,
xact etc. There are a few comments in general, and most of them only
comment obvious facts, like "ignore in-memory tuples" right before a
trivial if statement.
What is needed is an overview of the approach, so that the reviewers can
read that first, instead of assembling the knowledge from pieces
scattered over comments in many pieces. But I see the fasttab.c contains
this:
/* TODO TODO comment the general idea - in-memory tuples and indexes,
hooks principle, FasttabSnapshots, etc */
The other thing that needs to happen is you need to modify comments in
front of some of the modified methods - e.g. the comments may need a
paragraph "But when the table is fast temporary, what happens is ..."
IMHO if this patch gets in, we should use it as the only temp table
implementation (Or can you think of cases where keeping rows in
pg_class has advantages?). That'd also eliminate the need for FAST
keyword in the CREATE TABLE command.
Probably has zero value to have slow and fast temp tables (from
catalogue cost perspective). So the FAST implementation should be used
everywhere.
If there are no objections I see no reason no to do it in a next
version of a patch.
I believe there will be a lot of discussion about this.
I'm getting failures in the regression suite
I've run regression suite like 10 times in a row in different
environments with different build flags but didn't manage to reproduce
it. Also our DBAs are testing this feature for weeks now on real-world
applications and they didn't report anything like this. Could you
please describe how to reproduce this issue?
Nothing special:
$ ./configure --prefix=/home/user/pg-temporary --enable-debug \
--enable-cassert
$ make -s clean && make -s -j4 install
$ export PATH=/home/user/pg-temporary/bin:$PATH
$ pg_ctl -D ~/tmp/data-temporary init
$ pg_ctl -D ~/tmp/data-temporary -l ~/temporary.log start
$ make installcheck
I get the failures every time - regression diff attached. The first
failure in "rolenames" is expected, because of clash with existing user
name. The remaining two failures are not.
I only get the failure for "installcheck" but not "check" for some reason.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
*** /home/user/work/postgres/src/test/regress/expected/rolenames.out
2016-07-18 20:48:39.421444507 +0200
--- /home/user/work/postgres/src/test/regress/results/rolenames.out
2016-08-01 15:07:04.597000000 +0200
***************
*** 42,47 ****
--- 42,48 ----
CREATE ROLE "current_user";
CREATE ROLE "session_user";
CREATE ROLE "user";
+ ERROR: role "user" already exists
CREATE ROLE current_user; -- error
ERROR: CURRENT_USER cannot be used as a role name here
LINE 1: CREATE ROLE current_user;
***************
*** 956,958 ****
--- 957,960 ----
DROP OWNED BY regress_testrol0, "Public", "current_user", regress_testrol1,
regress_testrol2, regress_testrolx CASCADE;
DROP ROLE regress_testrol0, regress_testrol1, regress_testrol2,
regress_testrolx;
DROP ROLE "Public", "None", "current_user", "session_user", "user";
+ ERROR: current user cannot be dropped
======================================================================
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
