I can agree with that. Another supertiny nitpick on the above is to not end a
single-line comment with a period.
> Daniel: Are you intending to commit this?
Yes, my plan is to get it in before feature freeze. I notice now that I had
missed setting myself as committer in the CF to signal this intent, sorry about
that.
--
Daniel Gustafsson
> On 5 Apr 2023, at 16:30, Robert Haas wrote:
>
> On Wed, Apr 5, 2023 at 4:57 AM Daniel Gustafsson wrote:
>>> - Being able to write a list of event triggers working would be much
>>> more interesting than just individual elements.
>>> - There may be an argume
ar (I'm terrible with names)?
> I am, however, wondering if extensions expect to have access to the guc
> variable or the global variable -- or both?
Extensions have access to all exported symbols, and I think it's not uncommon
for extension authors to expect to have access to at least read GUC variables.
--
Daniel Gustafsson
NALYZE -- even if the
> launcher doesn't use these variables?
>
> I've removed the check, because I do agree with you that it may be
> unnecessarily confusing in the code.
+1
> On Tue, Apr 4, 2023 at 9:36 AM Daniel Gustafsson wrote:
>>> On 4 Apr 2023, at 00:35
oncerns about replacing the current
coverage report with one from an unreleased (and potentially buggy) lcov, we
could perhaps use diff.coverage.pg.o until it's released?
--
Daniel Gustafsson
> On 5 Apr 2023, at 10:10, Michael Paquier wrote:
>
> On Mon, Apr 03, 2023 at 11:35:14PM +0200, Daniel Gustafsson wrote:
>> Yeah. The patch as it stands allow for disabling specific types rather than
>> all-or-nothing, which is why the name was "ignore".
>
&g
hecks and user mappings are user-visible behaviour that
> are easy to overlook when making changes with unexpected side effects.
>
> It seems like the test would be just as easy to commit as to not
> commit and I don't see anything tricky about it that would necessitate
> a more in
> On 5 Apr 2023, at 07:27, Koshi Shibagaki (Fujitsu)
> wrote:
> I found out that there is a mistake written in
> contrib/postgres_fdw/postgres_fdw.c.
Thanks for the report, fixed.
--
Daniel Gustafsson
ne that explain why they are initialized to
those values.
+ /* There is at least 1 autovac worker (this worker). */
+ Assert(nworkers_for_balance > 0);
Is there a scenario where this is expected to fail? If so I think this should
be handled and not just an Assert.
--
Daniel Gustafsson
> On 3 Apr 2023, at 23:46, Tom Lane wrote:
>
> Daniel Gustafsson writes:
>> On 29 Sep 2022, at 21:33, Tom Lane wrote:
>>> I find this behavior a bit surprising:
>>>
>>> +SELECT
>>> array_dims(array_sample('[-1:2][2:3]={{1,2},{
> On 3 Apr 2023, at 16:09, Robert Haas wrote:
> On Mon, Apr 3, 2023 at 9:15 AM Daniel Gustafsson wrote:
>>> On 3 Apr 2023, at 15:09, Robert Haas wrote:
>>> I continue to think it's odd that the sense of this is inverted as
>>> compared with row_security.
aft but I'm not sure I follow why not preserving here, can you
explain?
The rest of your comments have been addressed in the attached v6 I think
(although I'm pretty sure the docs part is just as bad now, explaining these in
concise words is hard, will take another look with fr
> On 3 Apr 2023, at 21:04, Jacob Champion wrote:
>
> On Sun, Apr 2, 2023 at 1:36 PM Daniel Gustafsson wrote:
>>> On 31 Mar 2023, at 19:59, Jacob Champion wrote:
>>> I can make that change; note that it'll also skip some of the new tests
>>
> On 3 Apr 2023, at 15:09, Robert Haas wrote:
>
> On Mon, Apr 3, 2023 at 8:46 AM Daniel Gustafsson wrote:
>> All comments above addressed in the attached v5, thanks for review!
>
> I continue to think it's odd that the sense of this is inverted as
> compared with
gt;
> It doesn't technically "ensure that they're enabled", since they can be
> disabled by ALTER. Better to say that it "doesn't disable any even triggers".
All comments above addressed in the attached v5, thanks for review!
--
Daniel Gustafsson
v5-0001-Add-GUC-for-temporarily-disabling-event-triggers.patch
Description: Binary data
dded Amit who committed it in 216a784829c on cc: to see if he remembers
the comment in question and can shed some light. Skimming the linked thread
yields no immediate clues.
--
Daniel Gustafsson
and checking it ourselves. (And even that would look different
> between OpenSSL and LibreSSL...)
Right, that's clearly not something we want to do.
> Is there something we could document that's more helpful than "make sure
> your installation isn't broken"?
I wonder if there is an openssl command line example for verifying defaults
that we can document and refer to?
--
Daniel Gustafsson
> On 31 Mar 2023, at 22:33, Daniel Gustafsson wrote:
>
> The attached v4 fixes some incorrect documentation (added by me in v3), and
> fixes that background_psql() didn't honor on_error_stop and extraparams passed
> by the user. I've also added a commit which impl
iginated),
there are still lots of ways to break things with other ev triggers.
Any objections?
--
Daniel Gustafsson
invent our own parallel scheme?
Alternatively, the protocol in the.PROXY patch by Magnus [0] which stalled a
few CF's ago has similar functionality for the client to pass a hostname.
--
Daniel Gustafsson
[0]
https://www.postgresql.org/message-id/flat/CABUevExJ0ifpUEiX4uOREy0s2kHBrBrb=pxlehhpmtr1vvr...@mail.gmail.com
d up a few IPC::Run
includes from test scripts.
--
Daniel Gustafsson
v4-0002-Add-test-SCRAM-iteration-changes-with-psql-passwo.patch
Description: Binary data
v4-0001-Refactor-background-psql-TAP-functions.patch
Description: Binary data
> On 29 Mar 2023, at 22:08, Daniel Gustafsson wrote:
>
>> On 29 Mar 2023, at 21:14, Daniel Gustafsson wrote:
>
>> Ugh, I clearly should've stayed on the couch yesterday.
>
> Maybe today as well, just as I had sent this I realized there is mention of
> the
L_CTX_set_default_verify_paths(SSL_context) != 1)
OpenSSL documents this as "A missing default location is still treated as a
success.", is that something we need to document or in any way deal with?
(Skimming the OpenSSL code I'm not sure it's actually correct in v3+, but I
might very well have missed something.)
--
Daniel Gustafsson
6), but I suppose
> it was an oversight.
LGTM, it indeed seems to be an accidental oversight.
--
Daniel Gustafsson
> On 30 Mar 2023, at 22:29, Tom Lane wrote:
> Daniel Gustafsson writes:
>>> On 30 Mar 2023, at 20:44, Tom Lane wrote:
>>> Another idea could be for pg_regress to enforce that "select count(*)
>>> from pg_roles" gives the same answer before and afte
cros for extracting the boolean active/inactive that
most of the code needs to be concerned with would more for more readable code I
think.
--
Daniel Gustafsson
" gives the same answer before and after the test run.
That wouldn't prevent the contents of pg_roles to have changed though, so there
is a (slim) false positive risk with that no?
--
Daniel Gustafsson
ch
> came out in 2011.
I assume you then mean tying this to 1.44 (or another version?), since AFAICT
there has been both features and bugfixes well after 2003?
https://metacpan.org/dist/Text-Template/changes
--
Daniel Gustafsson
> On 30 Mar 2023, at 10:21, Daniel Gustafsson wrote:
>
>> On 30 Mar 2023, at 10:00, Julien Rouhaud wrote:
>>
>> On Thu, Mar 30, 2023 at 3:03 PM Daniel Gustafsson wrote:
>>>
>>>> On 30 Mar 2023, at 03:48, Hayato Kuroda (Fujitsu)
>>>>
> On 30 Mar 2023, at 10:00, Julien Rouhaud wrote:
>
> On Thu, Mar 30, 2023 at 3:03 PM Daniel Gustafsson wrote:
>>
>>> On 30 Mar 2023, at 03:48, Hayato Kuroda (Fujitsu)
>>> wrote:
>>
>>> While checking the buildfarm, I found a failure on NetBSD
oday to get a fix committed.
--
Daniel Gustafsson
I took another couple of looks at this and pushed it after a few small tweaks
to the docs.
--
Daniel Gustafsson
> On 29 Mar 2023, at 21:14, Daniel Gustafsson wrote:
> Ugh, I clearly should've stayed on the couch yesterday.
Maybe today as well, just as I had sent this I realized there is mention of the
output format in the docs that I had missed. The attached changes that as well
to match the
> On 29 Mar 2023, at 09:08, Peter Eisentraut
> wrote:
>
> On 28.03.23 15:56, Daniel Gustafsson wrote:
>>> On 28 Mar 2023, at 15:26, Daniel Gustafsson wrote:
>>> I think the attached is a good candidate for going in, but I would like to
>>> see it
&
> On 29 Mar 2023, at 19:18, Bruce Momjian wrote:
> On Thu, Feb 23, 2023 at 09:36:00AM +0100, Daniel Gustafsson wrote:
>>> On 23 Feb 2023, at 05:48, Tom Lane wrote:
>>> Mass conversion of /* to // style would answer that,
>>> but would also create an impossible b
> On 28 Mar 2023, at 15:26, Daniel Gustafsson wrote:
> I think the attached is a good candidate for going in, but I would like to
> see it
> for another spin in the CF bot first.
Another candidate due to a thinko which raised a compiler warning.
--
Daniel Gustafsson
v19-000
> On 15 Mar 2023, at 11:36, Peter Eisentraut
> wrote:
>
> On 24.02.23 10:49, Daniel Gustafsson wrote:
>> Another rebase on top of 337903a16f. Unless there are conflicting reviews, I
>> consider this patch to be done and ready for going in during the next CF.
>
>
if (pg_prng_strong_seed(&conn->prng_state)))
return;
/* fallback seeding */
}
--
Daniel Gustafsson
> On 27 Mar 2023, at 16:33, Tom Lane wrote:
>
> Julien Rouhaud writes:
>> On Mon, Mar 27, 2023 at 02:06:34PM +0200, Daniel Gustafsson wrote:
>> On 27 Mar 2023, at 14:04, Dagfinn Ilmari Mannsåker wrote:
>>>> Doesn't this apply to Apple Silicon generall
> On 27 Mar 2023, at 14:04, Dagfinn Ilmari Mannsåker wrote:
> Daniel Gustafsson writes:
>> Applied with a tiny but of changes to make it look like the rest of the
>> paragraph more. Thanks!
>
> Doesn't this apply to Apple Silicon generally, not just M1? M2 alrea
e3 OK
> ▶ 6/6 - received 50 connections across all nodesOK
Good point.
> Finally, I changed a few small typos in your updated commit message
> (some of which originated from my earlier commit messages)
+1
--
Daniel Gustafsson
> On 27 Mar 2023, at 10:41, Julien Rouhaud wrote:
> On Mon, Mar 27, 2023 at 10:32:52AM +0200, Daniel Gustafsson wrote:
>>> On 27 Mar 2023, at 10:24, Julien Rouhaud wrote:
>>> FTR the documented XML_CATALOG_FILES environment variable is only valid for
>>>
all the functions
dealing with addrinfo does. Also moved the error reporting to inside there
where the error happened.
* Made the prng init function void as it always returned true anyways.
* Minor comment and docs tweaking.
* I removed the change to geqo, while I don't think
tc/xml/catalog
For reference on why Homebrew use a different structure on Apple M1 the below
issue has more details:
https://github.com/Homebrew/brew/issues/9177
--
Daniel Gustafsson
> On 25 Mar 2023, at 01:56, Michael Paquier wrote:
>
> On Fri, Mar 24, 2023 at 09:56:29AM +0100, Daniel Gustafsson wrote:
>> I've actually ripped out the test in question in the attached v9 to have it
>> ready and building green in CFbot.
>
> While reading
e buildfarm; francolin has errored in
recoveryCheck which I'm looking into but at first glance I don't think it's
related (other animals have since passed it and it works locally, but I'll keep
digging at it to make sure).
--
Daniel Gustafsson
le pattern.
> 2. With the change Daniel is proposing here, \set VERBOSITY verbose is
> not going to print as useful information to tracking down where any
> unexpected nulls in the catalogue originates.
Thats a fair point for the elog() removals, for the rather many assertions it
might be a net positive to get a non-local elog when failing in production.
--
Daniel Gustafsson
it.
I had no idea we had support for building a .chm until reading this, but I've
also never come across anyone asking for such a docset. FWIW, no objections to
it going.
--
Daniel Gustafsson
> On 24 Mar 2023, at 21:27, Andres Freund wrote:
> On 2023-03-23 10:18:35 +0100, Daniel Gustafsson wrote:
>>> On 22 Mar 2023, at 18:00, Andres Freund wrote:
>>
>>> It wasn't actually that much work to write a patch to remove
>>> vacuum_defer_cl
ernative approach to this before withdrawing the
> patch.
If this is mainly targeting extension use, is there a way in which an extension
could have all this functionality with no: a) not exposing any of it from the
server; b) not having to copy/paste lots into the extension and; c) have a
reasonable way to keep up with any changes made in the backend?
--
Daniel Gustafsson
> On 24 Mar 2023, at 00:33, Michael Paquier wrote:
>
> On Thu, Mar 23, 2023 at 10:46:56PM +0100, Daniel Gustafsson wrote:
>> I'm fairly convinced it's a timeout in the interactive psql session. Given
>> how
>> ugly the use of that is I'm sort of waitin
> On 24 Mar 2023, at 08:16, Junwang Zhao wrote:
>
> %s/pg_current_xact/pg_current_xact_id
Pushed, thanks!
--
Daniel Gustafsson
> On 18 Mar 2023, at 23:07, Andrew Dunstan wrote:
> BTW, a better test than the one above would be
>
>$package->isa("PostgreSQL::Test::Cluster")
Attached is a quick updated v3 of the patch which, to the best of my Perl
abilities, tries to address the comment
mit this such that I can rewrite the test in a saner and more robust way.
--
Daniel Gustafsson
[0] https://commitfest.postgresql.org/42/4228/
hroughs to figure out of VacuumFailsafeActive does what
I think it does, and should be doing, but in general I think this is a good
idea and a patch in good condition close to being committable.
--
Daniel Gustafsson
> On 23 Mar 2023, at 10:51, Etsuro Fujita wrote:
> seems more appropriate to me as well in this context, so I
> left it alone.
And just to be clear, I think you are right in leaving it alone given the
context.
> Attached is an updated version of the patch.
LGTM.
--
Daniel Gustafsson
sion", but I tend to think it's not worth caring the complication
> of vacuum_defer_cleanup_age forward.
It might be late in the cycle, but as it's not adding something that might
break but rather removing something that's known for being problematic (and not
useful) I think it's Ok.
--
Daniel Gustafsson
t.postgresql.org/42/4013/
This thread was then later revived by Mikhail Gribkov but without 0001 instead
referring to the above patch for that part.
The new patch for 0001 is quite different, and I welcome your eyes on that
since I agree with you that it would be a good idea.
--
Daniel Gustafsson
While not the fault of this patch I find it confusing that we mix
and for marking up "postgres_fdw", the latter seemingly more correct
(and less commonly used) than .
--
Daniel Gustafsson
r ones. I'll go ahead with your patch after some
testing.
--
Daniel Gustafsson
ing the conflicts are non-trivial/obvious then of course, but if only to
stay recent and avoid fuzz in applying then it's more distracting.
--
Daniel Gustafsson
patch is something we need. I
> would recommend to mark it as rejected and move on.
Unless the claimed performance improvement is measured and provides a speedup,
and the loss of zeroing memory is guaranteed safe, there doesn't seem to be
much value provided.
--
Daniel Gustafsson
as native
> Perl octet strings.
> Please find the patch attached.
Thanks for the patch, I recommend registering this in the currently open
Commitfest to make sure it's kept track of:
https://commitfest.postgresql.org/43/
--
Daniel Gustafsson
ith the EST
example
above, this is not necessarily the same as local civil time on that date.
--
Daniel Gustafsson
uch counts in sync is always error-prone.
--
Daniel Gustafsson
> On 17 Mar 2023, at 14:48, Andrew Dunstan wrote:
> On 2023-03-17 Fr 05:48, Daniel Gustafsson wrote:
>>> On 15 Mar 2023, at 02:03, Andres Freund
>>> wrote:
>>>
>>>> Returning a hash seems like a worse option since it will complicate
>>>&g
alling ->start but it doesn't exactly make the code cleaner.
As mentioned off-list I did some small POD additions when reviewing, so I've
attached them here in a v2 in the hopes that it might be helpful. I've also
included the above POC for restarting the timeout per query to show what I
meant.
--
Daniel Gustafsson
v2-0001-Refactor-background-psql-TAP-functions.patch
Description: Binary data
> On 16 Mar 2023, at 15:58, Tom Lane wrote:
>
> Daniel Gustafsson writes:
>> When looking at the report in [0] an API choice in the relevant pg_upgrade
>> code
>> path stood out as curious. check_is_install_user() runs this query to ensure
>> that only t
atistically - result in the same order for a large enough
n'.
* Remove random_seed and 004_loadbalance_dns.pl
Thoughts?
--
Daniel Gustafsson
ot to
change this back to atoi() for code clarity as we're not reading an Oid here?
--
Daniel Gustafsson
[0]
ve1p191mb1118e9752d4ead45205e995cd6...@ve1p191mb1118.eurp191.prod.outlook.com
of reports (that I was able to find)
indicate that it might be rare in production though?
--
Daniel Gustafsson
ensures that entry->conn is NULL. So I removed the if-test.
> Attached is a patch for that.
LGTM, nice catch.
> I think we could instead add an assertion, but I did not, because we already
> have it in make_new_connection().
Agreed, the assertion in make_new_connection is enough (an
TH makes
> sense.
Yeah, raising just bgw_library_name to MAXPGPATH seems reasonable here. While
the memory usage does grow it's still quite modest, and has an upper limit in
max_worker_processes.
While here, I wonder if we should document what BGW_MAXLEN is defined as in
bgworker.sgml?
--
Daniel Gustafsson
hacked up your patch to
provide $h->reset_timer_before_query() which then injects a {timeout}->start
before running each query without the caller having to do it. Not sure if I'm
alone in doing that but if not I think it makes sense to add.
--
Daniel Gustafsson
> On 23 Feb 2023, at 15:12, Daniel Gustafsson wrote:
>
>> On 22 Feb 2023, at 20:20, Nathan Bossart wrote:
>
>> One thing I noticed is that the
>> "failed check" log is only printed once, even if multiple data type checks
>> failed. I believe this is
copy of the
> code, ISTM there's a good case for doing the small extra work
> to report catalog and column by name.
Rebased v3 on top of recent conflicting ICU changes causing the patch to not
apply anymore. Also took another look around the tree to see if there were
missed callsites but found none new.
--
Daniel Gustafsson
v3-0001-Add-SysCacheGetAttrNotNull-for-guaranteed-not-nul.patch
Description: Binary data
> On 13 Mar 2023, at 11:06, Peter Eisentraut
> wrote:
> On 06.03.23 17:06, Daniel Gustafsson wrote:
>> fipshash() with an explanatory comments sounds like a good idea.
>
> committed like that
+1. Looks like there is a just a slight diff in the compression.sql test suite.
--
Daniel Gustafsson
> On 9 Mar 2023, at 15:12, Nazir Bilal Yavuz wrote:
>
> Hi,
>
> On Thu, 9 Mar 2023 at 16:54, Daniel Gustafsson wrote:
>>
>>> On 9 Mar 2023, at 14:45, Peter Eisentraut
>>> wrote:
>>
>>> How about we just hardcode "openssl" here
bet so +1 for leaving
additional complexity for when needed.
--
Daniel Gustafsson
> On 9 Mar 2023, at 08:09, Michael Paquier wrote:
>
> On Wed, Mar 08, 2023 at 05:21:20PM +0900, Michael Paquier wrote:
>> On Wed, Mar 08, 2023 at 09:07:36AM +0100, Daniel Gustafsson wrote:
>>> AFAIK a TAP test with psql_interactive is the only way to do this so that
pear on any
other platform outside of archeology? Removing the file manually would be a
trivial way to stabilize but if it's only expected to happen on platforms which
are long since EOL by the time 16 ships then the added complication could be
hard to justify.
--
Daniel Gustafsson
> On 8 Mar 2023, at 10:30, Peter Eisentraut
> wrote:
>
> On 08.03.23 10:21, Daniel Gustafsson wrote:
>>> On 8 Mar 2023, at 09:49, Peter Eisentraut
>>> wrote:
>>> It occurred to me that it would be easier to maintain this in the long run
>>>
the code clearly can return -1.
+#ifdef FAKE_FIPS_MODE
I'm not enthusiastic about this. If we use this rather than OpenSSL with FIPS
enabled we might end up missing bugs or weird behavior due to changes in
OpenSSL that we didn't test.
--
Daniel Gustafsson
> On 8 Mar 2023, at 08:48, Michael Paquier wrote:
>
> On Tue, Mar 07, 2023 at 02:03:05PM +0100, Daniel Gustafsson wrote:
>> On 7 Mar 2023, at 09:26, Daniel Gustafsson wrote:
>>> Right, what I meant was: can a pg_regress sql/expected test drive a psql
>>> i
r is
this another case of my fear of silent subtle truncation bugs? =)
--
Daniel Gustafsson
pipe_read_line.diff
Description: Binary data
> On 7 Mar 2023, at 09:26, Daniel Gustafsson wrote:
> Right, what I meant was: can a pg_regress sql/expected test drive a psql
> interactive prompt? Your comments suggested using password.sql so I was
> curious if I was missing a neat trick for doing this.
The attached v7 adds a
good
idea though.
--
Daniel Gustafsson
t something like:
ereport(WARNING,
(errmsg("%ld rows were skipped due to data type incompatibility",
cstate->ignored_errors),
errhint("Skipped rows can be inspected in the database log for
reprocessing.")));
--
Daniel Gustafsson
> On 7 Mar 2023, at 05:53, Michael Paquier wrote:
>
> On Fri, Mar 03, 2023 at 11:13:36PM +0100, Daniel Gustafsson wrote:
>> That would indeed be nice, but is there a way to do this without a
>> complicated
>> pump TAP expression? I was unable to think of a
> On 6 Mar 2023, at 23:10, Andres Freund wrote:
>
> Hi,
>
> On 2023-03-06 15:55:01 -0500, Gregory Stark (as CFM) wrote:
>> It looks like Daniel Gustafsson, Andres, and Tom have all weighed in
>> on this patch with at least a neutral comment (+-0 from Andres :)
&g
> On 6 Mar 2023, at 21:55, Gregory Stark (as CFM) wrote:
>
> It looks like Daniel Gustafsson, Andres, and Tom have all weighed in
> on this patch with at least a neutral comment (+-0 from Andres :)
I think the concept of a login event trigger has merits, even though it's ki
ser is sensible.
This is the part where a committer who wants to pursue the hand-written parser
need to step up. With the amount of review received it's hopefully pretty close.
--
Daniel Gustafsson
[0] 098531e1-fba9-4b7d-884e-0a4363eee...@yesql.se
> On 6 Mar 2023, at 15:55, Tom Lane wrote:
> Daniel Gustafsson writes:
>> For readers without all context, wouldn't it be better to encode in the
>> function name why we're not just calling a hash like md5? Something like
>> fips_allowed_hash() or similar?
uint64 {aka long long
unsigned int}
On that note though, it seems to me that this error message leaves a bit to be
desired with regards to the level of detail.
--
Daniel Gustafsson
can't exactly recall my reasoning, but I do think you're right that if we're
to have this GUC it should support the types of existing EVTs. The updated v4
implements that as well as a rebase on top of master and fixing a typo
discovered upthread.
--
Daniel Gustafsson
v4-0001-Ad
why we're not just calling a hash like md5? Something like
fips_allowed_hash() or similar?
--
Daniel Gustafsson
make sure that libpq
> gets the call when the GUC is updated by a SET command?
That would indeed be nice, but is there a way to do this without a complicated
pump TAP expression? I was unable to think of a way but I might be missing
something?
--
Daniel Gustafsson
v6-0001-Make-SCRAM-iteration-count-configurable.patch
Description: Binary data
as the discussion is archived and searchable.
--
Daniel Gustafsson
> On 2 Mar 2023, at 10:59, Peter Eisentraut
> wrote:
>
> On 28.02.23 21:14, Daniel Gustafsson wrote:
>> The attached refactoring introduce SysCacheGetAttrNotNull as a wrapper around
>> SysCacheGetAttr where a NULL value triggers an elog(). This removes a lot of
>&
I prefer to allow those cases rather than the strict mode where
attnotnull has to be true, do you think it's preferrable to align the API with
attnotnull and keep the current coding for cases like the above?
--
Daniel Gustafsson
801 - 900 of 1964 matches
Mail list logo
