Re: [HACKERS] Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]
On 3/11/13 2:48 AM, Amit Kapila wrote: 1) When you change a sighup or user setting, it writes the config file out. But it does not signal for a reload. Example: I think we cannot guarantee even after calling pg_reload_conf(), as this is an asynchronous function call. We already once had a discussion about this point and as I can understand it is concluded that it should not be handled. Refer the below mail: http://www.postgresql.org/message-id/21869.1360683...@sss.pgh.pa.us I wasn't complaining that the change isn't instant. I understand that can't be done. But I think the signal to reload should be sent. If people execute SET PERSISTENT, and it doesn't actually do anything until the server is next restarted, they will be very surprised. It's OK if it doesn't do anything for a second, or until new sessions connect, because that's just how SIGHUP/session variables work. That's a documentation issue. Not reloading the config at all, I think that's going to trigger a ton of future support problems. 2 things, you want as part of this comment: a. change the name of .auto file to persistent.auto.conf b. new comment in beginning of persistent.auto.conf file. Right. config/persistent.auto.conf seems like it addresses everyone's ideas on how the file should be named. If there's a warning not to edit it in the file, too, it would be making obvious to users what is happening. The file handles persistent changes, it's automatically configured, and it's a config file. Putting "postgresql" in the name seems unnecessary to me. The check for the include_dir entry needs to happen each time SET PERSISTENT runs. This can be handled, but for this we need to parse the whole postgresql.conf file and then give the warning. This will increase the time of SET PERSISTENT command. I don't think that's a problem. I can run the command 1800 times per second on my laptop right now. If it could only run once per second, that would still be fast enough. Doing this the most reliable way it can be handled is the important part; if that happens to be the slowest way, too, that is acceptable. -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.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] Btrfs clone WIP patch
On 3/9/13 11:39 PM, Jonathan Rogers wrote: In an earlier implementation, I did call "cp --reflink=auto" once per regular file, preserving the behavior of copydir. On Btrfs, this works well, though slightly slower due to extra processes. AFAIK, there's no way to do something equivalent on ZFS without coming up with a much more complicated scheme involving both links and clones. Chasing after the performance win of the copy acceleration seems worth it. We can't really do that if it results in an obvious behavior change though. If it takes a bit longer to do it file at a time, but that's the right thing to do, as long as it's still a net win this idea is still worthwhile. I don't think it will be possible to implement a scheme that works on ZFS and addresses your concerns about file and directory handling that is not many times more complex than what I have so far. OTOH, I think the approach I have already implemented which calls an external command for each regular file to copy might be acceptable. Since I don't personally have much interest in using ZFS, I think I'm going to just focus on Btrfs for now. Just be aware that OS specific features like this are a lot more likely to be accepted if they're demonstrated to work on two OSes. That's much better evidence that the API for the OS specific work is a good one. -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.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] Reproducible "Bus error" in 9.2.3 during database dump restoration (Ubuntu Server 12.04 LTS)
x86_64, PostgreSQL 9.2. is run within an OpenVZ container and generates SIGBUS. PostgreSQL 9.1 has no such problem. (OpenVZ is a linux kernel-level virtualization which adds namespaces for processes, networking, quotas etc. It works not like e.g. Xen or VMWare, because all containers share the same kernel.) On Wed, Mar 6, 2013 at 7:51 AM, Merlin Moncure wrote: > On Tue, Mar 5, 2013 at 3:04 PM, Kevin Grittner wrote: > > Dmitry Koterov wrote: > > > >> LOG: server process (PID 18705) was terminated by signal 7: Bus error > > > > So far I have only heard of this sort of error when PostgreSQL is > > running in a virtual machine and the VM software is buggy. If you > > are not running in a VM, my next two suspects would be > > hardware/BIOS configuration issues, or an antivirus product. > > for posterity, what's the hardware platform? software bus errors are > more likely on non x86 hardware. > > merlin >
Re: postgres_fdw vs data formatting GUCs (was Re: [HACKERS] [v9.3] writable foreign tables)
Daniel Farina writes: > On Sun, Mar 10, 2013 at 12:15 PM, Tom Lane wrote: >> Hmm ... the buildfarm just rubbed my nose in a more immediate issue, >> which is that postgres_fdw is vulnerable to problems if the remote >> server is using different GUC settings than it is for things like >> timezone and datestyle. > Forgive my naivety: why would a timestamptz value not be passed > through the _in/_out function locally at least once (hence, respecting > local GUCs) when using the FDW? Is the problem overhead in not > wanting to parse and re-format the value on the local server? No, the problem is that ambiguous dates may be transferred incorrectly to or from the remote server. Once a timestamp value is inside our server, we are responsible for getting it to the remote end accurately, IMO. Here's an example using the "loopback" server that's set up by the postgres_fdw regression test: contrib_regression=# show datestyle; -- this is the style the "remote" session will be using DateStyle --- ISO, MDY (1 row) contrib_regression=# create table remote (f1 timestamptz); CREATE TABLE contrib_regression=# create foreign table ft (f1 timestamptz) server loopback options (table_name 'remote'); CREATE FOREIGN TABLE contrib_regression=# set datestyle = german, dmy; SET contrib_regression=# select now(); now 11.03.2013 09:40:17.401173 EDT (1 row) contrib_regression=# insert into ft values(now()); INSERT 0 1 contrib_regression=# select *, now() from ft; f1 | now +--- 03.11.2013 08:40:58.682679 EST | 11.03.2013 09:41:30.96724 EDT (1 row) The remote end has entirely misinterpreted the day vs month fields. Now, to hit this you need to be using a datestyle which will print in an ambiguous format, so the "ISO" and "Postgres" styles are not vulnerable; but "German" and "SQL" are. > I suppose that means any non-immutable in/out function pair may have > to be carefully considered, and the list is despairingly long...but > finite: A look at pg_dump says that it only worries about setting datestyle, intervalstyle, and extra_float_digits. 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] Materialized View patch broke pg_dump
On 03/06/2013 10:55 AM, Kevin Grittner wrote:
Bernd Helmle wrote:
Looking into this issue, it seems the version check in getTables() of pg_dump.c
is wrong. Shouldn't the check be
if (fout->remoteVersion >= 90300)
{
}
since this is where pg_relation_is_scannable() is introduced?
Fixed.
Thanks for the report!
I noticed this morning that I am still getting failures on 9.0, 9.1 and
9.2 which cause my cross-version upgrade testing to fail for git tip.
For all I know this might apply to all back branches, but these are the
only ones tested for upgrade, so that's all I can report on reliably.
I'm chasing it up to find out exactly what's going on, but figured some
extra eyeballs would help.
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] Reproducible "Bus error" in 9.2.3 during database dump restoration (Ubuntu Server 12.04 LTS)
On 03/11/2013 09:20 PM, Dmitry Koterov wrote: > x86_64, PostgreSQL 9.2. is run within an OpenVZ container and > generates SIGBUS. > PostgreSQL 9.1 has no such problem. > > (OpenVZ is a linux kernel-level virtualization which adds namespaces > for processes, networking, quotas etc. It works not like e.g. Xen or > VMWare, because all containers share the same kernel.) Related to SHM vs mmapped files? Seems unlikely, but I guess it could affect low-enough level work like kernel TLB usage. At what point in Pg's execution does the SIGBUS occur? Is it always at the same place or few places in the code? It would be helpful if you could enable core files writing and get backtraces from core files or (since it's reproducible) by attaching a debugger directly to a Pg backend. See http://wiki.postgresql.org/wiki/Getting_a_stack_trace_of_a_running_PostgreSQL_backend_on_Linux/BSD If you restore just the first half of the table or just the last half does the crash still happen? If it still happens in one part but not in another, can you do a binary search* to isolate the smallest chunk of the input file that still reliably causes the crash? * (ie: split the file roughly in half at a record boundary and test each half. Discard the half that doesn't crash, keep the half that crashes. Repeat the process using the kept half as input until you find the smallest chunk that still crashes, or get down to a single record that causes the problem.) Does the same data cause a crash when restored in another VM on the same OpenVZ container? What about when restored to another machine with the same OS and Pg version outside OpenVZ? -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Re: [HACKERS] Materialized View patch broke pg_dump
On 03/11/2013 10:43 AM, Andrew Dunstan wrote:
On 03/06/2013 10:55 AM, Kevin Grittner wrote:
Bernd Helmle wrote:
Looking into this issue, it seems the version check in getTables()
of pg_dump.c
is wrong. Shouldn't the check be
if (fout->remoteVersion >= 90300)
{
}
since this is where pg_relation_is_scannable() is introduced?
Fixed.
Thanks for the report!
I noticed this morning that I am still getting failures on 9.0, 9.1
and 9.2 which cause my cross-version upgrade testing to fail for git
tip. For all I know this might apply to all back branches, but these
are the only ones tested for upgrade, so that's all I can report on
reliably.
I'm chasing it up to find out exactly what's going on, but figured
some extra eyeballs would help.
The problem is that pg_dump is sending an empty query in versions less
than 9.3, and choking on that. Suggested fix attached - there's really
no reason to be doing anything re mat views in versions < 9.3.
cheers
andrew
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 8404458..9458429 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -1769,7 +1769,7 @@ makeTableDataInfo(TableInfo *tbinfo, bool oids)
static void
buildMatViewRefreshDependencies(Archive *fout)
{
- PQExpBuffer query = createPQExpBuffer();
+ PQExpBuffer query;
PGresult *res;
int ntups,
i;
@@ -1777,38 +1777,41 @@ buildMatViewRefreshDependencies(Archive *fout)
i_objid,
i_refobjid;
+ /* No Mat Views before 9.3. */
+ if (fout->remoteVersion < 90300)
+ return;
+
/* Make sure we are in proper schema */
selectSourceSchema(fout, "pg_catalog");
- if (fout->remoteVersion >= 90300)
- {
- appendPQExpBuffer(query, "with recursive w as "
- "( "
- "select d1.objid, d2.refobjid, c2.relkind as refrelkind "
- "from pg_depend d1 "
- "join pg_class c1 on c1.oid = d1.objid "
- "and c1.relkind = 'm' "
- "join pg_rewrite r1 on r1.ev_class = d1.objid "
- "join pg_depend d2 on d2.classid = 'pg_rewrite'::regclass "
- "and d2.objid = r1.oid "
- "and d2.refobjid <> d1.objid "
- "join pg_class c2 on c2.oid = d2.refobjid "
- "and c2.relkind in ('m','v') "
- "where d1.classid = 'pg_class'::regclass "
- "union "
- "select w.objid, d3.refobjid, c3.relkind "
- "from w "
- "join pg_rewrite r3 on r3.ev_class = w.refobjid "
- "join pg_depend d3 on d3.classid = 'pg_rewrite'::regclass "
- "and d3.objid = r3.oid "
- "and d3.refobjid <> w.refobjid "
- "join pg_class c3 on c3.oid = d3.refobjid "
- "and c3.relkind in ('m','v') "
- ") "
- "select 'pg_class'::regclass::oid as classid, objid, refobjid "
- "from w "
- "where refrelkind = 'm'");
- }
+ query = createPQExpBuffer();
+
+ appendPQExpBuffer(query, "with recursive w as "
+ "( "
+ "select d1.objid, d2.refobjid, c2.relkind as refrelkind "
+ "from pg_depend d1 "
+ "join pg_class c1 on c1.oid = d1.objid "
+ "and c1.relkind = 'm' "
+ "join pg_rewrite r1 on r1.ev_class = d1.objid "
+ "join pg_depend d2 on d2.classid = 'pg_rewrite'::regclass "
+ "and d2.objid = r1.oid "
+ "and d2.refobjid <> d1.objid "
+ "join pg_class c2 on c2.oid = d2.refobjid "
+ "and c2.relkind in ('m','v') "
+ "where d1.classid = 'pg_class'::regclass "
+ "union "
+ "select w.objid, d3.refobjid, c3.relkind "
+ "from w "
+ "join pg_rewrite r3 on r3.ev_class = w.refobjid "
+ "join pg_depend d3 on d3.classid = 'pg_rewrite'::regclass "
+ "and d3.objid = r3.oid "
+ "and d3.refobjid <> w.refobjid "
+ "join pg_class c3 on c3.oid = d3.refobjid "
+ "and c3.relkind in ('m','v') "
+ ") "
+ "select 'pg_class'::regclass::oid as classid, objid, refobjid "
+ "from w "
+ "where refrelkind = 'm'");
res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
--
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] matview join view error
"Erikjan Rijkers" writes: > With 9.3devel, I can't seem to join a matview to a view; surely that should > be allowed? Fixed, thanks for the report. 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] Materialized View patch broke pg_dump
On Tue, Mar 12, 2013 at 12:43 AM, Andrew Dunstan wrote:
>
> On 03/11/2013 10:43 AM, Andrew Dunstan wrote:
>>
>>
>> On 03/06/2013 10:55 AM, Kevin Grittner wrote:
>>>
>>> Bernd Helmle wrote:
>>>
Looking into this issue, it seems the version check in getTables() of
pg_dump.c
is wrong. Shouldn't the check be
if (fout->remoteVersion >= 90300)
{
}
since this is where pg_relation_is_scannable() is introduced?
>>>
>>> Fixed.
>>>
>>> Thanks for the report!
>>>
>>>
>>
>>
>>
>> I noticed this morning that I am still getting failures on 9.0, 9.1 and
>> 9.2 which cause my cross-version upgrade testing to fail for git tip. For
>> all I know this might apply to all back branches, but these are the only
>> ones tested for upgrade, so that's all I can report on reliably.
>>
>> I'm chasing it up to find out exactly what's going on, but figured some
>> extra eyeballs would help.
>>
>>
>
> The problem is that pg_dump is sending an empty query in versions less than
> 9.3, and choking on that. Suggested fix attached - there's really no reason
> to be doing anything re mat views in versions < 9.3.
This is the same problem that I reported in another thread.
http://www.postgresql.org/message-id/CAHGQGwH+4vtyq==l6hrupxtggfqrnlf0mwj75bfisoske28...@mail.gmail.com
The patch looks good to me.
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: postgres_fdw vs data formatting GUCs (was Re: [HACKERS] [v9.3] writable foreign tables)
> The remote end has entirely misinterpreted the day vs month fields. > Now, to hit this you need to be using a datestyle which will print > in an ambiguous format, so the "ISO" and "Postgres" styles are > not vulnerable; but "German" and "SQL" are. Is the "timezone" GUC a problem, also, for this? Seems like that could be much worse ... -- 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] transforms
> Your error looks like youre erroring out in plperl not in hstore? Look again. Peter, is there any way for you to tackle this issue? I have no idea how to fix it, myself ... -- 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] transforms
On 2013-03-11 09:42:18 -0700, Josh Berkus wrote: > > > Your error looks like youre erroring out in plperl not in hstore? > > Look again. > ERROR: could not load library > "/home/josh/pg93/lib/postgresql/hstore_plperl.so": > /home/josh/pg93/lib/postgresql/hstore_plperl.so: undefined symbol: PL_thr_key Thats a perl symbol. > Peter, is there any way for you to tackle this issue? I have no idea > how to fix it, myself ... If you add a: DO LANGUAGE plperlu ; SELECT NULL::hstore; to the extension script, does it work? 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] transforms
On 03/11/2013 09:50 AM, Andres Freund wrote: > DO LANGUAGE plperlu ; > SELECT NULL::hstore; Aha, so that's what you meant! Yeah, it works if I reference both extensions before the CREATE EXTENSION. In that case, seems like that could be added to the extension SQL, no? -- 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] Materialized View patch broke pg_dump
On 03/11/2013 12:30 PM, Fujii Masao wrote:
On Tue, Mar 12, 2013 at 12:43 AM, Andrew Dunstan wrote:
On 03/11/2013 10:43 AM, Andrew Dunstan wrote:
On 03/06/2013 10:55 AM, Kevin Grittner wrote:
Bernd Helmle wrote:
Looking into this issue, it seems the version check in getTables() of
pg_dump.c
is wrong. Shouldn't the check be
if (fout->remoteVersion >= 90300)
{
}
since this is where pg_relation_is_scannable() is introduced?
Fixed.
Thanks for the report!
I noticed this morning that I am still getting failures on 9.0, 9.1 and
9.2 which cause my cross-version upgrade testing to fail for git tip. For
all I know this might apply to all back branches, but these are the only
ones tested for upgrade, so that's all I can report on reliably.
I'm chasing it up to find out exactly what's going on, but figured some
extra eyeballs would help.
The problem is that pg_dump is sending an empty query in versions less than
9.3, and choking on that. Suggested fix attached - there's really no reason
to be doing anything re mat views in versions < 9.3.
This is the same problem that I reported in another thread.
http://www.postgresql.org/message-id/CAHGQGwH+4vtyq==l6hrupxtggfqrnlf0mwj75bfisoske28...@mail.gmail.com
Oh, I missed that. Yes, either of these would work.
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: postgres_fdw vs data formatting GUCs (was Re: [HACKERS] [v9.3] writable foreign tables)
Josh Berkus writes: >> The remote end has entirely misinterpreted the day vs month fields. >> Now, to hit this you need to be using a datestyle which will print >> in an ambiguous format, so the "ISO" and "Postgres" styles are >> not vulnerable; but "German" and "SQL" are. > Is the "timezone" GUC a problem, also, for this? Seems like that could > be much worse ... I'm not sure why you think being off by an hour is "much worse" than being off by nine months ;-). But anyway, timezone seems to be mostly a cosmetic issue, since timestamptz values will always get printed with an explicit zone value. I think you could possibly burn yourself if a foreign table were declared as being type timestamp when the underlying column is really timestamptz ... but that kind of misconfiguration would probably create a lot of misbehaviors above and beyond this one. Having said that, I'd still be inclined to try to set the remote's timezone GUC just so that error messages coming back from the remote don't reflect a randomly different timezone, which was the basic issue in the buildfarm failures we saw yesterday. OTOH, there is no guarantee at all that the remote has the same timezone database we do, so it may not know the zone or may think it has different DST rules than we think; so it's not clear how far we can get with that. Maybe we should just set the remote session's timezone to GMT always. 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] transforms
On 2013-03-11 09:55:34 -0700, Josh Berkus wrote: > On 03/11/2013 09:50 AM, Andres Freund wrote: > > DO LANGUAGE plperlu ; > > SELECT NULL::hstore; > > Aha, so that's what you meant! > > Yeah, it works if I reference both extensions before the CREATE EXTENSION. > > In that case, seems like that could be added to the extension SQL, no? If we don't find a better solution, yes. Why don't we lookup type input/ouput function for parameters and return type during CREATE FUNCTION? That should solve the issue in a neater way? 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
[HACKERS] matview patch readability/correctness gripe
While looking for the cause of Erikjan Rijkers' recent report, my attention was drawn to this hunk of the matview patch: diff --git a/src/backend/rewrite/rewriteDefine.c b/src/backend/rewrite/rewriteDefine.c index a1a9808e5d94959218b415ed34c46579c478c177..896326615753f2344b466eb180080174ddeda31d 100644 *** a/src/backend/rewrite/rewriteDefine.c --- b/src/backend/rewrite/rewriteDefine.c *** DefineQueryRewrite(char *rulename, *** 356,362 */ checkRuleResultList(query->targetList, RelationGetDescr(event_relation), ! true); /* * ... there must not be another ON SELECT rule already ... --- 357,364 */ checkRuleResultList(query->targetList, RelationGetDescr(event_relation), ! event_relation->rd_rel->relkind != ! RELKIND_MATVIEW); /* * ... there must not be another ON SELECT rule already ... IMO this is either flat-out wrong, or an unmaintainable crock, or both. The third argument of checkRuleResultList is "bool isSelect", defined thus: * The targetList might be either a SELECT targetlist, or a RETURNING list; * isSelect tells which. (This is mostly used for choosing error messages, * but also we don't enforce column name matching for RETURNING.) So the above hunk has at the very least rendered the documentation of checkRuleResultList a lie, but I suspect it's broken the logic too. I see no very good reason why matviews should work differently from regular views at all here. But even if there is some aspect of checkRuleResultList that should work differently for matviews, I really doubt that we want error messages related to them to be saying "RETURNING list" when the error is about the SELECT list. So this either needs to be reverted, or refactored into two arguments not one, with checkRuleResultList being updated to account honestly and readably for whatever it's supposed to be doing here. 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] Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]
On Mon, Mar 11, 2013 at 4:39 PM, Greg Smith wrote: > On 3/11/13 2:48 AM, Amit Kapila wrote: >>> >>> 1) When you change a sighup or user setting, it writes the config file >>> out. But it does not signal for a reload. Example: >> >> >> I think we cannot guarantee even after calling pg_reload_conf(), as this >> is >> an >> asynchronous function call. >> We already once had a discussion about this point and as I can understand >> it >> is >> concluded that it should not be handled. Refer the below mail: >> http://www.postgresql.org/message-id/21869.1360683...@sss.pgh.pa.us > > > I wasn't complaining that the change isn't instant. I understand that can't > be done. But I think the signal to reload should be sent. If people > execute SET PERSISTENT, and it doesn't actually do anything until the server > is next restarted, they will be very surprised. It's OK if it doesn't do > anything for a second, or until new sessions connect, because that's just > how SIGHUP/session variables work. That's a documentation issue. Not > reloading the config at all, I think that's going to trigger a ton of future > support problems. I agree with you if SET PERSISTENT reloads only postgresql.auto.conf. But in current conf reload mechanism, other configuration files like pg_hba.conf are also reloaded when pg_read_conf() is executed. Probably I don't like this behavior. Users might get surprised that the configuration changes by their manual operation (by not SET PERSISTENT) are also activated by SET PERSISTENT. And, this change would make the patch bigger... >>> The check for the include_dir entry needs to happen each time SET >>> PERSISTENT runs. >> >> This can be handled, but for this we need to parse the whole >> postgresql.conf >> >> file >> and then give the warning. This will increase the time of SET PERSISTENT >> command. > > > I don't think that's a problem. I can run the command 1800 times per second > on my laptop right now. If it could only run once per second, that would > still be fast enough. Doing this the most reliable way it can be handled is > the important part; if that happens to be the slowest way, too, that is > acceptable. Why should the include_dir entry for SET PERSISTENT exist in postgresql.conf? Is there any use case that users change that include_dir setting? If not, what about hardcoding the include_dir entry for SET PERSISTENT in the core? If we do this, we don't need to handle this problem at all. 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: postgres_fdw vs data formatting GUCs (was Re: [HACKERS] [v9.3] writable foreign tables)
> Having said that, I'd still be inclined to try to set the remote's > timezone GUC just so that error messages coming back from the remote > don't reflect a randomly different timezone, which was the basic issue > in the buildfarm failures we saw yesterday. OTOH, there is no guarantee > at all that the remote has the same timezone database we do, so it may > not know the zone or may think it has different DST rules than we think; > so it's not clear how far we can get with that. Maybe we should just > set the remote session's timezone to GMT always. Yeah, that seems the safest choice. What are the potential drawbacks, if any? -- 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] Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]
> I agree with you if SET PERSISTENT reloads only postgresql.auto.conf. > But in current conf reload mechanism, other configuration files like > pg_hba.conf are also reloaded when pg_read_conf() is executed. Probably > I don't like this behavior. Users might get surprised that the configuration > changes by their manual operation (by not SET PERSISTENT) are also > activated by SET PERSISTENT. I'm not sure I see this as a problem if it's well-documented. Basically you're saying that if a user does: 1. make some changes to pg_hba.conf 2. make some changes via SET PERSISTENT 3. pg_hba.conf changes will be automatically loaded It's a little surprising for some users, but if the behavior is well-documented, then I really don't see it as a problem. Consider the parallel of this: 1. make some changes to postgresql.conf 2. make some changes via SET PERSISTENT 3. manual pg.conf changes are automatically loaded I don't see this path as a problem ... in fact, I kind of think that as many users will expect the above as be surprised by it. -- 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
Column defaults for foreign tables (was Re: [HACKERS] [v9.3] writable foreign tables)
I wrote: > Thom Brown writes: >> Out of curiosity, is there any way to explicitly force a foreign >> DEFAULT with column-omission? > That's one of the things that would have to be worked out before > we could implement anything here. The easy answer would be that DEFAULT > specifies the local default, and only if you omit the column entirely > from the local command (including not having a local default) does the > remote default take effect. But whether that would be convenient to > use is hard to tell. > Another thing that would be easy to implement is to say that the new row > value is fully determined locally (including defaults if any) and remote > defaults have nothing to do with it. But I think that's almost > certainly a usability fail --- imagine that the remote has a > sequence-generated primary key, for instance. I think it's probably > necessary to permit remote insertion of defaults for that sort of table > definition to work conveniently. I looked into this a bit, and realize that the code-as-committed is already not self-consistent, because these give different results: insert into foreigntable default values; insert into foreigntable values(default, default, ...); The former case inserts whatever the remote-side default values are. The latter case inserts NULLs, regardless of the remote defaults, because that's what the query is expanded to by the rewriter. So it seems like this is something we must fix for 9.3, because otherwise we're going to have backwards-compatibility issues if we try to change the behavior later. I've concluded that the "ideal behavior" probably is that if you have declared a DEFAULT expression for a foreign table's column, then that's what the default is for the purpose of inserts or updates through the foreign table; but if you haven't, then (at least for postgres_fdw) the effective default is whatever the remote table has. I thought at first that we could fix this, and the related case update foreigntable set somecolumn = default with some relatively localized hacking in the rewriter. However, that idea fell down when I looked at multi-row inserts: insert into foreigntable values (x, y, z), (a, default, b), (c, d, default), ... The current implementation of this requires substituting the appropriate column default expressions into the VALUES lists at rewrite time. That's okay for a default expression that is known locally and should be evaluated locally; but I see absolutely no practical way to make it work if we'd like to have the defaults inserted remotely. We'd need to have some out-of-band indication that a row being returned by the ValuesScan node had had "default" placeholders in particular columns --- and I just can't see us doing the amount of violence that would need to be done to the executor to make that happen. Especially not in the 9.3 timeframe. So one possible answer is to adopt the ignore-remote-defaults semantics I suggested above, but I don't much like that from a usability standpoint. Another idea is to throw a "not implemented" error on the specific case of a multi-row VALUES with DEFAULT placeholders when the target is a foreign table. That's pretty grotty, not least because it would have to reject the case for all foreign tables not just postgres_fdw ones. But it would give us some wiggle room to implement more desirable semantics in the cases that we can handle reasonably. Thoughts? 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] [v9.3] writable foreign tables
On Sun, Mar 10, 2013 at 10:01 PM, Tom Lane wrote: > Another thing that would be easy to implement is to say that the new row > value is fully determined locally (including defaults if any) and remote > defaults have nothing to do with it. But I think that's almost > certainly a usability fail --- imagine that the remote has a > sequence-generated primary key, for instance. I think it's probably > necessary to permit remote insertion of defaults for that sort of table > definition to work conveniently. It feels a bit like unpredictable magic to have "DEFAULT" mean one thing and omitted columns mean something else. Perhaps we should have an explicit LOCAL DEFAULT and REMOTE DEFAULT and then have DEFAULT and omitted columns both mean the same thing. This starts getting a bit weird if you start to ask what happens when the remote table is itself an FDW though -- greg -- 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] [v9.3] writable foreign tables
Greg Stark writes: > It feels a bit like unpredictable magic to have "DEFAULT" mean one > thing and omitted columns mean something else. Agreed. The current code behaves that way, but I think that's indisputably a bug not behavior we want to keep. > Perhaps we should have > an explicit LOCAL DEFAULT and REMOTE DEFAULT and then have DEFAULT and > omitted columns both mean the same thing. I don't think we really want to introduce new syntax for this, do you? Especially not when many FDWs won't have a notion of a remote default at all. My thought was that the ideal behavior is that there's only one default for a column, with any local definition of it taking precedence over any remote definition. But see later message about how that may be hard to implement correctly. 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] [v9.3] writable foreign tables
On 11 March 2013 19:00, Greg Stark wrote: > On Sun, Mar 10, 2013 at 10:01 PM, Tom Lane wrote: >> Another thing that would be easy to implement is to say that the new row >> value is fully determined locally (including defaults if any) and remote >> defaults have nothing to do with it. But I think that's almost >> certainly a usability fail --- imagine that the remote has a >> sequence-generated primary key, for instance. I think it's probably >> necessary to permit remote insertion of defaults for that sort of table >> definition to work conveniently. > > It feels a bit like unpredictable magic to have "DEFAULT" mean one > thing and omitted columns mean something else. Perhaps we should have > an explicit LOCAL DEFAULT and REMOTE DEFAULT and then have DEFAULT and > omitted columns both mean the same thing. > > This starts getting a bit weird if you start to ask what happens when > the remote table is itself an FDW though We could have something like: CREATE FOREIGN TABLE ... ... OPTION (default ); where is 'local' or 'remote'. But then this means it still can't be specified in individual queries, or have a different locality between columns on the same foreign table. -- Thom -- 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] Using indexes for partial index builds
On Thu, Mar 7, 2013 at 12:51 AM, Jim Nasby wrote: > Something worth considering on this... I suspect it's possible to use an > index-only scan to do this, regardless of whether the heap page is all > visible. The reason is that the newly created index would just use the same > access methodology as the original index, so any dead rows would be ignored. This is actually quite clever. I wonder how many other cases can use similar logic. > We'd almost certainly need to block vacuums during the build however. > Obviously not an issue for regular index builds, but it would be for > concurrent ones. Concurrent index builds block vacuums already. -- greg -- 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: Proposal for Allow postgresql.conf values to be changed via SQL [review]
On Mon, Mar 11, 2013 at 7:39 AM, Greg Smith wrote: > I wasn't complaining that the change isn't instant. I understand that can't > be done. But I think the signal to reload should be sent. If people > execute SET PERSISTENT, and it doesn't actually do anything until the server > is next restarted, they will be very surprised. It's OK if it doesn't do > anything for a second, or until new sessions connect, because that's just > how SIGHUP/session variables work. That's a documentation issue. Not > reloading the config at all, I think that's going to trigger a ton of future > support problems. Think also about the case where someone wants to change multiple values together and having just some set and not others would be inconsistent. I can see you're right about surprising users but is there not another way to solve the same problem without making that impossible? -- greg -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: postgres_fdw vs data formatting GUCs (was Re: [HACKERS] [v9.3] writable foreign tables)
Josh Berkus writes: >> Having said that, I'd still be inclined to try to set the remote's >> timezone GUC just so that error messages coming back from the remote >> don't reflect a randomly different timezone, which was the basic issue >> in the buildfarm failures we saw yesterday. OTOH, there is no guarantee >> at all that the remote has the same timezone database we do, so it may >> not know the zone or may think it has different DST rules than we think; >> so it's not clear how far we can get with that. Maybe we should just >> set the remote session's timezone to GMT always. > Yeah, that seems the safest choice. What are the potential drawbacks, > if any? Hard to tell if there are any without testing it. I remembered that there's a relatively inexpensive way to set GUC values transiently within an operation, which is GUC_ACTION_SAVE; both extension.c and ri_triggers.c are relying on that. So here's my proposal for a fix: * To make the remote end transmit values unambiguously, send SET commands for the GUCs listed below during remote session setup. (postgres_fdw is already assuming that such SETs will persist for the whole session.) * To make our end transmit values unambiguously, use GUC_ACTION_SAVE to transiently change the GUCs listed below whenever we are converting values to text form to send to the remote end. (This would include deparsing of Const nodes as well as transmission of query parameters.) * Judging from the precedent of pg_dump, these are the things we ought to set this way: DATESTYLE = ISO INTERVALSTYLE = POSTGRES (skip on remote side, if version < 8.4) EXTRA_FLOAT_DIGITS = 3 (or 2 on remote side, if version < 9.0) * In addition I propose we set TIMEZONE = UTC on the remote side only. This is, I believe, just a cosmetic hack so that timestamptz values coming back in error messages will be printed consistently; it would let us revert the kluge solution I put in place for this type of regression failure: http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=rover_firefly&dt=2013-03-10%2018%3A30%3A00 BTW, it strikes me that dblink is probably subject to at least some of these same failure modes. I'm not personally volunteering to fix any of this in dblink, but maybe someone ought to look into that. Thoughts? 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: postgres_fdw vs data formatting GUCs (was Re: [HACKERS] [v9.3] writable foreign tables)
On Mon, Mar 11, 2013 at 12:30 PM, Tom Lane wrote: > BTW, it strikes me that dblink is probably subject to at least some of > these same failure modes. I'm not personally volunteering to fix any > of this in dblink, but maybe someone ought to look into that. I will try to make time for this, although it seems like the general approach should match pgsql_fdw if possible. Is the current thinking to forward the settings and then use the GUC hooks to track updates? -- 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] Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]
On 03/12/2013 03:19 AM, Greg Stark wrote: > Think also about the case where someone wants to change multiple > values together and having just some set and not others would be > inconsistent. Yeah, that's a killer. The reload would need to be scheduled for COMMIT time, it can't be done by `SET PERSISTENT` directly. -- Craig Ringer 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] transforms
On Mon, 2013-03-11 at 18:11 +0100, Andres Freund wrote: > If we don't find a better solution, yes. Why don't we lookup type > input/ouput function for parameters and return type during CREATE > FUNCTION? That should solve the issue in a neater way? A function in general has no particular use for a type's input or output function. Also, a type's input/output functions are not necessarily in the same library as other things about that type that you might want or need. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: postgres_fdw vs data formatting GUCs (was Re: [HACKERS] [v9.3] writable foreign tables)
Daniel Farina writes: > I will try to make time for this, although it seems like the general > approach should match pgsql_fdw if possible. Is the current thinking > to forward the settings and then use the GUC hooks to track updates? That's not what I had in mind for postgres_fdw --- rather the idea is to avoid needing on-the-fly changes in remote-side settings, because those are so expensive to make. However, postgres_fdw is fortunate in that the SQL it expects to execute on the remote side is very constrained. dblink might need a different solution that would leave room for user-driven changes of remote-side settings. 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] Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]
On Monday, March 11, 2013 11:02 PM Fujii Masao wrote: > On Mon, Mar 11, 2013 at 4:39 PM, Greg Smith > wrote: > > On 3/11/13 2:48 AM, Amit Kapila wrote: > >>> > >>> 1) When you change a sighup or user setting, it writes the config > file > >>> out. But it does not signal for a reload. Example: > >> > >> > >> I think we cannot guarantee even after calling pg_reload_conf(), as > this > >> is > >> an > >> asynchronous function call. > >> We already once had a discussion about this point and as I can > understand > >> it > >> is > >> concluded that it should not be handled. Refer the below mail: > >> http://www.postgresql.org/message-id/21869.1360683...@sss.pgh.pa.us > > > > > > I wasn't complaining that the change isn't instant. I understand > that can't > > be done. But I think the signal to reload should be sent. If people > > execute SET PERSISTENT, and it doesn't actually do anything until the > server > > is next restarted, they will be very surprised. It's OK if it > doesn't do > > anything for a second, or until new sessions connect, because that's > just > > how SIGHUP/session variables work. That's a documentation issue. > Not > > reloading the config at all, I think that's going to trigger a ton of > future > > support problems. > > I agree with you if SET PERSISTENT reloads only postgresql.auto.conf. > But in current conf reload mechanism, other configuration files like > pg_hba.conf are also reloaded when pg_read_conf() is executed. Probably > I don't like this behavior. Users might get surprised that the > configuration > changes by their manual operation (by not SET PERSISTENT) are also > activated by SET PERSISTENT. > > And, this change would make the patch bigger... it's more of issue in consistency as pg_reload_conf() is asynchronous. > >>> The check for the include_dir entry needs to happen each time SET > >>> PERSISTENT runs. > >> > >> This can be handled, but for this we need to parse the whole > >> postgresql.conf > >> > >> file > >> and then give the warning. This will increase the time of SET > PERSISTENT > >> command. > > > > > > I don't think that's a problem. I can run the command 1800 times per > second > > on my laptop right now. If it could only run once per second, that > would > > still be fast enough. Doing this the most reliable way it can be > handled is > > the important part; if that happens to be the slowest way, too, that > is > > acceptable. > > Why should the include_dir entry for SET PERSISTENT exist in > postgresql.conf? > Is there any use case that users change that include_dir setting? If > not, what > about hardcoding the include_dir entry for SET PERSISTENT in the core? > If we do this, we don't need to handle this problem at all. It will be for users who want to use the configuration settings as per previous version, means by manually setting in conf file. With Regards, Amit Kapila. -- 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 for Allow postgresql.conf values to be changed via SQL [review]
> From: gsst...@gmail.com [mailto:gsst...@gmail.com] On Behalf Of Greg > Stark > Sent: Tuesday, March 12, 2013 12:50 AM > To: Greg Smith > Cc: Amit Kapila; Andres Freund; Boszormenyi Zoltan; pgsql- > hack...@postgresql.org > Subject: Re: Re: Proposal for Allow postgresql.conf values to be > changed via SQL [review] > > On Mon, Mar 11, 2013 at 7:39 AM, Greg Smith > wrote: > > > I wasn't complaining that the change isn't instant. I understand > that can't > > be done. But I think the signal to reload should be sent. If people > > execute SET PERSISTENT, and it doesn't actually do anything until the > server > > is next restarted, they will be very surprised. It's OK if it > doesn't do > > anything for a second, or until new sessions connect, because that's > just > > how SIGHUP/session variables work. That's a documentation issue. > Not > > reloading the config at all, I think that's going to trigger a ton of > future > > support problems. > > Think also about the case where someone wants to change multiple > values together and having just some set and not others would be > inconsistent. Do you mean to say that because some variables can only be set after restart can lead to inconsistency, or is it because of asynchronous nature of pg_reload_conf()? > I can see you're right about surprising users but is there not another > way to solve the same problem without making that impossible? With Regards, Amit Kapila. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Incorrect handling of timezones with extract
Hi all, When running some QE tests at VMware, we found an error with extract handling timezones. Please see below: postgres=# show timezone; TimeZone Asia/Tokyo (1 row) postgres=# select now(); now --- 2013-03-12 14:54:28.911298+09 (1 row) postgres=# select extract(day from ((CAST(-3 || 'day' as interval)+now()) - now())); date_part --- -3 (1 row) postgres=# set timezone = 'US/Pacific'; SET postgres=# select now(); now --- 2013-03-11 22:56:10.317431-07 (1 row) postgres=# select extract(day from ((CAST(-3 || 'day' as interval)+now()) - now())); date_part --- -2 (1 row) Here I believe that the correct result should be -3. Note that it passes with values upper than -2 and lower than -127: postgres=# select extract(day from ((CAST(-128 || 'day' as interval)+now()) - now())); date_part --- -128 (1 row) postgres=# select extract(day from ((CAST(-127 || 'day' as interval)+now()) - now())); date_part --- -126 (1 row) postgres=# select extract(day from ((CAST(-2 || 'day' as interval)+now()) - now())); date_part --- -1 (1 row) postgres=# select extract(day from ((CAST(-1 || 'day' as interval)+now()) - now())); date_part --- -1 (1 row) Also note that this happens only with the timezone set where time -1day. postgres=# set timezone to 'Asia/Tokyo'; SET postgres=# select extract(day from ((CAST(-127 || 'day' as interval)+now()) - now())); date_part --- -127 (1 row) postgres=# select extract(day from ((CAST(-100 || 'day' as interval)+now()) - now())); date_part --- -100 (1 row) postgres=# select extract(day from ((CAST(-2 || 'day' as interval)+now()) - now())); date_part --- -2 (1 row) I also tested with PG on master until 8.4 and could reproduce the problem. Regards, -- Michael
[HACKERS] Fix document typo
I ran into a typo in the reference page on the SELECT command. Please find attached a patch. Best regards, Etsuro Fujita typo_fix_20130312.patch 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
