On Fri, Dec 18, 2020 at 11:51:55AM +0200, Heikki Linnakangas wrote:
> On 18/12/2020 11:35, Heikki Linnakangas wrote:
> > BTW, looking at pg_cryptohash_ctx and pg_cryptohash_state, why do we
> > need two structs? They're both allocated and controlled by the
> > cryptohash implementation. It would se
On Fri, Dec 18, 2020 at 11:30:16AM -0500, Stephen Frost wrote:
> * Heikki Linnakangas (hlinn...@iki.fi) wrote:
>> +1
>
> Yeah, in a quick look, this looks like a good improvement.
Thanks. This has been applied as of 93e8ff8.
--
Michael
signature.asc
Description: PGP signature
On Mon, Dec 21, 2020 at 04:02:29PM +0900, Masahiko Sawada wrote:
> Is it a bug? Since the created schema obviously depends on the
> extension when we created the schema specified in the schema option, I
> think we might want to create the dependency so that DROP EXTENSION
> drops the schema as well
On Wed, Dec 16, 2020 at 10:01:11AM +0900, Michael Paquier wrote:
> On Tue, Dec 15, 2020 at 09:45:17PM -0300, Alvaro Herrera wrote:
> > I don't like this idea too much, because adding an option causes an ABI
> > break. I don't think we commonly add options in backbranches
06520d6f023163024ec5f215b45e9a89f093884 Mon Sep 17 00:00:00 2001
From: Michael Paquier
Date: Tue, 22 Dec 2020 17:35:22 +0900
Subject: [PATCH v4] Fail Fast In CTAS/CMV If Relation Already Exists
Currently, for CTAS or CREATE MATERIALIZED VIEW(CMV) without
IF-NOT-EXISTS clause, the existence of the
On Tue, Dec 22, 2020 at 02:32:05AM -0600, Justin Pryzby wrote:
> Also, this one is going to be subsumed by ExecReindex(), so the palloc will go
> away (otherwise I would ask to pass it in from the caller):
Yeah, maybe. Still you need to be very careful if you have any
allocated variables like a t
Hi all,
While looking at a patch from David, I have noticed $subject:
https://www.postgresql.org/message-id/CAApHDvpgB+vxk=w6opkidwzzeo6knifqidnomzr8p4rotyk...@mail.gmail.com
adminpack and old_snapshot have no need for those references as they
don't use libpq. Any objections to a small-ish clean
On Tue, Dec 22, 2020 at 11:24:40PM +1300, David Rowley wrote:
> On Wed, 11 Nov 2020 at 13:44, Michael Paquier wrote:
>> It seems to me that your patch is doing the right thing for adminpack
>> and that its Makefile has no need to include a reference to libpq
>> source path, n
On Tue, Dec 22, 2020 at 03:15:37PM -0600, Justin Pryzby wrote:
> Now, I really think utility.c ought to pass in a pointer to a local
> ReindexOptions variable to avoid all the memory context, which is unnecessary
> and prone to error.
Yeah, it sounds right to me to just bite the bullet and do this
On Tue, Dec 22, 2020 at 03:12:15PM +0530, Bharath Rupireddy wrote:
> On Tue, Dec 22, 2020 at 2:07 PM Michael Paquier wrote:
>> Note: I'd like to think that we could choose a better name for
>> CheckRelExistenceInCTAS().
>
> I changed it to IsCTASRelCreationAllowed
On Wed, Dec 23, 2020 at 07:07:54PM -0600, Justin Pryzby wrote:
> On Wed, Dec 23, 2020 at 09:14:18PM -0300, Alvaro Herrera wrote:
>> On 2020-Dec-23, Justin Pryzby wrote:
>>> This was getting ugly:
>>>
>>> extern void reindex_index(Oid indexId, bool skip_constraint_checks,
>>>
On Wed, Dec 23, 2020 at 07:13:33PM +0530, Bharath Rupireddy wrote:
> +1. Shall we add some test cases(with xml, yaml, json formats as is
> currently being done in explain.sql) to cover that? We can have the
> explain_filter() function to remove the unstable parts in the output,
> it looks something
On Thu, Dec 24, 2020 at 10:50:34AM +0900, Michael Paquier wrote:
> FWIW, it still makes the most sense to me to keep the options that are
> extracted from the grammar or things that apply to all the
> sub-routines of REINDEX to be tracked in a single structure, so this
> should incl
On Thu, Dec 24, 2020 at 09:10:22AM +0530, Bharath Rupireddy wrote:
> Since I tested that with all the formats manually here and it works,
> so I don't want to make the test cases complicated with adding
> explain_filter() function into matview.sql and select_into.sql and all
> that. I'm okay withou
On Mon, Dec 21, 2020 at 10:11:53PM -0600, Justin Pryzby wrote:
> As I did last 2 years, I reviewed docs for v14...
Thanks for gathering all that!
> This year I've started early, since it takes more than a little effort and
> it's
> not much fun to argue the change in each individual hunk.
0001-
On Mon, Dec 21, 2020 at 04:28:26PM -0500, Robert Haas wrote:
> TBH, I think there's no point in return an error here at all, because
> it's totally non-specific. You have no idea what failed, just that
> something failed. Blech. If we want to check that ctx is non-NULL, we
> should do that with an
On Fri, Dec 25, 2020 at 09:59:09AM +0900, Ian Lawrence Barwick wrote:
> Basically a wrapper around PQconninfoParse(), I've had the code knocking
> around
> for a while now and finally got round to packaging it into an
> extension [1]. It's
> on my todo list to submit a patch based on this to core,
On Thu, Dec 24, 2020 at 07:29:37PM +0900, Masahiko Sawada wrote:
> Thank you. After re-logging in it looks the same as before but
> something will change on the CommitFest page?
There should be a link to a new menu called "administration" on the
left of the existing logout button at the top. From
On Wed, Dec 09, 2020 at 02:13:29PM -0600, Justin Pryzby wrote:
> I thought this was a good idea, but didn't hear back when I raised it before.
>
> Failing to preserve access method is arguably a bug, reminiscent of CREATE
> STATISTICS and 5564c1181. But maybe it's not important to backpatch a fix
On Thu, Dec 03, 2020 at 03:52:47AM +, tsunakawa.ta...@fujitsu.com wrote:
> The code looks good, and the performance seems to be nice, so I
> marked this ready for committer.
FWIW, I am extremely afraid of this proposal because this is basically
a footgun able to corrupt customer instances, and
On Wed, Dec 09, 2020 at 09:52:17AM -0300, Alvaro Herrera wrote:
> On 2020-Dec-09, tsunakawa.ta...@fujitsu.com wrote:
>> The new partition will have a property specified when the user creates
>> it. That is, while the storage property of each storage unit
>> (=partition) is basically independent, A
On Wed, Dec 09, 2020 at 10:56:45AM -0300, Alvaro Herrera wrote:
> Sounds good. I think this thread is a good place to collect those
> patches, but if you would prefer to have a new thread, feel free to
> start one (I'd suggest CC'ing me and Tsunakawa-san).
There is an entry listed in the CF for t
On Fri, Dec 04, 2020 at 03:58:22PM -0300, Alvaro Herrera wrote:
> I don't know if it's possible to determine (at function execution time)
> that we're running with the old extension version; if so it might
> suffice to throw a warning but still have the SQL function run the same
> C function.
Hmm.
On Fri, Dec 25, 2020 at 04:35:30PM +0900, Masahiko Sawada wrote:
> Hmm, on the left of the logout button, I can see only the 'edit
> profile' button and 'Activity log' button.
Maybe that's a cache issue with your browser? Magnus, any ideas?
I cannot control the permissions of the app, but if tha
On Mon, Dec 28, 2020 at 11:42:03AM +0100, Magnus Hagander wrote:
> Not as much "tightly controlled" as "nobody's really bothered to grant any
> permissions".
Magnus, do I have an access to that? This is the second time I am
crossing an issue with this issue, but I don't really know if I should
ac
On Tue, Dec 29, 2020 at 01:59:58PM +1300, Thomas Munro wrote:
> LGTM.
Thanks, I have done this one then.
--
Michael
signature.asc
Description: PGP signature
On Sun, Dec 27, 2020 at 02:26:05PM -0600, Justin Pryzby wrote:
> I think a couple of these should be backpatched.
> doc/src/sgml/ref/pg_dump.sgml
This part can go down to 9.5.
> doc/src/sgml/sources.sgml
Yes, I have done an extra effort on those fixes where needed. On top
of that, I have includ
Hi all,
Buildfarm member gombessa just had an interesting failure:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=gombessa&dt=2020-12-29%2000%3A16%3A49
Seq Scan on int8_tbl i8 (cost=N.N..N.N rows=N width=N) (actual
time=N.N..N.N rows=N loops=N)
Planning Time: N.N ms
- Execution Tim
On Tue, Dec 29, 2020 at 05:06:20PM -0500, Tom Lane wrote:
> 629b3af2 just moved around the existing -I switch in adminpack/Makefile.
> AFAICS that switch goes back to adminpack's introduction, fe59e5666.
> It was probably just copied-and-pasted from some other contrib Makefile;
> since this Makefil
On Tue, Dec 29, 2020 at 04:16:06PM -0500, Tom Lane wrote:
> Hmph, no, a look at explain.c shows that the "Execution Time" is just
> based on the difference of INSTR_TIME_SET_CURRENT measurements taken
> within the current process. It's difficult to conclude anything except
> that the clock went ba
On Tue, Dec 29, 2020 at 02:26:19PM -0600, Paul Martinez wrote:
> The checks for whether the current user can create a user with the SUPERUSER,
> REPLICATION, or BYPASSRLS attributes are chained together using if/else-if,
> before finally checking whether the user has CREATEROLE privileges in a
> fi
On Thu, Dec 24, 2020 at 01:23:40PM +0900, Michael Paquier wrote:
> Please note that I have added an entry in the CF app for the moment so
> as we don't lose track of it:
> https://commitfest.postgresql.org/31/2892/
I have been able to look at that again today, and applied it. I
On Mon, Dec 28, 2020 at 11:06:30AM +, Simon Riggs wrote:
> Agreed, it is a footgun. -1 to commit the patch as-is.
>
> The patch to avoid WAL is simple but it is dangerous for both the user
> and the PostgreSQL project.
Something that has not been mentioned on this thread is that if you
could
On Wed, Dec 30, 2020 at 08:22:07PM -0500, Bruce Momjian wrote:
> So, I am learning this cfbot thing. Seems I need -M100% to disable
> rename detection for diffs to work with cfbot --- makes sense.
A good way to make sure that a patch format is correct for the CF bot
would be to use "git format-pa
TH is a good idea either, I'd like to get
rid of it in the long-term) and ECPG, so that's clearly a gain.
I don't have a Windows host at hand, though I think that it should
work there correctly. What do you think about the ideas in the
attached patch?
--
Michael
From 8952fb2d5e5a7a0
On Fri, Jan 01, 2021 at 08:34:34PM +0100, Michael Banck wrote:
> I think enough people use data checksums these days that it warrants to
> be moved into the "normal part", like in the attached.
+1. Let's see first what others think about this change.
--
Michael
signature.asc
Description: PGP si
On Tue, Dec 29, 2020 at 06:22:43PM +0900, Michael Paquier wrote:
> Yes, I have done an extra effort on those fixes where needed. On top
> of that, I have included catalogs.sgml, pgstatstatements.sgml,
> explain.sgml, pg_verifybackup.sgml and wal.sgml in 13.
Justin, I got to look at th
On Sun, Jan 03, 2021 at 12:33:54AM -0600, Justin Pryzby wrote:
>
> But actually, maybe we should just use the comment that exists everywhere else
> for that.
>
> /* Propagate context related error context to libxml2 */
> xmlSetStructuredErrorFunc((void *) xtCxt->xmlerrcxt,
> xml_
On Mon, Jan 04, 2021 at 10:47:39PM -0500, Bruce Momjian wrote:
> I can see the value of passing the destination length to the hex
> functions, and I think you have to pass the src length to pg_hex_encode
> since the input can be binary. I assume the pg_hex_decode doesn't need
> the source length b
On Tue, Jan 05, 2021 at 09:35:21AM +0530, Amit Kapila wrote:
> As noted in [1], without this the subscriber might again ask for
> rollback prepared lsn after restart.
>
> Attached patch addresses this problem.
Is it possible to add some tests in test_decoding?
/* dump transaction origin info
On Mon, Jan 04, 2021 at 04:12:34PM +0300, Amine Tengilimoglu wrote:
>When I read the pg_rewind PG12 doc. It says:
>
> "... but if the target cluster ran for a long time after the divergence,
> the old WAL files might no longer be present. In that case, they can be
> manually copied from the W
On Sun, Jan 03, 2021 at 09:05:09PM +0900, Michael Paquier wrote:
> So let's use this version and call it a day for this part.
This has been done as of b49154b.
--
Michael
signature.asc
Description: PGP signature
On Mon, Jan 04, 2021 at 07:11:43PM +0100, Michael Banck wrote:
> Am Samstag, den 02.01.2021, 10:47 -0500 schrieb Stephen Frost:
>> * Michael Paquier (mich...@paquier.xyz) wrote:
>>> On Fri, Jan 01, 2021 at 08:34:34PM +0100, Michael Banck wrote:
>>> > I think enough p
On Tue, Jan 05, 2021 at 12:21:09PM -0500, Bruce Momjian wrote:
> Well, if the backend uses /common for hex like I suggested, and like we
> do now, it has to match the function signatures of bytea and esc, see
> struct pg_encoding. I don't see the point in changing those.
Not necessarily with some
On Sat, Dec 19, 2020 at 03:13:50PM +0900, Michael Paquier wrote:
> At the same time, I have taken care of your comment from upthread to
> return a failure if the caller passes NULL for the context, and
> adjusted some comments. What do you think of the attached?
I have looked agai
On Tue, Jan 05, 2021 at 04:24:21PM +0530, Amit Kapila wrote:
> There are already tests [1] in one of the upcoming patches for logical
> decoding of 2PC which covers this code using which I have found this
> problem. So, I thought those would be sufficient. I have not checked
> the feasibility of us
On Mon, Dec 14, 2020 at 12:48:15PM +0900, Michael Paquier wrote:
> This is a nice cleanup, so I have moved ahead and applied it. A
> rebased version of the SHA1 business is attached.
Rebased version attached to address the conflicts caused by 55fe26a.
I have fixed three places in pgcrypto
On Wed, Jan 06, 2021 at 03:27:03PM +0200, Heikki Linnakangas wrote:
> Looks fine to me.
Thanks, I have been able to get this part done as of 55fe26a.
--
Michael
signature.asc
Description: PGP signature
On Wed, Jan 06, 2021 at 03:58:22PM +0200, Heikki Linnakangas wrote:
> contrib/pgcrypto/internal-sha2.c and
> src/backend/utils/adt/cryptohashfuncs.c: the call to pg_cryptohash_create()
> is missing check for NULL result. With your latest patch, that's OK because
> the subsequent pg_cryptohash_updat
On Tue, Jan 05, 2021 at 11:54:42AM +0300, Amine Tengilimoglu wrote:
> Thank you Michael. I agree with you. Relevant part can be removed from the
> document and eliminate the confusion at least.
Okay, I got around this stuff, and committed a fix for 9.6~12. Thanks
for the report, Amine!
--
Micha
On Thu, Jan 07, 2021 at 09:51:00AM +0200, Heikki Linnakangas wrote:
> Hmm. Perhaps it would be best to change all the errors in mock
> authentication to COMMERROR. It'd be nice to have an accurate error message
> in the log, but no need to send it to the client.
Yeah, we could do that. Still, thi
On Fri, Dec 18, 2020 at 03:46:42PM +0900, Michael Paquier wrote:
> This has been tested on Windows and Linux across all the versions of
> OpenSSL we support on HEAD. I am also attaching a small module called
> hmacfuncs that I used as a way to validate this patch across all the
>
On Fri, Jan 08, 2021 at 04:54:47PM +0100, Peter Eisentraut wrote:
> Updated patch that does that.
Thanks. Looks sane seen from here.
+/* LCOV_EXCL_START */
Does it really make sense to add those markers here? It seems to me
that we would ignore any new coverage if regression tests based on
olde
On Fri, Feb 12, 2021 at 03:21:40PM +0900, Kyotaro Horiguchi wrote:
> The v3 drops the changes of the uuid_ossp contrib. I'm not sure the
> change of scram_HMAC_final is needed.
Meaning that v3 would fail to compile uuid-ossp. v3 also produces
compilation warnings in auth-scram.c.
> In v2, int_m
On Fri, Feb 12, 2021 at 04:42:26PM +, Anastasia Lubennikova wrote:
> I wonder, why this patch hangs on commitfest for so long.
> The idea of the fix is clear, the code is correct and all tests pass, so, I
> move it to ReadyForCommitter status.
>
> The new status of this patch is: Ready for C
On Thu, Feb 11, 2021 at 02:50:13PM -0500, Robert Haas wrote:
> +1 for not back-patching whatever we do here.
+1.
--
Michael
signature.asc
Description: PGP signature
On Fri, Feb 12, 2021 at 03:56:02PM +0900, Kyotaro Horiguchi wrote:
> If the return from the first call is a subtransaction, the second call
> always obtain the top transaction. If the top transaction actualy did
> not exist, it's rather the correct behavior to cause SEGV, than
> creating a bogus r
On Fri, Feb 12, 2021 at 10:32:14AM +0900, Ian Lawrence Barwick wrote:
> In the documentation, the "[ NO ]" option is listed in the synopsis for
> ALTER TRIGGER and ALTER FUNCTION, but not the others.
> Trivial patch attached.
There are two flavors to cover for 6 commands per gram.y, and you are
co
On Sat, Feb 13, 2021 at 05:37:32PM -0300, Ranier Vilela wrote:
> IMO there is no necessity in back-patching.
You are missing the point here. What you are proposing here would not
be backpatched. However, reusing the same words as upthread, this has
a cost in terms of *future* maintenance. In sh
On Sat, Feb 13, 2021 at 09:33:48PM -0300, Ranier Vilela wrote:
> Em sáb., 13 de fev. de 2021 às 20:32, Michael Paquier
> escreveu:
>
>> You are missing the point here. What you are proposing here would not
>> be backpatched. However, reusing the same words as upthread,
On Sun, Feb 14, 2021 at 09:29:08AM +0800, Andy Fan wrote:
> Thank you tom for the reply. What would be the difference between the
> SPI and "write a pure SQL UDF" and call it with DirectFunctionCall1? I
> just ran into a similar situation some days before. Currently I think
> DirectFunctionCall1
On Sun, Feb 14, 2021 at 11:39:47AM -0300, Ranier Vilela wrote:
> What do you think?
That's not a good idea for two reasons:
1) There is CRC32 to worry about, which relies on a different logic.
2) It would become easier to miss the new option as compilation would
not warn anymore if a new checksum
On Mon, Feb 15, 2021 at 02:15:51PM +1300, Thomas Munro wrote:
> guc.c: In function ‘RestoreGUCState’:
> guc.c:9455:4: error: ‘varsourceline’ may be used uninitialized in this
> function [-Werror=maybe-uninitialized]
> 9455 |set_config_sourcefile(varname, varsourcefile, varsourceline);
>
On Sun, Feb 14, 2021 at 08:10:50PM -0600, Justin Pryzby wrote:
> Isn't this dead code ?
Nope, it's not dead. Those two code paths can be hit when attempting
a reidex with a tablespace move directly on toast tables and indexes,
see:
=# create table aa (a text);
CREATE TABLE
=# select relname from
On Tue, Feb 09, 2021 at 04:27:34PM +0900, Michael Paquier wrote:
> Putting sanity checks within all the table_* functions of tableam.h
> would not be a good idea, as nothing prevents the call of what's
> stored in rel->rd_tableam.
I have been playing with this idea, and
On Thu, Feb 11, 2021 at 11:30:13PM +0900, Fujii Masao wrote:
> Yes, so what about the attached patch?
I see. So the first error triggering the spinlock error would cause
a transaction failure because the fallback implementation of atomics
uses a spinlock for this variable, and it may not initiali
On Mon, Feb 15, 2021 at 10:47:05PM +1300, Thomas Munro wrote:
> Why not initialise it in WalRcvShmemInit()?
I was thinking about doing that as well, but we have no real need to
initialize this stuff in most cases, say standalone deployments. In
particular for the fallback implementation of atomic
On Sat, Jan 23, 2021 at 01:43:20PM +0900, Michael Paquier wrote:
> Rebased patch is attached wiht SHA1 added as of a8ed6bb. Now that
> SHA1 is part of the set of options for cryptohashes, a lot of code of
> pgcrypto can be cleaned up thanks to the refactoring done here, but
> I am lea
On Mon, Feb 15, 2021 at 09:37:53AM +, Zohar Gofer wrote:
> In my mind the requirement for superuser is too strong. I think that
> requiring privileges of a replication user is more suitable. This
> way we can require that only a user with replication privileges will
> actually do replication, e
On Mon, Feb 15, 2021 at 03:57:04PM +0900, Ian Lawrence Barwick wrote:
> Indeed it does. Not the most exciting of use cases, though I imagine it
> might come in handy for anyone developing an extension, and the
> existing implementation is inconsistent (in place for ALTER INDEX,
> and partially for
On Tue, Feb 16, 2021 at 12:43:42PM +0900, Fujii Masao wrote:
> On 2021/02/16 6:28, Andres Freund wrote:
>> So what? It's just about free to initialize a spinlock, whether it's
>> using the fallback implementation or not. Initializing upon walsender
>> startup adds a lot of complications, because e.
On Tue, Feb 16, 2021 at 07:54:32AM +, Zohar Gofer wrote:
> Thanks. This seems to be the fix we need.
> Would it be possible to push it to previous versions? 12 or 13?
New features don't go into stable branches, only bug fixes do. And
this is not a bug fix, but a feature.
--
Michael
signatur
On Tue, Feb 16, 2021 at 11:18:47AM +0900, Ian Lawrence Barwick wrote:
> Hmm, with the current implementation "alter index my_index no "
> doesn't work
> anyway; you'd need to add this before the above lines:
>
> + else if (Matches("ALTER", "INDEX", MatchAny, "NO"))
> + COMPLETE
On Tue, Feb 16, 2021 at 11:47:52PM +0900, Fujii Masao wrote:
> On 2021/02/16 15:50, Michael Paquier wrote:
>> + /*
>> +* Read "writtenUpto" without holding a spinlock. So it may not be
>> +* consistent with other WAL receiver's shared variables protec
On Tue, Feb 16, 2021 at 12:39:08PM +0100, Matthias van de Meent wrote:
> These were added to report the index and table that are currently
> being worked on in concurrent reindexes of tables, schemas and
> databases. Before that commit, it would only report up to the last
> index being prepared in
On Wed, Feb 17, 2021 at 12:10:43AM -0600, Justin Pryzby wrote:
> On Wed, Feb 17, 2021 at 02:55:04PM +0900, Michael Paquier wrote:
>> I see no bug here.
>
> pg_stat_progress_create_index includes partitions_{done,total} for
> CREATE INDEX p, so isn't it strange if it w
On Wed, Feb 17, 2021 at 03:08:36PM +1300, Thomas Munro wrote:
> tp = SearchSysCache1(COLLOID, ObjectIdGetDatum(oid));
> if (!HeapTupleIsValid(tp))
> + {
> + if (found)
> + {
> + *found = fals
On Wed, Feb 17, 2021 at 06:34:36PM +, Jacob Champion wrote:
> This would only affect threaded libpq clients running OpenSSL 1.0.2 and
> below, and it looks like the most likely code path to be affected is
> the OpenSSL error stack. So if anything went wrong with one of those
> hash calls, it's
On Wed, Feb 17, 2021 at 10:24:37AM -0600, Justin Pryzby wrote:
> When we implemented REINDEX of partitioned tables, it should've handled
> progress reporting in the fields where that's reported for CREATE INDEX.
> Or else we should document that "partitions_total/done are not populated for
> REINDE
On Thu, Feb 18, 2021 at 10:45:53AM +1300, Thomas Munro wrote:
> I guess I was trying to preserve a distinction between "unknown OID"
> and "this is a collation OID, but I don't have version information for
> it" (for example, "C.utf8"). But it hardly matters, and your
> suggestion works for me. T
On Tue, Feb 16, 2021 at 06:14:15PM +0900, Kyotaro Horiguchi wrote:
> The attached is just fixing that. I tried to make it generic but
> didn't find a clean and translatable way.
>
> Also I found that only three cases in the function are excecised by
> make check.
>
> ATT_TABLE
On Thu, Feb 18, 2021 at 11:04:05AM +0900, Michael Paquier wrote:
> We have the code in place to properly initialize the crypto locking in
> libpq with ENABLE_THREAD_SAFETY, but the root of the issue is that the
> SSL and crypto initializations are grouped together. What we need to
> d
On Thu, Feb 18, 2021 at 02:17:00PM +0900, Michael Paquier wrote:
> I have no issues with documenting more precisely on which commands
> partitions_total and partitions_done apply currently, by citing the
> commands where these are effective. We do that for index_relid for
> instance.
On Fri, Feb 19, 2021 at 10:54:05AM +0900, Kyotaro Horiguchi wrote:
> At Thu, 18 Feb 2021 18:51:37 +0530, Ashutosh Bapat
> wrote in
>> On Thu, Feb 18, 2021 at 6:19 PM Peter Eisentraut <
>> peter.eisentr...@2ndquadrant.com> wrote:
>>> Here is an updated patch that just introduces LSN_FORMAT_ARGS()
On Fri, Feb 19, 2021 at 01:42:38AM -0500, Tom Lane wrote:
> Antonin Houska writes:
>> When I run "autoreconf" on the master branch, git generates the diff
>> below. Shouldn't it just be applied? I suppose someone changed configure.ac
>> and forgot to update the generated file.
>
> Yeah, looks lik
On Fri, Feb 19, 2021 at 02:21:21AM -0500, Tom Lane wrote:
> Michael Paquier writes:
> > Indeed, thanks. It looks like a "git add" that was fat-fingered. I
> > would like to make things more consistent with the attached.
>
> +1, but I think the first peri
On Fri, Feb 19, 2021 at 09:57:22AM -0500, Tom Lane wrote:
> Hm. It should be consistent with the rest, for sure. Personally I'd put
> the only period at the end, but I suppose we should stick with the
> prevailing style if there is one.
Thanks. I have just used the same style as XML, LDAP and L
On Fri, Feb 19, 2021 at 12:12:54AM -0600, Justin Pryzby wrote:
> Looks fine.
Thanks, applied then to clarify things.
> Also, I noticed that vacuum recurses into partition heirarchies since v10, but
> pg_stat_progress_vacuum also doesn't show anything about the parent table or
> the progress of re
On Sat, Feb 20, 2021 at 11:39:22AM +0530, Bharath Rupireddy wrote:
> Actually in the code base the style of that variable declaration and
> usage of pgstat_progress_update_multi_param is a mix. For instance, in
> lazy_scan_heap, ReindexRelationConcurrently, the variables are
> declared at the start
On Thu, Feb 18, 2021 at 11:13:06AM +0100, Guillaume Lelarge wrote:
> I finally managed to get a working TAP test for my patch. I have no idea if
> it's good, and if it's enough. Anyway, new version of the patch attached.
As presented in this patch, specifying both --extension and
--table/--schema
On Sat, Feb 13, 2021 at 10:49:26AM +0900, Michael Paquier wrote:
> So that's this patch: https://commitfest.postgresql.org/32/2941/.
> Alvaro is most likely going to take care of that, so let's wait for
> him.
Hearing nothing, I have looked at this stuff and the simplificatio
On Sat, Feb 20, 2021 at 10:39:24PM +0100, Guillaume Lelarge wrote:
> Le sam. 20 févr. 2021 à 17:31, Tom Lane a écrit :
>> I haven't read the patch, but the behavior I would expect is:
>>
>> 1. If --extension=pattern is given, then extensions matching the
>> pattern are included in the dump, regard
On Sat, Feb 20, 2021 at 12:25:58PM -0300, Álvaro Herrera wrote:
> On 2021-Feb-20, Michael Paquier wrote:
>> Hearing nothing, I have looked at this stuff and the simplification
>> makes sense. Any comments?
>
> No further comments ... I think the patch is simple enough.
On Sat, Feb 20, 2021 at 02:29:44PM +0530, Bharath Rupireddy wrote:
> Yeah. We could use pgstat_progress_update_multi_param instead of
> pgstat_progress_update_param to update multiple params.
>
> On a quick scan through the code, I found that we can do the following. If
> okay, I can start a new t
On Sun, Feb 21, 2021 at 11:30:21AM +0530, Bharath Rupireddy wrote:
> Attached is a patch that replaces some subsequent multiple
> update_param calls with a single update_multi_param.
Looks mostly fine to me.
-if (OidIsValid(indexOid))
-pgstat_progress_update_param(PROGRESS_CLUSTER_COM
On Sun, Feb 21, 2021 at 04:43:23PM +0530, Bharath Rupireddy wrote:
> While we are at it, I wanted to use a single line statement instead of
> if else, just like we do it in do_analyze_rel as below.
>
> pgstat_progress_update_param(PROGRESS_ANALYZE_PHASE,
> inh
On Mon, Feb 22, 2021 at 06:34:22PM +1300, Thomas Munro wrote:
> On Thu, Feb 18, 2021 at 8:15 PM Michael Paquier wrote:
>> Could you just add a test with pg_collation_current_version(0)?
>
> Done.
>
>> + pg_strncasecmp("POSIX.", collcollate, 6) != 0)
>
On Sun, Feb 21, 2021 at 09:43:59AM -0600, Justin Pryzby wrote:
> If you apply this patch, will you want to actually revert those
> earlier changes?
That's not in the plan.
> Also (related), this still crashes if methods are omitted from the
> initializer,
> like:
>
> // .slot_callbacks = no_sto
On Mon, Feb 22, 2021 at 05:15:57PM -0300, Álvaro Herrera wrote:
> I changed my mind on this after noticing that
> ItemPointerIndicatesMovedPartitions has a few callers; leaving the
> interface incomplete/asymmetric would be worse. So I propose to do
> this.
Doing that looks fine to me as well.
--
On Mon, Feb 22, 2021 at 05:19:37PM -0800, Andres Freund wrote:
> This doesn't seem like an advantage to me. Isn't this just pushing logic
> away from a fairly obvious point into an AM that one would expect to
> never actually get called?
>
> If we want to go down this path what's the justification
201 - 300 of 10481 matches
Mail list logo
