Re: [HACKERS] proposal: schema variables
> Hi,
>
> I propose a new database object - a variable. The variable is persistent
> object, that holds unshared session based not transactional in memory value
> of any type. Like variables in any other languages. The persistence is
> required for possibility to do static checks, but can be limited to session
> - the variables can be temporal.
>
> My proposal is related to session variables from Sybase, MSSQL or MySQL
> (based on prefix usage @ or @@), or package variables from Oracle (access
> is controlled by scope), or schema variables from DB2. Any design is coming
> from different sources, traditions and has some advantages or
> disadvantages. The base of my proposal is usage schema variables as session
> variables for stored procedures. It should to help to people who try to
> port complex projects to PostgreSQL from other databases.
>
> The Sybase (T-SQL) design is good for interactive work, but it is weak for
> usage in stored procedures - the static check is not possible. Is not
> possible to set some access rights on variables.
>
> The ADA design (used on Oracle) based on scope is great, but our
> environment is not nested. And we should to support other PL than PLpgSQL
> more strongly.
>
> There is not too much other possibilities - the variable that should be
> accessed from different PL, different procedures (in time) should to live
> somewhere over PL, and there is the schema only.
>
> The variable can be created by CREATE statement:
>
> CREATE VARIABLE public.myvar AS integer;
> CREATE VARIABLE myschema.myvar AS mytype;
>
> CREATE [TEMP] VARIABLE [IF NOT EXISTS] name AS type
> [ DEFAULT expression ] [[NOT] NULL]
> [ ON TRANSACTION END { RESET | DROP } ]
> [ { VOLATILE | STABLE } ];
>
> It is dropped by command DROP VARIABLE [ IF EXISTS] varname.
>
> The access rights is controlled by usual access rights - by commands
> GRANT/REVOKE. The possible rights are: READ, WRITE
>
> The variables can be modified by SQL command SET (this is taken from
> standard, and it natural)
>
> SET varname = expression;
>
> Unfortunately we use the SET command for different purpose. But I am
> thinking so we can solve it with few tricks. The first is moving our GUC to
> pg_catalog schema. We can control the strictness of SET command. In one
> variant, we can detect custom GUC and allow it, in another we can disallow
> a custom GUC and allow only schema variables. A new command LET can be
> alternative.
>
> The variables should be used in queries implicitly (without JOIN)
>
> SELECT varname;
>
> The SEARCH_PATH is used, when varname is located. The variables can be used
> everywhere where query parameters are allowed.
>
> I hope so this proposal is good enough and simple.
>
> Comments, notes?
Just q quick follow up. Looks like a greate feature!
Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Lockable views
> I'm a bit confused. What is difference between tables and functions > in a subquery with regard to view locking? I think also none view queries > using a subquery do not care about the changes of tables in the > subquery while executing the query. I might be misnderstanding > the problem you mentioned. The difference is in the function cases we concern the function definition. While the table cases need to care about table definitions *and* contents of the table. If we are changing the table definition, AccessExclusiveLock will be held for the table and the updation will be blocked anyway. So we don't need to care about the table definition changes. On the other hand, table contents changes need to be cared because no automatic locking are held in some cases. I think whether tables in the subquery need locking or not is depending on use cases. So I dug into the previous candidates a little bit more: 1) Leave as it is (ignore tables appearing in a subquery) 2) Lock all tables including in a subquery 3) Check subquery in the view definition. If there are some tables involved, emit an error and abort. I think one of the problems with #2 is, we will lock tables involved by the view in random order, which could cause unwanted dead locks. This is not good and I cannot see any easy way to avoid this. Also some tables may not need to be locked. Problem with #3 is, it does not help a user who wants to control lockings by himself/herself. So it seem #1 is the most reasonable way to deal with the problem assuming that it's user's responsibility to take appropriate locks on the tables in the subquery. > BTW, I found that if we have to handle subqueries in where clause, we would > also have to care about subqueries in target list... The view defined as > below is also updatable. > > =# create view v7 as select (select * from tbl2 limit 1) from tbl; The view is not updatable. You will get something like if you try to update v7: DETAIL: Views that have no updatable columns are not automatically updatable. On the other hand this: create view v7 as select i, (select j from tbl2 limit 1) from tbl; will be updatable. In this case column j of v7 will never be updatable. And you should do something like: insert into v7(i) values... In short, you don't need to care about a subquery appearing in the TLE as far as the view locking concerns. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Lockable views
>> >> test=# CREATE VIEW v3 AS SELECT count(*) FROM v1; >> >> CREATE VIEW >> >> test=# BEGIN; >> >> BEGIN >> >> test=# LOCK TABLE v3; >> >> ERROR: cannot lock view "v3" >> >> DETAIL: Views that return aggregate functions are not automatically >> >> updatable. >> > >> > It would be nice if the message would be something like: >> > >> > DETAIL: Views that return aggregate functions are not lockable > > This uses messages from view_query_is_auto_updatable() of the rewrite system > directly. > Although we can modify the messages, I think it is not necessary for now > since we can lock only automatically updatable views. You could add a flag to view_query_is_auto_updatable() to switch the message between DETAIL: Views that return aggregate functions are not automatically updatable. and DETAIL: Views that return aggregate functions are not lockable >> > I wonder if we should lock tables in a subquery as well. For example, >> > >> > create view v1 as select * from t1 where i in (select i from t2); >> > >> > In this case should we lock t2 as well? >> >> Current the patch ignores t2 in the case above. >> >> So we have options below: >> >> - Leave as it is (ignore tables appearing in a subquery) >> >> - Lock all tables including in a subquery >> >> - Check subquery in the view definition. If there are some tables >> involved, emit an error and abort. >> >> The first one might be different from what users expect. There may be >> a risk that the second one could cause deadlock. So it seems the third >> one seems to be the safest IMO. > > Make sense. Even if the view is locked, when tables in a subquery is > modified, the contents of view can change. To avoid it, we have to > lock tables, or give up to lock such views. > > We can say the same thing for functions in a subquery. If the definition > of the functions are changed, the result of the view can change. > We cannot lock functions, but should we abtain row-level lock on pg_proc > in such cases? (of cause, or give up to lock such views) I think we don't need to care about function definition changes used in where clauses in views. None view queries using functions do not care about the definition changes of functions while executing the query. So why updatable views need to care them? > BTW, though you mentioned the risk of deadlocks, even when there > are no subquery, deadlock can occur in the current patch. > > For example, lock a table T in Session1, and then lock a view V > whose base relation is T in Session2. Session2 will wait for > Session1 to release the lock on T. After this, when Session1 try to > lock view V, the deadlock occurs and the query is canceled. You are right. Dealocks could occur in any case. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Lockable views
>> test=# CREATE VIEW v3 AS SELECT count(*) FROM v1; >> CREATE VIEW >> test=# BEGIN; >> BEGIN >> test=# LOCK TABLE v3; >> ERROR: cannot lock view "v3" >> DETAIL: Views that return aggregate functions are not automatically >> updatable. > > It would be nice if the message would be something like: > > DETAIL: Views that return aggregate functions are not lockable > >> test=# END; >> ROLLBACK >> >> test=# CREATE FUNCTION fnc() RETURNS trigger AS $$ BEGIN RETURN NEW; END; $$ >> LANGUAGE plpgsql; >> CREATE FUNCTION >> test=# CREATE TRIGGER trg INSTEAD OF INSERT ON v1 FOR EACH ROW EXECUTE >> PROCEDURE fnc(); >> CREATE TRIGGER >> test=# BEGIN; >> BEGIN >> test=# LOCK TABLE v1; >> ERROR: cannot lock view "v1" >> DETAIL: views that have an INSTEAD OF trigger are not lockable >> test=# END; >> ROLLBACK > > I wonder if we should lock tables in a subquery as well. For example, > > create view v1 as select * from t1 where i in (select i from t2); > > In this case should we lock t2 as well? Current the patch ignores t2 in the case above. So we have options below: - Leave as it is (ignore tables appearing in a subquery) - Lock all tables including in a subquery - Check subquery in the view definition. If there are some tables involved, emit an error and abort. The first one might be different from what users expect. There may be a risk that the second one could cause deadlock. So it seems the third one seems to be the safest IMO. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Lockable views
> Hi, > > Attached is a patch to enable views to be locked. Nice. > PostgreSQL has supported automatically updatable views since 9.3, so we can > udpate simply defined views like regular tables. However, currently, > table-level locks on views are not supported. We can not execute LOCK TABLE > for views, while we can get row-level locks by FOR UPDATE/SHARE. In some > situations that we need table-level locks on tables, we may also need > table-level locks on automatically updatable views. Although we can lock > base-relations manually, it would be useful if we can lock views without > knowing the definition of the views. > > In the attached patch, only automatically-updatable views that do not have > INSTEAD OF rules or INSTEAD OF triggers are lockable. It is assumed that > those views definition have only one base-relation. When an auto-updatable > view is locked, its base relation is also locked. If the base relation is a > view again, base relations are processed recursively. For locking a view, > the view owner have to have he priviledge to lock the base relation. > > * Example > > test=# CREATE TABLE tbl (i int); > CREATE TABLE > > test=# CREATE VIEW v1 AS SELECT * FROM tbl; > CREATE VIEW > test=# BEGIN; > BEGIN > test=# LOCK TABLE v1; > LOCK TABLE > test=# SELECT relname, locktype, mode FROM pg_locks,pg_class c WHERE > c.oid=relation AND relname NOT LIKE 'pg%'; > relname | locktype |mode > -+--+- > tbl | relation | AccessExclusiveLock > v1 | relation | AccessExclusiveLock > (2 rows) > > test=# END; > COMMIT > > test=# CREATE VIEW v2 AS SELECT * FROM v1; > CREATE VIEW > test=# BEGIN; > BEGIN > test=# LOCK TABLE v2; > LOCK TABLE > test=# SELECT relname, locktype, mode FROM pg_locks,pg_class c WHERE > c.oid=relation AND relname NOT LIKE 'pg%'; > relname | locktype |mode > -+--+- > v2 | relation | AccessExclusiveLock > tbl | relation | AccessExclusiveLock > v1 | relation | AccessExclusiveLock > (3 rows) > > test=# END; > COMMIT > > test=# CREATE VIEW v3 AS SELECT count(*) FROM v1; > CREATE VIEW > test=# BEGIN; > BEGIN > test=# LOCK TABLE v3; > ERROR: cannot lock view "v3" > DETAIL: Views that return aggregate functions are not automatically > updatable. It would be nice if the message would be something like: DETAIL: Views that return aggregate functions are not lockable > test=# END; > ROLLBACK > > test=# CREATE FUNCTION fnc() RETURNS trigger AS $$ BEGIN RETURN NEW; END; $$ > LANGUAGE plpgsql; > CREATE FUNCTION > test=# CREATE TRIGGER trg INSTEAD OF INSERT ON v1 FOR EACH ROW EXECUTE > PROCEDURE fnc(); > CREATE TRIGGER > test=# BEGIN; > BEGIN > test=# LOCK TABLE v1; > ERROR: cannot lock view "v1" > DETAIL: views that have an INSTEAD OF trigger are not lockable > test=# END; > ROLLBACK I wonder if we should lock tables in a subquery as well. For example, create view v1 as select * from t1 where i in (select i from t2); In this case should we lock t2 as well? Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] list of credits for release notes
> On Mon, Oct 2, 2017 at 5:09 PM, Tatsuo Ishii <is...@sraoss.co.jp> wrote:
>> Michael is correct.
>>
>> Sometimes people choose family name first in the emails. However I am
>> sure "Fujii" is the family name and "Masao" is the firstname.
>
> But I don't think that directly answers the question of how he would
> prefer to be credited.
Of course. It's different story.
> Since I am American, I prefer to be credited
> using the style "${FIRSTNAME} ${LASTNAME}", but the preferences of
> someone from another country might be the same or different. I don't
> think we should presume that someone prefers something other than the
> style in their email name unless they say so.
My concern is that the list seems implicitly assumes that each name
appears first name then last name. So people might misunderstand that
"Fujii" is the first name and "Masao" is the last name. I don't how he
actually feels about that, but if I were him, I would feel
uncomfortable. If the list explicitly stats that we do not guarantee
that the order of the last names and the first names are correct, then
that kind of misunderstanding could be avoided.
Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Conversion error
> Tatsuo Ishii <is...@sraoss.co.jp> writes: >> I saw this while restoring 9.6 database to 10.0 database. >> \connect: FATAL: conversion between UTF8 and MULE_INTERNAL is not supported >> Is this expected? I saw nothing releated to this in the release notes. > > Don't think that's ever been supported. Sorry, that was my misunderstanding. All versions of PostgreSQL allow to create MULE_INTERNAL database while executing restore, but after that it fails while loading rows into the mule internal database. So, if there's no table in the database, the restore succeeds. That's why I got wrong expressions. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Conversion error
> I saw this while restoring 9.6 database to 10.0 database. > > \connect: FATAL: conversion between UTF8 and MULE_INTERNAL is not supported > > Is this expected? I saw nothing releated to this in the release notes. This had been allowed in 9.6. So I think 10.0 silently drops the feature. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Conversion error
I saw this while restoring 9.6 database to 10.0 database. \connect: FATAL: conversion between UTF8 and MULE_INTERNAL is not supported Is this expected? I saw nothing releated to this in the release notes. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] list of credits for release notes
> On Wed, Sep 27, 2017 at 8:29 PM, Michael Paquier > <michael.paqu...@gmail.com> wrote: >> Looking at this list, the first name is followed by the family name, >> so there are inconsistencies with some Japanese names: >> - Fujii Masao should be Masao Fujii. >> - KaiGai Kohei should be Kohei Kaigai. > > But his emails say Fujii Masao, not Masao Fujii. Michael is correct. Sometimes people choose family name first in the emails. However I am sure "Fujii" is the family name and "Masao" is the firstname. > KaiGai's case is a bit trickier, as his email name has changed over time. Michael is correct about Kaigai too. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Statement timeout behavior in extended queries
> On 2017-09-10 17:12:19 -0700, Andres Freund wrote: >> On 2017-09-11 09:10:49 +0900, Tatsuo Ishii wrote: >> > If you don't mind, can you please commit/push the patch? >> >> Ok, will do so. > > And, done. Thanks for patch and reminder! Thanks! -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] More efficient truncation of pg_stat_activity query strings
> Check the information the pg_*_mblen use / how the relevant encodings
> work. Will be something like
> int
> pg_utf_mblen(const unsigned char *s)
> {
> int len;
>
> if ((*s & 0x80) == 0)
> len = 1;
> else if ((*s & 0xe0) == 0xc0)
> len = 2;
> else if ((*s & 0xf0) == 0xe0)
> len = 3;
> else if ((*s & 0xf8) == 0xf0)
> len = 4;
> #ifdef NOT_USED
> else if ((*s & 0xfc) == 0xf8)
> len = 5;
> else if ((*s & 0xfe) == 0xfc)
> len = 6;
> #endif
> else
> len = 1;
> return len;
> }
>
> As you can see, only the first character (*s) is accessed to determine
> the length/width of the multibyte-character. That's afaict the case for
> all server-side encodings.
So your idea is just storing cmd_str into st_activity, which might be
clipped in the middle of multibyte character. And the reader of the
string will call pg_mbclipen() when it needs to read the string. Yes,
I think it works except for gb18030 by the reason you said.
However, if we could have variants of mblen functions with additional
parameter: input string length, then we could remove this inconstancy.
I don't know if this is overkill or not, though.
Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] More efficient truncation of pg_stat_activity query strings
> read side. I think that should work because all *server side* encodings > store character lengths in the *first* byte of a multibyte character What do you mean? I don't see such data in a multibyte string. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Statement timeout behavior in extended queries
Andres, If you don't mind, can you please commit/push the patch? Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp >> What do you think? I've not really tested this with the extended >> protocol, so I'd appreciate if you could rerun your test from the >> older thread. > > Attached is your patch just rebased against master branch. > > Also I attached the test cases and results using pgproto on patched > master branch. All looks good to me. > > Best regards, > -- > Tatsuo Ishii > SRA OSS, Inc. Japan > English: http://www.sraoss.co.jp/index_en.php > Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Statement timeout behavior in extended queries
> What do you think? I've not really tested this with the extended
> protocol, so I'd appreciate if you could rerun your test from the
> older thread.
Attached is your patch just rebased against master branch.
Also I attached the test cases and results using pgproto on patched
master branch. All looks good to me.
Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 8d3fecf..6c53b9c 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -144,6 +144,11 @@ static bool doing_extended_query_message = false;
static bool ignore_till_sync = false;
/*
+ * Flag to keep track of whether statement timeout timer is active.
+ */
+static bool stmt_timeout_active = false;
+
+/*
* If an unnamed prepared statement exists, it's stored here.
* We keep it separate from the hashtable kept by commands/prepare.c
* in order to reduce overhead for short-lived queries.
@@ -182,6 +187,8 @@ static bool IsTransactionExitStmtList(List *pstmts);
static bool IsTransactionStmtList(List *pstmts);
static void drop_unnamed_stmt(void);
static void log_disconnections(int code, Datum arg);
+static void enable_statement_timeout(void);
+static void disable_statement_timeout(void);
/*
@@ -1228,7 +1235,8 @@ exec_parse_message(const char *query_string, /* string to execute */
/*
* Start up a transaction command so we can run parse analysis etc. (Note
* that this will normally change current memory context.) Nothing happens
- * if we are already in one.
+ * if we are already in one. This also arms the statement timeout if
+ * necessary.
*/
start_xact_command();
@@ -1516,7 +1524,8 @@ exec_bind_message(StringInfo input_message)
/*
* Start up a transaction command so we can call functions etc. (Note that
* this will normally change current memory context.) Nothing happens if
- * we are already in one.
+ * we are already in one. This also arms the statement timeout if
+ * necessary.
*/
start_xact_command();
@@ -2008,6 +2017,9 @@ exec_execute_message(const char *portal_name, long max_rows)
* those that start or end a transaction block.
*/
CommandCounterIncrement();
+
+ /* full command has been executed, reset timeout */
+ disable_statement_timeout();
}
/* Send appropriate CommandComplete to client */
@@ -2437,25 +2449,27 @@ start_xact_command(void)
{
StartTransactionCommand();
- /* Set statement timeout running, if any */
- /* NB: this mustn't be enabled until we are within an xact */
- if (StatementTimeout > 0)
- enable_timeout_after(STATEMENT_TIMEOUT, StatementTimeout);
- else
- disable_timeout(STATEMENT_TIMEOUT, false);
-
xact_started = true;
}
+
+ /*
+ * Start statement timeout if necessary. Note that this'll intentionally
+ * not reset the clock on an already started timeout, to avoid the timing
+ * overhead when start_xact_command() is invoked repeatedly, without an
+ * interceding finish_xact_command() (e.g. parse/bind/execute). If that's
+ * not desired, the timeout has to be disabled explicitly.
+ */
+ enable_statement_timeout();
}
static void
finish_xact_command(void)
{
+ /* cancel active statement timeout after each command */
+ disable_statement_timeout();
+
if (xact_started)
{
- /* Cancel any active statement timeout before committing */
- disable_timeout(STATEMENT_TIMEOUT, false);
-
CommitTransactionCommand();
#ifdef MEMORY_CONTEXT_CHECKING
@@ -4521,3 +4535,42 @@ log_disconnections(int code, Datum arg)
port->user_name, port->database_name, port->remote_host,
port->remote_port[0] ? " port=" : "", port->remote_port)));
}
+
+/*
+ * Start statement timeout timer, if enabled.
+ *
+ * If there's already a timeout running, don't restart the timer. That
+ * enables compromises between accuracy of timeouts and cost of starting a
+ * timeout.
+ */
+static void
+enable_statement_timeout(void)
+{
+ /* must be within an xact */
+ Assert(xact_started);
+
+ if (StatementTimeout > 0)
+ {
+ if (!stmt_timeout_active)
+ {
+ enable_timeout_after(STATEMENT_TIMEOUT, StatementTimeout);
+ stmt_timeout_active = true;
+ }
+ }
+ else
+ disable_timeout(STATEMENT_TIMEOUT, false);
+}
+
+/*
+ * Disable statement timeout, if active.
+ */
+static void
+disable_statement_timeout(void)
+{
+ if (stmt_timeout_active)
+ {
+ disable_timeout(STATEMENT_TIMEOUT, false);
+
+ stmt_timeout_active = false;
+ }
+}
== 001-without-trans ===
FE=> Query(query="SET statement_timeout = '4s'")
<= BE CommandComplete(SET)
<= BE ReadyForQuery(I)
FE=> Parse(stmt="", query="SELECT pg_sleep(3)")
FE=> Bind(stmt="", portal="")
FE=> Execute(portal="")
FE=> Parse(stmt=&qu
Re: [HACKERS] pgbench: faster version of tpcb-like transaction
> Don't think anybody is proposing to remove the existing way to run > pgbench, so I'm not sure what your point is? I know. I just wanted to point out that the proposal is not good for cluster environment tests. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgbench: faster version of tpcb-like transaction
> Jeff Janes <jeff.ja...@gmail.com> writes: >> If all the data is in memory and you have a system with fast fsyncs (or are >> running with fsync off, or unlogged tables, or synchronous_commit off), >> then the big bottleneck in pgbench is the amount of back and forth between >> the pgbench program and the backend. There are 7 commands per transaction. > > Yeah ... > >> It is easy to package 5 of those commands into a single PL/pgSQL function, >> with the other two being implicit via the standard auto-commit behavior >> when explicit transactions are not opened. The attached patch does that, >> under the name tpcb-func. I first named it tpcb-like-func, but one builtin >> name can't be a prefix or another so that won't work. > > I dunno, it seems like this proposal involves jacking up the test case > and driving a completely different one underneath. There is no reason > to consider that you've improved the benchmark results --- you've just > substituted a different benchmark, one with no historical basis, and > not a lot of field justification either. > >> Wanting to measure IPC overhead is a valid thing to do, but >> certainly isn't the most common thing people want to do with pgbench. > > I think that's nonsense. Measuring how fast PG can do client interactions > is EXACTLY what this is about. Certainly, pushing SQL operations into > server-side functions is a great way to reduce network overhead, but it > has nothing to do with what we choose as a benchmark. Current implementation of pgbench allows Pgpool-II (or any proxy type middle ware) to test the behavior on PostgreSQL clusters. For example it sends write queries to the master DB node and read queries to standby nodes to distribute loads among DB nodes. With the proposed implementation it is not possible to do that kind of test anymore since everything is packed into a function. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [BUGS] Replication to Postgres 10 on Windows is broken
> On Thu, Aug 17, 2017 at 9:15 AM, Tatsuo Ishii <is...@sraoss.co.jp> wrote: >>> He meant logical replication, >> >> Oh I could not find he meant logical replication in the original >> report. > > The second message of the thread says so, but the first does not > mention logical replication at all. >>From here are mentioned PG 9.6 and pg_basebackup: > https://www.postgresql.org/message-id/cahbggj8g2t+zdcacz2fmzx9ctxkwjkbshd6nkyb4i9ojf6k...@mail.gmail.com. > Explaining the confusion. Oh, I see it now. Thanks. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [BUGS] Replication to Postgres 10 on Windows is broken
> He meant logical replication, Oh I could not find he meant logical replication in the original report. > but the code in question here is the same > for streaming replication, or whatever it's called. Yes. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [BUGS] Replication to Postgres 10 on Windows is broken
> In practice it's largely about architecture, I think. You definitely > need the same endianness and floating-point format, which are hardware, > and you need the same MAXALIGN, which is partly hardware but in principle > could be chosen differently in different ABI conventions for the same > hardware. (At least for non-alignment-picky hardware like Intel. > Alignment-picky hardware is likely going to dictate that choice too.) > > We disclaim cross-OS portability partly because of the possible influence > of different ABI conventions, but it doesn't surprise me in the least if > in practice 64-bit Windows can interoperate with x86_64 Linux. (Less sure > about the 32-bit case --- it looks like pg_config.h.win32 chooses > MAXALIGN 8 in all cases, which would mean 32-bit Windows is more likely to > interoperate with 64-bit Linux than 32-bit Linux.) I feel like we need to be a little bit more cleaner about this in our official documentation. From section 26.2.1. Planning: "Hardware need not be exactly the same, but experience shows that maintaining two identical systems is easier than maintaining two dissimilar ones over the lifetime of the application and system. In any case the hardware architecture must be the same ― shipping from, say, a 32-bit to a 64-bit system will not work." Probably We should disclaim cross-OS portability here? > The thing that's really likely to bite you cross-platform is dependency > of textual index sort ordering on non-C locale definitions. But that > wouldn't show up in a quick smoke test of replication ... and if you > really wanted, you could use C locale on both ends. Of course. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [BUGS] Replication to Postgres 10 on Windows is broken
> Thank you Tom Lane, > This patch fixes the problem. > > With this patch, streaming replication started working (replication to > Windows) > > (Tested for Linux to Windows replication) Do we allow streaming replication among different OS? I thought it is required that primary and standbys are same platform (in my understanding this means the same hardware architecture and OS) in streaming replication. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Confusing error message in pgbench
>> Not really objecting, but an even better fix might be to remove the >> restriction on the order in which the options can be specified. > > Indeed. It doesn't look that hard: AFAICS the problem is just that > process_sql_command() is making premature decisions about whether to > extract parameters from the SQL commands. Proposed patch attached. Great. Patch looks good to me. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Confusing error message in pgbench
> Not really objecting, but an even better fix might be to remove the > restriction on the order in which the options can be specified. +100 :-) Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgbench: Skipping the creating primary keys after initialization
> I think we could probably do without this ... if you want a non-default > test setup, why do you need to use "pgbench -i" to create it? > > It's not that there's anything greatly wrong with this particular idea, > it's just that pgbench has too many switches already, and omitting random > subsets of the initialization actions doesn't seem like it contributes > fundamental new benchmarking capability. > > I could get behind a proposal that generalized pgbench's "-i" behavior > in some meaningful way. I wonder whether it would be possible to convert > that behavior into a script. Some of what it does is just SQL commands > with injected parameters, which pgbench does already. There's also > data-loading actions, which could be converted to backslash commands > perhaps. Then desires like this could be addressed by invoking a > customized script instead of complicating pgbench's option set. +1. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Confusing error message in pgbench
I found an error message in pgbench is quite confusing.
pgbench -S -M extended -c 1 -T 30 test
query mode (-M) should be specified before any transaction scripts (-f or -b)
Since there's no -f or -b option is specified, users will be
confused. Actually the error occurs because pgbench implicitly
introduces a built in script for -S. To eliminate the confusion, I
think the error message should be fixed like this:
query mode (-M) should be specified before transaction type (-S or -N) or any
transaction scripts (-f or -b)
Patch attached.
Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 4d364a1..f7ef0ed 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -3898,7 +3898,7 @@ main(int argc, char **argv)
benchmarking_option_set = true;
if (num_scripts > 0)
{
- fprintf(stderr, "query mode (-M) should be specified before any transaction scripts (-f or -b)\n");
+ fprintf(stderr, "query mode (-M) should be specified before transaction type (-S or -N) or any transaction scripts (-f or -b)\n");
exit(1);
}
for (querymode = 0; querymode < NUM_QUERYMODE; querymode++)
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Incorrect comment of XLByteToSeg() and XLByteToPrevSeg()
> Thanks for the patch. Looks good to me. I will commit/push into all > supported branches if there's no objection. Done. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Missing comment for max_logical_replication_workers in postgresql.conf.sample
> Attached patch looks good except the excessive tab stops: > + > # (change requires restart) > > I will commit/push this with removing the excessive tab stops if > there's no objection. Done. Each fix were pushed in separate commit because the version needed to back patch are differ. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Missing comment for max_logical_replication_workers in postgresql.conf.sample
Attached patch looks good except the excessive tab stops: + # (change requires restart) I will commit/push this with removing the excessive tab stops if there's no objection. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp > On Thu, 27 Jul 2017 14:38:29 +0900 > Masahiko Sawada <sawada.m...@gmail.com> wrote: > >> On Thu, Jul 27, 2017 at 10:14 AM, Yugo Nagata <nag...@sraoss.co.jp> wrote: >> > Hi, >> > >> > I found that postgresql.conf.sample is missing a comment >> > to note that changing max_logical_replication_workers requires >> > restart of the server. >> > >> > Other such parameters has the comments, so I think the new >> > parameter also needs this. Attached is a simple patch to fix >> > this. >> > >> >> Good point. Similarly, dynamic_shared_memory_type and event_source are >> required restarting to change but are not mentioned in >> postgresql.conf.sample. Should we add a comment as well? > > I think so. The updated patch is attached. > >> >> Regards, >> >> -- >> Masahiko Sawada >> NIPPON TELEGRAPH AND TELEPHONE CORPORATION >> NTT Open Source Software Center > > > -- > Yugo Nagata <nag...@sraoss.co.jp> -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Incorrect comment of XLByteToSeg() and XLByteToPrevSeg()
> I found a type in the comment for XLByteToSeg() and XLByteToPrevSeg(). > This says "Compute ID and segment from an XLogRecPtr", but these > macros compute only segment numbers. I think "Compute a segment number > from an XLogRecPtr" is correct. > > The definition of these macros were modified by the following commit, > but the comment were not. > > commit dfda6ebaec6763090fb78b458a979b558c50b39b > Author: Heikki Linnakangas <heikki.linnakan...@iki.fi> > Date: Sun Jun 24 18:06:38 2012 +0300 Thanks for the patch. Looks good to me. I will commit/push into all supported branches if there's no objection. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] More optimization effort?
Currently following query does not use an index: t-ishii@localhost: psql -p 5433 test Pager usage is off. psql (9.6.3) Type "help" for help. test=# explain select * from pgbench_accounts where aid*100 < 1; QUERY PLAN Seq Scan on pgbench_accounts (cost=0.00..3319.00 rows=3 width=97) Filter: ((aid * 100) < 1) (2 rows) While following one does use the index. test=# explain select * from pgbench_accounts where aid < 1/100; QUERY PLAN -- Index Scan using pgbench_accounts_pkey on pgbench_accounts (cost=0.29..11.08 rows=102 width=97) Index Cond: (aid < 100) (2 rows) Is it worth to make our optimizer a little bit smarter to convert the the first query into the second form? Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Dealing with logical replication
> On 19 July 2017 at 16:34, Tatsuo Ishii <is...@sraoss.co.jp> wrote: > >> Now that we are going to have logical replication in PostgreSQL 10, I >> have started thinking how Pgpool-II can deal with it. For example, the >> logical replication does not replicate DDLs. Isn't it convenient for >> users to do it automatically in Pgpool-II? Or even doing it for >> TRUNCATE? >> >> Or are they against the design philosophy of the logical replication? >> > > (Disclaimer - Petr Jelinek and Peter Eisentraut were the main ones working > on in in-core logical rep, and I haven't had time to play with it much). > > TL;DR: a pooler can only solve a limited subset of the problem, fairly > unreliably, and this is really something that needs further work in core. > > Not replicating TRUNCATE and schema changes is more an implementation > limitation than anything else. It's desirable to get to the point where > logical replication can transparently replicate DDL. There are some hurdles > for doing so, but it'll be possible to get something in place that's good > enough when time/release progress permits. > > Similarly, with TRUNCATE, AFAIK support just didn't make the cut for pg10. > > A pooler could well help in the mean time, but you have to consider > ordering with care. For example, given "U" upstream, "D" downstream: > > U: CREATE TABLE x (a integer, b integer); > D: CREATE TABLE x (a); -- user script syncs > U: INSERT INTO x (a,b) VALUES (1,0); > D: [applies INSERT 1,0] > U: INSERT INTO x (a,b) VALUES (2,0); > U: ALTER TABLE x DROP COLUMN b; > D: ALTER TABLE x DROP COLUMN b; -- user script syncs > U: INSERT INTO x (a) VALUES (3); > D: [ERROR on INSERT of 2,0: no column 'b'] > > Because the DDL here is transported out of band vs the row data, you can > easily create situations where the schema change is applied before the last > committed-but-not-yet-replicated data from the upstream that was based on > the old schema. What I am thinking for now is, do not allow to issue DDLs on the downstream. > To achieve correct ordering, the simplest approach is to record DDL in a > table when you perform it on the upstream, and replay it when you see rows > in that table appear on the downstream. You know it's safe to replay it > now. This is the essence of what BDR and pglogical do with their DDL > replication, but they handle DDL in the middle of transactions that also > make row data changes by intercepting writes to the queue table and > performing the DDL at the exact point in the transaction where it happened > on the upstream. I don't think that's possible with the in-core logical > replication yet, and certainly not something a pooler can do. > > To do it externally, you have to take note of when a schema change happened > on the upstream and apply it on the downstream at or after the point where > the downstream has replayed and confirmed up to the upstream lsn where the > schema change happened. Then apply the schema change. > > A pooler trying to help here must also be very aware of the impact of > multi-statements. If you send a single simple query message with a mixture > of schema change commands and normal DML, you probably don't want to repeat > the DML on downstream nodes or you'll get duplicate rows etc. But ... by > unless it embeds transaction control commands, a simple query message > executes in a single implicit transaction, so if you extract just the DDL > you'll again have ordering issues of upstream vs downstream. Pgpool-II will avoid the problem by rejecting such multi-statements. > There are even a variety of difficulties to overcome with doing it in core: > event triggers don't capture ddl command text and have no facility to turn > the internal command representation back into SQL command text, nor do we > have any way to turn the internal representation back into a parsenode tree > for execution on a downstream's standard_ProcessUtility. However, we can > capture raw command text with ProcessUtility_hook now that we have > byte-ranges for the query-parts of a multi-part query (yay!), and that > works well enough if you also capture the active search_path and apply with > the same search_path. It can match the wrong object if extra objects with > the same name are present earlier on the search_path on the downstream than > on the upstream, so it's not ideal, but that's a weird corner case. > > If we had a hook in the logical apply worker's insert or wal-message > routines it'd be possible to write an extension to do this for pg10, but > AFAICS we don't. > > So schema changes in logical replication currently require more care than > in physical replication. Thank you for the explanation regardin
[HACKERS] Dealing with logical replication
Now that we are going to have logical replication in PostgreSQL 10, I have started thinking how Pgpool-II can deal with it. For example, the logical replication does not replicate DDLs. Isn't it convenient for users to do it automatically in Pgpool-II? Or even doing it for TRUNCATE? Or are they against the design philosophy of the logical replication? Comments are welcome. -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SCRAM auth and Pgpool-II
>> The comment in pg_hba.conf.sample seem to prefer md5 over clear text >> password. >> >> # Note that "password" sends passwords in clear text; "md5" or >> # "scram-sha-256" are preferred since they send encrypted passwords. > > Should that be reworded to eliminate "md5"? I'd consider "scram-sha-256" > suitable over a clear channel, but I've never recommended "md5" for that. I don't think so unless clear text password is superior than md5. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SCRAM auth and Pgpool-II
>> Using a clear text password would not be acceptable for users even >> through an encrypted connection, I think. > > Really, I don't think users who are concerned with security should be > using the md5 method either. The comment in pg_hba.conf.sample seem to prefer md5 over clear text password. # Note that "password" sends passwords in clear text; "md5" or # "scram-sha-256" are preferred since they send encrypted passwords. > What would be really nice for such cases is support for Kerberos and > delegated Kerberos credentials. Having pgpool support that would remove > the need to deal with passwords at all. > > Ditto for having postgres_fdw support same. More often than not, > Kerberos deployments (via AD, generally) is what I find in the > enterprises that I work with and they're happy to see we have Kerberos > but it's disappointing when they can't use Kerberos with either > connection poolers or with FDWs. I would add supporting Kerberos to the Pgpool-II todo list. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SCRAM auth and Pgpool-II
> What I am suggesting here is that in order to handle properly SCRAM > with channel binding, pgpool has to provide a different handling for > client <-> pgpool and pgpool <-> Postgres. In short, I don't have a > better answer than having pgpool impersonate the server and request > for a password in cleartext through an encrypted connection between > pgpool and the client if pgpool does not know about it, and then let > pgpool do by itself the SCRAM authentication on a per-connection basis > with each Postgres instances. When using channel binding, what would > matter is the TLS finish (for tls-unique) or the hash server > certificate between Postgres and pgpool, not between the client and > pgpool. But that's actually the point you are raising here: Using a clear text password would not be acceptable for users even through an encrypted connection, I think. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SCRAM auth and Pgpool-II
Hi Alvaro, > Hi Tatsuo. > > There's definitely an important concern here that should be addressed: > how poolers/proxies/middleware/etc can deal with SCRAM, specifically > in the context of channel binding. > > If there is to be a single connection from client to PostgreSQL > server, intercepted by pgpool to perform the magic foo, then channel > binding is, indeed, designed to defeat this. If, however, pgpool or > the middleware manages two separate connections (client<->pool and > pool<->PG) then there is some light here. > > One SCRAM feature not implemented as of today is the authzid > (authorization identity: see > https://tools.ietf.org/html/rfc5802#page-10, SCRAM attribute "a" and > https://tools.ietf.org/html/rfc5801). Authzid is basically "I want to > authenticate as user X and once authenticated, consider I'm user > Y". With authzid, a pool/proxy may have a common user name with its > own SCRAM credentials to authenticate with the backend PostgreSQL, and > pass the authzid with the real username (the one provided on the > client<->pool connection). > > This would require: > > a) That authzid is implemented in PostgreSQL. > b) A mechanism in PG to name which user(s) Y are allowed to be > authenticated by user X. This is similar, but not identical, to the > current SET ROLE. > > From a SCRAM protocol perspective, this is very simple, just an extra > attribute. Part b) may need more discussion. > > I believe this could be of help to your case and other middleware. That's ambitious. Thank you for the info! Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SCRAM auth and Pgpool-II
>> For more details what Pgpool-II actually does in md5 auth, please see: >> >> https://pgpool.net/mediawiki/index.php/FAQ#How_does_pgpool-II_handle_md5_authentication.3F > > I see. In short, that's not going to be able to work with SCRAM. That's my understanding with SCRAM too. > We also certainly can't weaken or break SCRAM to address Pgpool's needs. Right, that's not what I want too. > I'm afraid an alternative approach will need to be considered for Pgpool > to be able to do what it does today, as I outlined in my other email. Thank your for your comment. Probably now is the time to give up to support SCRAM in Pgpool-II. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SCRAM auth and Pgpool-II
> I recall vaguely Ishii-san mentioning me at PGcon that pgpool was > actually cheating, but my memories are a bit fuzzy for this part. What I meant by "cheating" was, Pgpool-II behaves as if PostgreSQL server in md5 auth. For more details what Pgpool-II actually does in md5 auth, please see: https://pgpool.net/mediawiki/index.php/FAQ#How_does_pgpool-II_handle_md5_authentication.3F >> IIUC, things will get even worse once channel binding is committed, >> presumably for PostgreSQL 11. The point of channel binding is to >> guarantee that you are conducting the authentication exchange with the >> target server, not some intermediate proxy that might be conducting a >> hostile MITM attack. pgpool may not be a hostile attack, but it is >> acting as a MITM, and channel binding is going to figure that out and >> fail the authentication. So unless I'm misunderstanding, the solution >> you are proposing figures to have a very limited shelf life. > > We may be misunderstanding each other then. The solution I am > proposing, or at least the solution that I imagine should be done but > my pgpool-fu is limited, is to keep around connection context and SSL > context to handle properly connections messages, and switch between > them. When using channel binding, the client needs the server > certificate for end-point and the TLS finish message which are both > stored in the SSL context after the handshake. So you need to cache > that for each server. > > One problem is that pgpool does not use directly libpq but tries to > handle things by itself at protocol level, so it needs to replicate a > lot of existing APIs. Postgres-XC/XL use a connection pooler, > presumably XL uses even a background worker for this stuff since it > has been integrated with Postgres 9.3. And this stuff use libpq. This > makes life way easier to handle context data on a connection basis. > Those don't need to handle protocol-level messages either, which is > perhaps different from what pgpool needs. Yeah, I wish I could use libpq in Pgpool-II. Unfortunately libpq does not provide necessary features what Pgpool-II needs. > In short I would think that pgpool needs first to un-cheat its > handling with MD5, which is likely here to simplify its code. See the link above why it's not possible. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SCRAM auth and Pgpool-II
> I think it is. From a security point of view, the fact that the same > salt is always used for the same username is a weakness of md5 > authentication which SCRAM corrects. In my understanding, md5 does not always use the same salt for the same username. PostgreSQL keeps md5(password+username) in pg_authid. When md5 auth is required, the backend sends a random number (i.e. salt) to the client. The client replies back md5(salt+md5(password+username)) to the backend. The backend does the calculation (md5(salt+md5(password+username))). If the result matches the value sent from the user, md5 authentication succeeds. So the salt is differ in each session in md5. The weakness in md5 is , IMO, each PostgreSQL installation always keeps the same value (md5(password+username)). > IIUC, things will get even worse once channel binding is committed, > presumably for PostgreSQL 11. The point of channel binding is to > guarantee that you are conducting the authentication exchange with the > target server, not some intermediate proxy that might be conducting a > hostile MITM attack. pgpool may not be a hostile attack, but it is > acting as a MITM, and channel binding is going to figure that out and > fail the authentication. So unless I'm misunderstanding, the solution > you are proposing figures to have a very limited shelf life. Check... Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SCRAM auth and Pgpool-II
Michael, > Couldn't you cache one single SASL exchange status for each > connection, meaning one PGconn saved for each? As the challenge sent > by the server and the response generated by the client are different > by design, I am afraid you would need to do that anyway in this > context (Isn't PG-pool using already the weaknesses of MD5 to make > things easier?). As the server decides first which authentication type > should happen before beginning the real message exchange, that should > not be difficult. It seems to me that you would need something more > modular than you have now if you want for example to handle > automatically connections to multiple servers that have different > password hashes stored for the same user. The latter may be an edge > case with pgpool though. Thank you for the quick response. I will study your suggestion along with the SCRAM code in PostgreSQL whether it could be possible in Pgpool-II. Regarding your question on md5 auth handling in Pgpool-II, please look into: https://pgpool.net/mediawiki/index.php/FAQ#How_does_pgpool-II_handle_md5_authentication.3F Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] SCRAM auth and Pgpool-II
Hi PostgreSQL hackers, I would like to hear ideas how Pgpool-II can deal with SCRAM auth which will be in PostgreSQL 10. For those who are not familiar with Pgpool-II[1], it is an external OSS project to provide some additional features to PostgreSQL, including load balancing and automatic failover. Pgpool-II works as a proxy between PostgreSQL client and PostgreSQL server(s). When a client wants to connects to PostgreSQL and SCRAM auth is enabled, it sends user name to server. Then the server sends information including a salt to the client. The client computes a "ClientProof" using the salt and other information, and sends it to the server[2]. For Pgpool-II, things would go as follows: 1) clients sends user name to Pgpool-II. 2) Pgpool-II forwards it to PostgreSQL servers. 3) Each PostgreSQL server sends their own salt to Pgpool-II. 4) Pgpool-II is confused because there are multiple salts and each has different values. The client only accepts single salt obviously. So my question is, is there any solution or workaround for the problem #4. Someone at PGCon 2017 suggested that the problem could be avoided if the auth method between the client and Pgpool-II is "trust" (which means no auth). But this does not seem to be a best solution for me because it would weaken the security. I suspect this problem may not be specific to Pgpool-II. Any middle ware which handles multiple PostgreSQL servers could have the similar problem. Any suggestion would be appreciated. Thanks in advance. [1] https://pgpool.net [2] https://tools.ietf.org/html/rfc5802#section-3 -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] REPLICA IDENTITY FULL
> Thanks for the feedback. I have committed it after removing the > datumIsEqual() call. Thanks for the patch! I confirmed my examples now work. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] shift_sjis_2004 related autority files are remaining
> I don't have a clear opinion on this particular issue, but I think we > should have clarity on why particular files or code exist. So why do > these files exist and the others don't? Is it just the license? I think so. Many of those files are from http://ftp.unicode.org. There's no license description there, so I think we should not copy those files for safety reason. (I vaguely recall that they explicitly prohibited to distribute the files before but I could no find such a statement at this moment). gb-18030-2000.xml and windows-949-2000.xml are from https://ssl.icu-project.org/. I do not know what licenses those files use (maybe Apache). Regarding euc-jis-2004-std.txt and sjis-0213-2004-std.txt are from http://x0213.org. The license are described in the files. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] shift_sjis_2004 related autority files are remaining
> For clarity, I personally perfer to keep all the source text file > in the repository, especially so that we can detect changes of > them. But since we decide that at least most of them not to be > there (from a reason of license), I just don't see a reason to > keep only the rest even without the restriction. So are you saying that if n/m of authority files are not kept because of license issue, then m-n authority files should not be kept as well? What's the benefit for us by doing so? Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] shift_sjis_2004 related autority files are remaining
> Hm. I am wondering about licensing issues here to keep those files in > the tree. I am no lawyer. Of course. Regarding euc-jis-2004-std.txt and sjis-0213-2004-std.txt, it seems safe to keep them. > ## Date: 13 May 2006 > ## License: > ##Copyright (C) 2001 earth...@tama.or.jp, All Rights Reserved. > ##Copyright (C) 2001 I'O, All Rights Reserved. > ##Copyright (C) 2006 Project X0213, All Rights Reserved. > ##You can use, modify, distribute this table freely. >> - It allows to track the changes in the original file if we decide to >> change the map files. > > You have done that in the past for a couple of codepoints, didn't you? I believe the reason why I didn't keep other txt files were they were prohibited to have copies according to their license. >> - The site http://x0213.org/ may disappear in the future. If that >> happens, we will lose track data how we create the map files. > > There are other problems then as there are 3 sites in use to fetch the data: > - GB2312.TXT comes from greenstone.org. > - Some from icu-project.org. > - The rest is from unicode.org. Maybe, but I don't know how to deal with them. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] shift_sjis_2004 related autority files are remaining
>> Why do you believe so? > > Unicode/Makefile includes that: > euc-jis-2004-std.txt sjis-0213-2004-std.txt: > $(DOWNLOAD) http://x0213.org/codetable/$(@F) > > So those files ought to be downloaded when rebuilding the maps, and > they should not be in the tree. In short, I think that Horiguchi-san > is right. On top of the two pointed out by Horiguchi-san, > gb-18030-2000.xml should not be in the tree. I think we should keep the original .txt files because: - It allows to track the changes in the original file if we decide to change the map files. - The site http://x0213.org/ may disappear in the future. If that happens, we will lose track data how we create the map files. I believe we'd better to follow the same way how src/timezone keeps the original timezone data. Above reasoning will not valid if we have a way to reconstruct the original txt files from the map files, I doubt it's worth the trouble to create such tools however. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] shift_sjis_2004 related autority files are remaining
> Hi, I happned to notice that backend/utils/mb/Unicode directory > contains two encoding authority files, which I believe are not to > be there. > > euc-jis-2004-std.txt > sjis-0213-2004-std.txt Why do you believe so? -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] REPLICA IDENTITY FULL
>> For example, if the table consists of only INT types, REPLICA IDENTITY >> FULL works with UPDATE/DELETE (i.e. replicated), but if some of them >> are TEXT types, then UPDATE/DELETE does not work. >> >> See up thread for more details. > > Right, but that's just a bug, nothing else. If my understanding is correct, it would not be easy to fix, no? > We might be able to refine that, but there is a general problem that > without an index and an operator class, we are just doing our random > best to match the values. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] REPLICA IDENTITY FULL
>> Yes, that's my understanding too. However, the feature may or may not >> work depending on the data types of columns, probably I will not >> recommend users/my customers to use it. > > I'm not sure how datatypes are playing into this? For example, if the table consists of only INT types, REPLICA IDENTITY FULL works with UPDATE/DELETE (i.e. replicated), but if some of them are TEXT types, then UPDATE/DELETE does not work. See up thread for more details. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] REPLICA IDENTITY FULL
> Nothing, and that's what I do. I just think it's a bit fuzzy. Maybe > I'm misunderstanding the purpose of REPLICA IDENTITY, but I read it as > saying "this is the replication key for this relation". Yes, that's my understanding too. However, the feature may or may not work depending on the data types of columns, probably I will not recommend users/my customers to use it. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] REPLICA IDENTITY FULL
> This is apparently because for replica identity full the comparison of > the search key against the tuple value goes through datumIsEqual(), > which doesn't work for TOAST values. > > We might be able to refine that, but there is a general problem that > without an index and an operator class, we are just doing our random > best to match the values. In other word, pass-by-value types work in this case? If so, we can document it or throw an error while executing ALTER REPLICA IDENTITY FULL on tables consisting of non pass-by-values column types to mitigate the problem. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] REPLICA IDENTITY FULL
While playing around with logical replication, I am confused by the
behavior of REPLICA IDENTITY FULL.
First I created a table having 2 INT columns with no keys. If I
execute ALTER TABLE REPLICA IDENTITY FULL, replication for
UPDATE/DELETE works.
In the session below, port 11002 is the publisher side, while 11003 is
the subscriber side.
+ psql -e -p 11002 -c create table t3(i int, j int); test
create table t3(i int, j int);
CREATE TABLE
+ psql -e -p 11003 -c create table t3(i int, j int); test
create table t3(i int, j int);
CREATE TABLE
+ psql -e -p 11002 -c alter table t3 replica identity full test
alter table t3 replica identity full
ALTER TABLE
+ psql -e -p 11002 -c insert into t3 values(1,1); test
insert into t3 values(1,1);
INSERT 0 1
+ psql -e -p 11002 -c insert into t3 values(2,2); test
insert into t3 values(2,2);
INSERT 0 1
+ psql -e -p 11002 -c insert into t3 values(2,2); test
insert into t3 values(2,2);
INSERT 0 1
+ psql -e -p 11003 -c ALTER SUBSCRIPTION mysub REFRESH PUBLICATION; test
ALTER SUBSCRIPTION mysub REFRESH PUBLICATION;
NOTICE: added subscription for table public.t3
ALTER SUBSCRIPTION
+ sleep 3
+ psql -e -p 11003 -c select * from t3; test
select * from t3;
i | j
---+---
1 | 1
2 | 2
2 | 2
(3 rows)
+ psql -e -p 11002 -c update t3 set j = 10 where i = 2 and j = 2; test
update t3 set j = 10 where i = 2 and j = 2;
UPDATE 2
+ psql -e -p 11003 -c select * from t3; test
select * from t3;
i | j
---+
1 | 1
2 | 10
2 | 10
(3 rows)
+ psql -e -p 11002 -c delete from t3 where i = 2; test
delete from t3 where i = 2;
DELETE 2
+ psql -e -p 11003 -c select * from t3; test
Pager usage is off.
select * from t3;
i | j
---+---
1 | 1
(1 row)
However, if a table has text columns, UPDATE/DELETE replication does
not work any more. Am I missing something?
+ psql -e -p 11002 -c create table t4(i text, j text); test
create table t4(i text, j text);
CREATE TABLE
+ psql -e -p 11003 -c create table t4(i text, j text); test
create table t4(i text, j text);
CREATE TABLE
+ psql -e -p 11002 -c alter table t4 replica identity full test
alter table t4 replica identity full
ALTER TABLE
+ psql -e -p 11002 -c insert into t4 values('a','a'); test
insert into t4 values('a','a');
INSERT 0 1
+ psql -e -p 11002 -c insert into t4 values('b','b'); test
insert into t4 values('b','b');
INSERT 0 1
+ psql -e -p 11002 -c insert into t4 values('b','b'); test
insert into t4 values('b','b');
INSERT 0 1
+ psql -e -p 11003 -c ALTER SUBSCRIPTION mysub REFRESH PUBLICATION; test
ALTER SUBSCRIPTION mysub REFRESH PUBLICATION;
NOTICE: added subscription for table public.t4
ALTER SUBSCRIPTION
+ sleep 3
+ psql -e -p 11003 -c select * from t4; test
select * from t4;
i | j
---+---
a | a
b | b
b | b
(3 rows)
+ psql -e -p 11002 -c update t4 set j = 'c' where i = 'b' and j = 'b'; test
update t4 set j = 'c' where i = 'b' and j = 'b';
UPDATE 2
+ psql -e -p 11003 -c select * from t4; test
select * from t4;
i | j
---+---
a | a
b | b
b | b
(3 rows)
+ psql -e -p 11002 -c delete from t4 where i = 'b'; test
delete from t4 where i = 'b';
DELETE 2
+ psql -e -p 11003 -c select * from t4; test
select * from t4;
i | j
---+---
a | a
b | b
b | b
(3 rows)
Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] improve release-note for pg_current_logfile()
>>> Add function pg_current_logfile() to read syslog's current stderr and >>> csvlog output file names (Gilles Darold) >> >> This confused me because "syslog" is one of method for logging as well >> as stderr and csvlog. I guess it is intended syslogger, but I think that >> "logging collector" is more convenient for users because this is used in >> the pg_current_logfile() documentation. > > His proposal is changing the line above to: > > Add function pg_current_logfile() to read logging collector's current stderr > and csvlog output file names (Gilles Darold) > > Looks reasonable fix to me. If there's no objection, I will commit > this. Done. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Restrictions of logical replication
>> Some of that information was sprinkled around, but I have now added a >> new section that collects them all in one place. (section 31.4) > > > Shouldn't we mention that COPY is supported? I think any commands that are not mentioned in the section are considered to be supported. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Restrictions of logical replication
> Docs stated "Publications can choose to limit the changes they produce to > any combination of INSERT, UPDATE, and DELETE". It is clear that only those > DMLs are supported. What about COPY? > However, we should mention that large objects are not > supported. Right. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Restrictions of logical replication
> On 6/16/17 03:00, Tatsuo Ishii wrote: >> Maybe I am missing something, but I could not find any description >> that logical replication does not support large objects and TRUNCATE >> in the doc. Do we want to add them to the doc as the restrictions of >> the logical replication? > > Some of that information was sprinkled around, but I have now added a > new section that collects them all in one place. (section 31.4) Great. Thanks! -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Restrictions of logical replication
Maybe I am missing something, but I could not find any description that logical replication does not support large objects and TRUNCATE in the doc. Do we want to add them to the doc as the restrictions of the logical replication? Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP: Data at rest encryption
> Yeah, I guess we will just have to wait to see it since other people are > excited about it. My concern is code complexity and usability > challenges, vs punting the problem to the operating system, though > admittedly there are some cases where that is not possible. Oracle sells this feature only with the expensive enterprise edition. And people actually buy it. I guess the feature is pretty important for some users. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] improve release-note for pg_current_logfile()
>> Add function pg_current_logfile() to read syslog's current stderr and csvlog >> output file names (Gilles Darold) > > This confused me because "syslog" is one of method for logging as well > as stderr and csvlog. I guess it is intended syslogger, but I think that > "logging collector" is more convenient for users because this is used in > the pg_current_logfile() documentation. His proposal is changing the line above to: Add function pg_current_logfile() to read logging collector's current stderr and csvlog output file names (Gilles Darold) Looks reasonable fix to me. If there's no objection, I will commit this. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] type of release note of PG10
> Hi, > > I found a typo in the PG10 release note and attached is a patch > to fix it. Fix committed. Thanks! -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Document bug regarding read only transactions
> It used to be true. Tom changed it in commit > 05d8a561ff85db1545f5768fe8d8dc9d99ad2ef7, back in 2010. Thank you for the info. For a record, I will add it to the commit message. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Document bug regarding read only transactions
> Your modification does not look completely correct to me either. > Temporary sequences can be updated in read-only transactions. Not sure. Temporary sequences are meaningless on standby because "create temporary sequence" command cannot be executed on standby anyway (and temporary sequence are not replicated to standby of course). > I think > that the page listing the sequence-related functions should as well > mention those restrictions for nextval() and setval(). If we do so, ANALYZE, VACUUM, LISTEN and NOTIFY man pages should also be updated to mention that they can be executed in read only transaction but not in standby servers. I'm not sure it's worth the trouble. Moreover, that will create maintenance headache once we decide to remove some of the restrictions, because we need to update multiple places in the doc. -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Document bug regarding read only transactions
In https://www.postgresql.org/docs/10/static/hot-standby.html#hot-standby-users
It is explained that read only transactions (not in standby) allow to
update sequences.
In normal operation, read-only transactions are allowed to
update sequences and to use LISTEN, UNLISTEN, and
NOTIFY, so Hot Standby sessions operate under slightly tighter
restrictions than ordinary read-only sessions. It is possible that some
of these restrictions might be loosened in a future release.
This is plain wrong.
BEGIN;
BEGIN
test=# SET transaction_read_only TO on;
SET
test=# SELECT nextval('t1_i_seq');
ERROR: cannot execute nextval() in a read-only transaction
test=# \q
Attached is the patch against master branch.
Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp
diff --git a/doc/src/sgml/high-availability.sgml b/doc/src/sgml/high-availability.sgml
index 92e3b45..91cbabd 100644
--- a/doc/src/sgml/high-availability.sgml
+++ b/doc/src/sgml/high-availability.sgml
@@ -1824,7 +1824,7 @@ if (!triggered)
In normal operation, read-only transactions are allowed to
-update sequences and to use LISTEN, UNLISTEN, and
+use LISTEN, UNLISTEN, and
NOTIFY, so Hot Standby sessions operate under slightly tighter
restrictions than ordinary read-only sessions. It is possible that some
of these restrictions might be loosened in a future release.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] documentation typo in ALTER TABLE example
>> Amit Langote, which one was your intention? > > I wanted to show DETACH PARTITION command's usage with a range partitioned > table (detaching the "oldest" partition). > > So, we should apply Nagata-san's patch. Thank you for the confirmation. I have pushed the patch. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] documentation typo in ALTER TABLE example
> Hi, > > Attached is a simple patch to fix a documentation typo in > the ALTER TABLE example. Or the original author's intention might have been something like this: --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -1399,7 +1399,7 @@ ALTER TABLE cities Detach a partition from partitioned table: ALTER TABLE cities -DETACH PARTITION measurement_y2015m12; +DETACH PARTITION cities_ab; Amit Langote, which one was your intention? -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgbench - allow to store select results into variables
> Hello, > >> I think you'd better to change the following comments because there's >> no psqlscan.l or psqlscanslash.l in pgbench source tree. >> >> + * underscore. Keep this in sync with the definition of >> variable_char in >> + * psqlscan.l and psqlscanslash.l. > > Here is a v3 with a more precise comment. Looks good to me. I have marked the patch status as "Ready for committer". Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgbench - allow to store select results into variables
Fabien, > Hello Tatsuo-san, > >> Thank you for the patch. I tested a little bit and found that it does >> not allow value replacement against non ascii variables in given SQL >> statements . Is it intentional? > > No, this is a bug. > >> If not, I think you need to fix parseVariable() as well. > > Indeed. Here is v2. I think you'd better to change the following comments because there's no psqlscan.l or psqlscanslash.l in pgbench source tree. + * underscore. Keep this in sync with the definition of variable_char in + * psqlscan.l and psqlscanslash.l. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgbench - allow to store select results into variables
Fabien, >> As the variable infrastructures are pretty different between psql & >> pgbench (typed vs untyped values, sorted array vs linked list data >> structure, no hook vs 2 hooks, name spaces vs no such thing...), I >> have chosen the simplest option of just copying the name checking >> function and extending the lexer to authorize non-ascii letters, so >> that psql/pgbench would accept the same variable names with the same >> constraint about encodings. >> >> See patch attached & test script. > > Argh, I'm jet-lagged, wrong patch suffix... Here it is with the right > suffix. Thank you for the patch. I tested a little bit and found that it does not allow value replacement against non ascii variables in given SQL statements . Is it intentional? If not, I think you need to fix parseVariable() as well. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Warn users about duplicate configuration parameters
>> Recently I've discovered that if there are multiple values of the same >> parameter in postgresql.conf PostgreSQL will silently use the last one. >> It looks like not the best approach to me. For instance, user can find >> the first value in the config file and expect that it will be used, etc. >> >> I suggest to warn users about duplicated parameters. Here is a >> corresponding patch. >> >> Thoughts? > > -1 - I frequently just override earlier parameters by adding an include > at the end of the file. Also, with postgresql.auto.conf it's even more > common to override parameters. -1 from me too by the same reason Andres said. -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] pg_export_snapshot doc
pg_export_snapshot() cannot be used during recovery (i.e. on standby servers), but it's not documented. IMO this is a bug and should be fixed. Patch attached. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index cb0a36a..9923e67 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -18778,6 +18778,7 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup()); Snapshots are exported with the pg_export_snapshot function, shown in , and imported with the command. +This function cannot be used during recovery. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgbench - allow to store select results into variables
> If I understand correctly, the patch is moved because of the unrelated > issue that variables cannot be utf8 in pgbench, and it is a condition > to consider this patch that existing pgbench variables (set with \set) > can be utf8? I'm not sure if it is "unrelated" because the new feature relies on existing pgbench variable infrastructure. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgbench - allow to store select results into variables
>> Having said all that, I think we're at the point in the commitfest >> where if there's any design question at all about a patch, it should >> get booted to the next cycle. We are going to have more than enough >> to do to stabilize what's already committed, we don't need to be >> adding more uncertainty. > > Ok, I will move the patch to the next cf. Done. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgbench - allow to store select results into variables
> Well, personally, as an all-ASCII guy I'm not too fussed about that, > but I can see that other people might be annoyed. > > The main problem in dealing with it seems to be whether you're willing > to support pgbench running in non-backend-safe encodings (eg SJIS). > If we rejected that case then it'd be a relatively simple change to allow > pgbench to treat any high-bit-set byte as a valid variable name character. > (I think anyway, haven't checked the code.) > > Although ... actually, psql allows any high-bit-set byte in variable > names (cf valid_variable_name()) without concern about encoding. > That means it's formally incorrect in SJIS, but it's been like that > for an awful lot of years and nobody's complained. Maybe it'd be fine > for pgbench to act the same. That's my thought too. > Having said all that, I think we're at the point in the commitfest > where if there's any design question at all about a patch, it should > get booted to the next cycle. We are going to have more than enough > to do to stabilize what's already committed, we don't need to be > adding more uncertainty. Ok, I will move the patch to the next cf. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgbench - allow to store select results into variables
Tom and others, I still wonder whether I should commit this or not because this patch does not allow none ascii column names. We know pgbench variable name has been restricted since the functionality was born. When users explicitly define a pgbench variable using \set, it is not a too strong limitation, because it's in a "pgbench world" anyway and nothing is related to PostgreSQL core specs. However, \gset is not happy with perfectly valid column names in PostgreSQL core, which looks too inconsistent and confusing for users. So the choices are: 1) commit the patch now with documenting the limitation. (the patch looks good to me except the issue above) 2) move it to next cf hoping that someone starts the implementation to eliminate the limitation of none ascii variable names. Comments? Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp > Hello Tatsuo-san, > >> It seems the new feature \gset doesn't work with tables having none >> ascii column names: > > Indeed. The same error is triggered with the \set syntax, which does > not involve any query execution. > > I have added a sentence mentionning the restriction when variables are > first discussed in the documentation, see attached patch. > > -- > Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Statement timeout behavior in extended queries
> Then, the following sequence should have occurred. The test result is valid. Yes, I remembered that and was about to make a posting :-) > # Execute statement which takes 2 seconds. > 'P' "S1""SELECT pg_sleep(2)"0 > -> start transaction T1 > 'B' "S2""S1"0 0 0 Yes, an extended query automatically starts a transaction if there's no ongoing transaction. > 'P' "" "SET statement_timeout = '1s'" 0 > 'B' "" "" 0 0 0 > 'E' "" 0 > > # Execute statement which takes 2 seconds (statement timeout expected). > 'E' "S2"0 > -> timeout error occurred, T1 aborted Right. The automatically started transaction is aborted and the effect of the set statement is canceled. In summary, as far as I know Andres's patch is working as expected. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Statement timeout behavior in extended queries
> From: pgsql-hackers-ow...@postgresql.org >> [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Tatsuo Ishii >> I have done tests using pgproto. One thing I noticed a strange behavior. >> Below is an output of pgproto. The test first set the timeout to 3 seconds, >> and parse/bind for "SELECT pg_sleep(2)" then set timeout to 1 second using >> extended query. Subsequent Execute emits a statement timeout error as >> expected, but next "SELECT pg_sleep(2)" >> call using extended query does not emit a statement error. The test for >> this is "007-timeout-twice". Attached is the test cases including this. > > What's the handling of transactions like in pgproto? I guess the first > statement timeout error rolled back the effect of "SET statement_timeout = > '1s'", and the timeout reverted to 3s or some other value. Since pgproto is a dumb protocol machine, it does not start a transaction automatically (user needs to explicitly send a start transaction command via either simple or extended query). In this particular case no explicit transaction has started. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Statement timeout behavior in extended queries
>> What do you think? I've not really tested this with the extended protocol, >> so I'd appreciate if you could rerun your test from the older thread. > > The patch looks good and cleaner. It looks like the code works as expected. > As before, I ran one INSERT statement with PgJDBC, with gdb's breakpoints set > on enable_timeout() and disable_timeout(). I confirmed that enable_timeout() > is called just once at Parse message, and disable_timeout() is called just > once at Execute message. > > I'd like to wait for Tatsuo-san's thorough testing with pgproto. I have done tests using pgproto. One thing I noticed a strange behavior. Below is an output of pgproto. The test first set the timeout to 3 seconds, and parse/bind for "SELECT pg_sleep(2)" then set timeout to 1 second using extended query. Subsequent Execute emits a statement timeout error as expected, but next "SELECT pg_sleep(2)" call using extended query does not emit a statement error. The test for this is "007-timeout-twice". Attached is the test cases including this. FE=> Query(query="SET statement_timeout = '3s'") <= BE CommandComplete(SET) <= BE ReadyForQuery(I) FE=> Parse(stmt="S1", query="SELECT pg_sleep(2)") FE=> Bind(stmt="S1", portal="S2") FE=> Parse(stmt="", query="SET statement_timeout = '1s'") FE=> Bind(stmt="", portal="") FE=> Execute(portal="") FE=> Execute(portal="S2") FE=> Sync <= BE ParseComplete <= BE BindComplete <= BE ParseComplete <= BE BindComplete <= BE CommandComplete(SET) <= BE ErrorResponse(S ERROR V ERROR C 57014 M canceling statement due to statement timeout F postgres.c L 2968 R ProcessInterrupts ) <= BE ReadyForQuery(I) FE=> Parse(stmt="S3", query="SELECT pg_sleep(2)") FE=> Bind(stmt="S3", portal="S2") FE=> Execute(portal="S2") FE=> Sync <= BE ParseComplete <= BE BindComplete <= BE DataRow <= BE CommandComplete(SELECT 1) <= BE ReadyForQuery(I) FE=> Terminate tests.tar.gz Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Statement timeout behavior in extended queries
> On 2017-04-04 16:56:26 -0700, 'Andres Freund' wrote: >> On 2017-04-04 23:52:28 +, Tsunakawa, Takayuki wrote: >> > From: Andres Freund [mailto:and...@anarazel.de] >> > > Looks to me like npgsql doesn't do that either. None of libpq, pgjdbs >> > > and >> > > npgsql doing it seems like some evidence that it's ok. >> > >> > And psqlODBC now uses always libpq. >> > >> > Now time for final decision? >> >> I'll send an updated patch in a bit, and then will wait till tomorrow to >> push. Giving others a chance to chime in seems fair. > > Attached. I did not like that the previous patch had the timeout > handling duplicated in the individual functions, I instead centralized > it into start_xact_command(). Given that it already activated the > timeout in the most common cases, that seems to make more sense to > me. In your version we'd have called enable_statement_timeout() twice > consecutively (which behaviourally is harmless). > > What do you think? I've not really tested this with the extended > protocol, so I'd appreciate if you could rerun your test from the > older thread. Ok, I will look into your patch and test it out using pgproto. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Statement timeout behavior in extended queries
>> What's your point of the question? What kind of problem do you expect >> if the timeout starts only once at the first parse meesage out of >> bunch of parse messages? > > It's perfectly valid to send a lot of Parse messages without > interspersed Sync or Bind/Execute message. There'll be one timeout > covering all of those Parse messages, which can thus lead to a timeout, > even though nothing actually takes long individually. Hmm. IMO, that could happen even with the current statement timeout implementation as well. Or we could start/stop the timeout in exec_execute_message() only. This could avoid the problem above. Also this is more consistent with log_duration/log_min_duration_statement behavior than now. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Statement timeout behavior in extended queries
> Hm. I started to edit it, but I'm halfway coming back to my previous > view that this isn't necessarily ready. > > If a client were to to prepare a large number of prepared statements > (i.e. a lot of parse messages), this'll only start the timeout once, at > the first statement sent. It's not an issue for libpq which sends a > sync message after each PQprepare, nor does it look to be an issue for > pgjdbc based on a quick look. > > Does anybody think there might be a driver out there that sends a bunch > of 'parse' messages, without syncs? What's your point of the question? What kind of problem do you expect if the timeout starts only once at the first parse meesage out of bunch of parse messages? Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgbench - allow to store select results into variables
>> Please find attached a v8 which hopefully fixes these two issues. > Looks good to me, marking as ready for committer. I have looked into this a little bit. It seems the new feature \gset doesn't work with tables having none ascii column names: $ src/bin/pgbench/pgbench -t 1 -f /tmp/f test starting vacuum...end. gset: invalid variable name: "数字" client 0 file 0 command 0 compound 0: error storing into var 数字 transaction type: /tmp/f scaling factor: 1 query mode: simple number of clients: 1 number of threads: 1 number of transactions per client: 1 number of transactions actually processed: 0/1 This is because pgbench variable names are limited to ascii ranges. IMO the limitation is unnecessary and should be removed. (I know that the limitation was brought in by someone long time ago and the patch author is not responsible for that). Anyway, now that the feature reveals the undocumented limitation, we should document the limitation of \gset at least. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Statement timeout behavior in extended queries
Andres,
>> I think the code needs a few clarifying comments around this, but
>> otherwise seems good. Not restarting the timeout in those cases
>> obviously isn't entirely "perfect"/"correct", but a tradeoff - the
>> comments should note that.
>>
>> Tatsuo-san, do you want to change those, and push? I can otherwise.
>
> Andres,
>
> If you don't mind, could you please fix the comments and push it.
I have changed the comments as you suggested. If it's ok, I can push
the patch myself (today I have time to work on this).
Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index a2282058..1fd93359 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -149,6 +149,11 @@ static bool doing_extended_query_message = false;
static bool ignore_till_sync = false;
/*
+ * Flag to keep track of whether we have started statement timeout timer.
+ */
+static bool stmt_timer_started = false;
+
+/*
* If an unnamed prepared statement exists, it's stored here.
* We keep it separate from the hashtable kept by commands/prepare.c
* in order to reduce overhead for short-lived queries.
@@ -188,6 +193,8 @@ static bool IsTransactionStmtList(List *pstmts);
static void drop_unnamed_stmt(void);
static void SigHupHandler(SIGNAL_ARGS);
static void log_disconnections(int code, Datum arg);
+static void enable_statement_timeout(void);
+static void disable_statement_timeout(void);
/*
@@ -1239,6 +1246,15 @@ exec_parse_message(const char *query_string, /* string to execute */
start_xact_command();
/*
+ * Set statement timeout running if it's not already set. The timer will
+ * be reset in a subsequent execute message. Ideally the timer should be
+ * reset in this function so that each parse/bind/execute message catches
+ * the timeout, but it needs gettimeofday() call for each, which is too
+ * expensive.
+ */
+ enable_statement_timeout();
+
+ /*
* Switch to appropriate context for constructing parsetrees.
*
* We have two strategies depending on whether the prepared statement is
@@ -1526,6 +1542,15 @@ exec_bind_message(StringInfo input_message)
*/
start_xact_command();
+ /*
+ * Set statement timeout running if it's not already set. The timer will
+ * be reset in a subsequent execute message. Ideally the timer should be
+ * reset in this function so that each parse/bind/execute message catches
+ * the timeout, but it needs gettimeofday() call for each, which is too
+ * expensive.
+ */
+ enable_statement_timeout();
+
/* Switch back to message context */
MemoryContextSwitchTo(MessageContext);
@@ -1942,6 +1967,11 @@ exec_execute_message(const char *portal_name, long max_rows)
start_xact_command();
/*
+ * Set statement timeout running if it's not already set.
+ */
+ enable_statement_timeout();
+
+ /*
* If we re-issue an Execute protocol request against an existing portal,
* then we are only fetching more rows rather than completely re-executing
* the query from the start. atStart is never reset for a v3 portal, so we
@@ -2014,6 +2044,11 @@ exec_execute_message(const char *portal_name, long max_rows)
* those that start or end a transaction block.
*/
CommandCounterIncrement();
+
+ /*
+ * We need to reset statement timeout if already set.
+ */
+ disable_statement_timeout();
}
/* Send appropriate CommandComplete to client */
@@ -2443,14 +2478,10 @@ start_xact_command(void)
{
StartTransactionCommand();
- /* Set statement timeout running, if any */
- /* NB: this mustn't be enabled until we are within an xact */
- if (StatementTimeout > 0)
- enable_timeout_after(STATEMENT_TIMEOUT, StatementTimeout);
- else
- disable_timeout(STATEMENT_TIMEOUT, false);
-
xact_started = true;
+
+ /* Set statement timeout running, if any */
+ enable_statement_timeout();
}
}
@@ -2460,7 +2491,7 @@ finish_xact_command(void)
if (xact_started)
{
/* Cancel any active statement timeout before committing */
- disable_timeout(STATEMENT_TIMEOUT, false);
+ disable_statement_timeout();
CommitTransactionCommand();
@@ -4511,3 +4542,53 @@ log_disconnections(int code, Datum arg)
port->user_name, port->database_name, port->remote_host,
port->remote_port[0] ? " port=" : "", port->remote_port)));
}
+
+/*
+ * Set statement timeout if any.
+ */
+static void
+enable_statement_timeout(void)
+{
+ if (!stmt_timer_started)
+ {
+ if (StatementTimeout > 0)
+ {
+
+ /*
+ * Sanity check
+ */
+ if (!xact_started)
+ {
+ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("Transaction must have been already started to set statement timeout")));
+
Re: [HACKERS] Statement timeout behavior in extended queries
>> If what Tatsuo-san said to Tom is correct (i.e. each Parse/Bind/Execute >> starts and stops the timer), then it's a concern and the patch should not be >> ready for committer. However, the current patch is not like that -- it >> seems to do what others in this thread are expecting. > > Oh, interesting - I kind of took the author's statement as, uh, > authoritative ;). A quick look over the patch confirms your > understanding. Yes, Tsunakawa-san is correct. Sorry for confusion. > I think the code needs a few clarifying comments around this, but > otherwise seems good. Not restarting the timeout in those cases > obviously isn't entirely "perfect"/"correct", but a tradeoff - the > comments should note that. > > Tatsuo-san, do you want to change those, and push? I can otherwise. Andres, If you don't mind, could you please fix the comments and push it. > Unfortunately I can't move the patch back to the current CF, but I guess > we can just mark it as committed in the next. That will be great. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Statement timeout behavior in extended queries
> The patch doesn't seem to behave like that. The Parse message calls > start_xact_command() -> enable_statement_timeout() -> enable_timeout(), and > set stmt_timer_started to true. Subsequent Bind and Execute messages call > enable_statement_timeout(), but enable_statement_timeout() doesn't call > enable_timeout() because stmt_timer_started is already true. > > >> > It looks like the patch does the following. I think this is desirable, >> because starting and stopping the timer for each message may be costly as >> Tom said. >> > Parse(statement1) >> > start timer >> > Bind(statement1, portal1) >> > Execute(portal1) >> > stop timer >> > Sync > > I ran one INSERT statement using JDBC with log_min_messages = DEBUG3, and the > server log shows what I said. > > DEBUG: parse : insert into a values(2) > DEBUG: Set statement timeout > LOG: duration: 0.431 ms parse : insert into a values(2) > DEBUG: bind to > LOG: duration: 0.127 ms bind : insert into a values(2) > DEBUG: Disable statement timeout > LOG: duration: 0.184 ms execute : insert into a values(2) > DEBUG: snapshot of 1+0 running transaction ids (lsn 0/163BF28 oldest xid 561 > latest complete 560 next xid 562) Check. >> This doesn't work in general use cases. Following pattern appears frequently >> in applications. >> >> Parse(statement1) >> Bind(statement1, portal1) >> Execute(portal1) >> Bind(statement1, portal1) >> Execute(portal1) >> : >> : >> Sync > > It works. The first Parse-Bind-Execute is measured as one unit, then > subsequent Bind-Execute pairs are measured as other units. That's because > each Execute ends the statement_timeout timer and the Bind message starts it > again. I think this is desirable, so the current patch looks good. May I > mark this as ready for committer? FYI, make check-world passed successfully. It's too late. Someone has already moved the patch to the next CF (for PostgreSQL 11). >> Also what would happen if client just send a parse message and does nothing >> after that? > > It's correct to trigger the statement timeout in this case, because the first > SQL statement started (with the Parse message) and its execution is not > finished (with Execute message.) > > > Regards > Takayuki Tsunakawa > > > -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Statement timeout behavior in extended queries
> Where is the statement_timeout timer stopped when processing Parse and Bind > messages? Actually the statement timer is replaced with new statement timer value in enable_statement_timeout(). > Do you mean the following sequence of operations are performed in this patch? > > Parse(statement1) > start timer > stop timer > Bind(statement1, portal1) > start timer > stop timer > Execute(portal1) > start timer > stop timer > Sync Yes. > It looks like the patch does the following. I think this is desirable, > because starting and stopping the timer for each message may be costly as Tom > said. > Parse(statement1) > start timer > Bind(statement1, portal1) > Execute(portal1) > stop timer > Sync This doesn't work in general use cases. Following pattern appears frequently in applications. Parse(statement1) Bind(statement1, portal1) Execute(portal1) Bind(statement1, portal1) Execute(portal1) : : Sync Also what would happen if client just send a parse message and does nothing after that? So I think following is better: Parse(statement1) Bind(statement1, portal1) Execute(portal1) start timer stop timer Bind(statement1, portal1) Execute(portal1) start timer stop timer : : Sync Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Statement timeout behavior in extended queries
> That seems like it could represent quite a lot of added overhead, > on machines where gettimeofday() is slow ... which is still a lot > of them, unfortunately. Maybe. I think we could eliminate restarting the timer for parse and bind. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Statement timeout behavior in extended queries
Thank you for reviewing my patch!
> Hello,
>
>
> I've reviewed the patch. My understanding is as follows. Please correct me
> if I'm wrong.
>
> * The difference is that the Execute message stops the statement_timeout
> timer,
No. Parse, bind and Execute message indivually stops and starts the
statement_timeout timer with the patch. Unless there's a lock
conflict, parse and bind will take very short time. So actually users
could only observe the timeout in execute message though.
> so that the execution of one statement doesn't shorten the timeout
> period of subsequent statements when they are run in batch followed by
> a single Sync message.
This is true. Currently multiple set of parse/bind/execute will not
trigger statement timeout until sync message is sent to
backend. Suppose statement_timeout is set to 3 seconds, combo A
(parse/bind/execute) takes 2 seconds and combo B (parse/bind/execute)
takes 2 seconds then a sync message is followed. Currently statement
timeout is fired in the run of combo B (assuming that parse and bind
take almost 0 seconds). With my patch, no statement timeout will be
fired because both combo A and combo B runs within 3 seconds.
> * This patch is also necessary (or desirable) for the feature
> "Pipelining/batch mode support for libpq," which is being developed for PG
> 10. This patch enables correct timeout handling for each statement execution
> in a batch. Without this patch, the entire batch of statements is subject to
> statement_timeout. That's different from what the manual describes about
> statement_timeout. statement_timeout should measure execution of each
> statement.
True.
> Below are what I found in the patch.
>
>
> (1)
> +static bool st_timeout = false;
>
> I think the variable name would better be stmt_timeout_enabled or
> stmt_timer_started, because other existing names use stmt to abbreviate
> statement, and thhis variable represents a state (see xact_started for
> example.)
Agreed. Chaged to stmt_timer_started.
> (2)
> +static void enable_statement_timeout(void)
> +{
>
> +static void disable_statement_timeout(void)
> +{
>
> "static void" and the remaining line should be on different lines, as other
> functions do.
Fixed.
> (3)
> + /*
> + * Sanity check
> + */
> + if (!xact_started)
> + {
> + ereport(ERROR,
> +
> (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> + errmsg("Transaction must have
> been already started to set statement timeout")));
> + }
>
> I think this fragment can be deleted, because enable/disable_timeout() is
> used only at limited places in postgres.c, so I don't see the chance of
> misuse.
I'd suggest leave it as it is. Because it might be possible that the
function is used in different place in the future. Or at least we
should document the pre-condition as a comment.
revised patch attached.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index a2282058..68a739e 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -149,6 +149,11 @@ static bool doing_extended_query_message = false;
static bool ignore_till_sync = false;
/*
+ * Flag to keep track of whether we have started statement timeout timer.
+ */
+static bool stmt_timer_started = false;
+
+/*
* If an unnamed prepared statement exists, it's stored here.
* We keep it separate from the hashtable kept by commands/prepare.c
* in order to reduce overhead for short-lived queries.
@@ -188,6 +193,8 @@ static bool IsTransactionStmtList(List *pstmts);
static void drop_unnamed_stmt(void);
static void SigHupHandler(SIGNAL_ARGS);
static void log_disconnections(int code, Datum arg);
+static void enable_statement_timeout(void);
+static void disable_statement_timeout(void);
/*
@@ -1239,6 +1246,11 @@ exec_parse_message(const char *query_string, /* string to execute */
start_xact_command();
/*
+ * Set statement timeout running, if any
+ */
+ enable_statement_timeout();
+
+ /*
* Switch to appropriate context for constructing parsetrees.
*
* We have two strategies depending on whether the prepared statement is
@@ -1526,6 +1538,11 @@ exec_bind_message(StringInfo input_message)
*/
start_xact_command();
+ /*
+ * Set statement timeout running, if any
+ */
+ enable_statement_timeout();
+
/* Switch back to message context */
MemoryContextSwitchTo(MessageContext);
@
Re: [HACKERS] [BUGS] Bug in Physical Replication Slots (at least 9.5)?
> On Thu, Mar 30, 2017 at 9:12 AM, Tatsuo Ishii <is...@sraoss.co.jp> wrote: >> Committers will not apply patches which has trailing whitespace >> issues. So the patch submitter needs to fix them anyway. > > I cannot comment on that point (committers are free to pick up things > the way they want), but just using git commands to apply a patch > should not be an obstacle for a review if a patch can be easily > applied as long as they roughly respect GNU's diff format. My point is, the coding standard. Having trainling whitespace is against our coding standard and committers should not accept such a code, I believe. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [BUGS] Bug in Physical Replication Slots (at least 9.5)?
> On Thu, Mar 30, 2017 at 8:49 AM, Venkata B Nagothi <nag1...@gmail.com> wrote: >> On Tue, Mar 28, 2017 at 5:51 PM, Kyotaro HORIGUCHI >> <horiguchi.kyot...@lab.ntt.co.jp> wrote: >> I tried applying this patch to latest master, it is not getting applied >> >> [dba@buildhost postgresql]$ git apply >> /data/postgresql-patches/9.5-ReplicationSlots-Bug-Patch/0001-Fix-a-bug-of-physical-replication-slot_a6f22e8.patch >> /data/postgresql-patches/9.5-ReplicationSlots-Bug-Patch/0001-Fix-a-bug-of-physical-replication-slot_a6f22e8.patch:28: >> trailing whitespace. >> /* >> /data/postgresql-patches/9.5-ReplicationSlots-Bug-Patch/0001-Fix-a-bug-of-physical-replication-slot_a6f22e8.patch:29: >> trailing whitespace. >> * This variable corresponds to restart_lsn in pg_replication_slots for a >> /data/postgresql-patches/9.5-ReplicationSlots-Bug-Patch/0001-Fix-a-bug-of-physical-replication-slot_a6f22e8.patch:30: >> trailing whitespace. >> * physical slot. This has a valid value only when it differs from the >> current >> /data/postgresql-patches/9.5-ReplicationSlots-Bug-Patch/0001-Fix-a-bug-of-physical-replication-slot_a6f22e8.patch:31: >> trailing whitespace. >> * flush pointer. >> /data/postgresql-patches/9.5-ReplicationSlots-Bug-Patch/0001-Fix-a-bug-of-physical-replication-slot_a6f22e8.patch:32: >> trailing whitespace. >> */ >> error: patch failed: src/backend/replication/walsender.c:210 >> error: src/backend/replication/walsender.c: patch does not apply > > git apply and git am can be very picky sometimes, so you may want to > fallback to patch -p1 if things don't work. In this case it does. Committers will not apply patches which has trailing whitespace issues. So the patch submitter needs to fix them anyway. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [POC] hash partitioning
> Please post an explanation for the delay and a schedule for the new > patch. If no patch or explanation is posted by 2017-03-17 AoE I will > mark this submission "Returned with Feedback". Depite the fact that Yugo has posted a new patch on 2017-03-17, this item had been marked as "Returned with Feedback". I don't know why. -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Statement timeout behavior in extended queries
> On Wed, Feb 22, 2017 at 11:50:44AM +0900, Tatsuo Ishii wrote: >> Last year I have proposed an enhancement regarding behavior of the >> statement timeout in extended queries. >> >> https://www.postgresql.org/message-id/20160528.220442.1489791680347556026.t-ishii%40sraoss.co.jp >> >> IMO the current behavior is counter intuitive and I would like to >> change it toward PostgreSQL 10.0. >> >> For example, suppose that the timeout is set to 4 seconds and the >> first query takes 2 seconds and the second query takes 3 seconds. Then >> the statement timeout is triggered if a sync message is sent to >> backend after the second query. >> >> Moreover, log_duration or log_min_duration_statement shows that each >> query took 2 or 3 seconds of course, which is not very consistent with >> the statement timeout IMO. >> >> Attached patch tries to change the behavior, by checking statement >> timeout against each phase of an extended query. >> >> To test the patch, I have created a small tool called "pgproto", which >> can issue arbitrary sequence of frontend/backend message, reading from a >> text file. >> >> https://github.com/tatsuo-ishii/pgproto >> (to build the program, you need C compiler and libpq) > > Does it seem reasonable to start making this into a regression test > and/or fuzz test for the protocol itself? I personally think the regression tests ought to include tests for extended query protocols and pgproto could be an useful tool to implement that. Of course if we are going for that direction, pgproto needs to be a contrib module first. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Statement timeout behavior in extended queries
Last year I have proposed an enhancement regarding behavior of the
statement timeout in extended queries.
https://www.postgresql.org/message-id/20160528.220442.1489791680347556026.t-ishii%40sraoss.co.jp
IMO the current behavior is counter intuitive and I would like to
change it toward PostgreSQL 10.0.
For example, suppose that the timeout is set to 4 seconds and the
first query takes 2 seconds and the second query takes 3 seconds. Then
the statement timeout is triggered if a sync message is sent to
backend after the second query.
Moreover, log_duration or log_min_duration_statement shows that each
query took 2 or 3 seconds of course, which is not very consistent with
the statement timeout IMO.
Attached patch tries to change the behavior, by checking statement
timeout against each phase of an extended query.
To test the patch, I have created a small tool called "pgproto", which
can issue arbitrary sequence of frontend/backend message, reading from a
text file.
https://github.com/tatsuo-ishii/pgproto
(to build the program, you need C compiler and libpq)
A test data is here:
--
#
# Test case for statement timeout patch.
#
'Q' "SET statement_timeout = '4s'"
# Receive response from backend
'Y'
# Execute statement which takes 3 seconds.
'P' "S1""SELECT pg_sleep(3)"0
'B' "" "S1"0 0 0
'E' "" 0
'C' 'S' "S1"
# Execute statement which takes 2 seconds.
'P' "S2""SELECT pg_sleep(2)"0
'B' "" "S2"0 0 0
'E' "" 0
'C' 'S' "S2"
# Issue Sync message
'S'
# Receive response from backend
'Y'
# Send terminate message
'X'
--
In each row, the first column corresponds to the message type defined
in frontend/backend protocol (except 'Y', which asks pgproto to
collect responses from backend). Each column is separated with a tab
character.
To run the test:
pgproto -f data_file -p port_number -d database_name
With the attached patch, "SELECT pg_sleep(3)" and "SELECT pg_sleep(2)"
does not trigger the statement timeout as expected, while existing
code triggers the statement timeout after the sync message is sent.
Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index b07d6c6..e35a1dd 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -149,6 +149,11 @@ static bool doing_extended_query_message = false;
static bool ignore_till_sync = false;
/*
+ * Flag to keep track of whether we have started statement timeout timer.
+ */
+static bool st_timeout = false;
+
+/*
* If an unnamed prepared statement exists, it's stored here.
* We keep it separate from the hashtable kept by commands/prepare.c
* in order to reduce overhead for short-lived queries.
@@ -188,6 +193,8 @@ static bool IsTransactionStmtList(List *pstmts);
static void drop_unnamed_stmt(void);
static void SigHupHandler(SIGNAL_ARGS);
static void log_disconnections(int code, Datum arg);
+static void enable_statement_timeout(void);
+static void disable_statement_timeout(void);
/*
@@ -1234,6 +1241,11 @@ exec_parse_message(const char *query_string, /* string to execute */
start_xact_command();
/*
+ * Set statement timeout running, if any
+ */
+ enable_statement_timeout();
+
+ /*
* Switch to appropriate context for constructing parsetrees.
*
* We have two strategies depending on whether the prepared statement is
@@ -1521,6 +1533,11 @@ exec_bind_message(StringInfo input_message)
*/
start_xact_command();
+ /*
+ * Set statement timeout running, if any
+ */
+ enable_statement_timeout();
+
/* Switch back to message context */
MemoryContextSwitchTo(MessageContext);
@@ -1937,6 +1954,11 @@ exec_execute_message(const char *portal_name, long max_rows)
start_xact_command();
/*
+ * Set statement timeout running, if any
+ */
+ enable_statement_timeout();
+
+ /*
* If we re-issue an Execute protocol request against an existing portal,
* then we are only fetching more rows rather than completely re-executing
* the query from the start. atStart is never reset for a v3 portal, so we
@@ -2008,6 +2030,11 @@ exec_execute_message(const char *portal_name, long max_rows)
* those that start or end a transaction block.
*/
CommandCounterIncrement();
+
+ /*
+ * We need to reset statement timeout if already set.
+ */
+ disable_statement_timeout();
}
/* Send appropriate CommandComplete to client */
@@ -2437,14 +2464,10 @@ start_xact_command(void)
{
StartTransactionCommand();
- /*
[HACKERS] Sync message
Sync message causes backend to return a "Ready for query" response even if there's no query previously sent to the backend. I don't think this is a bug but I'd think it would be better to write this behavior in the doc, because it might help someone who want to write an API which needs to handle frontend/backend protocol in the future. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix a comment in feelist.c
>> I've found a mistake in a comment of StrategyNotifyBgWriter >> in freelist.c. bgwriterLatch was replaced by bgwprocno in >> the following commit, but this is remained in the comment. >> >> commit d72731a70450b5e7084991b9caa15cb58a2820df >> Author: Andres Freund <and...@anarazel.de> >> Date: Thu Dec 25 18:24:20 2014 +0100 >> >> Lockless StrategyGetBuffer clock sweep hot path. > > Looks good to me. I will commit/push the patch if there's no > objection. Committed/pushed to all supported branches. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix a comment in feelist.c
> I've found a mistake in a comment of StrategyNotifyBgWriter > in freelist.c. bgwriterLatch was replaced by bgwprocno in > the following commit, but this is remained in the comment. > > commit d72731a70450b5e7084991b9caa15cb58a2820df > Author: Andres Freund <and...@anarazel.de> > Date: Thu Dec 25 18:24:20 2014 +0100 > > Lockless StrategyGetBuffer clock sweep hot path. Looks good to me. I will commit/push the patch if there's no objection. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Questionable tag usage
In: https://www.postgresql.org/docs/devel/static/runtime-config-file-locations.html --- ident_file (string) Specifies the configuration file for Section 20.2, “User Name Maps” user name mapping (customarily called pg_ident.conf). This parameter can only be set at server start. --- "Specifies the configuration file for Section 20.2, “User Name Maps” user name mapping" looks pretty strange to me because a raw section name appears. This is due to the corresponding SGML coding: Specifies the configuration file for user name mapping (customarily called pg_ident.conf). This parameter can only be set at server start. Shouldn't we use a link tag instead of the xref tag here? Attached is a patch to fix this. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 30dd54c..1707d40 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -532,10 +532,10 @@ include_dir 'conf.d' - Specifies the configuration file for - user name mapping - (customarily called pg_ident.conf). - This parameter can only be set at server start. + Specifies the configuration file + for user name mapping + (customarily called pg_ident.conf). This parameter can + only be set at server start. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Questions regarding signal handler of postmaster
> But we keep signals blocked almost all the time in the postmaster, > so in reality no signal handler can interrupt anything except the > select() wait call. People complain about that coding technique > all the time, but no one has presented any reason to believe that > it's broken. Ok, there seems no better solution than always blocking signals. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Questions regarding signal handler of postmaster
> I encountered that problem with postmaster and fixed it in 9.4.0 (it's not > back-patched to earlier releases because it's relatively complex). > > https://www.postgresql.org/message-id/20DAEA8949EC4E2289C6E8E58560DEC0@maumau > > > [Excerpt from 9.4 release note] > During crash recovery or immediate shutdown, send uncatchable termination > signals (SIGKILL) to child processes that do not shut down promptly (MauMau, > Álvaro Herrera) > This reduces the likelihood of leaving orphaned child processes behind after > postmaster shutdown, as well as ensuring that crash recovery can proceed if > some child processes have become “stuck”. Looks wild to me:-) I hope there exists better way to solve the problem... Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Questions regarding signal handler of postmaster
In postmaster.c signal handler pmdie() calls ereport() and errmsg_internal(), which could call palloc() then malloc() if necessary. Because it is possible that pmdie() gets called while malloc() gets called in postmaster, I think it is possible that a deadlock situation could occur through an internal locking inside malloc(). I have not observed the exact case in PostgreSQL but I see a suspected case in Pgpool-II. In the stack trace #14, malloc() is called by Pgpool-II. It is interrupted by a signal in #11, and the signal handler calls malloc() again, and it is stuck at #0. So my question is, is my concern about PostgreSQL valid? If so, how can we fix it? #0 __lll_lock_wait_private () at ../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:95 #1 0x7f67fe20ccba in _L_lock_12808 () from /lib/x86_64-linux-gnu/libc.so.6 #2 0x7f67fe20a6b5 in __GI___libc_malloc (bytes=15) at malloc.c:2887 #3 0x7f67fe21072a in __GI___strdup (s=0x7f67fe305dd8 "/etc/localtime") at strdup.c:42 #4 0x7f67fe239f51 in tzset_internal (always=, explicit=explicit@entry=1) at tzset.c:444 #5 0x7f67fe23a913 in __tz_convert (timer=timer@entry=0x7ffce1c1b7f8, use_localtime=use_localtime@entry=1, tp=tp@entry=0x7f67fe54bde0 <_tmbuf>) at tzset.c:632 #6 0x7f67fe2387d1 in __GI_localtime (t=t@entry=0x7ffce1c1b7f8) at localtime.c:42 #7 0x0045627b in log_line_prefix (buf=buf@entry=0x7ffce1c1b8d0, line_prefix=, edata=) at ../../src/utils/error/elog.c:2059 #8 0x0045894d in send_message_to_server_log (edata=0x753320 ) at ../../src/utils/error/elog.c:2084 #9 EmitErrorReport () at ../../src/utils/error/elog.c:1129 #10 0x00456d8e in errfinish (dummy=) at ../../src/utils/error/elog.c:434 #11 0x00421f57 in die (sig=2) at protocol/child.c:925 #12 #13 _int_malloc (av=0x7f67fe546760 , bytes=4176) at malloc.c:3302 #14 0x7f67fe20a6c0 in __GI___libc_malloc (bytes=4176) at malloc.c:2891 Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Back-patch use of unnamed POSIX semaphores for Linux?
> Potential risks involving minor upgrades are far higher than the risks > involved by systemd, so -1 for a backpatch seen from here. As long as we would have a compile time switch to enable/disable the back-patched feature, it seems it would be acceptable. In the worst case, the back-patching could bring disasters, but in that case packagers could turn off the switch and ship updated version of packages. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
