The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: tested, passed
Spec compliant: not tested
Documentation:not tested
Looks good to me. It only allows valid number between 1 and 65535, disal
Hello,
This is my first patch review ever, so please bear with me.
The patch[1] is in the context diff format and applies cleanly to
current git HEAD. Documentation is updated by the patch.
The code does implement what's the patch is supposed to do.
Do we want that? According to Greg's orig
Greg Smith writes:
Thank you for the review, Greg!
> Given that, there really isn't a useful path forward that helps out
> all those developers without supporting both prefixes. That's where
> this left off before, I just wanted to emphasize how clear that need
> seems now.
OK, I've used the c
Florian Weimer writes:
> * Alex Shulgin:
>
>> Yeah, this is really appealing, however how do you tell if the part
>> after the last slash is a socket directory name or a dbname? E.g:
>>
>> psql postgres:///path/to/different/socket/dir(default dbnam
Harold Giménez writes:
>I have interest in the URI connection string support patch[1], so I'm
>in the process of reviewing it. I have a couple of comments and
>questions:
Hello, thank you for your interest and the review!
>1. I see no tests in the patch. I'd like to start getti
Peter Eisentraut writes:
>> I've included a (separate) test shell script based on Greg's cases in
>> one of the updates. What would be preferred place to plug it in?
>> Override installcheck in libpq Makefile?
>
> I think that would be the right place.
I figured that adding this right into sr
Daniel Farina writes:
> I reviewed this and so far have not found any serious problems,
> although as is par for the course it contains some of the fiddly bits
> involved in any string manipulations in C. I made a few edits -- none
> strictly necessary for correctness -- that the original author
Alex writes:
> Marko Kreen writes:
>
>> On Thu, Mar 15, 2012 at 11:29:31PM +0200, Alex wrote:
>>> https://github.com/a1exsh/postgres/commits/uri
>>
>> The point of the patch is to have one string with all connection options,
>> in standard format, yes? So why does not this work:
>>
>> db = PQ
Heikki Linnakangas writes:
> On 22.03.2012 23:42, Alex wrote:
>> Okay, at last here's v9, rebased against current master branch.
>
> Some quick comments on this patch:
Heikki, thank you for taking a look at this!
> I see a compiler warning:
> fe-connect.c: In function ‘conninfo_parse’:
> fe-co
Hello Hackers,
After reading up through archives on the two $subj related TODO items
I'm under impression that the patches[1,2] didn't make it mainly because
of the risk of breaking SSL internals if we try to longjump out of the
signal handler in the middle of a blocking SSL read and/or if we try
Andres Freund writes:
>
> On 2014-11-15 00:11:36 +0300, Alex Shulgin wrote:
>> After reading up through archives on the two $subj related TODO items
>> I'm under impression that the patches[1,2] didn't make it mainly because
>> of the risk of breaking SSL inte
Andres Freund writes:
> On 2014-11-15 00:11:36 +0300, Alex Shulgin wrote:
>> After reading up through archives on the two $subj related TODO items
>> I'm under impression that the patches[1,2] didn't make it mainly because
>> of the risk of breaking SSL interna
Andres Freund writes:
>
> I've invested some more time in this:
> 0002 now makes sense on its own and doesn't change anything around the
> interrupt handling. Oh, and it compiles without 0003.
In this patch, the endif appears to be misplaced in PostgresMain:
+ if (MyProcPort != NULL)
Bruce Momjian writes:
>
> Here is a cleaned-up version of the unreference file patch that was
> discussed extensively in May of 2005. I want to get it into the
> archives in case someone else want to work on it.
>
> Here is a reference to the work still needed on the patch:
>
> http://arch
Dag-Erling Smørgrav writes:
>
> The attached patches add an ssl_protocols configuration option which
> control which versions of SSL or TLS the server will use. The syntax is
> similar to Apache's SSLProtocols directive, except that the list is
> colon-separated instead of whitespace-separated,
Dag-Erling Smørgrav writes:
> Alex Shulgin writes:
>> * The patch works as advertised, though the only way to verify that
>> connections made with the protocol disabled by the GUC are indeed
>> rejected is to edit fe-secure-openssl.c to only allow specific TLS
&
Andres Freund writes:
>
> On 2014-11-19 15:09:10 -0800, Josh Berkus wrote:
>> One patch that got deferred from 9.4 was the merger of recovery.conf and
>> postgresql.conf, due to conflicts with ALTER SYSTEM SET. However, this
>> is still a critical TODO, because recovery.conf is still an ongoing
Dag-Erling Smørgrav writes:
> Alex Shulgin writes:
>> I can do that too, just need a hint where to look at in libpq/psql to
>> add the option.
>
> The place to *enforce* the option is src/interfaces/libpq/fe-secure.c
> (look for SSLv23_method() and SSL_CTX_set_optio
GET_UNSET;
>> +}
>> +
>
> I don't think it's correct to do such hangups in the assign hook - you
> have no ideas in which order they will be called and such. Imo that
> should happen at startup, like we also do it for other interdependent
> variables like wa
Alex Shulgin writes:
>
>>> > * Why do you include xlog_internal.h in guc.c and not xlog.h?
>>
>>> we actually need both but including xlog_internal.h also includes xlog.h
>>> i added xlog.h and if someone things is enough only putting
>>> xlog_i
Josh Berkus writes:
>>
>> Well, the latest version of this patch fails to start when it sees
>> 'recovery.conf' in PGDATA:
>>
>> FATAL: "recovery.conf" is not supported anymore as a recovery method
>> DETAIL: Refer to appropriate documentation about migration methods
>>
>> I've missed al
Hackers,
It appears that replication connection doesn't support URI but only the
traditional conninfo string.
src/backend/replication/libpqwalreceiver/libpqwalreceiver.c:99: in
libpqrcv_connect():
snprintf(conninfo_repl, sizeof(conninfo_repl),
"%s dbname=replication replicatio
n" pseudo-database name.
Have a nice day!
--
Alex
>From 156e6faa96ad6a2ce58055ad72883ed78c576e5b Mon Sep 17 00:00:00 2001
From: Alex Shulgin
Date: Mon, 24 Nov 2014 16:55:50 +0300
Subject: [PATCH 1/3] Add missing check on OOM in expand_dbname path of
conninfo_array_parse().
---
src/int
Jaime Casanova writes:
>>>
>>> Either way, from the code it is clear that we only stay in recovery if
>>> standby_mode is directly turned on. This makes the whole check for a
>>> specially named file unnecessary, IMO: we should just check the value of
>>> standby_mode (which is off by default).
Josh Berkus writes:
>>
>> only that you need to start in recovery mode to start replication
>
> Right, but my point is that having a trigger file *is not necessary for
> replication, only for PITR* -- and maybe not necessary even for PITR.
> That is, in a streaming replication configuration, hav
Josh Berkus writes:
>>>
>>> Before I go into my ideas, though, what does the current patch do
>>> regarding non-replication PITR?
>>
>> It removes that $PGDATA/standby.enable trigger file it relies on to
>> start the PITR in the first place.
>
> OK, and that's required for replication too? I'm
Alex Shulgin writes:
> Heikki Linnakangas writes:
>>>
>>> It appears that replication connection doesn't support URI but only the
>>> traditional conninfo string.
>>>
>>> src/backend/replication/libpqwalreceiver/libpqwalreceiver.c:
Heikki Linnakangas writes:
> On 11/24/2014 06:05 PM, Alex Shulgin wrote:
>> The first patch is not on topic, I just spotted this missing check.
>
>> *** a/src/interfaces/libpq/fe-connect.c
>> --- b/src/interfaces/libpq/fe-connect.c
>> *** conninfo
Heikki Linnakangas writes:
>>>
>>> I think we need to fix all of those, and backpatch. Per attached.
>>
>> Yikes! Looks sane to me.
>
> Ok thanks, committed. It didn't apply cleanly to 9.0, 9.1 and 9.2, so
> the patch for those branches looks a bit different.
Great. Are you looking at the actu
Heikki Linnakangas writes:
>>>
>>> The second one is a self-contained fix, but the third one which is the
>>> actual patch depends on the second one, because it specifies the dbname
>>> keyword two times: first to parse the conninfo/URI, then to override any
>>> dbname provided by the user with "
Tom,
First of all, thanks for your help on IRC last time with that CREATE
INDEX memory consumption problem.
As has been pointed out in a stackexchange answer to my question[1], it
is indeed the limitation of pre-9.4 versions, but the limit is imposed
on memtuples array, rather than total memory
Tom Lane writes:
> Stephen Frost writes:
>> * Alex Shulgin (a...@commandprompt.com) wrote:
>>> Tom,
>>>
>>> First of all, thanks for your help on IRC last time with that CREATE
>>> INDEX memory consumption problem.
>
>> Doubt it was Tom,
Andrew Gierth writes:
>>>>>> "Alex" == Alex Shulgin writes:
>
> > Tom Lane writes:
> >> Must've been my evil twin.
>
> Alex> Sorry, I must be under false impression that RhodiumToad is
> Alex> *your* nick on #postgresql
Alex Shulgin writes:
>>>
>>> I can do that too, just need a hint where to look at in libpq/psql to
>>> add the option.
>>
>> The place to *enforce* the option is src/interfaces/libpq/fe-secure.c
>> (look for SSLv23_method() and SSL_CTX_set_option
Jay Levitt writes:
>
> (A quick Google shows redmine and especially Trac having spam issues
> of their own.)
Ugh, redmine (or trac for that matters) has nothing to with handling
spam. I believe a typical bug tracker doesn't handle spam itself, it
lets the mailing system do that.
Surely you can
Jay Levitt writes:
>
>> No meaningful search, eh? Works for me.
>
> Redmine searches return partial-word matches, and there's no way to
> disable that. Searching for "test" finds "latest". To me, that's
> broken.
Well, I believe one can plug in a different search engine, like lucene
or xapian.
Robert Haas writes:
> On Tue, Apr 17, 2012 at 1:47 AM, Magnus Hagander wrote:
>> That's probably one reason people aren't jumping on this. Because
>> there is no tracker out there that people actually *like*...
>
> I think this is a point worth serious thought.
I wonder why do people keep comp
Alex writes:
> Peter Eisentraut writes:
>
>> I have been reviewing how our new libpq URL syntax compares against
>> existing implementations of URL syntaxes in other drivers or
>> higher-level access libraries. In the case of SQLAlchemy, there is an
>> incompatibility regarding how Unix-domain
Dave Page writes:
> On Tue, Jun 19, 2012 at 4:43 AM, Robert Haas wrote:
>>
>>
>> Please add this patch here so that it doesn't get lost in the shuffle:
>>
>> https://commitfest.postgresql.org/action/commitfest_view/open
>
> Hmm, that raises an interesting question (though maybe I've just
> miss
Dave Page writes:
> On Tue, Jun 19, 2012 at 11:04 AM, Alex Shulgin wrote:
>>
>> In a real bug-tracking system we would create a new bug/ticket and set
>> it's target version to 'candidate for next minor release' or something
>> like that. This way, if
On Sat, Apr 2, 2016 at 8:57 PM, Shulgin, Oleksandr <
oleksandr.shul...@zalando.de> wrote:
> On Apr 2, 2016 18:38, "Tom Lane" wrote:
>
>> I did not like the fact that the compute_scalar_stats logic
>> would allow absolutely anything into the MCV list once num_hist falls
>> below 2. I think it's imp
On Sat, Apr 2, 2016 at 11:41 PM, Tom Lane wrote:
> "Shulgin, Oleksandr" writes:
> > On Mon, Mar 14, 2016 at 7:55 PM, Tom Lane wrote:
> >> Yeah, I don't much like that either. But I don't think we can avoid
> >> some refactoring there; as designed, conversion of an error message into
> >> user-
On Sun, Apr 3, 2016 at 3:43 AM, Alex Shulgin wrote:
>
> I'm not sure yet about the 1% rule for the last value, but would also love
> to see if we can avoid the arbitrary limit here. What happens with a last
> value which is less than 1% popular in the current code anyway?
&g
On Sun, Apr 3, 2016 at 7:18 AM, Tom Lane wrote:
> Alex Shulgin writes:
> > On Sun, Apr 3, 2016 at 3:43 AM, Alex Shulgin
> wrote:
> >> I'm not sure yet about the 1% rule for the last value, but would also
> love
> >> to see if we can avoid the arbitrary l
On Sun, Apr 3, 2016 at 7:49 AM, Tom Lane wrote:
> Alex Shulgin writes:
> > On Sun, Apr 3, 2016 at 7:18 AM, Tom Lane wrote:
> >> Well, we have to do *something* with the last (possibly only) value.
> >> Neither "include always" nor "omit always"
On Sun, Apr 3, 2016, 18:32 Tom Lane wrote:
> Peter Geoghegan writes:
> > On Thu, Feb 11, 2016 at 9:50 AM, Robert Haas
> wrote:
> >> Wow, that's a fabulous idea. I see Oleksandr has tried to implement
> >> it, although I haven't looked at the patch. But I think this would be
> >> REALLY helpfu
On Sun, Apr 3, 2016, 18:40 Tom Lane wrote:
> Alex Shulgin writes:
>
> > Well, if it's the only value it will be accepted simply because we are
> > checking that special case already and don't even bother to loop through
> > the track list.
>
> That was
On Sun, Apr 3, 2016 at 8:24 AM, Alex Shulgin wrote:
>
> On Sun, Apr 3, 2016 at 7:49 AM, Tom Lane wrote:
>>
>> Alex Shulgin writes:
>> > On Sun, Apr 3, 2016 at 7:18 AM, Tom Lane wrote:
>> >> Well, we have to do *something* with the last (possibly only) va
On Sun, Apr 3, 2016 at 10:53 PM, Tom Lane wrote:
> Alex Shulgin writes:
> > This recalled observation can now also explain to me why in the
> regression
> > you've seen, the short path was not followed: my bet is that stadistinct
> > appeared negative.
>
> Ye
On Mon, Apr 4, 2016 at 1:06 AM, Tom Lane wrote:
> Alex Shulgin writes:
> > On Sun, Apr 3, 2016 at 10:53 PM, Tom Lane wrote:
> >> The reason for checking toowide_cnt is that if it's greater than zero,
> >> then in fact the track list does NOT include all val
Dag-Erling Smørgrav writes:
> Alex Shulgin writes:
>> OK, looks like I've come up with something workable: I've added
>> sslprotocol connection string keyword similar to pre-existing
>> sslcompression, etc. Please see attached v2 of the original patch.
>>
Julien Tachoires writes:
>
> 2011/12/15 Alvaro Herrera :
>>
>> Uhm, surely you could compare the original toast tablespace to the heap
>> tablespace, and if they differ, handle appropriately when creating the
>> new toast table? =A0Just pass down the toast tablespace into
>> AlterTableCreateToastT
Christoph Berg writes:
> Re: Petr Jelinek 2014-11-25 <5474efea.2040...@2ndquadrant.com>
>> >Patch committed.
Before I go and rebase that recovery.conf -> GUC patch on top of
this... is it final?
>>
>> Thanks!
>
> I'm a bit late to the party, but wouldn't
>
> recovery_target_action = ...
>
>
Alex Shulgin writes:
>
> Here's an attempt to revive this patch.
Here's the patch rebased against current HEAD, that is including the
recently committed action_at_recovery_target option.
The default for the new GUC is 'pause', as in HEAD, and
pause_at_recovery_targe
Michael Paquier writes:
> On Tue, Dec 2, 2014 at 1:58 AM, Alex Shulgin wrote:
>> Here's the patch rebased against current HEAD, that is including the
>> recently committed action_at_recovery_target option.
> If this patch gets in, it gives a good argument to jump to 1
Andres Freund writes:
> On 2014-12-02 17:25:14 +0300, Alex Shulgin wrote:
>> I'd be in favor of a solution that works the same way as before the
>> patch, without the need for extra trigger files, etc., but that doesn't
>> seem to be nearly possible. Whatever tri
Heikki Linnakangas writes:
> On 12/03/2014 04:54 PM, Alvaro Herrera wrote:
>> ir commit timestamp directly as they commit,
>> or an external transaction c
>
> Sorry, I'm late to the party, but here's some random comments on this
> after a quick review:
Also this commit breaks initdb of `make ch
Alvaro Herrera writes:
> Alex Shulgin wrote:
>
>> Also this commit breaks initdb of `make check' for me:
>>
>> creating template1 database in
>> /home/ash/build/postgresql/HEAD/src/test/regress/./tmp_check/data/base/1
>> ... TRAP: FailedAssertion(&quo
Alvaro Herrera writes:
>
> Uh, that's odd. Can you please get a stack trace? Do you have unusual
> settings or a patched build?
Is there a way to pause the bootstrap process so I can attach gdb to it?
--
Alex
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make cha
Craig Ringer writes:
> On 12/04/2014 10:50 PM, Alex Shulgin wrote:
>> Is there a way to pause the bootstrap process so I can attach gdb to it?
>
> With a newer gdb, you can instead tell gdb to follow all forks. I wrote
> some notes on it recently.
>
> http://blog.2
Alvaro Herrera writes:
> Alex Shulgin wrote:
>
>> DEBUG: inserting column 7 value "varchar_transform"
>>
>> Breakpoint 1, GetSnapshotData (snapshot=0xdb2d60 )
>> at /home/ash/src/postgresql/src/backend/storage/ipc/procarray.c:1413
&
Alex Shulgin writes:
>
> Figured it out with a pg_usleep in bootstrap.c anyway. Does this look sane?
>
>
> DEBUG: inserting column 6 value "0"
> DEBUG: inserted -> 0
> DEBUG: inserting column 7 value "varchar_transform"
> TRAP: FailedAsser
Alvaro Herrera writes:
> Alex Shulgin wrote:
>
>> DEBUG: inserting column 7 value "varchar_transform"
>>
>> Breakpoint 1, GetSnapshotData (snapshot=0xdb2d60 )
>> at /home/ash/src/postgresql/src/backend/storage/ipc/procarray.c:1413
&
17 00:00:00 2001
From: Alex Shulgin
Date: Tue, 9 Dec 2014 16:35:14 +0300
Subject: [PATCH] WIP: track TRUNCATEs in pgstat transaction stats.
The n_live_tup and n_dead_tup counters need to be set to 0 after a
TRUNCATE on the relation. We can't issue a special message to the stats
collector becau
Alex Shulgin writes:
>
> Bruce Momjian writes:
>>
>> Added to TODO:
>>
>> o Clear table counters on TRUNCATE
>>
>> http://archives.postgresql.org/pgsql-hackers/2008-04/msg00169.php
>
> Hello,
>
> Attached is a WIP patch
; they exist; see check_for_prepared_transactions().
Alright, that's good to know. So I'm just adding a new bool field to
the 2PC pgstat record instead of the bit magic.
Attached is v0.2, now with a regression test included.
--
Alex
>From 4c8fae27ecd9c94e7c3da0997f03099045a152d9
Magnus Hagander writes:
>>
>> You should add it to the next CF for proper tracking, there are already many
>> patches in the queue waiting for reviews :)
>
> Absolutely - I just wanted those that were already involved in the
> thread to get a chance to look at it early :) didn't want to submit it
Alex Shulgin writes:
> Alex Shulgin writes:
>>
>> Here's an attempt to revive this patch.
>
> Here's the patch rebased against current HEAD, that is including the
> recently committed action_at_recovery_target option.
>
> The default for
*.[ch] files and removed incorrect copyright line.
I'm moving this to the current CF.
--
Alex
>From 18388300f9ed34fa47c66b8a2da098aeb19a7f71 Mon Sep 17 00:00:00 2001
From: Alex Shulgin
Date: Wed, 19 Nov 2014 19:49:29 +0300
Subject: [PATCH] DRAFT: ssl_protocols GUC
---
doc/src/sgml/config.sgml
Peter Eisentraut writes:
> On 10/16/14 11:34 PM, Craig Ringer wrote:
>> psql: FATAL: Peer authentication failed for user "fred"
>> HINT: See the server error log for additional information.
>
> I think this is wrong for many reasons.
>
> I have never seen an authentication system that responds
ls/git-external-diff'.
Now that I try to apply it using git, looks like git doesn't like the
copied context diff very much...
>From cc51823a01a194ef6fcd90bc763fa26498837322 Mon Sep 17 00:00:00 2001
From: Alex Shulgin
Date: Tue, 9 Dec 2014 16:35:14 +0300
Subject: [PATCH] WIP: track TRUNCATEs in pgstat transaction s
Alvaro Herrera writes:
>>
>> Another idea would be exposing pgstat_report_stat(true) at SQL level.
>> That would eleminate the need for explicit pg_sleep(>=0.5), but we'll
>> still need the wait_for_... call to make sure the collector has picked
>> it up.
>
> We already have a stats test that sl
Alvaro Herrera writes:
> Alex Shulgin wrote:
>
>> Alvaro Herrera writes:
>> >>
>> >> Another idea would be exposing pgstat_report_stat(true) at SQL level.
>> >> That would eleminate the need for explicit pg_sleep(>=0.5), but we'll
need to
use the stats gathered prior to truncate. Thus the need to track
insert/update/deletes that happened before first truncate separately.
To the point of making a dedicated pgstat testing tool: let's have
another TODO item?
--
Alex
>From 0b3161191a3ddb999cd9d0da08e1b6088ce07a84 Mon Sep
Alvaro Herrera writes:
> Alex Shulgin wrote:
>
>> OK, I think I have now all bases covered, though the updated patch is
>> not that "pretty".
>>
>> The problem is that we don't know in advance if the (sub)transaction is
>> going to succeed o
Steve Singer writes:
> On 12/15/2014 11:38 AM, Alex Shulgin wrote:
>
>> These are all valid concerns IMHO. Attached is the modified version
>> of the original patch by Craig, addressing the handling of the new
>> hint_log error data field and removing the client-side
Craig Ringer writes:
> On 12/19/2014 11:41 PM, Alex Shulgin wrote:
>> I don't think so. The scenario this patch relies on assumes that the
>> DBA will remember to look in the log if something goes wrong
>
> Well, actually, the whole point was that the user who'
Petr Jelinek writes:
>
> I had a quick look, the patch does not apply cleanly anymore but it's
> just release notes so nothing too bad.
Yes, there were some ongoing changes that touched some parts of this and
I must have missed the latest one (or maybe I was waiting for it to
settle down).
> I
Alex Shulgin writes:
> Petr Jelinek writes:
>>
>> I had a quick look, the patch does not apply cleanly anymore but it's
>> just release notes so nothing too bad.
>
> Yes, there were some ongoing changes that touched some parts of this and
> I must have mi
..
vmstat
zoneinfo
postgres=#
Limited performance testing shows no significant difference between
error-catching and plain code path. For example, timing
copy test_copy1 from program 'seq 100' [exceptions to stdout]
shows similar numbers with or without the added "exceptions to&
On Mon, Oct 3, 2011 at 00:05, Tom Lane wrote:
>
> So at this point I'd vote for just dropping it and always allowing
> custom (that is, qualified) GUC names to be set, whether the prefix
> corresponds to any loaded module or not.
>
> Comments, other proposals?
While working on E.164 telephone num
Greg Smith writes:
> One unicorn I would like to have here would give the CF app a database
> of recent e-mails to pgsql-hackers. I login to the CF app, click on
> "Add recent submission", and anything matching my e-mail address
> appears with a checkbox next to it. Click on the patch submissi
Nick Roosevelt writes:
I am sorry, seems like my new MUA was misconfigured so the two previous
attempts to reply to -hackers@ failed. So here goes again.
>Just reviewed the patch for adding URI connection string support for
>libpg.
Thank you for taking your time on this.
>There s
Andrew Dunstan writes:
> On 01/19/2012 12:59 PM, Alex Shulgin wrote:
>
>> Another idea: introduce some simple tag system in mails sent to -hackers
>> to be treated specially, e.g:
>>
>> @fest add-to-current
>>
>> to add new patch to the commit fest cu
84 matches
Mail list logo
