Re: [HACKERS] sql_drop Event Trigger
Alvaro Herrera alvhe...@2ndquadrant.com writes: I thought there was the idea that the list of objects to drop was to be acquired before actually doing the deletion; so that the trigger function could, for instance, get the name of the table being dropped. I don't see that it works if we only provide pg_event_trigger_dropped_objects to ddl_command_end events. Am I missing something? Tom and Robert have been rightfully insisting on how delicate it has been to come up with the right behavior for performMultipleDeletions, and that's not something we can easily reorganise. So the only way to get at the information seems to be what Robert insisted that I do, that is storing the information about the objects being dropped for later processing. Of course, later processing means that the objects are already dropped and that you can't do much. The idea is to provide more than just the OID of the object, we have yet to decide if adding a catalog cache lookup within performMultipleDeletions() is ok. If it is, we will extend the pg_event_trigger_dropped_objects() definition to also return the object name and its schema name, at a minimum. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] function for setting/getting same timestamp during whole transaction
Hi all,
I have deferred constraint update trigger in which I need to set same
timestamp to all modified rows. The time needs to be the time of first
invocation of this trigger fuction in transaciton. My intention is to set
commit time to rows modified in transaction.
So I need function that will store and return given timestamp on first call
in transaction and on subsequent calls will return stored timestamp. This
function have to be as fast as possible to minimize the inpact on
performance of trigger.
I have created a plpgsql function that uses temporal table for this task.
On first invocation in transaction row with timestamp is inserted and on
commit deleted. What I don't like is overhead with checks on table
existence on each invocation. Here is code:
CREATE OR REPLACE FUNCTION get_my_timestamp (
IN in_initial_timestamp TIMESTAMPTZ
) RETURNS TIMESTAMPTZ AS
$$
DECLARE
v_ret TIMESTAMPTZ;
BEGIN
--check temp table existence
PERFORM
1
FROM
pg_catalog.pg_class c
JOIN pg_catalog.pg_namespace n
ON n.oid = c.relnamespace
WHERE
c.relkind IN ('r','') AND
c.relname = 'timestamp_storage' AND
pg_catalog.pg_table_is_visible(c.oid) AND
n.nspname LIKE 'pg_temp%';
IF NOT FOUND THEN
CREATE TEMP TABLE timestamp_storage (
my_timestamp TIMESTAMPTZ
) ON COMMIT DELETE ROWS;
END IF;
--select timestamp
SELECT
my_timestamp
INTO
v_ret
FROM
timestamp_storage;
IF NOT FOUND THEN
INSERT INTO timestamp_storage(my_timestamp)
VALUES (in_initial_timestamp)
RETURNING my_timestamp
INTO v_ret;
END IF;
RETURN v_ret;
END;
$$ LANGUAGE plpgsql;
Example:
begin;
select get_my_timestamp(clock_timestamp());
get_my_timestamp
2013-02-06 11:07:33.698+01
select get_my_timestamp(clock_timestamp());
get_my_timestamp
2013-02-06 11:07:33.698+01
commit;
select get_my_timestamp(clock_timestamp());
get_my_timestamp
2013-02-06 11:09:02.406+01
Is there any more effective way of accomplishing this? Maybe in different
language.
Regards,
Miroslav Simulcik
Re: [HACKERS] function for setting/getting same timestamp during whole transaction
Hi,
I dont have access to pg at this moment... But:
BEGIN;
SELECT now();
SELECT clock_timestamp();
SELECT now();
SELECT pg_sleep(100);
SELECT now();
cCOMMIT;
Now() should always return the same, very first, result...
On Wednesday, February 6, 2013, Miroslav Šimulčík wrote:
Hi all,
I have deferred constraint update trigger in which I need to set same
timestamp to all modified rows. The time needs to be the time of first
invocation of this trigger fuction in transaciton. My intention is to set
commit time to rows modified in transaction.
So I need function that will store and return given timestamp on first
call in transaction and on subsequent calls will return stored timestamp.
This function have to be as fast as possible to minimize the inpact on
performance of trigger.
I have created a plpgsql function that uses temporal table for this task.
On first invocation in transaction row with timestamp is inserted and on
commit deleted. What I don't like is overhead with checks on table
existence on each invocation. Here is code:
CREATE OR REPLACE FUNCTION get_my_timestamp (
IN in_initial_timestamp TIMESTAMPTZ
) RETURNS TIMESTAMPTZ AS
$$
DECLARE
v_ret TIMESTAMPTZ;
BEGIN
--check temp table existence
PERFORM
1
FROM
pg_catalog.pg_class c
JOIN pg_catalog.pg_namespace n
ON n.oid = c.relnamespace
WHERE
c.relkind IN ('r','') AND
c.relname = 'timestamp_storage' AND
pg_catalog.pg_table_is_visible(c.oid) AND
n.nspname LIKE 'pg_temp%';
IF NOT FOUND THEN
CREATE TEMP TABLE timestamp_storage (
my_timestamp TIMESTAMPTZ
) ON COMMIT DELETE ROWS;
END IF;
--select timestamp
SELECT
my_timestamp
INTO
v_ret
FROM
timestamp_storage;
IF NOT FOUND THEN
INSERT INTO timestamp_storage(my_timestamp)
VALUES (in_initial_timestamp)
RETURNING my_timestamp
INTO v_ret;
END IF;
RETURN v_ret;
END;
$$ LANGUAGE plpgsql;
Example:
begin;
select get_my_timestamp(clock_timestamp());
get_my_timestamp
2013-02-06 11:07:33.698+01
select get_my_timestamp(clock_timestamp());
get_my_timestamp
2013-02-06 11:07:33.698+01
commit;
select get_my_timestamp(clock_timestamp());
get_my_timestamp
2013-02-06 11:09:02.406+01
Is there any more effective way of accomplishing this? Maybe in different
language.
Regards,
Miroslav Simulcik
Re: [HACKERS] function for setting/getting same timestamp during whole transaction
This is not what I'm looking for. now() returns transaction start time. I
need to set my own time anytime in transaction and then use that time later.
Miro
2013/2/6 Misa Simic misa.si...@gmail.com
Hi,
I dont have access to pg at this moment... But:
BEGIN;
SELECT now();
SELECT clock_timestamp();
SELECT now();
SELECT pg_sleep(100);
SELECT now();
cCOMMIT;
Now() should always return the same, very first, result...
On Wednesday, February 6, 2013, Miroslav Šimulčík wrote:
Hi all,
I have deferred constraint update trigger in which I need to set same
timestamp to all modified rows. The time needs to be the time of first
invocation of this trigger fuction in transaciton. My intention is to set
commit time to rows modified in transaction.
So I need function that will store and return given timestamp on first
call in transaction and on subsequent calls will return stored timestamp.
This function have to be as fast as possible to minimize the inpact on
performance of trigger.
I have created a plpgsql function that uses temporal table for this task.
On first invocation in transaction row with timestamp is inserted and on
commit deleted. What I don't like is overhead with checks on table
existence on each invocation. Here is code:
CREATE OR REPLACE FUNCTION get_my_timestamp (
IN in_initial_timestamp TIMESTAMPTZ
) RETURNS TIMESTAMPTZ AS
$$
DECLARE
v_ret TIMESTAMPTZ;
BEGIN
--check temp table existence
PERFORM
1
FROM
pg_catalog.pg_class c
JOIN pg_catalog.pg_namespace n
ON n.oid = c.relnamespace
WHERE
c.relkind IN ('r','') AND
c.relname = 'timestamp_storage' AND
pg_catalog.pg_table_is_visible(c.oid) AND
n.nspname LIKE 'pg_temp%';
IF NOT FOUND THEN
CREATE TEMP TABLE timestamp_storage (
my_timestamp TIMESTAMPTZ
) ON COMMIT DELETE ROWS;
END IF;
--select timestamp
SELECT
my_timestamp
INTO
v_ret
FROM
timestamp_storage;
IF NOT FOUND THEN
INSERT INTO timestamp_storage(my_timestamp)
VALUES (in_initial_timestamp)
RETURNING my_timestamp
INTO v_ret;
END IF;
RETURN v_ret;
END;
$$ LANGUAGE plpgsql;
Example:
begin;
select get_my_timestamp(clock_timestamp());
get_my_timestamp
2013-02-06 11:07:33.698+01
select get_my_timestamp(clock_timestamp());
get_my_timestamp
2013-02-06 11:07:33.698+01
commit;
select get_my_timestamp(clock_timestamp());
get_my_timestamp
2013-02-06 11:09:02.406+01
Is there any more effective way of accomplishing this? Maybe in different
language.
Regards,
Miroslav Simulcik
Re: [HACKERS] function for setting/getting same timestamp during whole transaction
2013/2/6 Miroslav Šimulčík simulcik.m...@gmail.com:
This is not what I'm looking for. now() returns transaction start time. I
need to set my own time anytime in transaction and then use that time later.
Miro
2013/2/6 Misa Simic misa.si...@gmail.com
Hi,
I dont have access to pg at this moment... But:
BEGIN;
SELECT now();
SELECT clock_timestamp();
SELECT now();
SELECT pg_sleep(100);
SELECT now();
cCOMMIT;
Now() should always return the same, very first, result...
On Wednesday, February 6, 2013, Miroslav Šimulčík wrote:
Hi all,
I have deferred constraint update trigger in which I need to set same
timestamp to all modified rows. The time needs to be the time of first
invocation of this trigger fuction in transaciton. My intention is to set
commit time to rows modified in transaction.
So I need function that will store and return given timestamp on first
call in transaction and on subsequent calls will return stored timestamp.
This function have to be as fast as possible to minimize the inpact on
performance of trigger.
I have created a plpgsql function that uses temporal table for this task.
On first invocation in transaction row with timestamp is inserted and on
commit deleted. What I don't like is overhead with checks on table existence
on each invocation. Here is code:
CREATE OR REPLACE FUNCTION get_my_timestamp (
IN in_initial_timestamp TIMESTAMPTZ
) RETURNS TIMESTAMPTZ AS
$$
DECLARE
v_ret TIMESTAMPTZ;
BEGIN
--check temp table existence
PERFORM
1
FROM
pg_catalog.pg_class c
JOIN pg_catalog.pg_namespace n
ON n.oid = c.relnamespace
WHERE
c.relkind IN ('r','') AND
c.relname = 'timestamp_storage' AND
pg_catalog.pg_table_is_visible(c.oid) AND
n.nspname LIKE 'pg_temp%';
IF NOT FOUND THEN
CREATE TEMP TABLE timestamp_storage (
my_timestamp TIMESTAMPTZ
) ON COMMIT DELETE ROWS;
END IF;
--select timestamp
SELECT
my_timestamp
INTO
v_ret
FROM
timestamp_storage;
IF NOT FOUND THEN
INSERT INTO timestamp_storage(my_timestamp)
VALUES (in_initial_timestamp)
RETURNING my_timestamp
INTO v_ret;
END IF;
RETURN v_ret;
END;
$$ LANGUAGE plpgsql;
Example:
begin;
select get_my_timestamp(clock_timestamp());
get_my_timestamp
2013-02-06 11:07:33.698+01
select get_my_timestamp(clock_timestamp());
get_my_timestamp
2013-02-06 11:07:33.698+01
commit;
select get_my_timestamp(clock_timestamp());
get_my_timestamp
2013-02-06 11:09:02.406+01
Is there any more effective way of accomplishing this? Maybe in different
language.
probably you can use a little bit cheaper session variables
test to system tables is slower then trapping error - just try to read
from tmp and when a read fails, then create table
probably C trigger can be very effective, possible to use this
technique - http://postgres.cz/wiki/Funkce_rownum%28%29 (sorry, it is
in Czech language)
Regards
Pavel Stehule
Regards,
Miroslav Simulcik
--
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] sql_drop Event Trigger
Alvaro Herrera alvhe...@2ndquadrant.com writes:
The bigger change I mentioned was the stuff in dependency.c -- I wasn't
too happy about exposing the whole ObjectAddresses stuff to the outside
world. The attached version only exposes simple accessors to let an
external user of that to iterate on such arrays. Some more commentary
is probably needed on those new functions. Also, if we're going to
extend things in this way we probably need to get extra out alongside
the object array itself.
Thanks for that.
I added some comments on those new functions, and made it so that we
call get_object_addresses_numelements() only once per loop rather than
at each step in the loop. See attached.
A larger issue with the patch is handling of subxacts. A quick test
doesn't reveal any obvious misbehavior, but having the list of objects
dropped by a global variable might be problematic. What if, say, the
event trigger function does something funny in an EXCEPTION block and it
fails? Some clever test case is needed here, I think. Also, if we
reset the variable at EOXact, do we also need to do something at
EOSubXact?
Now that you mention it, the case I'd be worried about is:
BEGIN;
SAVEPOINT a;
DROP TABLE foo;
ROLLBACK TO SAVEPOINT a;
DROP TABLE bar;
COMMIT;
As we currently have no support for on-commit triggers or the like, the
user function is going to run as part of the DROP TABLE foo; command,
and its effects are all going to disappear at the next ROLLBACK TO
SAVEPOINT anyway.
If the event trigger user function fails in an EXCEPTION block, I
suppose that the whole transaction is going to get a ROLLBACK, which
will call AbortTransaction() or CleanupTransaction(), which will reset
the static variable EventTriggerSQLDropInProgress. And the list itself
is gone away with the memory context reset.
I think the only missing detail is to force EventTriggerSQLDropList back
to NULL from within AtEOXact_EventTrigger(), and I've done so in the
attached. As we're only looking at the list when the protecting boolean
is true, I don't think it's offering anything else than clarity, which
makes it worthwile already.
You will find both the patch-on-top-of-your-patch (named .2.b) and the
new whole patch attached (named .3), as it makes things way easier IME.
Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index 6f95f50..3732fc8 100644
--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -2152,12 +2152,24 @@ record_object_address_dependencies(const ObjectAddress *depender,
behavior);
}
+/*
+ * get_object_addresses_numelements
+ *
+ * Return the number of object addresses in the given ObjectAddresses, allowing
+ * external modules to loop over the array.
+ */
int
get_object_addresses_numelements(const ObjectAddresses *addresses)
{
return addresses-numrefs;
}
+/*
+ * get_object_addresses_element
+ *
+ * Return the ObjectAddress at position i, allowing to fetch it from an
+ * external module.
+ */
ObjectAddress *
get_object_addresses_element(const ObjectAddresses *addresses, int i)
{
diff --git a/src/backend/commands/event_trigger.c b/src/backend/commands/event_trigger.c
index 9ed5715..4e112af 100644
--- a/src/backend/commands/event_trigger.c
+++ b/src/backend/commands/event_trigger.c
@@ -862,6 +862,8 @@ AtEOXact_EventTrigger(bool isCommit)
{
/* even on success we want to reset EventTriggerSQLDropInProgress */
EventTriggerSQLDropInProgress = false;
+ /* the list is palloc()ed and has already been taken care of */
+ EventTriggerSQLDropList = NULL;
}
/*
@@ -878,7 +880,7 @@ pg_event_trigger_dropped_objects(PG_FUNCTION_ARGS)
Tuplestorestate *tupstore;
MemoryContext per_query_ctx;
MemoryContext oldcontext;
- int i;
+ int i, n;
/*
* This function is meant to be called from within an event trigger in
@@ -914,7 +916,10 @@ pg_event_trigger_dropped_objects(PG_FUNCTION_ARGS)
MemoryContextSwitchTo(oldcontext);
- for (i = 0; i get_object_addresses_numelements(EventTriggerSQLDropList); i++)
+ /* only call the get_object_addresses_numelements accessor function once */
+ n = get_object_addresses_numelements(EventTriggerSQLDropList);
+
+ for (i = 0; i n; i++)
{
ObjectAddress *object;
Datum values[3];
dropped_objects.3.patch.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] sql_drop Event Trigger
Dimitri Fontaine escribió:
Alvaro Herrera alvhe...@2ndquadrant.com writes:
I thought there was the idea that the list of objects to drop was to be
acquired before actually doing the deletion; so that the trigger
function could, for instance, get the name of the table being dropped.
I don't see that it works if we only provide
pg_event_trigger_dropped_objects to ddl_command_end events. Am I
missing something?
Tom and Robert have been rightfully insisting on how delicate it has
been to come up with the right behavior for performMultipleDeletions,
and that's not something we can easily reorganise.
Well, I don't necessarily suggest that. But how about something like
this in performMultipleDeletions:
/*
* Fire event triggers for all objects to be dropped
*/
if (EventTriggerSQLDropInProgress)
{
for (i = 0; i targetObjects-numrefs; i++)
{
ObjectAddress *thisobj;
thisobj = targetObjects-refs + i;
if (EventTriggerSQLDropInProgress
EventTriggerSupportsObjectType(getObjectClass(thisobj)))
{
add_exact_object_address(thisobj,
EventTriggerSQLDropList);
}
}
/* invoke sql_drop triggers */
EventTriggerSQLDrop();
/* EventTriggerSQLDropList remains set for ddl_command_end
triggers */
}
/* and delete them */
for (i = 0; i targetObjects-numrefs; i++)
{
ObjectAddress *thisobj;
thisobj = targetObjects-refs + i;
deleteOneObject(thisobj, depRel, flags);
}
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
--
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] function for setting/getting same timestamp during whole transaction
probably you can use a little bit cheaper session variables I rejected session variables, because they don't get cleared at the end of transaction if somebody set value on session level. So I can't decide if new transaction started. this is good (variable is cleared at the end of transaction): begin; set local test.value to 123; show test.value; test.value 123 commit; show test.value; --cleared = transaction ended test.haha --- but this is bad: begin; set local test.value to 123; show test.value; test.value 123 set test.value to 456; commit; show test.value; --not cleared test.haha --- 456 test to system tables is slower then trapping error - just try to read from tmp and when a read fails, then create table Ok I will try reading from temp table directly with error trapping and compare times. probably C trigger can be very effective, possible to use this technique - http://postgres.cz/wiki/Funkce_rownum%28%29 (sorry, it is in Czech language) I'm from Slovakia so I don't have problem with czech language, but I'm not sure how to do it in C function without using temp table, because I need to clear variable at the end/start of transaction. Any hints? Miro
Re: [HACKERS] function for setting/getting same timestamp during whole transaction
On 02/06/2013 06:19 PM, Miroslav Šimulčík wrote:
Hi all,
I have deferred constraint update trigger in which I need to set same
timestamp to all modified rows. The time needs to be the time of first
invocation of this trigger fuction in transaciton. My intention is to
set commit time to rows modified in transaction.
So I need function that will store and return given timestamp on first
call in transaction and on subsequent calls will return stored
timestamp. This function have to be as fast as possible to minimize
the inpact on performance of trigger.
I have created a plpgsql function that uses temporal table for this
task. On first invocation in transaction row with timestamp is
inserted and on commit deleted. What I don't like is overhead with
checks on table existence on each invocation.
As fast as possible and PL/PgSQL function don't go that well
together. PL/PgSQL is well and good for a great many jobs, but I doubt
this is one of them.
If you're willing to spend the time to do it, consider writing a simple
C extension function to do this job. It'll be a heck of a lot faster,
though you'd need to be pretty careful about handing subtransactions.
Alternately, you might be able to use a custom GUC from a rather smaller
PL/PgSQL function. At transaction start, issue:
set_config('myapp.trigger_time', '', 't');
to define the var and make sure that subsequent current_setting() calls
will not report an error. Then in your trigger, check the value and set
it if it's empty:
current_setting('myapp.trigger_time')
followed by a:
set_config('myapp.trigger_time',clock_timestamp::text,'t')
if it's empty. I haven't tested this approach. You could avoid the need
for the initial set_config by using a BEGIN ... EXCEPTION block to trap
the error, but this uses subtransactions and would affect performance
quite significantly.
http://www.postgresql.org/docs/current/static/functions-admin.html
http://www.postgresql.org/docs/9.1/static/functions-admin.html
http://www.postgresql.org/docs/current/static/functions-datetime.html
http://www.postgresql.org/docs/8.2/static/functions-datetime.html
Custom GUCs don't seem to appear in the pg_settings view or be output by
the pg_show_all_settings() function the view is based on, so I don't
think you can use an EXISTS test on pg_settings as an alternative. Run
the set_config on transaction start, or consider implementing a C
function to do the job.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
Re: [HACKERS] function for setting/getting same timestamp during whole transaction
As fast as possible and PL/PgSQL function don't go that well together.
PL/PgSQL is well and good for a great many jobs, but I doubt this is one of
them.
Yes, I know. It was just example to demostrate functionality I need.
If you're willing to spend the time to do it, consider writing a simple C
extension function to do this job. It'll be a heck of a lot faster, though
you'd need to be pretty careful about handing subtransactions.
I don't know much about writing C extensions. Are there any good resources
explaining this topic in deep? I also need some tips on how to ensure that
variable will be cleared at the start/end of transaction.
Alternately, you might be able to use a custom GUC from a rather smaller
PL/PgSQL function. At transaction start, issue:
set_config('myapp.trigger_time', '', 't');
This is problem with using custom GUC - clearing variable at transaction
start. Without clearing it's not sufficient solution (see my response to
Pavel's mail). I don't want to do clearing from application and as far as i
know there is not transaction start trigger.
to define the var and make sure that subsequent current_setting() calls
will not report an error. Then in your trigger, check the value and set it
if it's empty:
current_setting('myapp.trigger_time')
followed by a:
set_config('myapp.trigger_time',clock_timestamp::text,'t')
if it's empty. I haven't tested this approach. You could avoid the need
for the initial set_config by using a BEGIN ... EXCEPTION block to trap the
error, but this uses subtransactions and would affect performance quite
significantly.
http://www.postgresql.org/docs/current/static/functions-admin.htmlhttp://www.postgresql.org/docs/9.1/static/functions-admin.html
http://www.postgresql.org/docs/current/static/functions-datetime.htmlhttp://www.postgresql.org/docs/8.2/static/functions-datetime.html
Custom GUCs don't seem to appear in the pg_settings view or be output by
the pg_show_all_settings() function the view is based on, so I don't think
you can use an EXISTS test on pg_settings as an alternative. Run the
set_config on transaction start, or consider implementing a C function to
do the job.
Thanks for advices. Maybe with some help I will be able to write C function
that can handle my problem.
Miro
Re: [HACKERS] sql_drop Event Trigger
Alvaro Herrera alvhe...@2ndquadrant.com writes: Well, I don't necessarily suggest that. But how about something like this in performMultipleDeletions: [edited snippet of code] /* invoke sql_drop triggers */ EventTriggerSQLDrop(); /* EventTriggerSQLDropList remains set for ddl_command_end triggers */ } /* and delete them */ for (i = 0; i targetObjects-numrefs; i++) ... deleteOneObject(thisobj, depRel, flags); My understanding of Tom and Robert comments is that it is very unsafe to run random user code at this point, so that can not be an Event Trigger call point. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] function for setting/getting same timestamp during whole transaction
2013/2/6 Miroslav Šimulčík simulcik.m...@gmail.com:
As fast as possible and PL/PgSQL function don't go that well together.
PL/PgSQL is well and good for a great many jobs, but I doubt this is one of
them.
Yes, I know. It was just example to demostrate functionality I need.
If you're willing to spend the time to do it, consider writing a simple C
extension function to do this job. It'll be a heck of a lot faster, though
you'd need to be pretty careful about handing subtransactions.
I don't know much about writing C extensions. Are there any good resources
explaining this topic in deep? I also need some tips on how to ensure that
variable will be cleared at the start/end of transaction.
Alternately, you might be able to use a custom GUC from a rather smaller
PL/PgSQL function. At transaction start, issue:
set_config('myapp.trigger_time', '', 't');
This is problem with using custom GUC - clearing variable at transaction
start. Without clearing it's not sufficient solution (see my response to
Pavel's mail). I don't want to do clearing from application and as far as i
know there is not transaction start trigger.
probably you cannot initialize variable on start transaction, but you
can add some callback function on
google, postgresql src: RegisterXactCallback
http://grokbase.com/t/postgresql/pgsql-hackers/055a7qgery/adding-callback-support
and some basic introduction to C PostgreSQL development
http://postgres.cz/wiki/C_a_PostgreSQL_-_intern%C3%AD_mechanismy
Regards
Pavel
to define the var and make sure that subsequent current_setting() calls
will not report an error. Then in your trigger, check the value and set it
if it's empty:
current_setting('myapp.trigger_time')
followed by a:
set_config('myapp.trigger_time',clock_timestamp::text,'t')
if it's empty. I haven't tested this approach. You could avoid the need
for the initial set_config by using a BEGIN ... EXCEPTION block to trap the
error, but this uses subtransactions and would affect performance quite
significantly.
http://www.postgresql.org/docs/current/static/functions-admin.html
http://www.postgresql.org/docs/current/static/functions-datetime.html
Custom GUCs don't seem to appear in the pg_settings view or be output by
the pg_show_all_settings() function the view is based on, so I don't think
you can use an EXISTS test on pg_settings as an alternative. Run the
set_config on transaction start, or consider implementing a C function to do
the job.
Thanks for advices. Maybe with some help I will be able to write C function
that can handle my problem.
Miro
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Facing authentication error on postgres 9.2 - dblink functions
Hello Everyone,
I am using postgres 9.2 and when executing function dblink facing a fatal
error while trying to execute dblink_connect as follows:
* SELECT * FROM dblink_connect('host=127.0.0.1 port=5432 dbname=postgres
password=test')*
*ERROR*: could not establish connection DETAIL: FATAL: password
authentication failed for user NETWORK SERVICE
What this error is related to? Do I need to modify pg_hba.conf file by any
chance?
Thanks..
Re: [HACKERS] sql_drop Event Trigger
Dimitri Fontaine escribió: Alvaro Herrera alvhe...@2ndquadrant.com writes: Well, I don't necessarily suggest that. But how about something like this in performMultipleDeletions: [edited snippet of code] /* invoke sql_drop triggers */ EventTriggerSQLDrop(); /* EventTriggerSQLDropList remains set for ddl_command_end triggers */ } /* and delete them */ for (i = 0; i targetObjects-numrefs; i++) ... deleteOneObject(thisobj, depRel, flags); My understanding of Tom and Robert comments is that it is very unsafe to run random user code at this point, so that can not be an Event Trigger call point. Hmm, quoth http://www.postgresql.org/message-id/23345.1358476...@sss.pgh.pa.us : I'd really like to get to a point where we can define things as happening like this: * collect information needed to interpret the DDL command (lookup and lock objects, etc) * fire before event triggers, if any (and if so, recheck info) * do the command * fire after event triggers, if any Note that in the snippet I posted above objects have already been looked up and locked (first phase above). One thing worth considering, of course, is the if so, recheck info parenthical remark; for example, what happens if the called function decides to, say, add a column to every dropped table? Or, worse, what happens if the event trigger function adds a column to table foo_audit when table foo is dropped, and you happen to drop both in the same command. Also, what if the function decides to drop table foo_audit when table foo is dropped? I think these things are all worth adding to a regress test for this feature, to ensure that behavior is sane, and also to verify that system catalogs after this remains consistent (for example we don't leave dangling pg_attribute rows, etc). Maybe the answer to all this is to run the lookup algorithm all over again and ensure that the two lists are equal, and throw an error otherwise, i.e. all scenarios above should be considered unsupported. That seems safest. But I don't think code structure convenience is the only reason to do things this way instead of postponing firing the trigger until the end. I think complete support for drop event triggers really needs to have the objects to be dropped still in catalogs, so that they can be looked up; for instance, user code might want to check the names of the columns and take particular actions if particular names or particular types are present. That doesn't seem an easy thing to do if all you get is pg_dropped_objects(), because how do you know which columns belong to which tables? -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [ADMIN] Facing authentication error on postgres 9.2 - dblink functions
Dev Kumkar wrote:
I am using postgres 9.2 and when executing function dblink facing a fatal
error while trying to
execute dblink_connect as follows:
SELECT * FROM dblink_connect('host=127.0.0.1 port=5432 dbname=postgres
password=test')
ERROR: could not establish connection DETAIL: FATAL: password
authentication failed for user
NETWORK SERVICE
What this error is related to? Do I need to modify pg_hba.conf file by any
chance?
You should specify a user=username option in the connection string,
otherwise that defaults to your operating system user name.
Yours,
Laurenz Albe
--
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] Facing authentication error on postgres 9.2 - dblink functions
On 02/06/2013 08:09 AM, Dev Kumkar wrote:
Hello Everyone,
I am using postgres 9.2 and when executing function dblink facing a
fatal error while trying to execute dblink_connect as follows:
/SELECT * FROM dblink_connect('host=127.0.0.1 port=5432
dbname=postgres password=test')/
*ERROR*: could not establish connection DETAIL: FATAL: password
authentication failed for user NETWORK SERVICE
What this error is related to? Do I need to modify pg_hba.conf file by
any chance?
Thanks..
Do NOT send questions to multiple lists. That is a waste of everybody's
time. So do NOT follow up this email. This question belongs on
pgsql-general. If you have further questions pleease ask there.
The short answer is that you need to provide the user name in your
connect string.
cheers
andrew
--
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] sql_drop Event Trigger
Alvaro Herrera alvhe...@2ndquadrant.com writes: Hmm, quoth http://www.postgresql.org/message-id/23345.1358476...@sss.pgh.pa.us : I'd really like to get to a point where we can define things as happening like this: * collect information needed to interpret the DDL command (lookup and lock objects, etc) * fire before event triggers, if any (and if so, recheck info) * do the command * fire after event triggers, if any Note that in the snippet I posted above objects have already been looked up and locked (first phase above). Ok, I like what you did, but what you did is reinstall the sql_drop event and is not complete, as you mention we have some hard problems to solve there. But I don't think code structure convenience is the only reason to do things this way instead of postponing firing the trigger until the end. I think complete support for drop event triggers really needs to have the objects to be dropped still in catalogs, so that they can be looked up; for instance, user code might want to check the names of the columns and take particular actions if particular names or particular types are present. That doesn't seem an easy thing to do if all you get is pg_dropped_objects(), because how do you know which columns belong to which tables? Agreed. In terms of Robert's reviewing, though, I think you're talking about another patch entirely, that will get worked on in the 9.4 cycle. And in my terms, doing all that work now is useless anyway because we are not exposing any object specific information that allow the user to do any actual catalog lookup anyway, yet. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] sql_drop Event Trigger
On Tue, Feb 5, 2013 at 10:42 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: A larger issue with the patch is handling of subxacts. A quick test doesn't reveal any obvious misbehavior, but having the list of objects dropped by a global variable might be problematic. What if, say, the event trigger function does something funny in an EXCEPTION block and it fails? Some clever test case is needed here, I think. Also, if we reset the variable at EOXact, do we also need to do something at EOSubXact? This is an awfully good point, although I think the issue has to do with command boundaries more than subtransactions. Suppose you create two ddl_command_end event triggers, A and B. A contains a DROP IF EXISTS command. Someone runs a toplevel DROP command. Now, A is going to fire first, and that's going to recursively invoke A (which will do nothing the second time) and then B; on return from B, we'll finish running the event triggers for the toplevel command, executing B again. If the list of dropped objects is stored in a global variable, it seems like there are a number of ways this can go wrong. I have not tested the actual behavior of the latest patch, but I think we want to define things so that the pg_event_trigger_dropped_objects() function returns, specifically, the list of objects dropped by the command which caused the event trigger to fire. In other words, in the above example, the first, recursive invocation of B should see the object removed by A's DROP-IF-EXISTS, and the second invocation should see the object removed by the toplevel command. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] palloc unification
On Mon, Feb 4, 2013 at 5:50 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: There was some discussion about unifying backend and frontend code/headers for palloc et al, particularly so that programs that want to mix both can be easily compiled; see http://www.postgresql.org/message-id/20130118150629.gc29...@alap2.anarazel.de for what I believe to be the latest and greatest patch in that area. The good news from that patch is that there is a reported (small) performance increase from the approach suggested there, on Intel and non-Intel platforms. So it seems we're okay on that front. On the source code organization front, however, I'm not too happy with that particular proposal. There would be two files named palloc.h, for one thing, and also the changes to utils/palloc.h leave way too much of the backend-only implementation exposed to the frontend world. I propose to have a new subdirectory src/include/shared, and two header files: src/include/utils/palloc.h (same as today) src/include/shared/fe_memutils.h (pg_malloc, pg_free, pg_strdup decls) utils/palloc.h would be modified so that #ifdef FRONTEND, only the basic function declarations (palloc, pfree, pstrdup, pnstrdup) are exposed; frontend environment would be safe from CurrentMemoryContext etc. The frontend (pg_malloc) function definitions would live somewhere in, say, src/shared/fe_memutils.c. For the palloc() implementation, I think we should create another file, so that frontend-only programs that do not require those symbols (most of them) are not unnecessarily bloated. Opinions on having a new subdir? We could eventually move some stuff from timestamp.c in there, so that pg_xlogdump could get timestamp_to_str from there; we could perhaps remove the ecpg implementation of that as well and make it use this one. I like the idea of having a place for shared frontend and backend code very much, but I don't think src/include/shared or src/shared is a good name, because shared can mean a lot of things - like shared library, for example. I think that this should be set up in a manner analogous to libpgport, except not for portability code, but instead for other stuff. Maybe we could call it libpgframework or something. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] sql_drop Event Trigger
Robert Haas robertmh...@gmail.com writes: variable, it seems like there are a number of ways this can go wrong. Yeah, I think the current behavior might be surprising. I have not tested the actual behavior of the latest patch, but I think we want to define things so that the pg_event_trigger_dropped_objects() function returns, specifically, the list of objects dropped by the command which caused the event trigger to fire. In other words, in the above example, the first, recursive invocation of B should see the object removed by A's DROP-IF-EXISTS, and the second invocation should see the object removed by the toplevel command. I disagree with that. I don't see why the enclosing event trigger shouldn't be aware of all the objects dropped by the command that just ran to completion, *including* the effects of any event trigger fired recursively or not. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] palloc unification
Robert Haas escribió: On Mon, Feb 4, 2013 at 5:50 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: I propose to have a new subdirectory src/include/shared, and two header files: The frontend (pg_malloc) function definitions would live somewhere in, say, src/shared/fe_memutils.c. For the palloc() implementation, I think we should create another file, so that frontend-only programs that do not require those symbols (most of them) are not unnecessarily bloated. Opinions on having a new subdir? I like the idea of having a place for shared frontend and backend code very much, but I don't think src/include/shared or src/shared is a good name, because shared can mean a lot of things - like shared library, for example. I think that this should be set up in a manner analogous to libpgport, except not for portability code, but instead for other stuff. Maybe we could call it libpgframework or something. Yeah, I am doing this right now and the shared name doesn't seem so good. libpgframework sounds decent. So since libpgport comes from src/port, are we okay with src/framework and src/include/framework? -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] sql_drop Event Trigger
On Wed, Feb 6, 2013 at 9:36 AM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Robert Haas robertmh...@gmail.com writes: variable, it seems like there are a number of ways this can go wrong. Yeah, I think the current behavior might be surprising. I have not tested the actual behavior of the latest patch, but I think we want to define things so that the pg_event_trigger_dropped_objects() function returns, specifically, the list of objects dropped by the command which caused the event trigger to fire. In other words, in the above example, the first, recursive invocation of B should see the object removed by A's DROP-IF-EXISTS, and the second invocation should see the object removed by the toplevel command. I disagree with that. I don't see why the enclosing event trigger shouldn't be aware of all the objects dropped by the command that just ran to completion, *including* the effects of any event trigger fired recursively or not. Well, that could result in some DROP events being reported more than once, which I assume would be undesirable for someone hoping to use this for replication. (Eventually, we'll have to face the same problem for CREATE events, too.) -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] palloc unification
On Wed, Feb 6, 2013 at 9:38 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Robert Haas escribió: On Mon, Feb 4, 2013 at 5:50 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: I propose to have a new subdirectory src/include/shared, and two header files: The frontend (pg_malloc) function definitions would live somewhere in, say, src/shared/fe_memutils.c. For the palloc() implementation, I think we should create another file, so that frontend-only programs that do not require those symbols (most of them) are not unnecessarily bloated. Opinions on having a new subdir? I like the idea of having a place for shared frontend and backend code very much, but I don't think src/include/shared or src/shared is a good name, because shared can mean a lot of things - like shared library, for example. I think that this should be set up in a manner analogous to libpgport, except not for portability code, but instead for other stuff. Maybe we could call it libpgframework or something. Yeah, I am doing this right now and the shared name doesn't seem so good. libpgframework sounds decent. So since libpgport comes from src/port, are we okay with src/framework and src/include/framework? That seems reasonable to me. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] sql_drop Event Trigger
Dimitri Fontaine escribió: Robert Haas robertmh...@gmail.com writes: I have not tested the actual behavior of the latest patch, but I think we want to define things so that the pg_event_trigger_dropped_objects() function returns, specifically, the list of objects dropped by the command which caused the event trigger to fire. In other words, in the above example, the first, recursive invocation of B should see the object removed by A's DROP-IF-EXISTS, and the second invocation should see the object removed by the toplevel command. I disagree with that. I don't see why the enclosing event trigger shouldn't be aware of all the objects dropped by the command that just ran to completion, *including* the effects of any event trigger fired recursively or not. Not sure about that. If the trigger records objects dropped in a table, aren't they going to show up there twice if you do that? -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] get_progname() should not be const char *?
On Tue, Feb 5, 2013 at 12:18 AM, Phil Sorber p...@omniti.com wrote: On Mon, Feb 4, 2013 at 10:52 PM, Tom Lane t...@sss.pgh.pa.us wrote: Phil Sorber p...@omniti.com writes: get_progname() returns a strdup()'d value. Shouldn't it then be simply char * and not const char *? Otherwise free() complains loudly without a cast. I don't believe that callers should be trying to free() the result. Whether it's been strdup'd or not is not any of their business. Is that just because of the nature of this specific function? I can't presume to speak for Tom, but I think so. Sometimes the API of a function includes the notion that the caller should pfree the result. Sometimes it doesn't. The advantage of NOT including that in the API contract is that you can sometimes do optimizations that would be impossible otherwise - e.g. you can return the same palloc'd string on successive calls to the function; or you can sometimes return a statically allocated string. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] sql_drop Event Trigger
Robert Haas robertmh...@gmail.com writes: I disagree with that. I don't see why the enclosing event trigger shouldn't be aware of all the objects dropped by the command that just ran to completion, *including* the effects of any event trigger fired recursively or not. Well, that could result in some DROP events being reported more than once, which I assume would be undesirable for someone hoping to use this for replication. Any command might have an event trigger attached doing a DROP, so that you don't know where to expect it, and it's well possible that in your example both the event triggers have been installed by different tools. -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] palloc unification
On 6 February 2013 14:38, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Robert Haas escribió: On Mon, Feb 4, 2013 at 5:50 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: I propose to have a new subdirectory src/include/shared, and two header files: The frontend (pg_malloc) function definitions would live somewhere in, say, src/shared/fe_memutils.c. For the palloc() implementation, I think we should create another file, so that frontend-only programs that do not require those symbols (most of them) are not unnecessarily bloated. Opinions on having a new subdir? I like the idea of having a place for shared frontend and backend code very much, but I don't think src/include/shared or src/shared is a good name, because shared can mean a lot of things - like shared library, for example. I think that this should be set up in a manner analogous to libpgport, except not for portability code, but instead for other stuff. Maybe we could call it libpgframework or something. Yeah, I am doing this right now and the shared name doesn't seem so good. libpgframework sounds decent. So since libpgport comes from src/port, are we okay with src/framework and src/include/framework? common ? src/backend/common src/include/common -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] palloc unification
On Wed, Feb 6, 2013 at 9:59 AM, Simon Riggs si...@2ndquadrant.com wrote: On 6 February 2013 14:38, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Robert Haas escribió: On Mon, Feb 4, 2013 at 5:50 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: I propose to have a new subdirectory src/include/shared, and two header files: The frontend (pg_malloc) function definitions would live somewhere in, say, src/shared/fe_memutils.c. For the palloc() implementation, I think we should create another file, so that frontend-only programs that do not require those symbols (most of them) are not unnecessarily bloated. Opinions on having a new subdir? I like the idea of having a place for shared frontend and backend code very much, but I don't think src/include/shared or src/shared is a good name, because shared can mean a lot of things - like shared library, for example. I think that this should be set up in a manner analogous to libpgport, except not for portability code, but instead for other stuff. Maybe we could call it libpgframework or something. Yeah, I am doing this right now and the shared name doesn't seem so good. libpgframework sounds decent. So since libpgport comes from src/port, are we okay with src/framework and src/include/framework? common ? src/backend/common src/include/common +1 -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- 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] pg_isready (was: [WIP] pg_ping utility)
On Tue, Feb 5, 2013 at 12:44 PM, Phil Sorber p...@omniti.com wrote: On Tue, Feb 5, 2013 at 9:08 AM, Phil Sorber p...@omniti.com wrote: On Tue, Feb 5, 2013 at 9:06 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Phil Sorber escribió: On Tue, Feb 5, 2013 at 6:41 AM, Robert Haas robertmh...@gmail.com wrote: On Sat, Feb 2, 2013 at 9:55 PM, Phil Sorber p...@omniti.com wrote: OK, here is the patch that handles the connection string in dbname. I'll post the other patch under a different posting because I am sure it will get plenty of debate on it's own. I'm sorry, can you remind me what this does for us vs. the existing coding? It's supposed to handle the connection string passed as dbname case to be able to get the right output for host:port. Surely the idea is that you can also give it a postgres:// URI, right? Absolutely. Here is it. I like this approach more than the previous one, but I'd like some feedback. Minor adjustment. There still seems to be a bit of a disconnect in libpq in my opinion. Taking options as a string (URI or conninfo) or a set of arrays, but returning info about connection parameters in PQconninfoOption? And nothing that takes that as an input. Seems odd to me. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services pg_isready_con_str_v3.diff 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: PATCH: Split stats file per database WAS: [HACKERS] autovacuum stress-testing our system
Pavel Stehule pavel.steh...@gmail.com writes: Nice. Another interesting numbers would be device utilization, average I/O speed and required space (which should be ~2x the pgstat.stat size without the patch). this point is important - with large warehouse with lot of databases and tables you have move stat file to some ramdisc - without it you lost lot of IO capacity - and it is very important if you need only half sized ramdisc [ blink... ] I confess I'd not been paying close attention to this thread, but if that's true I'd say the patch is DOA. Why should we accept 2x bloat in the already-far-too-large stats file? I thought the idea was just to split up the existing data into multiple files. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: PATCH: Split stats file per database WAS: [HACKERS] autovacuum stress-testing our system
Tom Lane escribió: Pavel Stehule pavel.steh...@gmail.com writes: Nice. Another interesting numbers would be device utilization, average I/O speed and required space (which should be ~2x the pgstat.stat size without the patch). this point is important - with large warehouse with lot of databases and tables you have move stat file to some ramdisc - without it you lost lot of IO capacity - and it is very important if you need only half sized ramdisc [ blink... ] I confess I'd not been paying close attention to this thread, but if that's true I'd say the patch is DOA. Why should we accept 2x bloat in the already-far-too-large stats file? I thought the idea was just to split up the existing data into multiple files. I think they are saying just the opposite: maximum disk space utilization is now half of the unpatched code. This is because when we need to write the temporary file to rename on top of the other one, the temporary file is not of the size of the complete pgstat data collation, but just that for the requested database. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: PATCH: Split stats file per database WAS: [HACKERS] autovacuum stress-testing our system
2013/2/6 Alvaro Herrera alvhe...@2ndquadrant.com: Tom Lane escribió: Pavel Stehule pavel.steh...@gmail.com writes: Nice. Another interesting numbers would be device utilization, average I/O speed and required space (which should be ~2x the pgstat.stat size without the patch). this point is important - with large warehouse with lot of databases and tables you have move stat file to some ramdisc - without it you lost lot of IO capacity - and it is very important if you need only half sized ramdisc [ blink... ] I confess I'd not been paying close attention to this thread, but if that's true I'd say the patch is DOA. Why should we accept 2x bloat in the already-far-too-large stats file? I thought the idea was just to split up the existing data into multiple files. I think they are saying just the opposite: maximum disk space utilization is now half of the unpatched code. This is because when we need to write the temporary file to rename on top of the other one, the temporary file is not of the size of the complete pgstat data collation, but just that for the requested database. +1 Pavel -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] sql_drop Event Trigger
Dimitri Fontaine dimi...@2ndquadrant.fr writes: Alvaro Herrera alvhe...@2ndquadrant.com writes: I thought there was the idea that the list of objects to drop was to be acquired before actually doing the deletion; so that the trigger function could, for instance, get the name of the table being dropped. Tom and Robert have been rightfully insisting on how delicate it has been to come up with the right behavior for performMultipleDeletions, and that's not something we can easily reorganise. So the only way to get at the information seems to be what Robert insisted that I do, that is storing the information about the objects being dropped for later processing. I might be forgetting something, but doesn't dependency.c work by first constructing a list of all the objects it's going to drop, and only then dropping them? Could we inject a pre deletion event trigger call at the point where the list is completed? regards, tom lane -- 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] pg_isready (was: [WIP] pg_ping utility)
On Thu, Feb 7, 2013 at 12:05 AM, Phil Sorber p...@omniti.com wrote: On Tue, Feb 5, 2013 at 12:44 PM, Phil Sorber p...@omniti.com wrote: On Tue, Feb 5, 2013 at 9:08 AM, Phil Sorber p...@omniti.com wrote: On Tue, Feb 5, 2013 at 9:06 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Phil Sorber escribió: On Tue, Feb 5, 2013 at 6:41 AM, Robert Haas robertmh...@gmail.com wrote: On Sat, Feb 2, 2013 at 9:55 PM, Phil Sorber p...@omniti.com wrote: OK, here is the patch that handles the connection string in dbname. I'll post the other patch under a different posting because I am sure it will get plenty of debate on it's own. I'm sorry, can you remind me what this does for us vs. the existing coding? It's supposed to handle the connection string passed as dbname case to be able to get the right output for host:port. Surely the idea is that you can also give it a postgres:// URI, right? Absolutely. Here is it. I like this approach more than the previous one, but I'd like some feedback. The patch looks complicated to me. I was thinking that we can address the problem just by using PQconninfoParse() and PQconndefaults() like uri-regress.c does. The patch should be very simple. Why do we need so complicated code? Regards, -- Fujii Masao -- 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] pg_isready (was: [WIP] pg_ping utility)
On Wed, Feb 6, 2013 at 11:11 AM, Fujii Masao masao.fu...@gmail.com wrote: On Thu, Feb 7, 2013 at 12:05 AM, Phil Sorber p...@omniti.com wrote: On Tue, Feb 5, 2013 at 12:44 PM, Phil Sorber p...@omniti.com wrote: On Tue, Feb 5, 2013 at 9:08 AM, Phil Sorber p...@omniti.com wrote: On Tue, Feb 5, 2013 at 9:06 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Phil Sorber escribió: On Tue, Feb 5, 2013 at 6:41 AM, Robert Haas robertmh...@gmail.com wrote: On Sat, Feb 2, 2013 at 9:55 PM, Phil Sorber p...@omniti.com wrote: OK, here is the patch that handles the connection string in dbname. I'll post the other patch under a different posting because I am sure it will get plenty of debate on it's own. I'm sorry, can you remind me what this does for us vs. the existing coding? It's supposed to handle the connection string passed as dbname case to be able to get the right output for host:port. Surely the idea is that you can also give it a postgres:// URI, right? Absolutely. Here is it. I like this approach more than the previous one, but I'd like some feedback. The patch looks complicated to me. I was thinking that we can address the problem just by using PQconninfoParse() and PQconndefaults() like uri-regress.c does. The patch should be very simple. Why do we need so complicated code? Did you like the previous version better? http://www.postgresql.org/message-id/cadakt-hnb3ohcpkr+pcg1c_bjrsb7j__bpv+-jrjs5opjr2...@mail.gmail.com Regards, -- Fujii Masao -- 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] sql_drop Event Trigger
Tom Lane t...@sss.pgh.pa.us writes: I might be forgetting something, but doesn't dependency.c work by first constructing a list of all the objects it's going to drop, and only then dropping them? Could we inject a pre deletion event trigger call at the point where the list is completed? What happens if the event trigger itself deletes objects? From the list? Then we have to redo all the preparatory steps, and I don't think we agreed on a way to do it. Plus, as we seem to be chasing minimal set of features per patch, I would think that getting the list of Objects OIDs that are already dropped is a well enough defined set of minimal feature for this very patch, that we will be in a position to extend later given some time. I still think we should think about the set of information we're going to be publishing first, because without publishing some more all we're doing here is moot anyway. Also, for most cases that I can think of, it's not a problem for the dropped object to not exist anymore in the catalogs by the time you get the information, if you get the object's name and schema and maybe some other properties. -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] palloc unification
Simon Riggs si...@2ndquadrant.com writes: On 6 February 2013 14:38, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Yeah, I am doing this right now and the shared name doesn't seem so good. libpgframework sounds decent. So since libpgport comes from src/port, are we okay with src/framework and src/include/framework? common ? src/backend/common src/include/common To me the term framework carries a lot of baggage that doesn't fit this usage. common seems better. regards, tom lane -- 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] sql_drop Event Trigger
Dimitri Fontaine escribió: Tom Lane t...@sss.pgh.pa.us writes: I might be forgetting something, but doesn't dependency.c work by first constructing a list of all the objects it's going to drop, and only then dropping them? Could we inject a pre deletion event trigger call at the point where the list is completed? Yes, this is what I'm proposing. What happens if the event trigger itself deletes objects? From the list? I replied to this above: raise an error. (How to do that is an open implementation question, but I proposed a simple idea upthread). -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] get_progname() should not be const char *?
Robert Haas robertmh...@gmail.com writes: On Tue, Feb 5, 2013 at 12:18 AM, Phil Sorber p...@omniti.com wrote: On Mon, Feb 4, 2013 at 10:52 PM, Tom Lane t...@sss.pgh.pa.us wrote: I don't believe that callers should be trying to free() the result. Whether it's been strdup'd or not is not any of their business. Is that just because of the nature of this specific function? I can't presume to speak for Tom, but I think so. Sometimes the API of a function includes the notion that the caller should pfree the result. Sometimes it doesn't. The advantage of NOT including that in the API contract is that you can sometimes do optimizations that would be impossible otherwise - e.g. you can return the same palloc'd string on successive calls to the function; or you can sometimes return a statically allocated string. Yeah. In this particular case, it seems rather obvious that the function should be returning the same string each time --- if it's actually doing a fresh malloc, that sounds like a bug. But in any case, adding or removing a const qualifier from a function's result typically goes along with an API-contract change as to whether the caller is supposed to free the result or not. My objection here was specifically that I don't believe the contract for get_progname includes caller-free now, and I don't want it to start being that. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: PATCH: Split stats file per database WAS: [HACKERS] autovacuum stress-testing our system
Dne 06.02.2013 16:53, Alvaro Herrera napsal: Tom Lane escribió: Pavel Stehule pavel.steh...@gmail.com writes: Nice. Another interesting numbers would be device utilization, average I/O speed and required space (which should be ~2x the pgstat.stat size without the patch). this point is important - with large warehouse with lot of databases and tables you have move stat file to some ramdisc - without it you lost lot of IO capacity - and it is very important if you need only half sized ramdisc [ blink... ] I confess I'd not been paying close attention to this thread, but if that's true I'd say the patch is DOA. Why should we accept 2x bloat in the already-far-too-large stats file? I thought the idea was just to split up the existing data into multiple files. I think they are saying just the opposite: maximum disk space utilization is now half of the unpatched code. This is because when we need to write the temporary file to rename on top of the other one, the temporary file is not of the size of the complete pgstat data collation, but just that for the requested database. Exactly. And I suspect the current (unpatched) code ofter requires more than twice the space because of open file descriptors to already deleted files. Tomas -- 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] sql_drop Event Trigger
Alvaro Herrera alvhe...@2ndquadrant.com writes: Dimitri Fontaine escribió: Tom Lane t...@sss.pgh.pa.us writes: I might be forgetting something, but doesn't dependency.c work by first constructing a list of all the objects it's going to drop, and only then dropping them? Could we inject a pre deletion event trigger call at the point where the list is completed? Yes, this is what I'm proposing. So, I add back the sql_drop event and implement it in dependency.c, and keep the list for later processing from ddl_command_end, right? Robert, you specifically opposed to sql_drop and I just removed it from the patch. What do you think now? Also, should that be a follow-up patch to the current one for your reviewing purposes? What happens if the event trigger itself deletes objects? From the list? I replied to this above: raise an error. (How to do that is an open implementation question, but I proposed a simple idea upthread). Well, can the list of objects that get dropped for CASCADE change in between releases? If it ever does, that means that your event trigger that used to work before is not broken after upgrade. Is that ok? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] sql_drop Event Trigger
Dimitri Fontaine dimi...@2ndquadrant.fr writes: Tom Lane t...@sss.pgh.pa.us writes: I might be forgetting something, but doesn't dependency.c work by first constructing a list of all the objects it's going to drop, and only then dropping them? Could we inject a pre deletion event trigger call at the point where the list is completed? What happens if the event trigger itself deletes objects? From the list? Throw an error. Per previous discussion, the trigger does not get to do anything that would affect the results of parse analysis of the command, and that list is exactly those results. Plus, as we seem to be chasing minimal set of features per patch, I would think that getting the list of Objects OIDs that are already dropped is a well enough defined set of minimal feature for this very patch, that we will be in a position to extend later given some time. Well, a list of object OIDs is of exactly zero use once the command has been carried out. So I don't think that that represents a useful or even very testable feature on its own, if there's no provision to fire user code while the OIDs are still in the catalogs. regards, tom lane -- 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] get_progname() should not be const char *?
On Wed, Feb 6, 2013 at 11:22 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Tue, Feb 5, 2013 at 12:18 AM, Phil Sorber p...@omniti.com wrote: On Mon, Feb 4, 2013 at 10:52 PM, Tom Lane t...@sss.pgh.pa.us wrote: I don't believe that callers should be trying to free() the result. Whether it's been strdup'd or not is not any of their business. Is that just because of the nature of this specific function? I can't presume to speak for Tom, but I think so. Sometimes the API of a function includes the notion that the caller should pfree the result. Sometimes it doesn't. The advantage of NOT including that in the API contract is that you can sometimes do optimizations that would be impossible otherwise - e.g. you can return the same palloc'd string on successive calls to the function; or you can sometimes return a statically allocated string. Yeah. In this particular case, it seems rather obvious that the function should be returning the same string each time --- if it's actually doing a fresh malloc, that sounds like a bug. It does, but it's noted in a comment that it's only expected to be run once. But in any case, adding or removing a const qualifier from a function's result typically goes along with an API-contract change as to whether the caller is supposed to free the result or not. My objection here was specifically that I don't believe the contract for get_progname includes caller-free now, and I don't want it to start being that. That's fair. Thanks for the explanation. regards, tom lane -- 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] sql_drop Event Trigger
Tom Lane t...@sss.pgh.pa.us writes: Well, a list of object OIDs is of exactly zero use once the command has been carried out. So I don't think that that represents a useful or even very testable feature on its own, if there's no provision to fire user code while the OIDs are still in the catalogs. For the same reason I want to talk about which information we publish. I can see why having access to the catalogs is a more general answer here though. Now, I've just been asked to remove sql_drop and I don't want to be adding it again before I know that whoever will commit the patch agrees with an explicit return of that feature from the dead. Please… -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] pg_isready (was: [WIP] pg_ping utility)
On Thu, Feb 7, 2013 at 1:15 AM, Phil Sorber p...@omniti.com wrote: On Wed, Feb 6, 2013 at 11:11 AM, Fujii Masao masao.fu...@gmail.com wrote: On Thu, Feb 7, 2013 at 12:05 AM, Phil Sorber p...@omniti.com wrote: On Tue, Feb 5, 2013 at 12:44 PM, Phil Sorber p...@omniti.com wrote: On Tue, Feb 5, 2013 at 9:08 AM, Phil Sorber p...@omniti.com wrote: On Tue, Feb 5, 2013 at 9:06 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Phil Sorber escribió: On Tue, Feb 5, 2013 at 6:41 AM, Robert Haas robertmh...@gmail.com wrote: On Sat, Feb 2, 2013 at 9:55 PM, Phil Sorber p...@omniti.com wrote: OK, here is the patch that handles the connection string in dbname. I'll post the other patch under a different posting because I am sure it will get plenty of debate on it's own. I'm sorry, can you remind me what this does for us vs. the existing coding? It's supposed to handle the connection string passed as dbname case to be able to get the right output for host:port. Surely the idea is that you can also give it a postgres:// URI, right? Absolutely. Here is it. I like this approach more than the previous one, but I'd like some feedback. The patch looks complicated to me. I was thinking that we can address the problem just by using PQconninfoParse() and PQconndefaults() like uri-regress.c does. The patch should be very simple. Why do we need so complicated code? Did you like the previous version better? http://www.postgresql.org/message-id/cadakt-hnb3ohcpkr+pcg1c_bjrsb7j__bpv+-jrjs5opjr2...@mail.gmail.com Yes because that version is simpler. But which version is better depends on the reason why you implemented new version. If you have some idea about the merit and demerit of each version, could you elaborate them? + set_connect_options(connect_options, pgdbname, pghost, pgport, connect_timeout, pguser); This code prevents us from giving options other than the above, for example application_name, in the conninfo. I think that pg_isready should accept all the libpq options. When more than one -d options are specified, psql always prefer the last one and ignore the others. OTOH, pg_isready with this patch seems to merge them. I'm not sure if there is specific rule about the priority order of -d option. But it seems better to follow the existing way, i.e., always prefer the last -d option. Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [COMMITTERS] pgsql: Fast promote mode skips checkpoint at end of recovery.
On 31.01.2013 21:33, Simon Riggs wrote: If anyone really wants me to revert, pls start new hackers thread to discuss, or comment on changes. Yes, I still think this needs fixing or reverting. Let me reiterate my my complaints: 1. I don't like the check in ReadCheckPointRecord() that the WAL containing last and previous checkpoint still exists. Several reasons for that: 1.1. I don't think such a check is necessary to begin with. We replayed that WAL record a while ago, so there's no reason to believe that it's gone now. If there is a bug that causes that to happen, you're screwed with or without this patch. 1.2. If we do that check, and it fails because the latest checkpoint is not present, there should be a big fat warning in the log because something's wrong. If you ask for fast promotion, and the system doesn't do that, a log message is the least we can do. 1.3. Why check for the prev checkpoint? The latest checkpoint is enough to recover, so why insist that also the previous one is present, too? There are normal scenarios where it won't be, like just after recovering from a base backup. I consider it a bug that fast promotion doesn't work right after restoring from a base backup. 2. I don't like demoting the trigger file method to a second class citizen. I think we should make all functionality available through both methods. If there was a good reason for deprecating the trigger file method, I could live with that, but this patch is not such a reason. 3. I don't like conflating the promotion modes and shutdown modes in the pg_ctl option. Shutdown modes and promotion modes are separate concepts. The fast option is pretty clear, but why does smart mean create an immediate checkpoint before promotion? How is that smarter than the fast mode? The pg_ctl --help on that is a bit confusing too: Options for stop, restart or promote: -m, --mode=MODEMODE can be smart, fast, or immediate The immediate mode is not actually valid for pg_ctl promote. That is clarified later in the output by listing out what the modes mean, but that above line is misleading, 4. I think fast promotion should be the default. Why not? There are cases where you want the promotion to happen ASAP, and there are cases where you don't care. But there are no scenarios where you want promotion to be slow, 5. Docs changes are missing. Here's what I think should be done: 1. Remove the check that prev checkpoint record exists. 2. Always do fast promotion if in standby mode. Remove the pg_ctl option. 3. Improve docs. - Heikki -- 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] Alias hstore's ? to ~ so that it works with JDBC
On Tue, Feb 5, 2013 at 11:29 AM, Seamus Abshere sea...@abshere.net wrote: hi, As reported in BUG #7715 [1], hstore's use of ? as an operator conflicts with JDBC's bind variables. I think we could just alias ? to ~ and tell JDBC users to use that instead. [2] This is not a bug with postgres, but with java/JDBC. There are many operators that use '?' besides hstore and JDBC should allow for escaping out of its statement interpretation. merlin -- 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] sql_drop Event Trigger
On Wed, Feb 6, 2013 at 9:44 AM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Robert Haas robertmh...@gmail.com writes: I disagree with that. I don't see why the enclosing event trigger shouldn't be aware of all the objects dropped by the command that just ran to completion, *including* the effects of any event trigger fired recursively or not. Well, that could result in some DROP events being reported more than once, which I assume would be undesirable for someone hoping to use this for replication. Any command might have an event trigger attached doing a DROP, so that you don't know where to expect it, and it's well possible that in your example both the event triggers have been installed by different tools. It certainly is; in fact, it's likely. So let's say that B is a replication trigger. Don't you want it to hear about each drop exactly once? If not, how will you avoid errors when you go to replay the events you've captured on another machine? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] pg_isready (was: [WIP] pg_ping utility)
On Wed, Feb 6, 2013 at 11:36 AM, Fujii Masao masao.fu...@gmail.com wrote: On Thu, Feb 7, 2013 at 1:15 AM, Phil Sorber p...@omniti.com wrote: On Wed, Feb 6, 2013 at 11:11 AM, Fujii Masao masao.fu...@gmail.com wrote: On Thu, Feb 7, 2013 at 12:05 AM, Phil Sorber p...@omniti.com wrote: On Tue, Feb 5, 2013 at 12:44 PM, Phil Sorber p...@omniti.com wrote: On Tue, Feb 5, 2013 at 9:08 AM, Phil Sorber p...@omniti.com wrote: On Tue, Feb 5, 2013 at 9:06 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Phil Sorber escribió: On Tue, Feb 5, 2013 at 6:41 AM, Robert Haas robertmh...@gmail.com wrote: On Sat, Feb 2, 2013 at 9:55 PM, Phil Sorber p...@omniti.com wrote: OK, here is the patch that handles the connection string in dbname. I'll post the other patch under a different posting because I am sure it will get plenty of debate on it's own. I'm sorry, can you remind me what this does for us vs. the existing coding? It's supposed to handle the connection string passed as dbname case to be able to get the right output for host:port. Surely the idea is that you can also give it a postgres:// URI, right? Absolutely. Here is it. I like this approach more than the previous one, but I'd like some feedback. The patch looks complicated to me. I was thinking that we can address the problem just by using PQconninfoParse() and PQconndefaults() like uri-regress.c does. The patch should be very simple. Why do we need so complicated code? Did you like the previous version better? http://www.postgresql.org/message-id/cadakt-hnb3ohcpkr+pcg1c_bjrsb7j__bpv+-jrjs5opjr2...@mail.gmail.com Yes because that version is simpler. But which version is better depends on the reason why you implemented new version. If you have some idea about the merit and demerit of each version, could you elaborate them? I didn't like the way that I had to hard code the options in the first one as you pointed out below. I also was looking through the code for something else and saw that a lot of the apps were starting with defaults then building from there, rather than trying to add the defaults at the end. I think they were still doing it wrong because they were using getenv() on their own rather than asking libpq for the defaults though. So the new version gets the defaults at the beginning and also makes it easy to add new params without changing function definitions. + set_connect_options(connect_options, pgdbname, pghost, pgport, connect_timeout, pguser); This code prevents us from giving options other than the above, for example application_name, in the conninfo. I think that pg_isready should accept all the libpq options. I'm with you there. The new version fixes that as well. When more than one -d options are specified, psql always prefer the last one and ignore the others. OTOH, pg_isready with this patch seems to merge them. I'm not sure if there is specific rule about the priority order of -d option. But it seems better to follow the existing way, i.e., always prefer the last -d option. The problem I am having here is resolving the differences between different -d options and other command line options. For example: -h foo -p 4321 -d host=bar port=1234 -d host=baz I would expect that to be 'baz:1234' but you are saying it should be 'baz:4321'? I look at -d as just a way to pass in multiple options (when you aren't strictly passing in dbname) and should be able to expand the above example to: -h foo -p 4321 -h bar -p 1234 -h baz If we hold off on parsing the value of -d until the end so we are sure we have the last one, then we might lose other parameters that were after the -d option. For example: -h foo -p 4321 -d host=bar port=1234 -d host=baz user=you -U me Should this be 'me@baz:1234' or 'you@baz:4321' or 'me@baz:4321'? So we would have to track the last instance of a parameter as well as the order those final versions came in. Sound to me like there is no clear answer there, but maybe there is a project consensus that has already been reached with regard to this? Or some general computing wisdom that applies? Regards, -- Fujii Masao -- 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] sql_drop Event Trigger
On Wed, Feb 6, 2013 at 11:04 AM, Tom Lane t...@sss.pgh.pa.us wrote: Dimitri Fontaine dimi...@2ndquadrant.fr writes: Alvaro Herrera alvhe...@2ndquadrant.com writes: I thought there was the idea that the list of objects to drop was to be acquired before actually doing the deletion; so that the trigger function could, for instance, get the name of the table being dropped. Tom and Robert have been rightfully insisting on how delicate it has been to come up with the right behavior for performMultipleDeletions, and that's not something we can easily reorganise. So the only way to get at the information seems to be what Robert insisted that I do, that is storing the information about the objects being dropped for later processing. I might be forgetting something, but doesn't dependency.c work by first constructing a list of all the objects it's going to drop, and only then dropping them? Could we inject a pre deletion event trigger call at the point where the list is completed? In theory, sure, but in practice, there are multiple ways that can break things. The possibility that the event trigger that runs at that point might drop one of the objects involved has already been mentioned, but there are others. For example, it might *create* a new object that depends on one of the objects to be dropped, potentially leading to an inconsistent pg_depend. It could also ALTER the object, or something that the object depends on. For example, suppose we're running an event trigger in the middle of a DROP TABLE command, and the event trigger creates a _SELECT rule on the table, transforming it into a view. Or, suppose it opens (and leaves open) a cursor scanning that table (normally, CheckTableNotInUse prevents that, but here we've already done that). Alternatively, suppose we're dropping a view which the event trigger redefines to depend on a completely different set of objects. I don't deny that code can be written to handle all of those cases correctly. But it's going to be a major refactoring, and the idea that we should be starting to design it in February seems ludicrous to me. It'll be May by the time we get this one patch right. Of 2014. And there are more than 70 other patches that need attention in this CommitFest. I have thus far mostly avoided getting sucked into the annual argument about which things should be evicted from this CommitFest because, frankly, I have better things to do with my time than argue about what the feature freeze date is with people who should know better. But the problem isn't going to go away because we don't talk about it. Has anyone else noticed that final triage is scheduled to end tomorrow, and we haven't done any triage of any kind and, at least with respect to this feature, are effectively still receiving new patch submissions? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] sql_drop Event Trigger
On Wed, Feb 6, 2013 at 12:05 PM, Robert Haas robertmh...@gmail.com wrote: On Wed, Feb 6, 2013 at 9:44 AM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Robert Haas robertmh...@gmail.com writes: I disagree with that. I don't see why the enclosing event trigger shouldn't be aware of all the objects dropped by the command that just ran to completion, *including* the effects of any event trigger fired recursively or not. Well, that could result in some DROP events being reported more than once, which I assume would be undesirable for someone hoping to use this for replication. Any command might have an event trigger attached doing a DROP, so that you don't know where to expect it, and it's well possible that in your example both the event triggers have been installed by different tools. It certainly is; in fact, it's likely. So let's say that B is a replication trigger. Don't you want it to hear about each drop exactly once? If not, how will you avoid errors when you go to replay the events you've captured on another machine? In this case, the hygenic change that we're thinking of making to Slony, at least initially, is for the trigger to check to see if the table is replicated, and raise an exception if it is. That forces the Gentle User to submit the Slony SET DROP TABLE command http://slony.info/documentation/2.1/stmtsetdroptable.html. Now, if we stipulate that imposition, then, for this kind of event, it becomes unnecessary for event triggers to get *overly* concerned about capturing more about dropping tables. After all, SET DROP TABLE already knows how to replicate its action, so what happens, in that case is: - User submits SET DROP TABLE - SET DROP TABLE drops the triggers for the table, cleans out Slony configuration surrounding the table, forwards request to other nodes - User submits DROP TABLE - Slony is no longer involved with that table; there's nothing special anymore about replicating this; perhaps we capture and forward it via event trigger. I'm not sure if that makes thinking about this easier, I hope so :-). -- When confronted by a difficult problem, solve it by reducing it to the question, How would the Lone Ranger handle this? -- 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] sql_drop Event Trigger
On Wed, Feb 6, 2013 at 11:30 AM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Alvaro Herrera alvhe...@2ndquadrant.com writes: Dimitri Fontaine escribió: Tom Lane t...@sss.pgh.pa.us writes: I might be forgetting something, but doesn't dependency.c work by first constructing a list of all the objects it's going to drop, and only then dropping them? Could we inject a pre deletion event trigger call at the point where the list is completed? Yes, this is what I'm proposing. So, I add back the sql_drop event and implement it in dependency.c, and keep the list for later processing from ddl_command_end, right? Robert, you specifically opposed to sql_drop and I just removed it from the patch. What do you think now? Also, should that be a follow-up patch to the current one for your reviewing purposes? Well, if it has a different firing point than ddl_command_end, then there could well be some point to having it after all. But I'm far from convinced that the proposed firing point can be made safe without a major refactoring. I think this is the sort of things where design before code ought to be the cardinal rule. I replied to this above: raise an error. (How to do that is an open implementation question, but I proposed a simple idea upthread). Well, can the list of objects that get dropped for CASCADE change in between releases? If it ever does, that means that your event trigger that used to work before is not broken after upgrade. Is that ok? That particular problem does not bother me, personally. But the possibility of ending up with corrupt system catalog contents does. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] sql_drop Event Trigger
On Wed, Feb 6, 2013 at 12:28 PM, Christopher Browne cbbro...@gmail.com wrote: On Wed, Feb 6, 2013 at 12:05 PM, Robert Haas robertmh...@gmail.com wrote: On Wed, Feb 6, 2013 at 9:44 AM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Robert Haas robertmh...@gmail.com writes: I disagree with that. I don't see why the enclosing event trigger shouldn't be aware of all the objects dropped by the command that just ran to completion, *including* the effects of any event trigger fired recursively or not. Well, that could result in some DROP events being reported more than once, which I assume would be undesirable for someone hoping to use this for replication. Any command might have an event trigger attached doing a DROP, so that you don't know where to expect it, and it's well possible that in your example both the event triggers have been installed by different tools. It certainly is; in fact, it's likely. So let's say that B is a replication trigger. Don't you want it to hear about each drop exactly once? If not, how will you avoid errors when you go to replay the events you've captured on another machine? In this case, the hygenic change that we're thinking of making to Slony, at least initially, is for the trigger to check to see if the table is replicated, and raise an exception if it is. That forces the Gentle User to submit the Slony SET DROP TABLE command http://slony.info/documentation/2.1/stmtsetdroptable.html. Now, if we stipulate that imposition, then, for this kind of event, it becomes unnecessary for event triggers to get *overly* concerned about capturing more about dropping tables. After all, SET DROP TABLE already knows how to replicate its action, so what happens, in that case is: - User submits SET DROP TABLE - SET DROP TABLE drops the triggers for the table, cleans out Slony configuration surrounding the table, forwards request to other nodes - User submits DROP TABLE - Slony is no longer involved with that table; there's nothing special anymore about replicating this; perhaps we capture and forward it via event trigger. I'm not sure if that makes thinking about this easier, I hope so :-). Well, that means you wouldn't necessarily mind getting duplicate DROP events for the same object, but they don't benefit you in any way, either. And, I'm not sure we can conclude from this that duplicate events will be OK in every use case. For example, for a logging hook, it's pessimal, because now you're logging duplicate messages for the same drops and there's no easy way to fix it so you don't. Also, it means you're really going to need the schema name and table name for this to be useful; Tom was just complaining upthread that without that information the features isn't useful, so perhaps we should conclude from your email that he is correct. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] Alias hstore's ? to ~ so that it works with JDBC
On Wed, Feb 6, 2013 at 11:20 AM, Seamus Abshere sea...@abshere.net wrote: merlin, Yes, you're correct, my phrasing was bad: all I meant was that it was a conflict, not a bug in Postgres or hstore. I personally don't know of any way around the conflict except changing JDBC or hstore, and I don't think JDBC is gonna change. Deciding not to accommodate JDBC on the Postgres side, though, is going to prevent hstore from being used properly with Java or any JVM-based language like JRuby. Please let me know if my assumptions are wrong. This problem is not unique to ? character. Hibernate for example reserves the use of : character for name parameter insertion with similarly ridiculous results. Basically every language I know of except for the java stack seems to understand that when embedding constructs into a foreign language there must be some type of escaping mechanism (note they may in fact allow this in some level: via googling it isn't clear). The point is that Postgres should not introduce language constraints because of broken driver technology. To move forward in your particular case, consider: *) switching to 'hstore defined()' function: *) hacking pg_operator (carefully look up and change oprname for the specific hstore operator) merlin -- 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] proposal: ANSI SQL 2011 syntax for named parameters
On Mon, Feb 4, 2013 at 3:32 PM, Simon Riggs si...@2ndquadrant.com wrote: On 4 February 2013 19:53, Robert Haas robertmh...@gmail.com wrote: This seems pretty close to an accusation of bad faith, which I don't believe to be present. Robert, this is not an accusation of bad faith, just an observation that we can move forwards more quickly. It's your opinion, to which you are certainly entitled, but it is not an observation of an objective fact. I find your attitude toward backward compatibility to be astonishingly inconsistent. We haven't made any progress on overhauling recovery.conf in two years because you've steadfastly stonewalled any forward progress on backwards-compatibility grounds, even though a change there can't possible break any working PostgreSQL *application*, only the admin tools. I stonewalled nothing; what exactly do you think I hoped to gain by such actions? In fact, I repeatedly encouraged change and showed how we could have both backwards compatibility and progress. Any lack of progress is not a result of my request for backwards compatibility, made in the interests of avoiding data loss and very broken applications. I don't agree with that characterization of what happened. There was a patch that basically everyone except you agreed on last year around this time, and neither it nor anything else has been committed. The issue here is rather complicated because there are a lot of subtly different things that could be done, and I think saying that you showed how we could have both backward compatibility and progress sweeps a significant amount of meaningful detail under the rug. As I recall, there were meaningful objections to your counter-proposal, you didn't agree with those objections, and that's where we got stuck. What we need on that issue is a detailed plan that meets with general agreement, not an assertion (without supporting detail) that you know how to solve the problem. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] [COMMITTERS] pgsql: Fast promote mode skips checkpoint at end of recovery.
On 6 February 2013 16:36, Heikki Linnakangas hlinnakan...@vmware.com wrote: On 31.01.2013 21:33, Simon Riggs wrote: If anyone really wants me to revert, pls start new hackers thread to discuss, or comment on changes. Yes, I still think this needs fixing or reverting. Let me reiterate my my complaints: I'm sorry that they are complaints rather than just feedback, and will work to address them. 1.3. Why check for the prev checkpoint? The latest checkpoint is enough to recover, so why insist that also the previous one is present, too? That was there from Kyotaro's patch and I left it as it was since it had been reviewed prior to me. I thought it was OK too, but now I think your arguments are good and I'm now happy to change to just the last checkpoint. That does bring into question what the value of the prev checkpoint is in any situation, not just this one... There are normal scenarios where it won't be, like just after recovering from a base backup. I consider it a bug that fast promotion doesn't work right after restoring from a base backup. OK 2. I don't like demoting the trigger file method to a second class citizen. I think we should make all functionality available through both methods. If there was a good reason for deprecating the trigger file method, I could live with that, but this patch is not such a reason. I don't understand why we introduced a second method if they both will continue to be used. I see no reason for that, other than backwards compatibility. Enhancing both mechanisms suggests both will be supported into the future. Please explain why the second mode exists? 3. I don't like conflating the promotion modes and shutdown modes in the pg_ctl option. Shutdown modes and promotion modes are separate concepts. The fast option is pretty clear, but why does smart mean create an immediate checkpoint before promotion? How is that smarter than the fast mode? The pg_ctl --help on that is a bit confusing too: Options for stop, restart or promote: -m, --mode=MODEMODE can be smart, fast, or immediate The immediate mode is not actually valid for pg_ctl promote. That is clarified later in the output by listing out what the modes mean, but that above line is misleading, We can always rename them, as you wish. 4. I think fast promotion should be the default. Why not? There are cases where you want the promotion to happen ASAP, and there are cases where you don't care. But there are no scenarios where you want promotion to be slow, Not true. Slow means safe and stable, and there are many scenarios where we want safe and stable. (Of course, nobody specifically requests slow). My feeling is that this is an area of exposure that we have no need and therefore no business touching. I will of course go with what others think here, but I don't find the argument that we should go fast always personally convincing. I am willing to relax it over time once we get zero field problems as a result. 5. Docs changes are missing. OK Here's what I think should be done: 1. Remove the check that prev checkpoint record exists. Agreed 2. Always do fast promotion if in standby mode. Remove the pg_ctl option. Disagreed, other viewpoints welcome. 3. Improve docs. Agreed -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] Alias hstore's ? to ~ so that it works with JDBC
On 02/06/2013 12:34 PM, Merlin Moncure wrote: The point is that Postgres should not introduce language constraints because of broken driver technology. +1 To move forward in your particular case, consider: *) switching to 'hstore defined()' function: good solution - but just use the existing exist() function. *) hacking pg_operator (carefully look up and change oprname for the specific hstore operator) bad solution. Why not just provide an additional operator? CREATE OPERATOR ~ ( LEFTARG = hstore, RIGHTARG = text, PROCEDURE = exist, RESTRICT = contsel, JOIN = contjoinsel ); cheers andrew -- 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] [COMMITTERS] pgsql: Fast promote mode skips checkpoint at end of recovery.
On Wed, Feb 6, 2013 at 12:43 PM, Simon Riggs si...@2ndquadrant.com wrote: 2. I don't like demoting the trigger file method to a second class citizen. I think we should make all functionality available through both methods. If there was a good reason for deprecating the trigger file method, I could live with that, but this patch is not such a reason. I don't understand why we introduced a second method if they both will continue to be used. I see no reason for that, other than backwards compatibility. Enhancing both mechanisms suggests both will be supported into the future. Please explain why the second mode exists? I agree that we should be pushing people towards pg_ctl promote. I have no strong opinion about whether backward-compatibility for the trigger file method is a good idea or not. It might be a little soon to relegate that to second-class status, but I'm not sure. 4. I think fast promotion should be the default. Why not? There are cases where you want the promotion to happen ASAP, and there are cases where you don't care. But there are no scenarios where you want promotion to be slow, Not true. Slow means safe and stable, and there are many scenarios where we want safe and stable. (Of course, nobody specifically requests slow). My feeling is that this is an area of exposure that we have no need and therefore no business touching. I will of course go with what others think here, but I don't find the argument that we should go fast always personally convincing. I am willing to relax it over time once we get zero field problems as a result. I'm skeptical of the idea that we shouldn't default to fast-promote because the fast-promote code might be buggy. We do sometimes default new features to off on the grounds that they might be buggy - Hot Standby got an on/off switch partly for that reason - but usually we only add a knob if there's some plausible reason for wanting to change the setting independently of the possibility of bugs. For instance, in the case of Hot Standby, another of the reasons for adding a knob was that people wanted a way to make sure that they wouldn't accidentally connect to the standby when they intended to connect to the master. That may or may not have been a sufficiently *good* reason, but it was accepted as justification at the time. So I would ask this question: why would someone want to turn off fast-promote mode, assuming for the sake of argument that it isn't buggy? I think there might be good reasons to do that, but I'm not sure what they are. I doubt it will be a common thing to want. I think most people are going to want fast-promote, but many may not know enough to request it, which means that if it isn't the default, the code may not get much testing anyway. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] proposal: ANSI SQL 2011 syntax for named parameters
On 6 February 2013 17:43, Robert Haas robertmh...@gmail.com wrote: On Mon, Feb 4, 2013 at 3:32 PM, Simon Riggs si...@2ndquadrant.com wrote: On 4 February 2013 19:53, Robert Haas robertmh...@gmail.com wrote: This seems pretty close to an accusation of bad faith, which I don't believe to be present. Robert, this is not an accusation of bad faith, just an observation that we can move forwards more quickly. It's your opinion, to which you are certainly entitled, but it is not an observation of an objective fact. And what? You expressed an opinion, as did I. I repeat: I don't see why waiting a year changes anything here. Can you please explain why the situation is improved by waiting a year? -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] [COMMITTERS] pgsql: Fast promote mode skips checkpoint at end of recovery.
On 06.02.2013 20:02, Robert Haas wrote: On Wed, Feb 6, 2013 at 12:43 PM, Simon Riggssi...@2ndquadrant.com wrote: 2. I don't like demoting the trigger file method to a second class citizen. I think we should make all functionality available through both methods. If there was a good reason for deprecating the trigger file method, I could live with that, but this patch is not such a reason. I don't understand why we introduced a second method if they both will continue to be used. I see no reason for that, other than backwards compatibility. Enhancing both mechanisms suggests both will be supported into the future. Please explain why the second mode exists? I agree that we should be pushing people towards pg_ctl promote. I have no strong opinion about whether backward-compatibility for the trigger file method is a good idea or not. It might be a little soon to relegate that to second-class status, but I'm not sure. Both the trigger file and pg_ctl promote methods are useful in different setups. If you point the trigger file on an NFS mount or similar, that allows triggering promotion from a different host without providing shell access. You might want to put the trigger file on an NFS mount that also contains the WAL archive, for example. A promotion script that also controls the network routers to redirect traffic and STONITH the dead node, can then simply touch /mnt/.../trigger to promote. Sure, it could also ssh to the server and run pg_ctl promote, but that requires more setup. - Heikki -- 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] palloc unification
Tom Lane escribió:
Simon Riggs si...@2ndquadrant.com writes:
On 6 February 2013 14:38, Alvaro Herrera alvhe...@2ndquadrant.com wrote:
Yeah, I am doing this right now and the shared name doesn't seem so
good. libpgframework sounds decent. So since libpgport comes from
src/port, are we okay with src/framework and src/include/framework?
common ?
src/backend/common
src/include/common
To me the term framework carries a lot of baggage that doesn't fit
this usage. common seems better.
Okay, here's an attempt at doing it that way. Notably this creates
libpgcommon, a static library, to be used by both frontend and backend.
There's only a frontend file now (fe_memutils.c); the backend side of it
is empty. I verified that the backend makefile rules work, but there's
no attempt to link it into the backend. libpgcommon piggybacks on
libpgport: for instance there is no separate submake-pgcommon rule on
which to depend, or LDFLAGS additions and the like, I just appended it
all to existing pgport rules. Duplicating it would be much more verbose
and pointless; if we ever need to distinguish these two libs, that can
easily be done.
Some existing pg_malloc() implementations tried to do something other
than exit(EXIT_FAILURE); pg_upgrade did pg_log(FATAL) for instance. But
I don't think there is much point in that, so I've just made them all
use fprintf(stderr); exit(EXIT_FAILURE); This is the same approach
Zoltan ended up with in his patch.
MSVC is known broken (I didn't touch Andres' hunk of that. I will look
into that later.)
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
*** a/contrib/oid2name/oid2name.c
--- b/contrib/oid2name/oid2name.c
***
*** 9,14
--- 9,16
*/
#include postgres_fe.h
+ #include common/fe_memutils.h
+
#include unistd.h
#ifdef HAVE_GETOPT_H
#include getopt.h
***
*** 50,58 struct options
/* function prototypes */
static void help(const char *progname);
void get_opts(int, char **, struct options *);
- void *pg_malloc(size_t size);
- void *pg_realloc(void *ptr, size_t size);
- char *pg_strdup(const char *str);
void add_one_elt(char *eltname, eary *eary);
char *get_comma_elts(eary *eary);
PGconn *sql_conn(struct options *);
--- 52,57
***
*** 201,253 help(const char *progname)
progname, progname);
}
- void *
- pg_malloc(size_t size)
- {
- void *ptr;
-
- /* Avoid unportable behavior of malloc(0) */
- if (size == 0)
- size = 1;
- ptr = malloc(size);
- if (!ptr)
- {
- fprintf(stderr, out of memory\n);
- exit(1);
- }
- return ptr;
- }
-
- void *
- pg_realloc(void *ptr, size_t size)
- {
- void *result;
-
- /* Avoid unportable behavior of realloc(NULL, 0) */
- if (ptr == NULL size == 0)
- size = 1;
- result = realloc(ptr, size);
- if (!result)
- {
- fprintf(stderr, out of memory\n);
- exit(1);
- }
- return result;
- }
-
- char *
- pg_strdup(const char *str)
- {
- char *result = strdup(str);
-
- if (!result)
- {
- fprintf(stderr, out of memory\n);
- exit(1);
- }
- return result;
- }
-
/*
* add_one_elt
*
--- 200,205
*** a/contrib/pg_upgrade/check.c
--- b/contrib/pg_upgrade/check.c
***
*** 10,15
--- 10,16
#include postgres.h
#include pg_upgrade.h
+ #include common/fe_memutils.h
static void set_locale_and_encoding(ClusterInfo *cluster);
*** a/contrib/pg_upgrade/controldata.c
--- b/contrib/pg_upgrade/controldata.c
***
*** 10,15
--- 10,16
#include postgres.h
#include pg_upgrade.h
+ #include common/fe_memutils.h
#include ctype.h
*** a/contrib/pg_upgrade/file.c
--- b/contrib/pg_upgrade/file.c
***
*** 10,15
--- 10,16
#include postgres.h
#include pg_upgrade.h
+ #include common/fe_memutils.h
#include fcntl.h
*** a/contrib/pg_upgrade/function.c
--- b/contrib/pg_upgrade/function.c
***
*** 10,15
--- 10,16
#include postgres.h
#include pg_upgrade.h
+ #include common/fe_memutils.h
#include access/transam.h
*** a/contrib/pg_upgrade/info.c
--- b/contrib/pg_upgrade/info.c
***
*** 10,15
--- 10,16
#include postgres.h
#include pg_upgrade.h
+ #include common/fe_memutils.h
#include access/transam.h
*** a/contrib/pg_upgrade/option.c
--- b/contrib/pg_upgrade/option.c
***
*** 9,14
--- 9,15
#include postgres.h
+ #include common/fe_memutils.h
#include miscadmin.h
#include pg_upgrade.h
*** a/contrib/pg_upgrade/pg_upgrade.c
--- b/contrib/pg_upgrade/pg_upgrade.c
***
*** 37,42
--- 37,43
#include postgres.h
+ #include common/fe_memutils.h
#include pg_upgrade.h
#ifdef HAVE_LANGINFO_H
*** a/contrib/pg_upgrade/pg_upgrade.h
--- b/contrib/pg_upgrade/pg_upgrade.h
***
*** 451,460 void
[HACKERS] JSON NULLs
Hackers,
While playing with Andrew’s JSON enhancements, I noticed this:
david=# select * From json_each_as_text('{baz: null}'::json);
key | value
-+---
baz | null
It is returning 'null'::text there, not NULL::text. I had expected the latter,
because otherwise it's not possible to tell the difference between '{foo:
null}' and '{foo: null}'.
But then I noticed that this seems to be true for JSON NULLs in general:
david=# select 'null'::json::text IS NULL;
?column?
--
f
Again, I expected a NULL there. I recognize that JSON NULLs are not the same as
SQL NULLs, but if there is no way to tell the difference, well, it’s annoying.
I see that 'null'::json::text resolves to 'null'::text, so that’s one way
to deal with it. But since json_each_as_text returns values as text, not quoted
JSON values, maybe *it* should return JSON NULLs as SQL NULLs?
Thanks,
David
--
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] palloc unification
On 2013-02-06 15:51:15 -0300, Alvaro Herrera wrote: Okay, here's an attempt at doing it that way. Notably this creates libpgcommon, a static library, to be used by both frontend and backend. There's only a frontend file now (fe_memutils.c); the backend side of it is empty. I verified that the backend makefile rules work, but there's no attempt to link it into the backend. libpgcommon piggybacks on libpgport: for instance there is no separate submake-pgcommon rule on which to depend, or LDFLAGS additions and the like, I just appended it all to existing pgport rules. Duplicating it would be much more verbose and pointless; if we ever need to distinguish these two libs, that can easily be done. Looks good to me. Although I have to admit I am wondering whether there still is a point in using pg_malloc() et al. instead of just using palloc() et al. There doesn't seem to be too much gained by that. But thats probably better done as another patch ontop of this if at all. Some existing pg_malloc() implementations tried to do something other than exit(EXIT_FAILURE); pg_upgrade did pg_log(FATAL) for instance. But I don't think there is much point in that, so I've just made them all use fprintf(stderr); exit(EXIT_FAILURE); This is the same approach Zoltan ended up with in his patch. If anything it should be plain write()s, as that wouldn't allocate memory... But otherwise I aggree. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] JSON NULLs
On Wed, Feb 6, 2013 at 1:08 PM, David E. Wheeler da...@justatheory.com wrote:
Hackers,
While playing with Andrew’s JSON enhancements, I noticed this:
david=# select * From json_each_as_text('{baz: null}'::json);
key | value
-+---
baz | null
It is returning 'null'::text there, not NULL::text. I had expected the
latter, because otherwise it's not possible to tell the difference between
'{foo: null}' and '{foo: null}'.
IMO, this is bug in proposed implementation. json unquoted null
should not map to string 'null' but to SQL, casting behavior from
text as implemented looks correct. (only SQL null should produce json
null)
merlin
--
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] JSON NULLs
On 02/06/2013 02:24 PM, Merlin Moncure wrote:
On Wed, Feb 6, 2013 at 1:08 PM, David E. Wheeler da...@justatheory.com wrote:
Hackers,
While playing with Andrew’s JSON enhancements, I noticed this:
david=# select * From json_each_as_text('{baz: null}'::json);
key | value
-+---
baz | null
It is returning 'null'::text there, not NULL::text. I had expected the latter, because otherwise it's not
possible to tell the difference between '{foo: null}' and '{foo: null}'.
IMO, this is bug in proposed implementation. json unquoted null
should not map to string 'null' but to SQL, casting behavior from
text as implemented looks correct. (only SQL null should produce json
null)
Probably. I'm on it.
cheers
andrew
--
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] Alias hstore's ? to ~ so that it works with JDBC
On Wed, Feb 6, 2013 at 12:00 PM, Andrew Dunstan and...@dunslane.net wrote: On 02/06/2013 12:34 PM, Merlin Moncure wrote: The point is that Postgres should not introduce language constraints because of broken driver technology. +1 To move forward in your particular case, consider: *) switching to 'hstore defined()' function: good solution - but just use the existing exist() function. *) hacking pg_operator (carefully look up and change oprname for the specific hstore operator) bad solution. Why not just provide an additional operator? CREATE OPERATOR ~ ( LEFTARG = hstore, RIGHTARG = text, PROCEDURE = exist, RESTRICT = contsel, JOIN = contjoinsel ); yeah, this is much less hacky way to go. merlin -- 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] [COMMITTERS] pgsql: Fast promote mode skips checkpoint at end of recovery.
On Wed, Feb 6, 2013 at 1:47 PM, Heikki Linnakangas hlinnakan...@vmware.com wrote: On 06.02.2013 20:02, Robert Haas wrote: On Wed, Feb 6, 2013 at 12:43 PM, Simon Riggssi...@2ndquadrant.com wrote: 2. I don't like demoting the trigger file method to a second class citizen. I think we should make all functionality available through both methods. If there was a good reason for deprecating the trigger file method, I could live with that, but this patch is not such a reason. I don't understand why we introduced a second method if they both will continue to be used. I see no reason for that, other than backwards compatibility. Enhancing both mechanisms suggests both will be supported into the future. Please explain why the second mode exists? I agree that we should be pushing people towards pg_ctl promote. I have no strong opinion about whether backward-compatibility for the trigger file method is a good idea or not. It might be a little soon to relegate that to second-class status, but I'm not sure. Both the trigger file and pg_ctl promote methods are useful in different setups. If you point the trigger file on an NFS mount or similar, that allows triggering promotion from a different host without providing shell access. You might want to put the trigger file on an NFS mount that also contains the WAL archive, for example. A promotion script that also controls the network routers to redirect traffic and STONITH the dead node, can then simply touch /mnt/.../trigger to promote. Sure, it could also ssh to the server and run pg_ctl promote, but that requires more setup. Good point. I hadn't thought about that. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] proposal: ANSI SQL 2011 syntax for named parameters
On Wed, Feb 6, 2013 at 1:06 PM, Simon Riggs si...@2ndquadrant.com wrote: On 6 February 2013 17:43, Robert Haas robertmh...@gmail.com wrote: On Mon, Feb 4, 2013 at 3:32 PM, Simon Riggs si...@2ndquadrant.com wrote: On 4 February 2013 19:53, Robert Haas robertmh...@gmail.com wrote: This seems pretty close to an accusation of bad faith, which I don't believe to be present. Robert, this is not an accusation of bad faith, just an observation that we can move forwards more quickly. It's your opinion, to which you are certainly entitled, but it is not an observation of an objective fact. And what? You expressed an opinion, as did I. I repeat: I don't see why waiting a year changes anything here. Can you please explain why the situation is improved by waiting a year? What was unclear or incomplete about the last two times I explained it? Here's what I wrote the first time: $ Keep in mind that, as recently as PostgreSQL 9.1, we shipped hstore $ with a =(text, text) operator. That operator was deprecated in 9.0, $ but it wasn't actually removed until PostgreSQL 9.2. Whenever we do $ this, it's going to break things for anyone who hasn't yet upgraded $ from hstore v1.0 to hstore v1.1. So I would prefer to wait one more $ release. That way, anyone who does an upgrade, say, every other major $ release cycle should have a reasonably clean upgrade path. And here's what I wrote the second time: $ Right now there is one and only one release in $ the field that contains hstore 1.1. If we go ahead and prohibit = as $ an operator name now, we're going to require everyone who is on 9.1 $ and uses hstore and wants to get to 9.3 to either (a) first upgrade to $ 9.2, then update hstore, then upgrade to 9.3; or (b) dig the $ hstore-1.1 update out of a future release, apply it to an earlier $ release on which it did not ship, and then upgrade. I don't know what to add to that. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Considering Gerrit for CFs
Hackers, As an occasional CommitFest manager, I'm keenly aware of the makeshift nature of the CommitFest app. If we want to go on using it -- and if we want to attract additional reviewers -- we need to improve it substantially. What Robert built for us was supposed to be a second draft, not a final version. The problem with doing it in-house is that the folks who can work on it and maintain it will be taking time away from developing PostgreSQL. So I've been keeping an eye on third-party OSS apps for contribution management, waiting for one of them to mature enough that we can seriously consider using it. I think one of them has, now: Gerrit. http://code.google.com/p/gerrit/ I spent some time with OpenStack's main Gerrit admin while at LCA, and was fairly encouraged that Gerrit would be a big step up compared to our current ad-hoc PHP. However, gerrit is designed to be git-centric rather than email-centric, so it would modify our current email-centric workflow (e.g. reviews are posted via a special git commit). Unlike other git tools, though, it expects patches and not branches, so that would integrate well with what we do now. It would also require supporting Java in our infrastructure. The advantages in features would be substantial: a better interface, ways to perform automated tasks (like remind submitters that a patch is waiting on author), online diffs, automated testing integration, and a configurable review workflow process. The existing Gerrit community would be keen to have the PostgreSQL project as a major user, though, and would theoretically help with modification needs. Current major users are OpenStack, Mediawiki, LibreOffice and QT. Thoughts? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- 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] Considering Gerrit for CFs
On Wed, Feb 6, 2013 at 10:07 PM, Josh Berkus j...@agliodbs.com wrote: Hackers, As an occasional CommitFest manager, I'm keenly aware of the makeshift nature of the CommitFest app. If we want to go on using it -- and if we want to attract additional reviewers -- we need to improve it substantially. What Robert built for us was supposed to be a second draft, not a final version. This is probably not something we should discuss right now - it's better discussed when we're not right inthe middle of a commitfest, no? The problem with doing it in-house is that the folks who can work on it and maintain it will be taking time away from developing PostgreSQL. So I've been keeping an eye on third-party OSS apps for contribution management, waiting for one of them to mature enough that we can seriously consider using it. I think one of them has, now: Gerrit. http://code.google.com/p/gerrit/ I spent some time with OpenStack's main Gerrit admin while at LCA, and was fairly encouraged that Gerrit would be a big step up compared to our current ad-hoc PHP. However, gerrit is designed to be git-centric We have no ad-hoc PHP, but I'm assume you're referring to the cf management app that's in perl? rather than email-centric, so it would modify our current email-centric workflow (e.g. reviews are posted via a special git commit). Unlike Previously, we've said we do not want to do this. And I think in general, it's a realliy bad idea to have a tool dictate the workflow. It should be the other way around. Now, if we *want' to change our workflow, that's a different story, of course. But a tool shouldn't dictate that. other git tools, though, it expects patches and not branches, so that would integrate well with what we do now. It would also require supporting Java in our infrastructure. We already have a certain amount of support for java in the infrastructure. It does mandate that it doesn't have any special requirements on the java environment, of course - but as long as it works with the one that ships on Debian, we can do it. The advantages in features would be substantial: a better interface, ways to perform automated tasks (like remind submitters that a patch is waiting on author), online diffs, automated testing integration, and a configurable review workflow process. Could you point to an example somewhere that we could check such features out? The existing Gerrit community would be keen to have the PostgreSQL project as a major user, though, and would theoretically help with modification needs. Current major users are OpenStack, Mediawiki, LibreOffice and QT. theoretically? Thoughts? I just took a quick look at their system, and when they start talking about requirements in the 100's of Gb of RAM, 24 core machines and SSD, I get scared :) But that's to scale it - doesn't mention when you need to do anything like that. I'm assuming we'd be tiny. FWIW, what we have now could easily run on a box with 128Mb RAM... -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- 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] Considering Gerrit for CFs
This is probably not something we should discuss right now - it's better discussed when we're not right inthe middle of a commitfest, no? Well, *if* we were to change tooling, the time to do it would be during beta. Hence, bringing it up now. We have no ad-hoc PHP, but I'm assume you're referring to the cf management app that's in perl? Sorry, ad-hoc perl. rather than email-centric, so it would modify our current email-centric workflow (e.g. reviews are posted via a special git commit). Unlike Previously, we've said we do not want to do this. And I think in general, it's a realliy bad idea to have a tool dictate the workflow. It should be the other way around. Yeah. It would be theoretically possible to change things so that Gerrit would accept email reviews, but anyone who's worked with RT can tell you that automated processing of email is error-prone. That's why nobody does it. Note that Gerrit is perfectly capable of *sending* email to the list, it's just *receiving* it which is an issue. Mind you, when I explained our current CF review workflow for the SF ReviewFest last year, the attendees thought I was insane. It's kept me from doing more reviewfests. Our current workflow and tooling is definitely a serious obstacle to gettng more reviewers. Seems like a good topic for the developer meeting. The advantages in features would be substantial: a better interface, ways to perform automated tasks (like remind submitters that a patch is waiting on author), online diffs, automated testing integration, and a configurable review workflow process. Could you point to an example somewhere that we could check such features out? This is a good place to start: https://review.openstack.org The existing Gerrit community would be keen to have the PostgreSQL project as a major user, though, and would theoretically help with modification needs. Current major users are OpenStack, Mediawiki, LibreOffice and QT. theoretically? Well, I think we learned from Gforge that folks are more willing to promise help than to deliver it. I just took a quick look at their system, and when they start talking about requirements in the 100's of Gb of RAM, 24 core machines and SSD, I get scared :) But that's to scale it - doesn't mention when you need to do anything like that. I'm assuming we'd be tiny. Yeah, I have no idea. Given that it's Java, I'd assume that requirements would go up. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- 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] Considering Gerrit for CFs
On 06/02/2013 22:25, Josh Berkus wrote: Mind you, when I explained our current CF review workflow for the SF ReviewFest last year, the attendees thought I was insane. It's kept me from doing more reviewfests. Our current workflow and tooling is definitely a serious obstacle to gettng more reviewers. Seems like a good topic for the developer meeting. I'm honestly having a hard time believing this. I've never thought of the commitfest workflow as complicated or burdensome. I actually quite like how things are there at the moment. Besides, if we can't expect people to spend 30 minutes figuring out the workflow, how on earth do we expect them to review patches? Regards, Marko Tiikkaja -- 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] Considering Gerrit for CFs
Magnus Hagander mag...@hagander.net writes: On Wed, Feb 6, 2013 at 10:07 PM, Josh Berkus j...@agliodbs.com wrote: As an occasional CommitFest manager, I'm keenly aware of the makeshift nature of the CommitFest app. If we want to go on using it -- and if we want to attract additional reviewers -- we need to improve it substantially. What Robert built for us was supposed to be a second draft, not a final version. This is probably not something we should discuss right now - it's better discussed when we're not right inthe middle of a commitfest, no? Judging from the last six months, it's doubtful whether we will ever again not be in the middle of a commitfest :-(. If better tooling might help us escape this state, we should talk about it. However ... rather than email-centric, so it would modify our current email-centric workflow (e.g. reviews are posted via a special git commit). Unlike Previously, we've said we do not want to do this. And I think in general, it's a realliy bad idea to have a tool dictate the workflow. ... if it's going to try to coerce us out of our email-centric habits, then I for one am very much against it. To me, the problems with the existing CF app are precisely that it's not well enough integrated with the email discussions. The way to fix that is not to abandon email (or at least, it's not a way I wish to pursue). regards, tom lane -- 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] Considering Gerrit for CFs
On 02/06/2013 01:53 PM, Tom Lane wrote: ... if it's going to try to coerce us out of our email-centric habits, then I for one am very much against it. To me, the problems with the existing CF app are precisely that it's not well enough integrated with the email discussions. The way to fix that is not to abandon email (or at least, it's not a way I wish to pursue). The email centric habits are by far the biggest limiting factor we have. Email was never designed for integral collaboration. That said, I see no way around it. I have brought up this idea before but, Redmine has by far served the purposes (with a little effort) of CMD and it also integrates with GIT. It also does not break the email work flow. It does not currently allow commands via email but that could be easily (when I say easily, I mean it) added. Just another thought. JD -- Command Prompt, Inc. - http://www.commandprompt.com/ PostgreSQL Support, Training, Professional Services and Development High Availability, Oracle Conversion, Postgres-XC @cmdpromptinc - 509-416-6579 -- 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] Considering Gerrit for CFs
On Wed, Feb 06, 2013 at 10:17:09PM +0100, Magnus Hagander wrote: I just took a quick look at their system, and when they start talking about requirements in the 100's of Gb of RAM, 24 core machines and SSD, I get scared :) But that's to scale it - doesn't mention when you need to do anything like that. I'm assuming we'd be tiny. FWIW, what we have now could easily run on a box with 128Mb RAM... I'm right now setting up a gerrit instance for another project (not as large as Postgres) on a VM with 1GB of RAM and it works alright. You'll want to run a Postgres database next to it for storing the actual reviews and such. It's a nice tool and integrates reasonably well with other thing. We have a bot connected to it which sends messages on IRC in response to various state changes. I've never seen an instance respond to email though. Have a nice day, -- Martijn van Oosterhout klep...@svana.org http://svana.org/kleptog/ He who writes carelessly confesses thereby at the very outset that he does not attach much importance to his own thoughts. -- Arthur Schopenhauer signature.asc Description: Digital signature
Re: PATCH: Split stats file per database WAS: [HACKERS] autovacuum stress-testing our system
On Tue, Feb 5, 2013 at 2:31 PM, Tomas Vondra t...@fuzzy.cz wrote: On 5.2.2013 19:23, Jeff Janes wrote: If I shutdown the server and blow away the stats with rm data/pg_stat/*, it recovers gracefully when I start it back up. If a do rm -r data/pg_stat then it has problems the next time I shut it down, but I have no right to do that in the first place. If I initdb a database without this patch, then shut it down and restart with binaries that include this patch, and need to manually make the pg_stat directory. Does that mean it needs a catalog bump in order to force an initdb? U, what you mean by catalog bump? There is a catalog number in src/include/catalog/catversion.h, which when changed forces one to redo initdb. Formally I guess it is only for system catalog changes, but I thought it was used for any on-disk changes during development cycles. I like it the way it is, as I can use the same data directory for both versions of the binary (patched and unpatched), and just manually create or remove the directory pg_stat directory when changing modes. That is ideal for testing this patch, probably not ideal for being committed into the tree along with all the other ongoing devel work. But I think this is something the committer has to worry about. I have a question about its completeness. When I first start up the cluster and have not yet touched it, there is very little stats collector activity, either with or without this patch. When I kick the cluster sufficiently (I've been using vacuumdb -a to do that) then there is a lot of stats collector activity. Even once the vacuumdb has long finished, this high level of activity continues even though the database is otherwise completely idle, and this seems to happen for every. This patch makes that high level of activity much more efficient, but it does not reduce the activity. I don't understand why an idle database cannot get back into the state right after start-up. What do you mean by stats collector activity? Is it reading/writing a lot of data, or is it just using a lot of CPU? Basically, the launching of new autovac workers and the work that that entails. Your patch reduces the size of data that needs to be written, read, and parsed for every launch, but not the number of times that that happens. Isn't that just a natural and expected behavior because the database needs to actually perform ANALYZE to actually collect the data. Although the tables are empty, it costs some CPU / IO and there's a lot of them (1000 dbs, each with 200 tables). It isn't touching the tables at all, just the stats files. I was wrong about the cluster opening quiet. It only does that if, while the cluster was shutdown, you remove the statistics files which I was doing, as I was switching back and forth between patched and unpatched. When the cluster opens, any databases that don't have statistics in the stat file(s) will not get an autovacuum worker process spawned. They only start getting spawned once someone asks for statistics for that database. But then once that happens, that database then gets a worker spawned for it every naptime (or, at least, as close to that as the server can keep up with) for eternity, even if that database is never used again. The only way to stop this is the unsupported way of blowing away the permanent stats files. I don't think there's a way around this. You may increase the autovacuum naptime, but that's about all. I do not think that the patch needs to solve this problem in order to be accepted, but if it can be addressed while the author and reviewers are paying attention to this part of the system, that would be ideal. And if not, then we should at least remember that there is future work that could be done here. If I understand that correctly, you see the same behaviour even without the patch, right? In that case I'd vote not to make the patch more complex, and try to improve that separately (if it's even possible). OK. I just thought that while digging through the code, you might have a good idea for fixing this part as well. If so, it would be a shame for that idea to be lost when you move on to other things. I created 1000 databases each with 200 single column tables (x serial primary key). After vacuumdb -a, I let it idle for a long time to see what steady state was reached. without the patch: vacuumdb -a real11m2.624s idle steady state: 48.17% user, 39.24% system, 11.78% iowait, 0.81% idle. with the patch: vacuumdb -areal6m41.306s idle steady state: 7.86% user, 5.00% system 0.09% iowait 87% idle, Nice. Another interesting numbers would be device utilization, average I/O speed I didn't gather that data, as I never figured out how to interpret those numbers and so don't have much faith in them. (But I am pretty impressed with the numbers I do understand) and required space (which should be ~2x the pgstat.stat size without
Re: PATCH: Split stats file per database WAS: [HACKERS] autovacuum stress-testing our system
Jeff Janes jeff.ja...@gmail.com writes: On Tue, Feb 5, 2013 at 2:31 PM, Tomas Vondra t...@fuzzy.cz wrote: U, what you mean by catalog bump? There is a catalog number in src/include/catalog/catversion.h, which when changed forces one to redo initdb. Formally I guess it is only for system catalog changes, but I thought it was used for any on-disk changes during development cycles. Yeah, it would be appropriate to bump the catversion if we're creating a new PGDATA subdirectory. I'm not excited about keeping code to take care of the lack of such a subdirectory at runtime, as I gather there is in the current state of the patch. Formally, if there were such code, we'd not need a catversion bump --- the rule of thumb is to change catversion if the new postgres executable would fail regression tests without a run of the new initdb. But it's pretty dumb to keep such code indefinitely, when it would have no more possible use after the next catversion bump (which is seldom more than a week or two away during devel phase). What do you mean by stats collector activity? Is it reading/writing a lot of data, or is it just using a lot of CPU? Basically, the launching of new autovac workers and the work that that entails. Your patch reduces the size of data that needs to be written, read, and parsed for every launch, but not the number of times that that happens. It doesn't seem very reasonable to ask this patch to redesign the autovacuum algorithms, which is essentially what it'll take to improve that. That's a completely separate layer of code. regards, tom lane -- 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] issues with range types, btree_gist and constraints
I wrote:
Tomas Vondra t...@fuzzy.cz writes:
I've managed to further simplify the test-case, and I've verified that
it's reproducible on current 9.2 and 9.3 branches.
It seems that
(1) gistfindgroup decides that SimpleTestString is equiv to something.
It's not too clear what; for sure there is no other leaf key of the same
value. The code in gistfindgroup looks a bit like it ought to think
that all the items are equiv to one of the union datums or the other,
but that's not the case --- no other item gets marked.
After looking closer at this, I see that it's marking left-side tuples
as equiv if they could be put in the right-side page for zero penalty,
and similarly marking right-side tuples as equiv if they could be put
in the left-side page for zero penalty. IOW it's finding the tuples for
which it doesn't matter which side they go to. So this is not quite as
insane as I thought, although the name equiv for the flag does not
seem like le mot juste, and the function name and comment are rather
misleading IMO.
(After further
investigation it seems that gbt_var_penalty is returning a negative
penalty at the critical spot here, which might have something to
do with it --- the business with the tmp[4] array seems many degrees
away from trustworthy.)
This is a bug, but the core gist code ought not generate invalid indexes
just because the type-specific penalty function is doing something
stupid.
(2) cleanupOffsets removes the item for SimpleTestString from
sv-spl_right[], evidently because it's equiv.
(3) placeOne() sticks the entry into the spl_left[] array, which is
an absolutely horrid decision: it means the left-side page will now
need a range spanning the right-side page's range, when a moment
ago they had been disjoint.
The above behaviors are fine, really, because they are driven by the
penalty function's claim that there's zero/negative cost to moving this
tuple to the other side.
(4) gistunionsubkey(), which apparently is supposed to fix the union
datums to reflect this rearrangement, does no such thing because it
only processes the index columns to the right of the one where the
damage has been done. (It's not clear to me that it could ever
be allowed to skip any columns, but that's what it's doing.)
This is a bug. It's also a bug that gistunionsubkey() fails to null out
the existing union keys for each side before calling gistMakeUnionItVec
--- that can result in the recomputed union keys being larger than
necessary.
Furthermore, there's also a bug in gbt_var_bin_union(), the same one
I saw previously that it does the wrong thing when both sides of the
existing range need to be enlarged.
The attached patch fixes these things, but not the buggy penalty
function, because we ought to try to verify that these changes are
enough to prevent creation of an incorrect index. I can't reproduce any
misbehavior anymore with this patch applied. However, I'm troubled by
your report that the behavior is unstable, because I get the same result
each time if I start from an empty (truncated) table, with or without
the patch. You may be seeing some further bug(s). Could you test this
fix against your own test cases?
There's a lot of other things I don't much like in gistsplit.c, but
they're in the nature of cosmetic and documentation fixes.
regards, tom lane
diff --git a/contrib/btree_gist/btree_utils_var.c b/contrib/btree_gist/btree_utils_var.c
index 9f8a132..691b103 100644
*** a/contrib/btree_gist/btree_utils_var.c
--- b/contrib/btree_gist/btree_utils_var.c
*** void
*** 225,237
gbt_var_bin_union(Datum *u, GBT_VARKEY *e, Oid collation,
const gbtree_vinfo *tinfo)
{
- GBT_VARKEY *nk = NULL;
- GBT_VARKEY *tmp = NULL;
- GBT_VARKEY_R nr;
GBT_VARKEY_R eo = gbt_var_key_readable(e);
if (eo.lower == eo.upper) /* leaf */
{
tmp = gbt_var_leaf2node(e, tinfo);
if (tmp != e)
eo = gbt_var_key_readable(tmp);
--- 225,237
gbt_var_bin_union(Datum *u, GBT_VARKEY *e, Oid collation,
const gbtree_vinfo *tinfo)
{
GBT_VARKEY_R eo = gbt_var_key_readable(e);
+ GBT_VARKEY_R nr;
if (eo.lower == eo.upper) /* leaf */
{
+ GBT_VARKEY *tmp;
+
tmp = gbt_var_leaf2node(e, tinfo);
if (tmp != e)
eo = gbt_var_key_readable(tmp);
*** gbt_var_bin_union(Datum *u, GBT_VARKEY *
*** 239,263
if (DatumGetPointer(*u))
{
-
GBT_VARKEY_R ro = gbt_var_key_readable((GBT_VARKEY *) DatumGetPointer(*u));
if ((*tinfo-f_cmp) (ro.lower, eo.lower, collation) 0)
{
nr.lower = eo.lower;
! nr.upper = ro.upper;
! nk = gbt_var_key_copy(nr, TRUE);
}
if ((*tinfo-f_cmp) (ro.upper, eo.upper, collation) 0)
{
nr.upper = eo.upper;
! nr.lower = ro.lower;
! nk = gbt_var_key_copy(nr, TRUE);
}
! if (nk)
! *u = PointerGetDatum(nk);
}
else
{
--- 239,264
if (DatumGetPointer(*u))
{
GBT_VARKEY_R ro =
Re: PATCH: Split stats file per database WAS: [HACKERS] autovacuum stress-testing our system
On 7.2.2013 00:40, Tom Lane wrote: Jeff Janes jeff.ja...@gmail.com writes: On Tue, Feb 5, 2013 at 2:31 PM, Tomas Vondra t...@fuzzy.cz wrote: U, what you mean by catalog bump? There is a catalog number in src/include/catalog/catversion.h, which when changed forces one to redo initdb. Formally I guess it is only for system catalog changes, but I thought it was used for any on-disk changes during development cycles. Yeah, it would be appropriate to bump the catversion if we're creating a new PGDATA subdirectory. I'm not excited about keeping code to take care of the lack of such a subdirectory at runtime, as I gather there is in the current state of the patch. Formally, if there were such code, we'd not need a No, there is nothing to handle that at runtime. The directory is created at initdb and the patch expects that (and fails if it's gone). catversion bump --- the rule of thumb is to change catversion if the new postgres executable would fail regression tests without a run of the new initdb. But it's pretty dumb to keep such code indefinitely, when it would have no more possible use after the next catversion bump (which is seldom more than a week or two away during devel phase). What do you mean by stats collector activity? Is it reading/writing a lot of data, or is it just using a lot of CPU? Basically, the launching of new autovac workers and the work that that entails. Your patch reduces the size of data that needs to be written, read, and parsed for every launch, but not the number of times that that happens. It doesn't seem very reasonable to ask this patch to redesign the autovacuum algorithms, which is essentially what it'll take to improve that. That's a completely separate layer of code. My opinion, exactly. Tomas -- 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] Alias hstore's ? to ~ so that it works with JDBC
tl;dr Scala/JRuby/Clojure (any JVM-based language) + Postgres + hstore = awesome... why not just add a few lines to hstore--1.2.sql and make sure that all operators are available and indexable? hi Andrew, hi merlin, use the existing exist() function EXIST() can't use hstore's GiST or GIN indexes. Why not just provide an additional operator? CREATE OPERATOR ~ ( LEFTARG = hstore, RIGHTARG = text, PROCEDURE = exist, RESTRICT = contsel, JOIN = contjoinsel ); Since the goal is to get ? working on JDBC with indexes, I think you also have to do CREATE OPERATOR CLASS [1] Best, Seamus [1] See my revision of hstore--1.1.sql at https://gist.github.com/seamusabshere/4715959/revisions -- Seamus Abshere sea...@abshere.net https://github.com/seamusabshere -- 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] Alias hstore's ? to ~ so that it works with JDBC
On Wed, Feb 6, 2013 at 11:42 PM, Merlin Moncure mmonc...@gmail.com wrote: *) hacking pg_operator (carefully look up and change oprname for the specific hstore operator) bad solution. Why not just provide an additional operator? CREATE OPERATOR ~ ( LEFTARG = hstore, RIGHTARG = text, PROCEDURE = exist, RESTRICT = contsel, JOIN = contjoinsel ); yeah, this is much less hacky way to go. But, you need to add new operator to opclasses in order to use GiST and GIN indexes. Another solution is to create SQL functionw which calls operator: CREATE FUNCTION exists_inline (hstore, text) RETURNS bool AS $$ SELECT $1 ? $2; $$ LANGUAGE sql; It will inline and use indexes. -- With best regards, Alexander Korotkov.
[HACKERS] Vacuum/visibility is busted
While stress testing Pavan's 2nd pass vacuum visibility patch, I realized that vacuum/visibility was busted. But it wasn't his patch that busted it. As far as I can tell, the bad commit was in the range 692079e5dcb331..168d3157032879 Since a run takes 12 to 24 hours, it will take a while to refine that interval. I was testing using the framework explained here: http://www.postgresql.org/message-id/CAMkU=1xoa6fdyoj_4fmlqpiczr1v9gp7clnxjdhu+iggqb6...@mail.gmail.com Except that I increased JJ_torn_page to 8000, so that autovacuum has a chance to run to completion before each crash; and I turned off archive_mode as it was not relevant and caused annoying noise. As far as I know, crashing is entirely irrelevant to the current problem, but I just used and adapted the framework I had at hand. A tarball of the data directory is available below, for those who would like to do a forensic inspection. The table jjanes.public.foo is clearly in violation of its unique index. https://docs.google.com/file/d/0Bzqrh1SO9FcEbk1lUWgwSk9Od28/edit?usp=sharing Thanks for any help, Jeff
Re: [HACKERS] Vacuum/visibility is busted
Jeff Janes jeff.ja...@gmail.com writes: While stress testing Pavan's 2nd pass vacuum visibility patch, I realized that vacuum/visibility was busted. But it wasn't his patch that busted it. As far as I can tell, the bad commit was in the range 692079e5dcb331..168d3157032879 Since a run takes 12 to 24 hours, it will take a while to refine that interval. Just eyeballing the commit logs, I'd have to guess that 0ac5ad5134f2769c (the foreign-key-locks patch) is the only change in that range that seems like it might account for a visibility-ish bug. If you're lacking patience for a bisection run, try that one first. regards, tom lane -- 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] Considering Gerrit for CFs
On Wed, Feb 6, 2013 at 3:00 PM, Joshua D. Drake j...@commandprompt.com wrote: On 02/06/2013 01:53 PM, Tom Lane wrote: ... if it's going to try to coerce us out of our email-centric habits, then I for one am very much against it. To me, the problems with the existing CF app are precisely that it's not well enough integrated with the email discussions. The way to fix that is not to abandon email (or at least, it's not a way I wish to pursue). The email centric habits are by far the biggest limiting factor we have. Email was never designed for integral collaboration. That said, I see no way around it. I have brought up this idea before but, Redmine has by far served the purposes (with a little effort) of CMD and it also integrates with GIT. It also does not break the email work flow. It does not currently allow commands via email but that could be easily (when I say easily, I mean it) added. Just another thought. I don't think controlling things by email is the issue. I have used the usual litany of bug trackers and appreciate the correspondence-driven model that -hackers and -bugs uses pretty pleasant. If nothing else, the uniform, well-developed, addressable, and indexed nature of the archives definitely provides me a lot of value that I haven't really seen duplicated in other projects that use structured bug or patch tracking. The individual communications tend to be of higher quality -- particularly to the purpose of later reference -- maybe a byproduct of the fact that prose is on the pedestal. There are obvious tooling gaps (aren't there always?), but I don't really see the model as broken, and I don't think I've been around pgsql-hackers exclusively or extensively enough to have developed Stockholm syndrome. I also happen to feel that the commitfest application works rather well for me in general. Sure, I wish that it might slurp up all submitted patches automatically or something like that with default titles or something or identify new versions when they appear, but I've always felt that skimming the commitfest detail page for a patch was useful. It was perhaps harder to know if the commitfest page I was looking at was complete or up to date or not, although it often is. Here's what I find most gut-wrenching in the whole submit-review-commit process: * When a reviewer shows up a couple of weeks later and says this patch doesn't apply or make check crash or fails to compile. It's especially sad because the reviewer has presumably cut out a chunk of time -- now probably aborted -- to review the patch. Machines can know these things automatically. * When on occasion patches are submitted with non-C/sh/perl suites that may not really be something that anyone wants to be a build/source tree, but seem like they might do a better job testing the patch. The inevitable conclusion is that the automated test harness is tossed, or never written because it is known it will have no place to live after a possible commit. Somehow I wish those could live somewhere and run against all submitted patches. I've toyed a very, very tiny bit with running build farm clients on Heroku with both of these in mind, but haven't revisited it in a while. -- fdr -- 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] Considering Gerrit for CFs
On 7 February 2013 08:07, Josh Berkus j...@agliodbs.com wrote: The existing Gerrit community would be keen to have the PostgreSQL project as a major user, though, and would theoretically help with modification needs. Current major users are OpenStack, Mediawiki, LibreOffice and QT. Do we actually have any requirements that are known to be uncatered for in gerrit's standard feature set? Cheers, BJ -- 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] Considering Gerrit for CFs
On 2013-02-06 13:25:31 -0800, Josh Berkus wrote: Mind you, when I explained our current CF review workflow for the SF ReviewFest last year, the attendees thought I was insane. It's kept me from doing more reviewfests. Our current workflow and tooling is definitely a serious obstacle to gettng more reviewers. Seems like a good topic for the developer meeting. I personally feel that the CF process isn't limited by technicalities like the commitfest UI or by the lack of random reviewers. Its limited by the amount of available reviewer time of people with in-depth knowledge of pg and committer bandwith. Trying to solve that problem via technical means doesn't seem likely to work out very well. The existing Gerrit community would be keen to have the PostgreSQL project as a major user, though, and would theoretically help with modification needs. Current major users are OpenStack, Mediawiki, LibreOffice and QT. There's also android as a rather major user... I just took a quick look at their system, and when they start talking about requirements in the 100's of Gb of RAM, 24 core machines and SSD, I get scared :) But that's to scale it - doesn't mention when you need to do anything like that. I'm assuming we'd be tiny. Well, with gerrit you do far more work inside the application in comparison to what we are doing today in the CF app, so part of this probably is just that more work is running through it... Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] Considering Gerrit for CFs
2013/2/7 Andres Freund and...@2ndquadrant.com: On 2013-02-06 13:25:31 -0800, Josh Berkus wrote: Mind you, when I explained our current CF review workflow for the SF ReviewFest last year, the attendees thought I was insane. It's kept me from doing more reviewfests. Our current workflow and tooling is definitely a serious obstacle to gettng more reviewers. Seems like a good topic for the developer meeting. I personally feel that the CF process isn't limited by technicalities like the commitfest UI or by the lack of random reviewers. Its limited by the amount of available reviewer time of people with in-depth knowledge of pg and committer bandwith. Trying to solve that problem via technical means doesn't seem likely to work out very well. +1 Pavel The existing Gerrit community would be keen to have the PostgreSQL project as a major user, though, and would theoretically help with modification needs. Current major users are OpenStack, Mediawiki, LibreOffice and QT. There's also android as a rather major user... I just took a quick look at their system, and when they start talking about requirements in the 100's of Gb of RAM, 24 core machines and SSD, I get scared :) But that's to scale it - doesn't mention when you need to do anything like that. I'm assuming we'd be tiny. Well, with gerrit you do far more work inside the application in comparison to what we are doing today in the CF app, so part of this probably is just that more work is running through it... Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- 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] Support for REINDEX CONCURRENTLY
Hi Michael, On 2013-02-07 16:45:57 +0900, Michael Paquier wrote: Please find attached a patch fixing 3 of the 4 problems reported before (the patch does not contain docs). Cool! 1) Removal of the quadratic dependency with list_append_unique_oid 2) Minimization of the wait phase for parent relations, this is done in a single transaction before phase 2 3) Authorization of the drop for invalid system indexes I think there's also the issue of some minor changes required to make exclusion constraints work. The problem remaining is related to toast indexes. In current master code, tuptoastter.c assumes that the index attached to the toast relation is unique This creates a problem when running concurrent reindex on toast indexes, because after phase 2, there is this problem: pg_toast_index valid ready pg_toast_index_cct valid !ready The concurrent toast index went though index_build is set as valid. So at this instant, the index can be used when inserting new entries. Um, isn't pg_toast_index_cct !valid ready? However, when inserting a new entry in the toast index, only the index registered in reltoastidxid is used for insertion in tuptoaster.c:toast_save_datum. toastidx = index_open(toastrel-rd_rel-reltoastidxid, RowExclusiveLock); This cannot work when there are concurrent toast indexes as in this case the toast index is thought as unique. In order to fix that, it is necessary to extend toast_save_datum to insert index data to the other concurrent indexes as well, and I am currently thinking about two possible approaches: 1) Change reltoastidxid from oid type to oidvector to be able to manage multiple toast index inserts. The concurrent indexes would be added in this vector once built and all the indexes in this vector would be used by tuptoaster.c:toast_save_datum. Not backward compatible but does it matter for toast relations? I don't see a problem breaking backward compat in that area. 2) Add new oidvector column in pg_class containing a vector of concurrent toast index Oids built but not validated. toast_save_datum would scan this vector and insert entries in index if there are any present in vector. What about 3) Use reltoastidxid if != InvalidOid and manually build the list (using RelationGetIndexList) otherwise? That should keep the additional overhead minimal and should be relatively straightforward to implement? I think your patch accidentially squashed in some other changes (like 5a1cd89f8f), care to repost without? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
