On Mon, Apr 28, 2025 at 7:51 PM Amit Kapila wrote:
>
> On Mon, Apr 28, 2025 at 8:08 AM Peter Smith wrote:
> >
> > On Wed, Apr 23, 2025 at 10:33 AM David G. Johnston
> > wrote:
> > ...
> > > In the column list I would stick to mentioning what cannot be spec
On 07.01.25 18:23, Nathan Bossart wrote:
+ /*
+* We chose the default for autovacuum_worker_slots during the
+* max_connections tests above, but we print a progress message anyway.
+*/
+ printf(_("selecting default \"autovacuum_worker_slots\" ... %d\n"),
+
stand why it was put there in the first place.
Anyway, PSA patch v2.
==
Kind Regards,
Peter Smith.
Fujitsu Australia
v2-0001-DOCS-create-publication-for-table.patch
Description: Binary data
. e.g. what's the point
of gathering publisher sequence lists and setting INIT states for
them, etc, when it won't synchronise anything because copy_data=false?
Everything will be synchronised later anyway when the user does
REFRESH PUBLICATION SEQUENCES.
==
Kind Regards,
Peter Smith.
Fujitsu Australia
:
trailing whitespace.
*
warning: 1 line adds whitespace errors.
==
Kind Regards,
Peter Smith.
Fujitsu Australia
On 24.04.25 12:53, Christoph Berg wrote:
Now you can say `psql -h db.example.com -p 5433 dbfoo`, but for
specifying the sslmode, you have to rewrite at least the last argument
to use connection string syntax, `psql "dbname=dbfoo sslmode=verify-full`.
This needs be be less cumbersome. (And the nam
On 24.04.25 13:16, Jelte Fennema-Nio wrote:
On Thu, 24 Apr 2025 at 10:54, Peter Eisentraut wrote:
The cut-off in practice for these things is usually RHEL. PG18
currently still supports RHEL 7, which appears to come with Python 3.6.
Seeing that the small problem with the test script was
On 23.04.25 00:24, Tomas Vondra wrote:
The patch that flips the default has been committed.
I also started a PG18 open items page and made a note that we follow up
on the upgrade experience, as was discussed in this thread.
https://wiki.postgresql.org/wiki/PostgreSQL_18_Open_Items
Regarding t
On 22.04.25 18:04, Tom Lane wrote:
Jacob Champion writes:
As for picking a version... 3.6 will have been EOL for almost three
years by the time 18 releases. It seems like we would drop it happily,
were it not for RHEL8.
Agreed, but RHEL8 is out there and I don't think we can just drop
support
ilure time'.
Those details belong at a lower level -- here, it is better to be more
generic.
SUGGESTION:
/* This is a clean exit, so no need for any sequence failure logic. */
==
Kind Regards,
Peter Smith.
Fujitsu Australia
d functions do.
~
4b.
I felt that all the SyncXXX functions exposed from syncutils.c should
be grouped together, and maybe even with a comment like /* from
syncutils.c */
==
Kind Regards,
Peter Smith.
Fujitsu Australia
theses.
==
Kind Regards,
Peter Smith.
Fujitsu Australia
Hi Ajin.
Here are some v17-0003 review comments (aka some v16-0003 comments
that were not yet addressed or rejected)
On Fri, Apr 11, 2025 at 4:05 PM Peter Smith wrote:
...
> ==
> Commit message
>
> 2. missing commit message
Not yet addressed.
> ~~~
>
> 8.
> # In
eady be assigned in both the *if* and the *else* blocks. So, why is
it being overwritten again outside that if/else?
==
Kind Regards,
Peter Smith.
Fujitsu Australia
uded in
this case only if publish_generated_columns is set to stored.
I've attached a patch to implement this suggested wording.
Thoughts?
==
[1]
https://www.postgresql.org/docs/devel/sql-createpublication.html#SQL-CREATEPUBLICATION-PARAMS-FOR-TABLE
Kind Regards,
Peter Smith.
Fujitsu Australia
v1-00
affected by this the most are index-only scans on
> all-visible tables that fit into shared buffers, with
> correlated/sequential pattern. Or even regular index scans with all data
> in shred buffers.
My hope is that the index scan manager can be taught to back off when
this is happening, to avoid the regressions. Or that it can avoid them
by only gradually ramping up the prefetching. Does that sound
plausible to you?
--
Peter Geoghegan
results, correct results).
Cool!
> Compared to the last patch version [1] shared on list (in November),
> there's a number of significant design changes - a lot of this is based
> on a number of off-list discussions I had with Peter Geoghegan, which
> was very helpful.
Thanks for
->try_to_filter_change = entry->filterable;
+ return entry->filterable;
+ }
+
Bad indentation.
==
src/include/replication/reorderbuffer.h
3.
+extern bool ReorderBufferCanFilterChanges(ReorderBuffer *rb);
+
Why is this extern here? This function is not implemented in patch 0001.
==
Kind Regar
cussed (earlier in this thread)
changes that are not in this v3* patch set. They haven't been
forgotten; I'll revisit those later after the above (simpler) changes
are dealt with.
==
Kind Regards,
Peter Smith.
Fujitsu Australia
v3-0004-DOCS-pg_waldump.-Some-option-tags-are-wrong-s
ight, I have changed that.
PS REPLY 17/4
Yes, the error message, but also I thought 'tblinfo' var and
FindTableByOid function name should refer to relations instead of
tables?
==
Kind Regards,
Peter Smith.
Fujitsu Australia
fferences between the publisher and the subscriber,
+ which might occur when copy_data = true.
+
ditto previous review comment #6.
==
Kind Regards,
Peter Smith.
Fujitsu Australia
copy_data)
12.
+
+ See
+ for recommendations on how to handle any warnings about
differences in
+ the sequence definition between the publisher and the subscriber,
+ which might occur when copy_data = true.
+
Ditto my previous review comment #10.
==
Kind Regards,
Peter Smith.
Fujitsu Australia
~~
29.
+# Check - newly published sequence values are not updated
+$result = $node_subscriber->safe_psql(
+ 'postgres', qq(
+ SELECT last_value, log_cnt, is_called FROM regress_s4;
+));
+is($result, '1|0|f',
+ 'REFRESH PUBLICATION will sync newly published sequence');
+
(This is the copy_data=false test)
29a.
Maybe it is good here also to show the sequence value at the publisher
to see if it is different.
~
29b.
The message 'REFRESH PUBLICATION will sync newly published sequence'
seems wrong because the values are NOT synced when copy_data=false
==
Kind Regards,
Peter Smith.
Fujitsu Australia
On 10.04.25 01:08, Jacob Champion wrote:
Christoph noted that this was also confusing from the packaging side,
earlier, and Daniel proposed -Doauth-client/--with-oauth-client as the
feature switch name instead. Any objections? Unfortunately it would
mean a buildfarm email is in order, so we shoul
)
{
elm->last = next; /* last returned number */
elm->last_valid = true;
error: patch failed: src/backend/commands/sequence.c:994
error: src/backend/commands/sequence.c: patch does not apply
==
Kind Regards,
Peter Smith.
Fujitsu Australia
#x27;t this same code already done a few lines above in the same
function? Maybe I misread something.
======
src/test/regress/sql/publication.sql
5.
+-- check that describe sequence lists all publications the sequence belongs to
Might be clearer to say: "lists both" instead of "lists all"
==
Kind Regards,
Peter Smith.
Fujitsu Australia
32 | t
+(1 row)
+
I think 32 may seem like a surprising value to anybody reading these
results. Perhaps it will help if there can be a comment for this .sql
test to explain why this is the expected value.
==
Kind Regards,
Peter Smith.
Fujitsu Australia
On Fri, Apr 11, 2025 at 4:26 PM Jonathan S. Katz wrote:
> Please join us in wishing Jacob much success and few reverts!
Well done, Jacob.
--
Peter Geoghegan
On 08.04.25 19:44, Jacob Champion wrote:
Would anybody following along be opposed to a situation where
- dynamiclib builds go through the dlopen() shim
- staticlib builds always rely on statically linked symbols
If this can be implemented in a straightforward way, that would be the
best way, I
I would like for someone else to prepare the list of acknowledgments in
the release notes this year.
I have been preparing the list of acknowledgments in the release notes
(example: [0]) since PostgreSQL 10 (launched from discussions at PGCon
2017 [1]). I'm looking to hand this off now, so that
On 09.04.25 12:02, Peter Eisentraut wrote:
I ran src/tools/mark_pgdllimport.pl and it detected a few new global
variables with missing markings. See attached patch. Please point out
if any of these should not be marked or if they are special cases in
some other way. I'm Cc'i
On 10.04.25 05:51, David Rowley wrote:
Looks like v18 has grown a few appendStringInfo misusages, e.g. using
appendStringInfo() when no formatting is needed or just using format
"%s" instead of using appendStringInfoString().
Would it be useful to augment appendStringInfo() something like this:
8b.
Change the comment or rearrange the tests so they are in the same
order as described
~
8c.
Looking at the expected logs I wondered if it might be nicer for the
pgoutput_filter_change doing this logging to also emit the relation
name.
~~~
9.
+$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION sub_all");
+$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION sub_insert_only");
+$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION sub_delete_only");
+
9a.
You could combine all those DDL
~
9b.
Add some simple comment like "# cleanup"
~
9c.
Any reason why you dropped the subscriptions but not the publications?
==
Kind Regards,
Peter Smith.
Fujitsu Australia
d be able
to filter INSERTS but not filter DELETES even for the same relation.
==
Kind Regards,
Peter Smith.
Fujitsu Australia
r names might be:
entry->filterable
rb->try_to_filter_change
Also these explanations/distinctions need to be made more clearly in
the commit message and/of file head comments, as well as where those
members are defined.
==
Kind Regards,
Peter Smith.
Fujitsu Australia.
==
[1] https://www.postgresql.org/docs/current/logical-replication-col-lists.html
[2]
https://www.postgresql.org/message-id/flat/CAH2L28vddB_NFdRVpuyRBJEBWjz4BSyTB%3D_ektNRH8NJ1jf95g%40mail.gmail.com
Kind Regards,
Peter Smith.
Fujitsu Australia
On 09.04.25 16:23, Tom Lane wrote:
Heikki Linnakangas writes:
Inconsistency is not good either though. I'm not sure it's worth the
churn, but I could get on board a patch to actually replace all
HeapTupleIsValid(tuple) calls with plain "tuple != NULL" checks. Keep
HeapTupleIsValid() just for co
On 09.04.25 14:26, Heikki Linnakangas wrote:
It's a matter of taste, but personally I find 'if (tuple != NULL)' more
clear than 'if (HeapTupleIsValid(tuple))'. The presence of a macro
suggests that there might be other kinds of invalid tuples than a NULL
pointer, which just adds mental load.
Relation() to see list of relations that are not
+ * decoded by the reorderbuffer.
/to see list of relations/to see the kinds of relation/
==
Kind Regards,
Peter Smith.
Fujitsu Australia
On Wed, Apr 9, 2025 at 8:08 PM Amit Kapila wrote:
>
> On Wed, Apr 9, 2025 at 7:48 AM Peter Smith wrote:
> >
> > I was looking at the recent push for the pg_createsubscription "--all"
> > option [1], because I was absent for several weeks prior.
> >
> &g
Btw., this new variable in memutils.h
extern dsa_area *area;
could probably do with a less generic name?
From e9d868a38b50c1fa449258926c40cef88bd73045 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
Date: Wed, 9 Apr 2025 11:55:48 +0200
Subject: [PATCH] WIP: Add missing PGDLLIMPORT markings
an't
backpatch this because of the ABI change, so once PG18 ships the window
is over.
For the -bundle -> -dynamiclib change, I don't have any particular
urgency right now.
From 57de7451676506f79a99681338efcb0a1f956c3c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
Date: We
On 14.09.24 21:14, Tom Lane wrote:
+Section: Class 10 - XQuery Error
+
+# recent SQL versions define quite a few codes in this class, but for now
+# we are only using this generic one
+10608EERRCODE_INVALID_ARGUMENT_FOR_XQUERY
invalid_argument_for_xquery
Coul
gt; it, but the main patch can be committed even without this.
>
I've created a separate thread [1] to continue the discussion about
the synopsis.
==
[1]
https://www.postgresql.org/message-id/CAHut%2BPueY_DyuBy6SuvJev2DWJVGtg%3D9WG9WXvYQDJu39gV6TQ%40mail.gmail.com
Kind Regards,
Peter Smith.
Fujitsu Australia
till works)
==
[1]
https://www.postgresql.org/message-id/CAHut%2BPuyBsOJTSygus2-yp60sw_phwYQ-JyC%2BU6fCBMos9x2LA%40mail.gmail.com
[2]
https://www.postgresql.org/message-id/CAHv8RjKU24jCHR2fOHocmdSTqhu7ige5UQsUQMkaTZniLc9DbA%40mail.gmail.com
[3]
https://github.com/postgres/postgres/commit/f
s part of the --all description to the
Note/Prerequisites section. (see #1c)
0003. Fixes to code comments. (see #2)
==
[1]
https://github.com/postgres/postgres/commit/fb2ea12f42b9453853be043b8ed107e136e1ccb7
[2] https://www.postgresql.org/docs/current/app-vacuumdb.html
Kind Regards,
Pete
On 08.04.25 14:22, Junwang Zhao wrote:
When I read the libpq source code, I found unnecessary static type qualifiers
in PQsetClientEncoding().
diff --git a/src/interfaces/libpq/fe-connect.c
b/src/interfaces/libpq/fe-connect.c
index 0258d9ace3c..300ddfffd55 100644
--- a/src/interfaces/libpq/fe-c
On 08.04.25 16:59, Bruce Momjian wrote:
On Tue, Apr 8, 2025 at 10:36:45AM -0400, Bruce Momjian wrote:
Since we recorded feature freeze as April 8, 2025 0:00 AoE (anywhere on
Earth):
https://wiki.postgresql.org/wiki/PostgreSQL_18_Open_Items#Important_Dates
https://www.timeandda
On Tue, Apr 8, 2025 at 11:20 AM Nathan Bossart wrote:
> +1 for UTC.
+1, I think that AoE is needlessly obscure
--
Peter Geoghegan
needs to be balanced
by a call to the other function: parallel_vacuum_end_work_phase.
~~~
12.
+static void
+parallel_vacuum_end_worke_phase(ParallelVacuumState *pvs)
Typo (worke)
==
[1]
https://www.postgresql.org/message-id/CAHut%2BPs9yTGk2TWdjgLQEzGfPTWafKT0H-Q6WvYh5UKNW0pCvQ%40mail.gmail.com
Kind Regards,
Peter Smith.
Fujitsu Australia
On 07.04.25 16:43, Andres Freund wrote:
While I was looking into this I found that Debian's going to use the
existence of an SONAME to check other things, which I assume will make
Christoph's life harder. I have switched over to
'libpq-oauth-.so', without any SONAME or symlinks.
Yes, this is cor
On 05.04.25 02:27, Jacob Champion wrote:
On Tue, Apr 1, 2025 at 3:40 PM Jacob Champion
wrote:
Maybe a better idea would be to ship an SONAME of
`libpq-oauth.so.0.`, without any symlinks, so that there's
never any ambiguity about which module belongs with which libpq.
While I was looking into
05 PM Peter Smith wrote:
>
> ==
> src/backend/access/table/tableamapi.c
>
> 2.
> Assert(routine->relation_vacuum != NULL);
> + Assert(routine->parallel_vacuum_compute_workers != NULL);
> Assert(routine->scan_analyze_next_block != NULL);
>
> Is it better
dry-run"
~~~
Please find attached a small patch that implements all these changes.
==
[1]
https://www.postgresql.org/message-id/CAKFQuwbaYnSBc5fgHsoFLW_cUq2u466-3ZpkA%2Bu1Z%3D-medNgwg%40mail.gmail.com
Kind Regards,
Peter Smith.
Fujitsu Australia
v1-0001-improvements-for-remove-option-documentation.patch
Description: Binary data
On Thu, Apr 3, 2025 at 10:26 AM Andres Freund wrote:
>
> Hi,
>
> On 2025-04-03 09:58:30 +1100, Peter Smith wrote:
> > I saw that a new errhint_internal() function was recently committed
> > [1]. I had also posted above asking about this same missing function a
> > mo
On 28.03.25 09:13, Peter Eisentraut wrote:
About patch 0003:
I had previously pointed out that the canonicalization might have
been intentional, and that we have canonicalization of ICU locale
names. But we don't have to keep the setlocale()-based locale
checking implementation just for
ation
for what you can implement yourself in your extension's makefile.
From d08218d36d4590f80c2d46f3d7c30bee3e35b7e2 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
Date: Thu, 20 Mar 2025 14:00:18 +0100
Subject: [PATCH v0] WIP: Support "make check" for PGXS extensions
---
s
e wrong idea
about the index. I think that it's much less important to help users
to understand exactly how well a skip scan performs, relative to some
theoretical ideal. The theoretical ideal is just too complicated.
> I'm not sure that I correctly understood about "separation optimizer index
> path/executor stage". Do you mean that it's better to do all the
> optimizations during index execution rather than during index execution?
Yes. In general it's good if we can delay decisions about the scan's
behavior until runtime, when we have a more complete picture of what
the index actually looks like.
--
Peter Geoghegan
nless a mark is
> done.
That's exactly what I had in mind.
--
Peter Geoghegan
I have committed these four patches (squashed into three). I made the
error handling change in 0003 that you requested, and also the error
handling change in 0002 discussed in an adjacent message.
On 12.03.25 16:52, Mark Dilger wrote:
On Wed, Mar 12, 2025 at 7:00 AM Peter Eisentraut
axTIDsPerBTreePage * sizeof(BTScanPosItem) array once per scan matters when
> that many tuples are returned.
I'm not arguing for or against anything. I'm just giving context.
--
Peter Geoghegan
On 24.03.25 10:24, Jakub Wartak wrote:
On Fri, Mar 14, 2025 at 11:31 AM vignesh C wrote:
On Wed, 5 Mar 2025 at 11:46, Amit Kapila wrote:
On Tue, Mar 4, 2025 at 4:44 PM Jakub Wartak
I can go with the last patch as you observed that in a real-world
case, and we can look at others (if any) on
On 12.03.25 17:08, Mark Dilger wrote:
On Wed, Mar 12, 2025 at 7:25 AM Tom Lane <mailto:t...@sss.pgh.pa.us>> wrote:
Peter Eisentraut mailto:pe...@eisentraut.org>> writes:
> 0002: Add get_opfamily_member_for_cmptype(). This was called
> get_opmethod_membe
ed to leak memory. Same with
all operators. This will be far from the only time that we expect
opclass authors to get that right. This mostly works just fine,
probably because an opclass that leaked memory like this would visibly
break quite quickly.
> You could Assert(inc_sk_argument == (Datum) 0) in the oflow branch?
> I'm certain that (Datum) 0 is an invalid representation of a pointer,
> so we know that no allocated value is returned (be it new or
> pre-existing).
I just don't see what the point would be. Nothing would stop a
decrement/increment callback that needs to allocate memory from
returning 0 and then leaking memory anyway.
--
Peter Geoghegan
lict type split into two, something like
'insert_multiple_conflicts' and 'update_multiple_conflicts'?
==
[1]
https://github.com/postgres/postgres/commit/73eba5004a06a744b6b8570e42432b9e9f75997b
Kind Regards,
Peter Smith.
Fujitsu Australia
TIDs
from a page, though. Any low cardinality index will tend to have
almost that many TIDs to return on any page that only stores
duplicates. And scan will necessarily have to return all of the TIDs
from such a page, if it has to return any.
--
Peter Geoghegan
On Fri, Apr 4, 2025 at 12:00 PM Tom Lane wrote:
> Peter Geoghegan writes:
> > Is somebody going to commit this soon? Alexander?
>
> Done.
Thanks!
> If it's still not stable I think the next step is to nuke both
> test queries, since I remain of the opinion that t
On Fri, Apr 4, 2025 at 11:19 AM Alena Rybakina
wrote:
> I fixed it - changed the tables and didn't use system tables.
Is somebody going to commit this soon? Alexander?
--
Peter Geoghegan
On 04.04.25 14:17, Andres Freund wrote:
skink/valgrind just started to die during the main regression tests:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=skink&dt=2025-04-04%2011%3A01%3A56
Thanks, fix is on the way.
The set of commits seem to point to the changes made as part of th
On 31.03.25 15:52, Tom Lane wrote:
Peter Eisentraut writes:
I'm not sure what to do with this. If setlocale() and newlocale()
indeed behave differently in what set of locale names they accept, then
technically we ought to test both of them, since we do use both of them
later on. Or may
On 17.03.25 19:54, Jeff Davis wrote:
On Thu, 2025-03-13 at 14:49 +0100, Peter Eisentraut wrote:
I think these test result changes are incorrect. AFAICT, nothing has
changed in the upstream data that would explain such a change.
I didn't get such test differences in my original patch. Di
On Wed, Apr 2, 2025 at 12:43 PM Álvaro Herrera wrote:
> I'm just saying that this is the reason to have the field be last in the
> struct.
Right. And I'm just saying that I don't think that it would take very
much effort to change it. That aspect isn't performance critical.
--
Peter Geoghegan
On 03.04.25 10:07, Alvaro Herrera wrote:
The new flag is there for quick access by get_relation_info. We could
easily not have it otherwise, because clients don't need it, but its
lack would probably make planning measurably slower because it'd have to
do syscache access for every single not-nul
nch right now is that we should probably take the patch that sets
the version option and consider it for backpatching. The patch with the
tests can be held for detailed review later.
From c1e85711e1b0c7efcd1fa55cc46db959e12d6cfb Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
Date: Thu, 3 Apr 202
It occurred to me that we will also want to have NOT NULL NOT ENFORCED
constraints eventually. As we have discussed elsewhere, the NOT
ENFORCED state is closely related to the NOT VALID state. So that
should probably be considered in the design here.
Reading up on this again now, I'm confuse
On 11.03.25 11:42, Peter Eisentraut wrote:
Here is an updated patch that works more along those lines. It adds a
pg_upgrade option --update-checksums, which activates the code to
rewrite the checksums. You must specify this option if the source and
target clusters have different checksum
gresql.org/message-id/CAHut%2BPtDHRif49G%2BbzspOGspETym5oKseD13v0tcBJXWUrTx9A%40mail.gmail.com
[3] https://www.postgresql.org/message-id/547688.1738630459%40sss.pgh.pa.us
Kind Regards,
Peter Smith.
Fujitsu Australia
he common case where there weren't too many tuples to
return from the page.
The way that we allocate BLCKSZ twice for index-only scans (one for
so->currTuples, the other for so->markTuples) is also pretty
inefficient. Especially because any kind of use of mark and restore is
exceedingly rare.
--
Peter Geoghegan
In
practice, merge joins tend to hardly ever do that (I know this because
it makes testing annoyingly difficult). In other words, the
optimization added by commit 08ae5edc5c seems to be very effective in
practice.
--
Peter Geoghegan
On 28.03.25 14:27, Amul Sul wrote:
On Fri, Mar 28, 2025 at 3:34 PM Ashutosh Bapat
wrote:
On Thu, Mar 27, 2025 at 6:25 PM Amul Sul wrote:
I am not sure how to make such tests stable enough since the trigger
name involves OIDs. In count check, I tried adjusting the join
condition to ensure t
Thanks for pushing.
==
Kind Regards,
Peter Smith.
Fujitsu Australia
thin
_bt_set_startikey itself this way, which doesn't add up to much.
_bt_set_startikey is only called once per page.
Besides, in general it's fairly likely that a range skip array that
_bt_set_startikey sees won't be against a column that we already know
(from the _bt_keep_natts_fa
ly address the opclass
author's responsibilities around NULLs? It specifically says that it's
not up to the opclass author to think about them at all.
> The current system is quite easy to build a leaking
> implementation for. Sure, there are other easy ways to break this, but
> I think it's an easy API modification that makes things just that bit
> safer.
How can I do that? The callbacks themselves (the callbacks for
functions that are called as the scan progresses) don't use the fmgr
interface.
They're simple C function pointers (rather like sort support
callbacks), and do not have any args related to NULLs. They accept a
raw Datum, which can never be a NULL. The convention followed by
similar functions that are passed a Datum that might just be a NULL is
for the function to also accept a separate "bool isnull" argument.
(Just not having such a separate bool arg is another existing
convention, and the one that I've followed here.)
> A renewed review for 0002+ will arrive at a later point.
Thanks for the review!
--
Peter Geoghegan
ritten away from that it's a
glibc-specific extension.
From e12c5d224a195b893f79c470bc4f21de1585c808 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
Date: Tue, 1 Apr 2025 19:00:39 +0200
Subject: [PATCH] WIP: Fix AC_CHECK_DECLS equivalent in meson
---
meson.build | 19
nce I uploaded a complete HTML code coverage report
> (~36 Mb) [1].
Thanks. I actually generated a similar coverage report myself
yesterday. I'm keeping an eye on this, but I don't think that it's
worth aiming for very high test coverage for these code paths. Writing
a failing test case for that one ISNULL _bt_advance_array_keys code
path was quite difficult, and required quite a big index. That isn't
going to be acceptable within the standard regression tests.
--
Peter Geoghegan
On 31.03.25 08:28, Kyotaro Horiguchi wrote:
If you're already aware of this and have taken it into account, please
feel free to ignore this.
As described in the recent commit a0ed19e0a9e, many %ll? format
specifiers are being replaced with %.
I hadn’t paid much attention to this before, but I h
On 20.03.25 12:59, Peter Eisentraut wrote:
v22-0006-Convert-from-StrategyNumber-to-CompareType.patch
This is all that remains now. I think with a bit more polishing around
the edges, some comment updates, etc., this is close to ready.
Here is an updated version of this patch. I have left
On 10.03.25 10:49, Peter Eisentraut wrote:
On 02.03.25 22:08, Thomas Munro wrote:
On Mon, Mar 3, 2025 at 6:21 AM Peter Eisentraut
wrote:
On 05.12.24 23:18, Thomas Munro wrote:
Old: errmsg("hello %llu", (unsigned long long) x)
New: errmsg("hello %" PRIu64, x
On 28.03.25 17:32, Peter Eisentraut wrote:
On 28.03.25 17:14, Masahiko Sawada wrote:
On Fri, Mar 28, 2025 at 8:30 AM Peter Eisentraut
wrote:
On 09.02.25 08:32, Peter Eisentraut wrote:
Checking the status of this thread ...
The patches that removed the configure checks for
of the big patch file, though. Maybe you're just not used to seeing
tests appear last like this?
I use "git config diff.orderfile ... " to get this behavior. I find it
useful to put the important changes (particularly header file changes)
first, and less important changes (like tests) much later.
Thanks for taking a look at my patch!
--
Peter Geoghegan
On 28.03.25 17:14, Masahiko Sawada wrote:
On Fri, Mar 28, 2025 at 8:30 AM Peter Eisentraut wrote:
On 09.02.25 08:32, Peter Eisentraut wrote:
Checking the status of this thread ...
The patches that removed the configure checks for _configthreadlocale(),
and related cleanup, have been
On 09.02.25 08:32, Peter Eisentraut wrote:
Checking the status of this thread ...
The patches that removed the configure checks for _configthreadlocale(),
and related cleanup, have been committed.
The original patch to "Tidy up locale thread safety in ECPG library" is
still o
On 24.03.25 04:26, jian he wrote:
rebase, and some minor code comments change.
I have committed this.
I did a bit more editing on the naming of various things. In terms of
functionality, in ExecConstraints() notnull_virtual_attrs and
virtual_notnull_check were redundant, so I simplified tha
On 02.03.25 22:08, Thomas Munro wrote:
And one more thing like that: in a couple of places we see warnings on
macOS CI that I'd missed: when printing the result of i64abs() as
PRId64, because it happens to use labs() and it happens to define
int64_t as long long, [...]. I
suppose we should cast
On 27.03.25 11:16, Peter Eisentraut wrote:
Patch 0002 also looks okay, except that the error handling could be
unified with existing code, as I had previously pointed out. Patch
0005 fixes that.
I plan to commit this one next, after the above had a bit of time to stew.
also done
About
f a missed opportunity. It doesn't guarantee
that the scan can end in the case of a SAOP (the very next leaf page
could satisfy the same scan key, given a SAOP array with "gaps" in the
elements). So it can only really end the scan with a scalar = key --
though never when it is preceded by a skip array (doesn't matter if
the skip array is definitely satisfied/has only one distinct attribute
value on the page). Is this idea related to your previous idea
involving "special-casing firstchangingattnum=1"?
If I was going to do something like this, I think that it'd work by
backing out of applying the optimization entirely. Right now, 0003-*
applies the optimization whenever _bt_readpage decides to call
_bt_skip_ikeyprefix, regardless of the details after that (it would be
easy to teach _bt_skip_ikeyprefix to decide against applying its
optimization entirely, but testing seems to show that it always makes
sense to go ahead when _bt_skip_ikeyprefix is called, regardless of
what _bt_skip_ikeyprefix sees on the page).
> Thank you for working on this.
Thanks for the review!
--
Peter Geoghegan
than it's worth.
The duplication isn't quite duplication, so much as it's two functions
that are mirror images of each other. The details/direction of things
is flipped in a number of places. The strategy number differs, even
though the same function is called.
--
Peter Geoghegan
On 20.03.25 18:20, David E. Wheeler wrote:
On Mar 20, 2025, at 09:06, Peter Eisentraut wrote:
This is a quick follow-up to the extension_control_path patch. With this little
additional patch, you can now run "make check" in PGXS-using extensions
(instead of having to do make ins
On 14.02.25 15:13, Peter Eisentraut wrote:
On 09.02.25 15:52, Peter Eisentraut wrote:
This patch set is still desirable. Here is a rebased version of the
v5 patches. I haven't applied any corrections or review comments.
Here is the same patch set with some review comments.
Patch 0001
On 24.03.25 21:33, Matheus Alcantara wrote:
I'm a bit confused about the refactoring patch 0001. There are some
details there that don't seem right. For example, you write that the
pfree(rconn) calls are no longer necessary, but AFAICT, it would still
be needed in dblink_get_conn(). Also, ther
1 - 100 of 2341 matches
Mail list logo