Hi, here are some initial review comments.
//
Patch v00-0001
1.
+
+ /* Check required parameters */
+ if (!protocol_version_given)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("proto_version parameter missing")));
+ if (!publication_names_given)
+ ereport(ERROR,
+
On Thu, Dec 14, 2023 at 12:31 PM Ashutosh Bapat
wrote:
>
> On Thu, Dec 14, 2023 at 10:53 AM Dilip Kumar wrote:
>
> > > >
> > >
> > > It is correct that we can make a wrong decision about whether a change
> > > is transactional or non-transactional when sequence DDL happens before
> > > the
On Thu, Dec 14, 2023 at 10:53 AM Dilip Kumar wrote:
> > >
> >
> > It is correct that we can make a wrong decision about whether a change
> > is transactional or non-transactional when sequence DDL happens before
> > the SNAPBUILD_FULL_SNAPSHOT state and the sequence operation happens
> > after
On Thu, 7 Dec 2023 at 13:09, David Rowley wrote:
>
> On Thu, 7 Dec 2023 at 10:09, Matthias van de Meent
> wrote:
> > PFA a patch that reduces the output size of nodeToString by 50%+ in
> > most cases (measured on pg_rewrite), which on my system reduces the
> > total size of pg_rewrite by 33% to
On Thursday, December 14, 2023 12:45 PM Peter Smith
wrote:
> A review comment for v47-0001
Thanks for the comment.
>
> ==
> src/backend/replication/slot.c
>
> 1. GetStandbySlotList
>
> +static void
> +WalSndRereadConfigAndReInitSlotList(List **standby_slots) {
> + char
On Wed, Dec 13, 2023 at 3:53 PM Peter Smith wrote:
>
> 12.
> +static void
> +synchronize_one_slot(WalReceiverConn *wrconn, RemoteSlot *remote_slot,
> + bool *slot_updated)
> +{
> + ReplicationSlot *s;
> + ReplicationSlot *slot;
> + char sync_state = '\0';
>
> In my previous review [1]#33a I
On Wed, Dec 13, 2023 at 6:26 PM Amit Kapila wrote:
>
> > > But can this even happen? Can we start decoding in the middle of a
> > > transaction? How come this wouldn't affect e.g. XLOG_HEAP2_NEW_CID,
> > > which is also skipped until SNAPBUILD_FULL_SNAPSHOT. Or logical
> > > messages, where we
A review comment for v47-0001
==
src/backend/replication/slot.c
1. GetStandbySlotList
+static void
+WalSndRereadConfigAndReInitSlotList(List **standby_slots)
+{
+ char*pre_standby_slot_names;
+
+ ProcessConfigFile(PGC_SIGHUP);
+
+ /*
+ * If we are running on a standby, there is no need
Dear hackers,
> It is correct that we can make a wrong decision about whether a change
> is transactional or non-transactional when sequence DDL happens before
> the SNAPBUILD_FULL_SNAPSHOT state and the sequence operation happens
> after that state.
I found a workload which decoder distinguish
On Wed, Dec 13, 2023 at 9:25 PM Emre Hasegeli wrote:
>
> I noticed that Logical Streaming Replication Protocol documentation
> [1] is missing the options added to "pgoutput" since version 14. A
> patch is attached to fix it together with another small one to give a
> nice error when
On Mon, Dec 11, 2023 at 10:42 AM Dilip Kumar wrote:
> On Thu, Nov 30, 2023 at 3:30 PM Dilip Kumar wrote:
> >
> > On Wed, Nov 29, 2023 at 4:58 PM Dilip Kumar
> wrote:
>
> Here is the updated patch based on some comments by tender wang (those
> comments were sent only to me)
>
few nitpicks:
+
On Mon, 4 Dec 2023 17:05:24 +0900
Michael Paquier wrote:
> On Tue, Sep 26, 2023 at 12:17:04PM -0400, Andrew Dunstan wrote:
> > On 2023-09-26 Tu 01:25, NINGWEI CHEN wrote:
> >> hamerkop is not yet prepared for Meson builds, but we plan to work on this
> >> support soon.
> >> If we go with Meson
On Thu, Dec 14, 2023 at 9:34 AM Zhang Mingli wrote:
>
> Hi,
>
> LGTM.
>
> + Assert(size >= sizeof(Node)); /* need the tag, at least */
> + result = (Node *) palloc0fast(size);
> + result->type = tag;
>
> + return result;
> +}
>
> How about moving the comments /* need the tag, at least */ after
Hi,
LGTM.
+ Assert(size >= sizeof(Node)); /* need the tag, at least */
+ result = (Node *) palloc0fast(size);
+ result->type = tag;
+ return result;
+}
How about moving the comments /* need the tag, at least */ after result->type =
tag; by the way?
Zhang Mingli
Hi, here are a few more review comments for the patch v47-0002
(plus my review comments of v45-0002 [1] are yet to be addressed)
==
1. General
For consistency and readability, try to use variables of the same
names whenever they have the same purpose, even when they declared are
in
Hi, all
By reading the codes, I found that we process limit option as
LIMIT_OPTION_WITH_TIES when using WITH TIES
and all others are LIMIT_OPTION_COUNT by commit
357889eb17bb9c9336c4f324ceb1651da616fe57.
And check actual limit node in limit_needed().
There is no need to maintain a useless
The newNode() macro can be turned into a static inline function, which
makes it a lot simpler. See attached. This was not possible when the
macro was originally written, as we didn't require compiler to have
static inline support, but nowadays we do.
This was last discussed in 2008, see
Thanks for taking a look!
On Wed, Dec 13, 2023 at 1:36 PM Tom Lane wrote:
>
> James Coleman writes:
> > Attached is a patch that solves that issue. It also teaches predtest about
> > quite a few more cases involving BooleanTest expressions (e.g., how they
> > relate to NullTest expressions).
On Tue, Dec 12, 2023 at 11:53 AM John Naylor wrote:
>
> On Mon, Dec 11, 2023 at 1:18 PM Masahiko Sawada wrote:
>
> > I've attached the updated patch set. From the previous patch set, I've
> > merged patches 0007 to 0010. The other changes such as adding RT_GET()
> > still are unmerged for now,
> What is the actual
> use case for such a setting?
I don't have exact details on the use-case, bit this is not a common
use-case.
> Doesn't it risk security problems?
I cannot see how setting it on the database being more problematic than
setting it on a session level.
> I'm rather
Hi,
In all releases, if bootstrap mode's checkpoint gets an error (ENOSPC,
EDQUOT, EIO, ...) or a short write in md.c, ERROR is promoted to FATAL
and the shmem_exit resowner machinery reaches this:
running bootstrap script ... 2023-12-14 10:38:02.320 NZDT [1409162]
FATAL: could not write block
On 2023-12-13 We 09:23, Michael Paquier wrote:
On Tue, Dec 05, 2023 at 07:29:59AM +0900, Michael Paquier wrote:
Okay. Thanks for the update.
While in Prague, Andres and Peter E. have mentioned me that we perhaps
had better move on with this patch sooner than later, without waiting
for the
"Imseih (AWS), Sami" writes:
> A recent case in the field in which a database session_authorization is
> altered to a non-superuser, non-owner of tables via alter database .. set
> session_authorization ..
> caused autovacuum to skip tables.
That seems like an extremely not-bright idea. What
On Wed Dec 13, 2023 at 2:35 PM CST, Andrew Dunstan wrote:
On 2023-12-12 Tu 18:02, Tom Lane wrote:
> "Tristan Partin" writes:
>> The big patch here is adding support for Mac. objdump -W doesn't work on
>> Mac. So, I used dsymutil and dwarfdump to achieve the same result.
> We should probably
On 2023-12-12 Tu 10:30, Alvaro Herrera wrote:
On 2023-Dec-12, Tom Lane wrote:
"Euler Taveira" writes:
When you add exceptions, it starts to complicate the UI.
Indeed. It seems like --silent-diff was poorly defined and poorly
named, and we need to rethink that option along the way to
Hi,
A recent case in the field in which a database session_authorization is
altered to a non-superuser, non-owner of tables via alter database .. set
session_authorization ..
caused autovacuum to skip tables.
The issue was discovered on 13.10, and the logs show such messages:
warning:
On 2023-12-12 Tu 18:02, Tom Lane wrote:
"Tristan Partin" writes:
The big patch here is adding support for Mac. objdump -W doesn't work on
Mac. So, I used dsymutil and dwarfdump to achieve the same result.
We should probably nuke the current version of src/tools/find_typedef
altogether in
Here is a thought that occurs to me, as I follow along with Jeff
Davis's evolving proposals for built-in collations and ctypes: What
would stop us from dropping support for the libc (sic) provider on
Windows? That may sound radical and likely to cause extra work for
people on upgrade, but how
By chance I discovered that checks on large object ownership
are broken in v16+. For example,
regression=# create user alice;
CREATE ROLE
regression=# \c - alice
You are now connected to database "regression" as user "alice".
regression=> \lo_import test
lo_import 40378
regression=> comment on
James Coleman writes:
> Attached is a patch that solves that issue. It also teaches predtest about
> quite a few more cases involving BooleanTest expressions (e.g., how they
> relate to NullTest expressions). One thing I could imagine being an
> objection is that not all of these warrant cycles
On Wed Dec 13, 2023 at 11:27 AM CST, Tom Lane wrote:
"Tristan Partin" writes:
> That makes sense to me. Where can I find the buildfarm code to propose
> a different patch, at least pulling in the current state of the
> buildfarm script? Or perhaps Andrew is the best person for this job.
I
"Tristan Partin" writes:
> That makes sense to me. Where can I find the buildfarm code to propose
> a different patch, at least pulling in the current state of the
> buildfarm script? Or perhaps Andrew is the best person for this job.
I think this is the authoritative repo:
Great results.
On Sat, Dec 9, 2023 at 5:12 AM Melanie Plageman
wrote:
> Values can be "removed" from the accumulator by simply decrementing its
> cardinality and decreasing the sum and sum squared by a value that will
> not change the mean and standard deviation of the overall distribution.
> To
On Tue Dec 12, 2023 at 5:02 PM CST, Tom Lane wrote:
"Tristan Partin" writes:
> The big patch here is adding support for Mac. objdump -W doesn't work on
> Mac. So, I used dsymutil and dwarfdump to achieve the same result.
We should probably nuke the current version of src/tools/find_typedef
On Wed, 2023-12-13 at 16:34 +0100, Daniel Verite wrote:
> But there are CLDR mappings on top of that.
I see, thank you.
Would it still be called "full" case mapping to only use the mappings
in SpecialCasing.txt? And would that be useful?
Regards,
Jeff Davis
I noticed that Logical Streaming Replication Protocol documentation
[1] is missing the options added to "pgoutput" since version 14. A
patch is attached to fix it together with another small one to give a
nice error when "proto_version" parameter is not provided.
[1]
Jeff Davis wrote:
> While "full" case mapping sounds more complex, there are actually
> very few cases to consider and they are covered in another (small)
> data file. That data file covers ~100 code points that convert to
> multiple code points when the case changes (e.g. "ß" -> "SS"), 7
Hi Andres
Thanks for your reply.
The reason I was suspicious with the warnings of the gcc build was, because
gmake check reported 138 out of 202 tests to have failed. I have attached the
output of gmake check.
After you mentioned that gcc did not report any errors, just warnings, we
installed
ywgrit writes:
> I forbid to create indexes on whole-row expression in the following patch.
> I'd like to hear your opinions.
As I said in the previous thread, I don't think this can possibly
be acceptable. Surely there are people depending on the capability.
I'm not worried so much about the
Abdul Matin writes:
> Can postgres client send subsequent requests without receiving response? If
> so, how does the postgres client correlate between a request and its
> response? Where can I get more hints about it?
On Tue, Dec 12, 2023 at 04:54:32PM -0300, Euler Taveira wrote:
> Couldn't it give up before starting if you apply your patch? My main concern
> is
> due to a slow system, the walrcv_connect() took to long in WalReceiverMain()
> and the code above kills the walreceiver while in the process to
On Tue, Dec 05, 2023 at 07:29:59AM +0900, Michael Paquier wrote:
> Okay. Thanks for the update.
While in Prague, Andres and Peter E. have mentioned me that we perhaps
had better move on with this patch sooner than later, without waiting
for the two buildfarm members to do the switch because much
On Tue, 2023-12-12 at 13:14 -0800, Jeremy Schneider wrote:
> My biggest concern is around maintenance. Every year Unicode is
> assigning new characters to existing code points, and those existing
> code points can of course already be stored in old databases before
> libs
> are updated.
Is the
On Wed, Dec 13, 2023 at 5:39 AM Jakub Wartak
wrote:
> > I can't exactly say that such a hint would be inaccurate, but I think
> > the impulse to add it here is misguided. One of my design goals for
> > this system is to make it so that you never have to take a new
> > incremental backup "just
On Thu, Dec 7, 2023 at 10:41 AM Dilip Kumar wrote:
>
> On Wed, Dec 6, 2023 at 7:09 PM Tomas Vondra
> wrote:
> >
> > Yes, if something like this happens, that'd be a problem:
> >
> > 1) decoding starts, with
> >
> >SnapBuildCurrentState(builder) < SNAPBUILD_FULL_SNAPSHOT
> >
> > 2)
> On 12 Dec 2023, at 18:28, Alvaro Herrera wrote:
>
> Andrey, do you have any stress tests or anything else that you used to
> gain confidence in this code?
We are using only first two steps of the patchset, these steps do not touch
locking stuff.
We’ve got some confidence after Yura
On Tue, Dec 12, 2023 at 11:09 AM Junwang Zhao wrote:
>
> On Mon, Dec 11, 2023 at 10:32 PM Masahiko Sawada
> wrote:
> >
> > On Mon, Dec 11, 2023 at 7:19 PM Michael Paquier wrote:
> > >
> > > On Mon, Dec 11, 2023 at 10:57:15AM +0900, Masahiko Sawada wrote:
> > > > IIUC we cannot create two same
On Mon, Dec 11, 2023 at 6:28 AM Thomas Munro wrote:
>
> Hi,
>
> This is not exhaustive, I just noticed in passing that we don't need these.
I was able to compile the changes with "--with-llvm" successfully, and
the changes look good to me.
Thanks and Regards,
Shubham Khanna.
On Wed, Dec 13, 2023 at 1:41 AM Alvaro Herrera wrote:
>
> I would replace the text in explain.sgml with this:
>
> + Include information on memory consumption by the query planning phase.
> + This includes the precise amount of storage used by planner in-memory
> + structures, as
Hi Robert,
On Mon, Dec 11, 2023 at 6:08 PM Robert Haas wrote:
>
> On Fri, Dec 8, 2023 at 5:02 AM Jakub Wartak
> wrote:
> > While we are at it, maybe around the below in PrepareForIncrementalBackup()
> >
> > if (tlep[i] == NULL)
> > ereport(ERROR,
> >
> >
Hi Shveta, here are some review comments for v45-0002.
==
doc/src/sgml/bgworker.sgml
1.
+
+
+ BgWorkerStart_PostmasterStart
+
+
+ BgWorkerStart_PostmasterStart
+ Start as soon as postgres itself has finished its own initialization;
+ processes
Hi. small issues I found...
typo:
+-- Test mutabilily od query functions
+ default:
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("only datetime, bool, numeric, and text types can be casted
to jsonpath types")));
transformJsonPassingArgs's function:
On Tue, Dec 12, 2023 at 3:25 AM Tomas Vondra
wrote:
> The attached 0001 patch fixes this by adding the call to validate_index,
which seems like the proper place as it's where the indexInfo is
allocated and destroyed.
Yes, and by reading validate_index's header comment, there is a clear
53 matches
Mail list logo
