Re: [HACKERS] Partition-wise join for join between (declaratively) partitioned tables
On 2017/09/09 2:38, Ashutosh Bapat wrote: > On Fri, Sep 8, 2017 at 11:17 AM, Amit Langote wrote: >> I updated the patch to include just those changes. I'm not sure about >> one of the Ashutosh's changes whereby the child PlanRowMark is also passed >> to expand_partitioned_rtentry() to use as the parent PlanRowMark. I think >> the child RTE, child RT index and child Relation are fine, because they >> are necessary for creating AppendRelInfos in a desired way for later >> planning steps. But PlanRowMarks are not processed within the planner >> afterwards and do not need to be marked with the immediate parent-child >> association in the same way that AppendRelInfos need to be. > > Passing top parent's row mark works today, since there is no > parent-child specific translation happening there. But if in future, > we introduce such a translation, row marks for indirect children in a > multi-level partitioned hierarchy won't be accurate. So, I think it's > better to pass row marks of the direct parent. IMHO, we should make it the responsibility of the future patch to set a child PlanRowMark's prti to the direct parent's RT index, when we actually know that it's needed for something. We clearly know today why we need to pass the other objects like child RT entry, RT index, and Relation, so we should limit this patch to pass only those objects to the recursive call. That makes this patch a relatively easy to understand change. >> I also included the changes to add_paths_to_append_rel() from my patch on >> the "path toward faster partition pruning" thread. We'd need that change, >> because while add_paths_to_append_rel() is called for all partitioned >> table RTEs in a given partition tree, expand_inherited_rtentry() would >> have set up a PartitionedChildRelInfo only for the root parent, so >> get_partitioned_child_rels() would not find the same for non-root >> partitioned table rels and crash failing the Assert. The changes I made >> are such that we call get_partitioned_child_rels() only for the parent >> rels that are known to correspond root partitioned tables (or as you >> pointed out on the thread, "the table named in the query" as opposed those >> added to the query as result of inheritance expansion). In addition to >> the relkind check on the input RTE, it would seem that checking that the >> reloptkind is RELOPT_BASEREL would be enough. But actually not, because >> if a partitioned table is accessed in a UNION ALL query, reloptkind even >> for the root partitioned table (the table named in the query) would be >> RELOPT_OTHER_MEMBER_REL. The only way to confirm that the input rel is >> actually the root partitioned table is to check whether its parent rel is >> not RTE_RELATION, because the parent in case of UNION ALL Append is a >> RTE_SUBQUERY RT entry. >> > > There was a change in my 0003 patch, which fixed the crash. It checked > for RELOPT_BASEREL and RELKIND_PARTITIONED_TABLE. I have pulled it in > my 0001 patch. It no more crashes. I tried various queries involving > set operations and bare multi-level partitioned table scan with my > patch, but none of them showed any anomaly. Do you have a testcase > which shows problem with my patch? May be your suggestion is correct, > but corresponding code implementation is slightly longer than I would > expect. So, we should go with it, if there is corresponding testcase > which shows why it's needed. If we go with your patch, partitioned tables won't get locked, for example, in case of the following query (p is a partitioned table): select 1 from p union all select 2 from p; That's because the RelOptInfos for the two instances of p in the above query are RELOPT_OTHER_MEMBER_REL, not RELOPT_BASEREL. They are children of the Append corresponding to the UNION ALL subquery RTE. So, partitioned_rels does not get set per your proposed code. > > In your patch > > +parent_rel = root->simple_rel_array[parent_relid]; > +get_pcinfo = (parent_rel->rtekind == RTE_SUBQUERY); > > Do you mean RTE_RELATION as you explained above? No, I mean RTE_SUBQUERY. If the partitioned table RTE in question corresponds to one named in the query, we should be able to find its pcinfo in root->pcinfo_list. If the partitioned table RTE is one added as result of inheritance expansion, it won't have an associated pcinfo. So, we should find a way to distinguish them from one another. The first idea that had occurred to me was the same as yours -- RelOptInfo of the partitioned table RTE named in the query would be of reloptkind RELOPT_BASEREL and those of the partitioned table RTE added as result of inheritance expansion will be of reloptkind RELOPT_OTHER_MEMBER_REL. Although the latter is always true, the former is not. If the partitioned table named in the query appears under UNION ALL query, then its reloptkind will be RELOPT_OTHER_MEMBER_REL. That means we have to use some other means to distinguish partitioned table RTEs that have an asso
Re: [JDBC] [HACKERS] Channel binding support for SCRAM-SHA-256
On Mon, Aug 21, 2017 at 9:51 PM, Michael Paquier wrote: > On Tue, Jun 20, 2017 at 1:11 PM, Michael Paquier > wrote: >> With the tests directly in the patch, things are easy to run. WIth >> PG10 stabilization work, of course I don't expect much feedback :) >> But this set of patches looks like the direction we want to go so as >> JDBC and libpq users can take advantage of channel binding with SCRAM. > > Attached is a new patch set, rebased as of c6293249. And again a new set to fix the rotten bits caused by 85f4d63. -- Michael 0001-Refactor-routine-to-test-connection-to-SSL-server.patch Description: Binary data 0002-Support-channel-binding-tls-unique-in-SCRAM.patch Description: Binary data 0003-Add-connection-parameters-saslname-and-saslchannelbi.patch Description: Binary data 0004-Implement-channel-binding-tls-server-end-point-for-S.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
Re: [HACKERS] CLUSTER command progress monitor
On 2017/09/08 18:55, Robert Haas wrote:
On Wed, Aug 30, 2017 at 10:12 PM, Tatsuro Yamada
wrote:
1. scanning heap
2. sort tuples
These two phases overlap, though. I believe progress reporting for
sorts is really hard. In the simple case where the data fits in
work_mem, none of the work of the sort gets done until all the data is
read. Once you switch to an external sort, you're writing batch
files, so a lot of the work is now being done during data loading.
But as the number of batch files grows, the final merge at the end
becomes an increasingly noticeable part of the cost, and eventually
you end up needing multiple merge passes. I think we need some smart
way to report on sorts so that we can tell how much of the work has
really been done, but I don't know how to do it.
Thanks for the comment.
As you know, CLUSTER command uses SEQ SCAN or INDEX SCAN as a scan method by
cost estimation. In the case of SEQ SCAN, these two phases not overlap.
However, in INDEX SCAN, it overlaps. Therefore I created the phase of "scan
heap and write new heap" when INDEX SCAN was selected.
I agree that progress reporting for sort is difficult. So it only reports
the phase ("sorting tuples") in the current design of progress monitor of
cluster.
It doesn't report counter of sort.
heap_tuples_total | bigint | | |
The patch is getting the value reported as heap_tuples_total from
OldHeap->rd_rel->reltuples. I think this is pointless: the user can
see that value anyway if they wish. The point of the progress
counters is to expose things the user couldn't otherwise see. It's
also not necessarily accurate: it's only an estimate in the best case,
and may be way off if the relation has recently be extended by a large
amount. I think it's pretty important that we try hard to only report
values that are known to be accurate, because users hate (and mock)
inaccurate progress reports.
Do you mean to use the number of rows by using below calculation instead
OldHeap->rd_rel->reltuples?
estimate rows = physical table size / average row length
I understand that OldHeap->rd_rel->reltuples is sometimes useless because
it is correct by auto analyze and it can't perform when under a threshold.
I'll add it in next patch and also share more detailed the current design of
progress monitor for cluster.
Regards,
Tatsuro Yamada
--
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] Setting pd_lower in GIN metapage
On Sun, Sep 10, 2017 at 9:56 PM, Tom Lane wrote: > Amit Kapila writes: >> On Sun, Sep 10, 2017 at 11:52 AM, Michael Paquier >> wrote: >>> Coordinating efforts here would be nice. If you, Amit K, are taking >>> care of a patch for btree and hash > >> I think here we should first agree on what we want to do. Based on >> Tom's comment, I was thinking of changing comments in btree/hash part >> and additionally for hash indexes, I can see if we can pass >> REGBUF_STANDARD for all usages of metapage. I am not sure if we want >> similar exercise for btree as well. > > FWIW, now that we've noticed the discrepancy, I'm for using > REGBUF_STANDARD or equivalent for all metapage calls. Even if it > saves no space, inconsistency is bad because it's confusing. > Agreed. However, I feel there is no harm in doing in two patches, one for hash/btree and second for all other indexes (or maybe separate patches for them as well; I haven't yet looked into the work involved for other indexes) unless you prefer to do it all at a one-shot. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.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] The case for removing replacement selection sort
On Sun, Sep 10, 2017 at 5:59 PM, Tomas Vondra wrote: > OK, so 1MB, 4MB, 8MB, 32MB? Right. > Ah, so you suggest doing all the tests on current master, by only > tweaking the replacement_sort_tuples value? I've been testing master vs. > your patch, but I guess setting replacement_sort_tuples=0 should have > the same effect. I think that there may be a very slight advantage to RS-free performance with my patch actually applied, because it removes the number of instructions that heap maintenance (merging) routines consist of. This is due to my removing HEAPCOMPARE()/tupindex handling. However, it's probably a low single digit percentage difference -- not enough to be worth taking into account, probably. I assume that just setting replacement_sort_tuples to zero is more convenient for you (that's what I did). To be clear, you'll still need to set replacement_sort_tuples high when testing RS, to make sure that we really use it for at least the first run when we're expected to. (There is no easy way to have testing mechanically verify that we really do only have one run in the end with RS, but I assume that such paranoia is unneeded.) > I probably won't eliminate the random/DESC data sets, though. At least > not from the two smaller data sets - I want to do a bit of benchmarking > on Heikki's polyphase merge removal patch, and for that patch those data > sets are still relevant. Also, it's useful to have a subset of results > where we know we don't expect any change. Okay. The DESC dataset is going to make my patch look good, because it won't change anything for merging (same number of runs in the end), but sorting will be slower for the first run with RS. > Meh, more data is probably better. And with the reduced work_mem values > and skipping of random/DESC data sets it should complete much faster. Note that my own test case had a much higher number of tuples than even 10 million -- it had 100 million tuples. I think that if any of your test cases bring about a new insight, it will not be due to the number of distinct tuples. But, if the extra time it takes doesn't matter to you, then it doesn't matter to me either. -- Peter Geoghegan -- 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] Setting pd_lower in GIN metapage
On Mon, Sep 11, 2017 at 1:26 AM, Tom Lane wrote: > FWIW, now that we've noticed the discrepancy, I'm for using > REGBUF_STANDARD or equivalent for all metapage calls. Even if it > saves no space, inconsistency is bad because it's confusing. OK, I don't mind having a more aggressive approach, but it would definitely be nicer if the REGBUF additions are done in a second patch that can be applied on top of the one setting pd_lower where needed. Both things are linked, still are aimed to solve different problems. -- Michael -- 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] Still another race condition in recovery TAP tests
On Sun, Sep 10, 2017 at 12:38 AM, Tom Lane wrote: > Michael Paquier writes: >> On Sat, Sep 9, 2017 at 1:43 PM, Tom Lane wrote: >>> Yeah, even if we fixed this particular call site, I'm sure the issue >>> would come up again. Certainly we expect hot backups to work with >>> a changing source directory. > >> In short, I'd still like to keep RecursiveCopy for now, but change its >> code so as a copy() is not a hard failure. What do you think? > > The specific case we need to allow is "ENOENT on a file/dir that was > there a moment ago". I think it still behooves us to complain about > anything else. If you think it's a simple fix, have at it. But > I see at least three ways for _copypath_recurse to fail depending on > exactly when the file disappears. I have finally been able to take a couple of hours and looked at problems in this module. Errno is supported down to 5.8.8 per the docs, and those are set also on Windows, copy() and opendir() complain with ENOENT for missing entries: http://perldoc.perl.org/5.8.8/Errno.html readdir() does not though. If for example a directory gets removed in the middle of a scan, then an empty list is returned. mkdir() would not fail on ENOENT either, the parent directory would have already been created if a child repository is being scanned. With the check for -d and -f, this brings indeed the count to three code paths. With the patch attached, I have added some manual sleep calls in RecursiveCopy.pm and triggered manual deletions of the source repository. The copy is able to complete in any case, warning about missing directories or files. I have added warn messages in the patch when ENOENT is triggered, though I think that those should be removed in the final patch. By the way, 010_logical_decoding_timelines.pl does not need to include RecursiveCopy. -- Michael tap-copypath-enoent.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
Re: [HACKERS] The case for removing replacement selection sort
On 09/11/2017 02:22 AM, Peter Geoghegan wrote: > On Sun, Sep 10, 2017 at 5:07 PM, Tomas Vondra > wrote: >> I'm currently re-running the benchmarks we did in 2016 for 9.6, but >> those are all sorts with a single column (see the attached script). But >> it'd be good to add a few queries testing sorts with multiple keys. We >> can either tweak some of the existing data sets + queries, or come up >> with entirely new tests. > > I see that work_mem is set like this in the script: > > "for wm in '1MB' '8MB' '32MB' '128MB' '512MB' '1GB'; do" > > I suggest that we forget about values over 32MB, since the question of > how well quicksort does there was settled by your tests in 2016. I > would also add '4MB' to the list of wm values that you'll actually > test. OK, so 1MB, 4MB, 8MB, 32MB? > > Any case with input that is initially in random order or DESC sort > order is not interesting, either. I suggest you remove those, too. > OK. > I think we're only interested in benchmarks where replacement > selection really does get its putative best case (no merge needed in > the end). Any (almost) sorted cases (the only cases that you are > interesting to test now) will always manage that, once you set > replacement_sort_tuples high enough, and provided there isn't even a > single tuple that is completely out of order. The "before" cases here > should have a replacement_sort_tuples of 1 billion (so that we're sure > to not have the limit prevent the use of replacement selection in the > first place), versus the "after" cases, which should have a > replacement_sort_tuples of 0 to represent my proposal (to represent > performance in a world where replacement selection is totally > removed). > Ah, so you suggest doing all the tests on current master, by only tweaking the replacement_sort_tuples value? I've been testing master vs. your patch, but I guess setting replacement_sort_tuples=0 should have the same effect. I probably won't eliminate the random/DESC data sets, though. At least not from the two smaller data sets - I want to do a bit of benchmarking on Heikki's polyphase merge removal patch, and for that patch those data sets are still relevant. Also, it's useful to have a subset of results where we know we don't expect any change. >> For the existing queries, I should have some initial results >> tomorrow, at least for the data sets with 100k and 1M rows. The >> tests with 10M rows will take much more time (it takes 1-2hours for >> a single work_mem value, and we're testing 6 of them). > > I myself don't see that much value in a 10M row test. > Meh, more data is probably better. And with the reduced work_mem values and skipping of random/DESC data sets it should complete much faster. regards -- Tomas Vondra http://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, 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] [Proposal] Allow users to specify multiple tables in VACUUM commands
On Mon, Sep 11, 2017 at 9:38 AM, Bossart, Nathan wrote: > I agree that it is nice to see when relations are skipped, but I do not > know if the WARNING messages would provide much value for this > particular use case (i.e. 'VACUUM;'). If a user does not provide a list > of tables to VACUUM, they might not care too much about WARNINGs for > dropped tables. Some users trigger manual VACUUM with cron jobs in moments of low-activity as autovacuum may sometimes not be able to keep up with hte bloat cleanup. It seems to me that getting WARNING messages is particularly important for partitioned tables. -- Michael -- 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] Allow users to specify multiple tables in VACUUM commands
On 9/9/17, 7:28 AM, "Michael Paquier" wrote:
> In the duplicate patch, it seems to me that you can save one lookup at
> the list of VacuumRelation items by checking for column duplicates
> after checking that all the columns are defined. If you put the
> duplicate check before closing the relation you can also use the
> schema name associated with the Relation.
I did this so that the ERROR prioritization would be enforced across all
relations. For example:
VACUUM ANALYZE table1 (a, a), table2 (does_not_exist);
If I combine the 'for' loops to save a lookup, this example behaves
differently. Instead of an ERROR for the nonexistent column, you would
hit an ERROR for the duplicate column in table1's list. However, I do
not mind changing this.
> + if (i == InvalidAttrNumber)
> + ereport(ERROR,
> + (errcode(ERRCODE_UNDEFINED_COLUMN),
> +errmsg("column \"%s\" of relation \"%s\" does not exist",
> + col, RelationGetRelationName(rel;
> This could use the schema name unconditionally as you hold a Relation
> here, using RelationGetNamespace().
Sure, I think this is a good idea. I'll make this change in the next
version of the patch.
> if (!onerel)
> + {
> + /*
> +* If one of the relations specified by the user has disappeared
> +* since we last looked it up, let them know so that they do not
> +* think it was actually analyzed.
> +*/
> + if (!IsAutoVacuumWorkerProcess() && relation)
> + ereport(WARNING,
> + (errmsg("skipping \"%s\" --- relation no longer exists",
> + relation->relname)));
> +
> return;
> + }
> Hm. So if the relation with the defined OID is not found, then we'd
> use the RangeVar, but this one may not be set here:
> + /*
> +* It is safe to leave everything except the OID empty here.
> +* Since no tables were specified in the VacuumStmt, we know
> +* we don't have any columns to keep track of. Also, we do
> +* not need the RangeVar, because it is only used for error
> +* messaging when specific relations are chosen.
> +*/
> + rel_oid = HeapTupleGetOid(tuple);
> + relinfo = makeVacuumRelation(NULL, NIL, rel_oid);
> + vacrels_tmp = lappend(vacrels_tmp, relinfo);
> So if the relation is analyzed but skipped, we would have no idea that
> it actually got skipped because there are no reports about it. That's
> not really user-friendly. I am wondering if we should not instead have
> analyze_rel also enforce the presence of a RangeVar, and adding an
> assert at the beginning of the function to undertline that, and also
> do the same for vacuum(). It would make things also consistent with
> vacuum() which now implies on HEAD that a RangeVar *must* be
> specified.
I agree that it is nice to see when relations are skipped, but I do not
know if the WARNING messages would provide much value for this
particular use case (i.e. 'VACUUM;'). If a user does not provide a list
of tables to VACUUM, they might not care too much about WARNINGs for
dropped tables.
> Are there opinions about back-patching the patch checking for
> duplicate columns? Stable branches now complain about an unhelpful
> error message.
I wouldn't mind drafting something up for the stable branches.
Nathan
--
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] The case for removing replacement selection sort
On Sun, Sep 10, 2017 at 5:07 PM, Tomas Vondra wrote: > I'm currently re-running the benchmarks we did in 2016 for 9.6, but > those are all sorts with a single column (see the attached script). But > it'd be good to add a few queries testing sorts with multiple keys. We > can either tweak some of the existing data sets + queries, or come up > with entirely new tests. I see that work_mem is set like this in the script: "for wm in '1MB' '8MB' '32MB' '128MB' '512MB' '1GB'; do" I suggest that we forget about values over 32MB, since the question of how well quicksort does there was settled by your tests in 2016. I would also add '4MB' to the list of wm values that you'll actually test. Any case with input that is initially in random order or DESC sort order is not interesting, either. I suggest you remove those, too. I think we're only interested in benchmarks where replacement selection really does get its putative best case (no merge needed in the end). Any (almost) sorted cases (the only cases that you are interesting to test now) will always manage that, once you set replacement_sort_tuples high enough, and provided there isn't even a single tuple that is completely out of order. The "before" cases here should have a replacement_sort_tuples of 1 billion (so that we're sure to not have the limit prevent the use of replacement selection in the first place), versus the "after" cases, which should have a replacement_sort_tuples of 0 to represent my proposal (to represent performance in a world where replacement selection is totally removed). > For the existing queries, I should have some initial results tomorrow, > at least for the data sets with 100k and 1M rows. The tests with 10M > rows will take much more time (it takes 1-2hours for a single work_mem > value, and we're testing 6 of them). I myself don't see that much value in a 10M row test. Thanks for volunteering to test! -- Peter Geoghegan -- 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] Automatic testing of patches in commit fest
On Mon, Sep 11, 2017 at 12:10 PM, Jeff Janes wrote: > On Sun, Sep 10, 2017 at 4:40 PM, Michael Paquier >> http://commitfest.cputube.org/ > > This looks very interesting. But when I click on a "build: failing" icon, > it takes me to a generic page, not one describing how that specific build > is failing. True. The problem is that travis-ci.org doesn't seem to provide a URL that can take you directly to the latest build for a given branch, instead I'd need to figure out how to talk to their API to ask for the build ID of the latest build for that branch. For now you have to note the Commitfest entry ID, and then when find the corresponding branch (ie commitfest/14/1234) on the page it dumps you on. It would be nice to fix that. -- Thomas Munro http://www.enterprisedb.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] Automatic testing of patches in commit fest
On Mon, Sep 11, 2017 at 11:40 AM, Michael Paquier wrote: > Thomas Munro has hacked up a prototype of application testing > automatically if patches submitted apply and build: > http://commitfest.cputube.org/ > > I would recommend have a look at it from time to time if you are a > patch author (or a reviewer) as any failure may say that your patch > has rotten over time and needs a rebase. It is good to keep the commit > fest entries build-clean at least for testers. I should add: this is a spare-time effort, a work-in-progress and building on top of a bunch of hairy web scraping, so it may take some time to perfect. One known problem at the moment is that the mail archive (where patches are fetched from) cuts off long threads so it doesn't manage to find the latest version of (for example) the Partition-Wise Join patch. Other problems include getting confused by incidental material posted in .patch form, patches that depend on other patches or patches whose apply order is not obvious, or ... etc etc. There are also a few other patches missing currently for various reasons, which I intend to fix, gradually. That said, it might already be useful for catching bitrot and things like TAP test failures, contrib regressions and documentation build errors which (based on the evidence I've seen so far) many submitters aren't actually running themselves. YMMV. I've been having conversations with Magnus and Stephen about what a more robust version integrated with the CF app might eventually look like. The basic idea here is: Commitfest submissions are analogous to pull request, which many comparable projects test automatically using CI technology that is generously made available to open source projects and well integrated with popular free git hosts. We should too! -- Thomas Munro http://www.enterprisedb.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] Statement timeout behavior in extended queries
Hi, On 2017-09-11 09:10:49 +0900, Tatsuo Ishii wrote: > If you don't mind, can you please commit/push the patch? Ok, will do so. Greetings, Andres Freund -- 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] Automatic testing of patches in commit fest
On Sun, Sep 10, 2017 at 4:40 PM, Michael Paquier wrote: > Hi all, > > Thomas Munro has hacked up a prototype of application testing > automatically if patches submitted apply and build: > http://commitfest.cputube.org/ > > I would recommend have a look at it from time to time if you are a > patch author (or a reviewer) as any failure may say that your patch > has rotten over time and needs a rebase. It is good to keep the commit > fest entries build-clean at least for testers. > Thanks, This looks very interesting. But when I click on a "build: failing" icon, it takes me to a generic page, not one describing how that specific build is failing. Cheers, Jeff
Re: [HACKERS] Statement timeout behavior in extended queries
Andres, If you don't mind, can you please commit/push the patch? Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp >> What do you think? I've not really tested this with the extended >> protocol, so I'd appreciate if you could rerun your test from the >> older thread. > > Attached is your patch just rebased against master branch. > > Also I attached the test cases and results using pgproto on patched > master branch. All looks good to me. > > Best regards, > -- > Tatsuo Ishii > SRA OSS, Inc. Japan > English: http://www.sraoss.co.jp/index_en.php > Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] The case for removing replacement selection sort
On 09/11/2017 01:03 AM, Peter Geoghegan wrote: > On Sat, Sep 9, 2017 at 8:28 AM, Greg Stark wrote: >> This may be a bit "how long is a piece of string" but how do those two >> compare with string sorting in an interesting encoding/locale -- say >> /usr/share/dict/polish in pl_PL for example. It's certainly true that >> people do sort text as well as numbers. > > I haven't looked at text, because I presume that it's very rare for > tuples within a table to be more or less ordered by a text > attribute. While the traditional big advantage of replacement > selection has always been its ability to double initial run length on > average, where text performance is quite interesting because > localized clustering still happens, that doesn't really seem relevant > here. The remaining use of replacement selection is expressly only > about the "single run, no merge" best case, and in particular about > avoiding regressions when upgrading from versions prior to 9.6 (9.6 > is the version where we began to generally prefer quicksort). > >> Also, people often sort on keys of more than one column > > Very true. I should test this. > I'm currently re-running the benchmarks we did in 2016 for 9.6, but those are all sorts with a single column (see the attached script). But it'd be good to add a few queries testing sorts with multiple keys. We can either tweak some of the existing data sets + queries, or come up with entirely new tests. The current tests construct data sets with different features (unique or low/high-cardinality, random/sorted/almost-sorted). How should that work for multi-key sorts? Same features for all columns, or some mix? For the existing queries, I should have some initial results tomorrow, at least for the data sets with 100k and 1M rows. The tests with 10M rows will take much more time (it takes 1-2hours for a single work_mem value, and we're testing 6 of them). But perhaps we can reduce the number of tests somehow, only testing the largest/smallest work_mem values? So that we could get some numbers now, possibly running the complete test suite later. regards -- Tomas Vondra http://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services sort-bench.sh Description: application/shellscript -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Automatic testing of patches in commit fest
Hi all, Thomas Munro has hacked up a prototype of application testing automatically if patches submitted apply and build: http://commitfest.cputube.org/ I would recommend have a look at it from time to time if you are a patch author (or a reviewer) as any failure may say that your patch has rotten over time and needs a rebase. It is good to keep the commit fest entries build-clean at least for testers. Thanks, -- Michael -- 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] Pattern based listeners for asynchronous messaging (LISTEN/NOTIFY)
Hi Marko! Thanks for the good feedback. Good point on the pg_listening_channels(). Do you think we could change the interface of the function? At least PG v10 has changed functions elsewhere quite dramatically eg. related to xlog functions. We could change the pg_listening_channels() return type to 'setof record' which would include name (text) and isPattern (bool). This would also allow some future additions (eg. time of last received notification). One other way with better backward compatibility would be to add some kind of postfix to patterns to identify them and keep the 'text' return type. It would return eg. 'foo% - PATTERN' for a pattern, but this is quite nasty. Second way would be to add totally new function eg. 'pg_listening_channels_info' with return type of 'setof record' and keep the original function as it is (just change the behaviour on duplicated names and patterns as you showed). You also have a good point on the UNLISTEN. At first I thought that UNLISTEN SIMILAR TO 'xxx' would be semantically confusing since it fells that it would do some pattern based unregistering. But by documenting the behaviour it might be ok. I also agree that it would be best to have distinction between unlistening patterns and regular names. I'll reflect on the rest of the feedback on the next patch if we reach some conclusion on the pg_listening_channels and UNLISTEN. BR Markus 2017-09-08 2:44 GMT+03:00 Marko Tiikkaja : > Hi Markus, > > On Sun, Aug 20, 2017 at 9:56 PM, Markus Sintonen < > markus.sinto...@gmail.com> wrote: > >> I also encountered this when I built it with different configuration. I >> attached updated patch with the correct number of arguments to >> 'similar_escape'. I also added preliminary documentation to the patch. >> (Unfortunately unable to currently compile the documentation for testing >> purpose on Windows probably because of commit https://github.com/post >> gres/postgres/commit/510074f9f0131a04322d6a3d2a51c87e6db243f9. I >> followed https://www.postgresql.org/docs/devel/static/install-windows >> -full.html#idm45412738673840.) >> >> What do you think about the syntax? There was a suggestion to specify >> type of the pattern (eg ltree extension) but to me this feels like a >> overkill. >> One option here would be eg: >> LISTEN PATTERN 'foo%' TYPE 'similar' >> LISTEN PATTERN 'foo.*' TYPE 'ltree' >> > > Not that my opinion matters, but I think we should pick one pattern style > and stick to it. SIMILAR TO doesn't seem like the worst choice. ltree > seems useless. > > As for the rest of the interface.. > > First, I think mixing patterns and non-patterns is weird. This is > apparent in at least two cases: > > marko=# listen "foo%"; > LISTEN > marko=# listen similar to 'foo%'; > LISTEN > marko=# select * from pg_listening_channels(); > pg_listening_channels > --- > foo% > (1 row) > > -- Not actually listening on the pattern. Confusion. > > The second case being the way UNLISTEN can be used to unlisten patterns, > too. It kind of makes sense given that you can't really end up with both a > channel name and a pattern with the same source string, but it's still > weird. I think it would be much better to keep these completely separate > so that you could be listening on both the channel "foo%" and the pattern > 'foo%' at the same time, and you'd use a bare UNLISTEN to unsubscribe from > the former, and UNLISTEN SIMILAR TO for the latter. As you said in the > original email: > > > Allow *UNLISTEN SIMILAR TO 'xxx'* which would unregister matching > listeners. To me this feels confusing therefore it is not in the patch. > > I agree, starting to match the patterns themselves would be confusing. So > I think we should use that syntax for unsubscribing from patterns. But > others should feel free to express their opinions on this. > > Secondly -- and this is a kind of continuation to my previous point of > conflating patterns and non-patterns -- I don't think you can get away with > not changing the interface for pg_listening_channels(). Not knowing which > ones are compared byte-for-byte and which ones are patterns just seems > weird. > > > As for the patch itself, I have a couple of comments. I'm writing this > based on the latest commit in your git branch, commit > fded070f2a56024f931b9a0f174320eebc362458. > > In queue_listen(), the new variables would be better declared at the > innermost scope possible. The datum is only used if isSimilarToPattern is > true, errormsg only if compile_regex didn't return REG_OKAY, etc.. > > I found this comment confusing at first: "If compiled RE was not applied > as a listener then it is freed at transaction commit." The past tense > makes it seem like something that has already happened when that code runs, > when in reality it happens later in the transaction. > > I'm not a fan of the dance you're doing with pcompreg. I think it would > be better to optimistically allocate the
[HACKERS] Constifying numeric.c's local vars
Hi,
For JIT inlining currently functions can't be inlined if they reference
non-constant static variables. That's because there's no way, at least
none I know of, to link to the correct variable, instead of duplicating,
the linker explicitly renames symbols after all (that's the whole point
of static vars). There's a bunch of weird errors if you ignore that ;)
One large user of unnecessary non-constant static variables is
numeric.c. More out of curiosity - numeric is slow enough in itself to
make inlining not a huge win - I converted it to use consts.
before:
andres@alap4:~/build/postgres/dev-assert/vpath$ size --format=SysV
src/backend/utils/adt/numeric.o |grep -v debug|grep -v '^.group'
src/backend/utils/adt/numeric.o :
section size addr
.text 68099 0
.data 64 0
.bss 2 0
.rodata 4256 0
.data.rel.local 224 0
.comment 29 0
.note.GNU-stack0 0
.eh_frame 5976 0
Total 395590
after:
$ size --format=SysV src/backend/utils/adt/numeric.o |grep -v debug|grep -v
'^.group'
src/backend/utils/adt/numeric.o :
sectionsize addr
.text 68108 0
.data 0 0
.bss 0 0
.rodata4288 0
.data.rel.ro.local 224 0
.comment 29 0
.note.GNU-stack 0 0
.eh_frame 5976 0
Total395586
Nicely visible that the data is moved from a mutable segment to a
readonly one.
It's a bit ugly that some consts have to be casted away in the constant
definitions, but aside from just inlining the values, I don't quite see
a better solution?
Leaving JIT aside, I think stuff like this is worthwhile on its own...
Greetings,
Andres Freund
>From 2b114f2c91c7bbf80e48ef5c9653748c957bf15f Mon Sep 17 00:00:00 2001
From: Andres Freund
Date: Sun, 10 Sep 2017 16:20:41 -0700
Subject: [PATCH] Constify numeric.c.
---
src/backend/utils/adt/numeric.c | 210 +---
1 file changed, 110 insertions(+), 100 deletions(-)
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index bc01f3c284..ddc44d5179 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -367,59 +367,59 @@ typedef struct NumericSumAccum
* Some preinitialized constants
* --
*/
-static NumericDigit const_zero_data[1] = {0};
-static NumericVar const_zero =
-{0, 0, NUMERIC_POS, 0, NULL, const_zero_data};
+static const NumericDigit const_zero_data[1] = {0};
+static const NumericVar const_zero =
+{0, 0, NUMERIC_POS, 0, NULL, (NumericDigit *) const_zero_data};
-static NumericDigit const_one_data[1] = {1};
-static NumericVar const_one =
-{1, 0, NUMERIC_POS, 0, NULL, const_one_data};
+static const NumericDigit const_one_data[1] = {1};
+static const NumericVar const_one =
+{1, 0, NUMERIC_POS, 0, NULL, (NumericDigit *) const_one_data};
-static NumericDigit const_two_data[1] = {2};
-static NumericVar const_two =
-{1, 0, NUMERIC_POS, 0, NULL, const_two_data};
+static const NumericDigit const_two_data[1] = {2};
+static const NumericVar const_two =
+{1, 0, NUMERIC_POS, 0, NULL, (NumericDigit *) const_two_data};
#if DEC_DIGITS == 4 || DEC_DIGITS == 2
-static NumericDigit const_ten_data[1] = {10};
-static NumericVar const_ten =
-{1, 0, NUMERIC_POS, 0, NULL, const_ten_data};
+static const NumericDigit const_ten_data[1] = {10};
+static const NumericVar const_ten =
+{1, 0, NUMERIC_POS, 0, NULL, (NumericDigit *) const_ten_data};
#elif DEC_DIGITS == 1
-static NumericDigit const_ten_data[1] = {1};
-static NumericVar const_ten =
-{1, 1, NUMERIC_POS, 0, NULL, const_ten_data};
+static const NumericDigit const_ten_data[1] = {1};
+static const NumericVar const_ten =
+{1, 1, NUMERIC_POS, 0, NULL, (NumericDigit *) const_ten_data};
#endif
#if DEC_DIGITS == 4
-static NumericDigit const_zero_point_five_data[1] = {5000};
+static const NumericDigit const_zero_point_five_data[1] = {5000};
#elif DEC_DIGITS == 2
-static NumericDigit const_zero_point_five_data[1] = {50};
+static const NumericDigit const_zero_point_five_data[1] = {50};
#elif DEC_DIGITS == 1
-static NumericDigit const_zero_point_five_data[1] = {5};
+static const NumericDigit const_zero_point_five_data[1] = {5};
#endif
-static NumericVar const_zero_point_five =
-{1, -1, NUMERIC_POS, 1, NULL, const_zero_point_five_data};
+static const NumericVar const_zero_point_five =
+{1, -1, NUMERIC_POS, 1, NULL, (NumericDigit *) const_zero_point_five_data};
#if DEC_DIGITS == 4
-static NumericDigit const_zero_point_nine_data[1] = {9000};
+static const NumericDigit const_zero_point_nine_data[1] = {9000};
#elif DEC_DIGITS == 2
-static NumericDigit const_zero_point_nine_data[1] = {90};
+static const NumericDigit const_zero_point_nine_data[1] = {90};
#elif DEC_DIGITS == 1
-static NumericDigit const_zero_point_nine_data[1] =
Re: [HACKERS] The case for removing replacement selection sort
On Sat, Sep 9, 2017 at 8:28 AM, Greg Stark wrote: > This may be a bit "how long is a piece of string" but how do those two > compare with string sorting in an interesting encoding/locale -- say > /usr/share/dict/polish in pl_PL for example. It's certainly true that > people do sort text as well as numbers. I haven't looked at text, because I presume that it's very rare for tuples within a table to be more or less ordered by a text attribute. While the traditional big advantage of replacement selection has always been its ability to double initial run length on average, where text performance is quite interesting because localized clustering still happens, that doesn't really seem relevant here. The remaining use of replacement selection is expressly only about the "single run, no merge" best case, and in particular about avoiding regressions when upgrading from versions prior to 9.6 (9.6 is the version where we began to generally prefer quicksort). > Also, people often sort on > keys of more than one column Very true. I should test this. -- Peter Geoghegan -- 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: Add --no-comments to skip COMMENTs with pg_dump
Michael, * Michael Paquier (michael.paqu...@gmail.com) wrote: > As there begins to be many switches of this kind and much code > duplication, I think that some refactoring into a more generic switch > infrastructure would be nicer. I have been thinking about this also and agree that it would be nicer, but then these options would just become "shorthand" for the generic switch. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] mysql_fdw + PG10: unrecognized node type: 217
Christoph Berg writes: > I'm not sure if this is a bug in mysql_fdw, or in PG10: > ! ERROR: unrecognized node type: 217 Hm, nodetag 217 is T_List according to gdb. Wouldn't expect that failure in very many places. If you could get a stack trace from the errfinish call, it might help narrow things down. Offhand my bet is on mysql_fdw needing an update for some PG10 change, but that's just a guess. 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
[HACKERS] mysql_fdw + PG10: unrecognized node type: 217
Hi, I'm not sure if this is a bug in mysql_fdw, or in PG10: == running regression test queries== test mysql_fdw... FAILED *** 345,359 NOTICE: Found number Three NOTICE: Found number Four NOTICE: Found number Five ! NOTICE: Found number Six ! NOTICE: Found number Seven ! NOTICE: Found number Eight ! NOTICE: Found number Nine ! test_param_where ! -- ! ! (1 row) ! DELETE FROM employee; DELETE FROM department; DELETE FROM empdata; --- 344,352 NOTICE: Found number Three NOTICE: Found number Four NOTICE: Found number Five ! ERROR: unrecognized node type: 217 ! CONTEXT: SQL statement "select bfrom numbers where a=x" ! PL/pgSQL function test_param_where() line 6 at SQL statement DELETE FROM employee; DELETE FROM department; DELETE FROM empdata; mysql_fdw master at 7d084c59, PG10 at 6913d066. The testsuite was running against this mysql schema: CREATE DATABASE testdb; USE testdb; CREATE USER 'foo'@'127.0.0.1' IDENTIFIED BY 'bar'; GRANT ALL PRIVILEGES ON testdb.* TO 'foo'@'127.0.0.1'; CREATE TABLE department(department_id int, department_name text, PRIMARY KEY (department_id)); CREATE TABLE employee(emp_id int, emp_name text, emp_dept_id int, PRIMARY KEY (emp_id)); CREATE TABLE empdata (emp_id int, emp_dat blob, PRIMARY KEY (emp_id)); CREATE TABLE numbers(a int PRIMARY KEY, b varchar(255)); Christoph -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] relation mapping file checksum failure
A pretty routine "make check-world" run on HEAD just blew up here, with the symptom 2017-09-10 14:27:33.318 EDT [2535] FATAL: relation mapping file "global/pg_filenode.map" contains incorrect checksum 2017-09-10 14:27:33.319 EDT [2519] LOG: autovacuum launcher process (PID 2535) exited with exit code 1 2017-09-10 14:27:33.319 EDT [2519] LOG: terminating any other active server processes This was during a REINDEX DATABASE operation in src/bin/scripts/t/200_connstr.pl, for what that's worth. Not sure what to make of this. Random bit flip? My machine does have ECC memory, though. Race condition against some updater? But the file is only 512 bytes, and we've always assumed that that size of disk read or write is atomic. (Platform is RHEL6/x86_64, kernel is Red Hat 2.6.32-696.10.1.el6.x86_64.) 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] Red-Black tree traversal tests
I wrote: > In the meantime, here's my version. Notable changes: I went ahead and pushed this, with the removal of the preorder/postorder code, so we can see if the buildfarm finds out anything interesting. Feel free to continue to submit improvements though. One thing that occurred to me is that as-is, this is entirely black-box testing. It doesn't try to check that the tree actually satisfies the RB invariants, which is something that is interesting for performance reasons. (That is, the code could pass these tests even though it produces an unbalanced tree with horrible performance.) Is it worth adding logic for that? Not sure. It's not like we are actively changing the RB code or have reason to think it is buggy. 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] Setting pd_lower in GIN metapage
Amit Kapila writes: > On Sun, Sep 10, 2017 at 11:52 AM, Michael Paquier > wrote: >> Coordinating efforts here would be nice. If you, Amit K, are taking >> care of a patch for btree and hash > I think here we should first agree on what we want to do. Based on > Tom's comment, I was thinking of changing comments in btree/hash part > and additionally for hash indexes, I can see if we can pass > REGBUF_STANDARD for all usages of metapage. I am not sure if we want > similar exercise for btree as well. FWIW, now that we've noticed the discrepancy, I'm for using REGBUF_STANDARD or equivalent for all metapage calls. Even if it saves no space, inconsistency is bad because it's confusing. And Michael is correct to point out that we can exploit this to improve WAL consistency checking. 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
[HACKERS] Re: DROP SUBSCRIPTION hangs if sub is disabled in the same transaction
On Thu, Sep 07, 2017 at 04:53:12AM +, Noah Misch wrote: > On Wed, Sep 06, 2017 at 03:28:47PM +0900, Masahiko Sawada wrote: > > On Mon, Sep 4, 2017 at 11:43 PM, Arseny Sher wrote: > > > Arseny Sher writes: > > > > > >> Attached patch fixes this by stopping workers before RO drop, as > > >> already done in case when we drop replication slot. > > > > > > Sorry, here is the patch. > > > > > > > I could reproduce this issue, it's a bug. Added this to the open item. > > The cause of this is commit 7e174fa7 which make subscription ddls kill > > the apply worker only when transaction commit. I didn't look the patch > > deeply yet but I'm not sure the approach that kills apply worker > > before commit would be good. > > [Action required within three days. This is a generic notification.] > > The above-described topic is currently a PostgreSQL 10 open item. Peter, > since you committed the patch believed to have created it, you own this open > item. If some other commit is more relevant or if this does not belong as a > v10 open item, please let us know. Otherwise, please observe the policy on > open item ownership[1] and send a status update within three calendar days of > this message. Include a date for your subsequent status update. Testers may > discover new open items at any time, and I want to plan to get them all fixed > well in advance of shipping v10. Consequently, I will appreciate your efforts > toward speedy resolution. Thanks. > > [1] > https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com This PostgreSQL 10 open item is past due for your status update. Kindly send a status update within 24 hours, and include a date for your subsequent status update. Refer to the policy on open item ownership: https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] Red-black trees: why would anyone want preorder or postorder traversal?
On Sun, Sep 10, 2017 at 3:25 AM, Tom Lane wrote: > In short, therefore, I propose we rip out the DirectWalk and InvertedWalk > options along with their support code, and then drop the portions of > test_rbtree that are needed to exercise them. Any objections? > +1, I don't see any point in leaving DirectWalk and InvertedWalk in RB-tree. -- Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
Re: [HACKERS] log_destination=file
> On 7 September 2017 at 15:46, Dmitry Dolgov <9erthali...@gmail.com> wrote: > > It seems that for this particular workload it was about 20-25% slower. Hmm...looks like I provided misleading data, sorry. The numbers from previous email are correct and I'm able to reproduce them, but surprisingly for me only for one particular situation when `log_statement=all; log_destination=stderr` and stderr is sent to a console with a tmux session running in it (and apparently it has some impact on the performance, but I'm not sure how exactly). In all other cases (when stderr is sent to a plain console, /dev/null, or we send logs to a file) numbers are different, and the difference between patch and master is quite less significant. average latency: clients patch master 10 0.321 0.286 20 0.669 0.602 30 1.016 0.942 40 1.358 1.280 50 1.727 1.637
Re: [HACKERS] Setting pd_lower in GIN metapage
On Sun, Sep 10, 2017 at 3:28 PM, Amit Kapila wrote: > I think here we should first agree on what we want to do. Based on > Tom's comment, I was thinking of changing comments in btree/hash part > and additionally for hash indexes, I can see if we can pass > REGBUF_STANDARD for all usages of metapage. I am not sure if we want > similar exercise for btree as well. Changing only comments for now looks like a good idea to me. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP: Separate log file for extension
On Fri, Aug 25, 2017 at 12:12 AM, Antonin Houska wrote:
> Attached is a draft patch to allow extension to write log messages to a
> separate file. It introduces a concept of a "log stream". The extension's
> shared library gets its stream assigned by calling this function from
> _PG_init()
> my_stream_id = get_log_stream("my_extension", &my_log_stream);
>
I like this idea in general.
> Then it's supposed to change some of its attributes
>
> adjust_log_stream_attr(&stream->filename, "my_extension.log");
>
This, however, seems to be wrong.
The logfile name does not belong in the extension, it belongs in the
configuration file. I think the extension should set it's "stream id" or
whatever you want to call it, and then it should be possible to control in
postgresql.conf where that log is sent.
Also, what if this extension is loaded on demand in a session and not via
shared_preload_libraries? It looks like the syslogger only gets the list of
configured streams when it starts?
In short, I think the solution should be more generic, and not "just for
extensions".
I completely missed this thread when I did my quick-wip at
https://www.postgresql.org/message-id/flat/cabuevexztl0gorywm9s4tr_ft3fmjbraxqdxj+bqzjpvmru...@mail.gmail.com#cabuevexztl0gorywm9s4tr_ft3fmjbraxqdxj+bqzjpvmru...@mail.gmail.com
-- some of the changes made were close enough that I got the two confused
:) Based on the feedback of that one, have you done any performance checks?
--
Magnus Hagander
Me: https://www.hagander.net/
