Re: Moving forward with TDE
Hi, I was re-reading the patches here and there was one thing I didn't understand. There are provisions for a separation of data encryption keys for primary and replica I see, and these share a single WAL key. But if I am setting up a replica from the primary, and the primary is already encrypted, then do these forceably share the same data encrypting keys? Is there a need to have (possibly in a follow-up patch) an ability to decrypt and re-encrypt in pg_basebackup (which would need access to both keys) or is this handled already and I just missed it? Best Wishes, Chris Travers
Re: [PoC] pg_upgrade: allow to upgrade publisher node
17.12.2023 07:02, Thomas Munro wrote: FYI fairywren failed in this test: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=fairywren&dt=2023-12-16%2022%3A03%3A06 ===8<=== Restoring database schemas in the new cluster *failure* Consult the last few lines of "C:/tools/nmsys64/home/pgrunner/bf/root/HEAD/pgsql.build/testrun/pg_upgrade/003_logical_slots/data/t_003_logical_slots_newpub_data/pgdata/pg_upgrade_output.d/20231216T221418.035/log/pg_upgrade_dump_1.log" for the probable cause of the failure. Failure, exiting [22:14:34.598](22.801s) not ok 10 - run of pg_upgrade of old cluster [22:14:34.600](0.001s) # Failed test 'run of pg_upgrade of old cluster' # at C:/tools/nmsys64/home/pgrunner/bf/root/HEAD/pgsql/src/bin/pg_upgrade/t/003_logical_slots.pl line 177. ===8<=== Without that log it might be hard to figure out what went wrong though :-/ Yes, but most probably it's the same failure as https://www.postgresql.org/message-id/flat/TYAPR01MB5866AB7FD922CE30A2565B8BF5A8A%40TYAPR01MB5866.jpnprd01.prod.outlook.com Best regards, Alexander
Re: [PoC] pg_upgrade: allow to upgrade publisher node
FYI fairywren failed in this test: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=fairywren&dt=2023-12-16%2022%3A03%3A06 ===8<=== Restoring database schemas in the new cluster *failure* Consult the last few lines of "C:/tools/nmsys64/home/pgrunner/bf/root/HEAD/pgsql.build/testrun/pg_upgrade/003_logical_slots/data/t_003_logical_slots_newpub_data/pgdata/pg_upgrade_output.d/20231216T221418.035/log/pg_upgrade_dump_1.log" for the probable cause of the failure. Failure, exiting [22:14:34.598](22.801s) not ok 10 - run of pg_upgrade of old cluster [22:14:34.600](0.001s) # Failed test 'run of pg_upgrade of old cluster' # at C:/tools/nmsys64/home/pgrunner/bf/root/HEAD/pgsql/src/bin/pg_upgrade/t/003_logical_slots.pl line 177. ===8<=== Without that log it might be hard to figure out what went wrong though :-/
Re: Synchronizing slots from primary to standby
On Fri, Dec 15, 2023 at 5:55 PM Nisha Moond wrote: > > (1) > When we try to create a subscription on standby using a synced slot > that is in 'r' sync_state, the subscription will be created at the > subscriber, and on standby, two actions will take place - > (i) As copy_data is true by default, it will switch the failover > state of the synced-slot to 'false'. > (ii) As we don't allow to use synced-slots, it will start giving > the expected error in the log file - > ERROR: cannot use replication slot "logical_slot" for logical > decoding > DETAIL: This slot is being synced from the primary server. > HINT: Specify another replication slot. > > The first one seems an issue, it toggles the failover to false and > then it remains false after that. I think it should be fixed. > +1. If we don't allow the slot to be used, we shouldn't allow its state to be changed as well. > (2) > With the patch, the 'CREATE SUBSCRIPTION' command with a 'slot_name' > of an 'active' logical slot fails and errors out - > ERROR: could not alter replication slot "logical_slot" on > publisher: ERROR: replication slot "logical_slot1" is active for PID > > > Without the patch, the create subscription with an 'active' slot_name > succeeds and the log file shows the error "could not start WAL > streaming: ERROR: replication slot "logical_slot" is active for PID > ". > > Given that the specified active slot_name has failover set to false > and the create subscription command also specifies failover=false, the > expected behavior of the "with-patch" case is anticipated to be the > same as that of the "without-patch" scenario. > Currently, we first acquire the slot to change its state but I guess if we want the behavior as you mentioned we first need to check the slot's 'failover' state without acquiring the slot. I am not sure if that is any better because anyway we are going to fail in the very next step as the slot is busy. -- With Regards, Amit Kapila.
Re: Improve eviction algorithm in ReorderBuffer
On Fri, Dec 15, 2023 at 11:29 AM Masahiko Sawada wrote: > > On Fri, Dec 15, 2023 at 12:37 PM Amit Kapila wrote: > > > > On Wed, Dec 13, 2023 at 6:01 AM Masahiko Sawada > > wrote: > > > > > > > IIUC, you are giving preference to multiple list ideas as compared to > > (a) because you don't need to adjust the list each time the > > transaction size changes, is that right? > > Right. > > > If so, I think there is a > > cost to keep that data structure up-to-date but it can help in > > reducing the number of times we need to serialize. > > Yes, there is a trade-off. > > What I don't want to do is to keep all transactions ordered since it's > too costly. The proposed idea uses multiple lists to keep all > transactions roughly ordered. The maintenance cost would be cheap > since each list is unordered. > > It might be a good idea to have a threshold to switch how to pick the > largest transaction based on the number of transactions in the > reorderbuffer. If there are many transactions, we can use the proposed > algorithm to find a possibly-largest transaction, otherwise use the > current way. > Yeah, that makes sense. > > > > > > > > > 1) A scenario where suppose you have one very large transaction that > > > > is consuming ~40% of the memory and 5-6 comparatively smaller > > > > transactions that are just above 10% of the memory limit. And now for > > > > coming under the memory limit instead of getting 1 large transaction > > > > evicted out, we are evicting out multiple times. > > > > > > Given the large transaction list will have up to 10 transactions, I > > > think it's cheap to pick the largest transaction among them. It's O(N) > > > but N won't be large. > > > > > > > 2) Another scenario where all the transactions are under 10% of the > > > > memory limit but let's say there are some transactions are consuming > > > > around 8-9% of the memory limit each but those are not very old > > > > transactions whereas there are certain old transactions which are > > > > fairly small and consuming under 1% of memory limit and there are many > > > > such transactions. So how it would affect if we frequently select > > > > many of these transactions to come under memory limit instead of > > > > selecting a couple of large transactions which are consuming 8-9%? > > > > > > Yeah, probably we can do something for small transactions (i.e. small > > > and on-memory transactions). One idea is to pick the largest > > > transaction among them by iterating over all of them. Given that the > > > more transactions are evicted, the less transactions the on-memory > > > transaction list has, unlike the current algorithm, we would still > > > win. Or we could even split it into several sub-lists in order to > > > reduce the number of transactions to check. For example, splitting it > > > into two lists: transactions consuming 5% < and 5% >= of the memory > > > limit, and checking the 5% >= list preferably. > > > > > > > Which memory limit are you referring to here? Is it > > logical_decoding_work_mem? > > logical_decoding_work_mem. > > > > > > The cost for > > > maintaining these lists could increase, though. > > > > > > > Yeah, can't we maintain a single list of all xacts that are consuming > > equal to or greater than the memory limit? Considering that the memory > > limit is logical_decoding_work_mem, then I think just picking one > > transaction to serialize would be sufficient. > > IIUC we serialize a transaction when the sum of all transactions' > memory usage in the reorderbuffer exceeds logical_decoding_work_mem. > In what cases are multiple transactions consuming equal to or greater > than the logical_decoding_work_mem? > The individual transactions shouldn't cross 'logical_decoding_work_mem'. I got a bit confused by your proposal to maintain the lists: "...splitting it into two lists: transactions consuming 5% < and 5% >= of the memory limit, and checking the 5% >= list preferably.". In the previous sentence, what did you mean by transactions consuming 5% >= of the memory limit? I got the impression that you are saying to maintain them in a separate transaction list which doesn't seems to be the case. -- With Regards, Amit Kapila.
Re: XID formatting and SLRU refactorings
Hi,
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=loach&dt=2023-12-16%2005%3A25%3A18
TRAP: failed Assert("epoch > 0"), File: "twophase.c", Line: 969, PID: 71030
0xa8edcd at
/usr/home/pgbf/buildroot/HEAD/pgsql.build/tmp_install/home/pgbf/buildroot/HEAD/inst/bin/postgres
0x613863 at
/usr/home/pgbf/buildroot/HEAD/pgsql.build/tmp_install/home/pgbf/buildroot/HEAD/inst/bin/postgres
That's the new assertion from 5a1dfde8:
+ * The wrap logic is safe here because the span of active xids cannot
exceed one
+ * epoch at any given time.
...
+ if (unlikely(xid > nextXid))
+ {
+ /* Wraparound occured, must be from a prev epoch. */
+ Assert(epoch > 0);
Re: Show WAL write and fsync stats in pg_stat_io
On Tue, Dec 12, 2023 at 02:29:03PM +0300, Nazir Bilal Yavuz wrote: > On Tue, 5 Dec 2023 at 09:16, Michael Paquier wrote: >> This interface from pgstat_prepare_io_time() is not really good, >> because we could finish by setting io_start in the existing code paths >> calling this routine even if track_io_timing is false when >> track_wal_io_timing is true. Why not changing this interface a bit >> and pass down a GUC (track_io_timing or track_wal_io_timing) as an >> argument of the function depending on what we expect to trigger the >> timings? > > Done in 0001. One thing that 0001 missed is an update of the header where the function is declared. I've edited a few things, and applied it to start on this stuff. The rest will have to wait a bit more.. -- Michael signature.asc Description: PGP signature
Re: useless LIMIT_OPTION_DEFAULT
On 2023-Dec-14, Tom Lane wrote: > Zhang Mingli writes: > > 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 default limit enum. > > I agree, that looks pretty useless. Our normal convention for > representing not having any LIMIT clause would be to not create > a SelectLimit node at all. I don't see why allowing a second > representation is a good idea, especially when the code is not > in fact doing that. > > git blame shows that this came in with 357889eb1. Alvaro, > Surafel, do you want to argue for keeping things as-is? I looked at the history of this. That enum member first appeared as a result of your review at [1], and the accompanying code at that time did use it a few times. The problem is that I later ([2]) proposed to rewrite that code and remove most of the uses of that enum member, but failed to remove it completely. So, I think we're OK to remove it. I'm going to push Zhang's patch soon unless there are objections. [1] https://postgr.es/m/3277.1567722...@sss.pgh.pa.us [2] https://postgr.es/m/20191125203442.GA30191@alvherre.pgsql -- Álvaro HerreraBreisgau, Deutschland — https://www.EnterpriseDB.com/ "Doing what he did amounts to sticking his fingers under the hood of the implementation; if he gets his fingers burnt, it's his problem." (Tom Lane)
Re: [PoC] Reducing planning time when tables have many partitions
Hi!
On 13.12.2023 09:21, Yuya Watari wrote:
Hello Alena, Andrei, and all,
I am sorry for my late response. I found that the current patches do
not apply to the master, so I have rebased those patches. I have
attached v22. For this later discussion, I separated the rebasing and
bug fixing that Alena did in v21 into separate commits, v22-0003 and
v22-0004. I will merge these commits after the discussion.
1. v22-0003 (solved_conflict_with_self_join_removal.txt)
Thank you!
Thank you for your rebase. Looking at your rebasing patch, I thought
we could do this more simply. Your patch deletes (more precisely, sets
to null) non-redundant members from the root->eq_sources list and
re-adds them to the same list. However, this approach seems a little
waste of memory. Instead, we can update
EquivalenceClass->ec_source_indexes directly. Then, we can reuse the
members in root->eq_sources and don't need to extend root->eq_sources.
I did this in v22-0003. What do you think of this approach?
I thought about this earlier and was worried that the index links of the
equivalence classes might not be referenced correctly for outer joins,
so I decided to just overwrite them and reset the previous ones.
The main concern with this idea is that it does not fix
RangeTblEntry->eclass_source_indexes. The current code works fine even
if we don't fix the index because get_ec_source_indexes() always does
bms_intersect() for eclass_source_indexes and ec_source_indexes. If we
guaranteed this behavior of doing bms_intersect, then simply modifying
ec_source_indexes would be fine. Fortunately, such a guarantee is not
so difficult.
And your patch removes the following assertion code from the previous
patch. May I ask why you removed this code? I think this assertion is
helpful for sanity checks. Of course, I know that this kind of
assertion will slow down regression tests or assert-enabled builds.
So, we may have to discuss which assertions to keep and which to
discard.
=
-#ifdef USE_ASSERT_CHECKING
- /* verify the results look sane */
- i = -1;
- while ((i = bms_next_member(rel_esis, i)) >= 0)
- {
- RestrictInfo *rinfo = list_nth_node(RestrictInfo, root->eq_sources,
- i);
-
- Assert(bms_overlap(relids, rinfo->clause_relids));
- }
-#endif
=
this is due to the fact that I explained before: we zeroed the values
indicated by the indexes,
then this check is not correct either - since the zeroed value indicated
by the index is correct.
That's why I removed this check.
Finally, your patch changes the name of the following function. I
understand the need for this change, but it has nothing to do with our
patches, so we should not include it and discuss it in another thread.
=
-update_eclasses(EquivalenceClass *ec, int from, int to)
+update_eclass(PlannerInfo *root, EquivalenceClass *ec, int from, int to)
=
I agree.
2. v22-0004 (bug_related_to_atomic_function.txt)
Thank you for fixing the bug. As I wrote in the previous mail:
On Wed, Nov 22, 2023 at 2:32 PM Yuya Watari wrote:
On Mon, Nov 20, 2023 at 1:45 PM Andrei Lepikhov
wrote:
During the work on committing the SJE feature [1], Alexander Korotkov
pointed out the silver lining in this work [2]: he proposed that we
shouldn't remove RelOptInfo from simple_rel_array at all but replace it
with an 'Alias', which will refer the kept relation. It can simplify
further optimizations on removing redundant parts of the query.
Thank you for sharing this information. I think the idea suggested by
Alexander Korotkov is also helpful for our patch. As mentioned above,
the indexes are in RangeTblEntry in the current implementation.
However, I think RangeTblEntry is not the best place to store them. An
'alias' relids may help solve this and simplify fixing the above bug.
I will try this approach soon.
I think that the best way to solve this issue is to move these indexes
from RangeTblEntry to RelOptInfo. Since they are related to planning
time, they should be in RelOptInfo. The reason why I put these indexes
in RangeTblEntry is because some RelOptInfos can be null and we cannot
store the indexes. This problem is similar to an issue regarding
'varno 0' Vars. I hope an alias RelOptInfo would help solve this
issue. I have attached the current proof of concept I am considering
as poc-alias-reloptinfo.txt. To test this patch, please follow the
procedure below.
1. Apply all *.patch files,
2. Apply Alexander Korotkov's alias_relids.patch [1], and
3. Apply poc-alias-reloptinfo.txt, which is attached to this email.
My patch creates a dummy (or an alias) RelOptInfo to store indexes if
the corresponding RelOptInfo is null. The following is the core change
in my patch.
=
@@ -627,9 +627,19 @@ add_eq_source(PlannerInfo *root, EquivalenceClass
*ec, RestrictInfo *rinfo)
i = -1;
while ((i = bms_next_member(rinfo->clause_relids, i)) >= 0)
{
- RangeTblEntry *rte = root->simple_rte_array[i];
+ RelOpt
Re: [PATCH] Add support function for containment operators
fix a typo and also did a minor change.
from
+ if (lowerExpr != NULL && upperExpr != NULL)
+ return (Node *) makeBoolExpr(AND_EXPR, list_make2(lowerExpr, upperExpr),
-1);
+ else if (lowerExpr != NULL)
+ return (Node *) lowerExpr;
+ else if (upperExpr != NULL)
+ return (Node *) upperExpr;
to
+ if (lowerExpr != NULL && upperExpr != NULL)
+ return (Node *) makeBoolExpr(AND_EXPR, list_make2(lowerExpr, upperExpr),
-1);
+ else if (lowerExpr != NULL)
+ return (Node *) lowerExpr;
+ else if (upperExpr != NULL)
+ return (Node *) upperExpr;
+ else
+ {
+ Assert(false);
+ return NULL;
+ }
because cfbot says:
15:04:38.116] make -s -j${BUILD_JOBS} clean
[15:04:38.510] time make -s -j${BUILD_JOBS} world-bin
[15:04:43.272] rangetypes.c:2908:1: error: non-void function does not
return a value in all control paths [-Werror,-Wreturn-type]
[15:04:43.272] }
[15:04:43.272] ^
[15:04:43.272] 1 error generated.
also add some commit messages, I hope it will be useful.
From 334566a62163469f53cfd941ed87e6d6e54d756b Mon Sep 17 00:00:00 2001
From: pgaddict
Date: Mon, 11 Dec 2023 19:14:01 +0800
Subject: [PATCH v5 1/1] Simplify containment in range constants with support function
doc:
https://www.postgresql.org/docs/current/extend-type-system.html#EXTEND-TYPES-POLYMORPHIC
Similarly, if there are positions declared anyrange and others declared anyelement or anyarray,
the actual range type in the anyrange positions must be a range
whose subtype is the same type appearing in the anyelement positions
and the same as the element type of the anyarray positions.
proname => 'range_contains_elem', prorettype => 'bool', proargtypes => 'anyrange anyelement',
proname => 'elem_contained_by_range', prorettype => 'bool', proargtypes => 'anyelement anyrange'
It will work, because the range constant base type will be the same as the element.
so we can safely rewrite "element within a range or not"
to element comparison with range lower bound
and (&&) element comparison with range upper bound.
and these simple same data type comparisons can be supported by btree index.
---
src/backend/utils/adt/rangetypes.c | 196
src/backend/utils/adt/rangetypes_selfuncs.c | 6 +-
src/include/catalog/pg_proc.dat | 12 +-
src/test/regress/expected/rangetypes.out| 193 +++
src/test/regress/sql/rangetypes.sql | 92 +
5 files changed, 494 insertions(+), 5 deletions(-)
diff --git a/src/backend/utils/adt/rangetypes.c b/src/backend/utils/adt/rangetypes.c
index 24bad529..0b3eab08 100644
--- a/src/backend/utils/adt/rangetypes.c
+++ b/src/backend/utils/adt/rangetypes.c
@@ -31,16 +31,24 @@
#include "postgres.h"
#include "access/tupmacs.h"
+#include "access/stratnum.h"
+#include "catalog/pg_range.h"
#include "common/hashfn.h"
#include "lib/stringinfo.h"
#include "libpq/pqformat.h"
#include "miscadmin.h"
#include "nodes/miscnodes.h"
+#include "nodes/makefuncs.h"
+#include "nodes/nodeFuncs.h"
+#include "nodes/pg_list.h"
+#include "nodes/supportnodes.h"
#include "port/pg_bitutils.h"
#include "utils/builtins.h"
+#include "utils/fmgroids.h"
#include "utils/date.h"
#include "utils/lsyscache.h"
#include "utils/rangetypes.h"
+#include "utils/syscache.h"
#include "utils/timestamp.h"
#include "varatt.h"
@@ -69,6 +77,10 @@ static Size datum_compute_size(Size data_length, Datum val, bool typbyval,
char typalign, int16 typlen, char typstorage);
static Pointer datum_write(Pointer ptr, Datum datum, bool typbyval,
char typalign, int16 typlen, char typstorage);
+static Expr *build_bound_expr(Oid opfamily, TypeCacheEntry *typeCache,
+ bool isLowerBound, bool isInclusive,
+ Datum val, Expr *otherExpr, Oid rng_collation);
+static Node *find_simplified_clause(Const *rangeConst, Expr *otherExpr);
/*
@@ -2173,6 +2185,57 @@ make_empty_range(TypeCacheEntry *typcache)
return make_range(typcache, &lower, &upper, true, NULL);
}
+/*
+ * Planner support function for the elem_contained_by_range (<@) operator.
+ */
+Datum
+elem_contained_by_range_support(PG_FUNCTION_ARGS)
+{
+ Node *rawreq = (Node *) PG_GETARG_POINTER(0),
+ *leftop,
+ *rightop;
+ FuncExpr *fexpr;
+
+ if (!IsA(rawreq, SupportRequestSimplify))
+ PG_RETURN_POINTER(NULL);
+
+ fexpr = ((SupportRequestSimplify *) rawreq)->fcall;
+
+ Assert(list_length(fexpr->args) == 2);
+ leftop = linitial(fexpr->args);
+ rightop = lsecond(fexpr->args);
+
+ if (!IsA(rightop, Const) || ((Const *) rightop)->constisnull)
+ PG_RETURN_POINTER(NULL);
+
+ PG_RETURN_POINTER(find_simplified_clause((Const *) rightop, (Expr *) leftop));
+}
+
+/*
+ * Planner support function for the range_contains_elem (@>) operator.
+ */
+Datum
+range_contains_elem_support(PG_FUNCTION_ARGS)
+{
+ Node *rawreq = (Node *) PG_GETARG_POINTER(0),
+ *leftop,
+ *rightop;
+ FuncExpr *fexpr;
+
+ if (!IsA(rawreq, SupportRequestSimplify))
+ PG_RETURN_POINTER(NULL);
+
+ fexpr = ((SupportRequestSimplify *) rawreq)->fc
Re: Adding OLD/NEW support to RETURNING
On Mon, Dec 4, 2023 at 8:15 PM Dean Rasheed wrote:
>
> I have been playing around with the idea of adding support for OLD/NEW
> to RETURNING, partly motivated by the discussion on the MERGE
> RETURNING thread [1], but also because I think it would be a very
> useful addition for other commands (UPDATE in particular).
>
> This was discussed a long time ago [2], but that previous discussion
> didn't lead to a workable patch, and so I have taken a different
> approach here.
>
> Thoughts?
>
/* get the tuple from the relation being scanned */
- scratch.opcode = EEOP_ASSIGN_SCAN_VAR;
+ switch (variable->varreturningtype)
+ {
+ case VAR_RETURNING_OLD:
+ scratch.opcode = EEOP_ASSIGN_OLD_VAR;
+ break;
+ case VAR_RETURNING_NEW:
+ scratch.opcode = EEOP_ASSIGN_NEW_VAR;
+ break;
+ default:
+ scratch.opcode = EEOP_ASSIGN_SCAN_VAR;
+ break;
+ }
I have roughly an idea of what this code is doing. but do you need to
refactor the above comment?
/* for EEOP_INNER/OUTER/SCAN_FETCHSOME */
in src/backend/executor/execExpr.c, do you need to update the comment?
create temp table foo (f1 int, f2 int);
insert into foo values (1,2), (3,4);
INSERT INTO foo select 11, 22 RETURNING WITH (old AS new, new AS old)
new.*, old.*;
--this works. which is fine.
create or replace function stricttest1() returns void as $$
declare x record;
begin
insert into foo values(5,6) returning new.* into x;
raise notice 'x.f1 = % x.f2 %', x.f1, x.f2;
end$$ language plpgsql;
select * from stricttest1();
--this works.
create or replace function stricttest2() returns void as $$
declare x record; y record;
begin
INSERT INTO foo select 11, 22 RETURNING WITH (old AS o, new AS n)
o into x, n into y;
raise notice 'x.f1: % x.f2 % y.f1 % y.f2 %', x.f1,x.f2, y.f1, y.f2;
end$$ language plpgsql;
--this does not work.
--because
https://www.postgresql.org/message-id/flat/CAFj8pRB76FE2MVxJYPc1RvXmsf2upoTgoPCC9GsvSAssCM2APQ%40mail.gmail.com
create or replace function stricttest3() returns void as $$
declare x record; y record;
begin
INSERT INTO foo select 11, 22 RETURNING WITH (old AS o, new AS n) o.*,n.*
into x;
raise notice 'x.f1 % x.f2 %, % %', x.f1, x.f2, x.f1,x.f2;
end$$ language plpgsql;
select * from stricttest3();
--this is not what we want. because old and new share the same column name
--so here you cannot get the "new" content.
create or replace function stricttest4() returns void as $$
declare x record; y record;
begin
INSERT INTO foo select 11, 22
RETURNING WITH (old AS o, new AS n)
o.f1 as of1,o.f2 as of2,n.f1 as nf1, n.f2 as nf2
into x;
raise notice 'x.0f1 % x.of2 % nf1 % nf2 %', x.of1, x.of2, x.nf1, x.nf2;
end$$ language plpgsql;
--kind of verbose, but works, which is fine.
create or replace function stricttest5() returns void as $$
declare x record; y record;
a foo%ROWTYPE; b foo%ROWTYPE;
begin
INSERT INTO foo select 11, 22
RETURNING WITH (old AS o, new AS n) o into a, n into b;
end$$ language plpgsql;
-- expect this to work.
Re: Clang optimiser vs preproc.c
Hi, On 2023-12-15 22:19:56 -0500, Tom Lane wrote: > Thomas Munro writes: > > On Sat, Dec 16, 2023 at 3:44 PM Tom Lane wrote: > >> Thomas Munro writes: > >>> FYI, it looks like there is a big jump in CPU time to compile preproc.c > >>> at -O2: > >>> clang15: ~16s > >>> clang16: ~211s > >>> clang17: ~233s Is this with non-assert clangs? Because I see times that seem smaller by more than what can be explained by hardware differences: preproc.c: 17 10.270s 169.685s 158.300s gram.c: 171.936s 162.131s 152.161s That's still bad, but a far cry away from 233s. > Huh. There's not that many more productions in the ecpg grammar > than the core, so it doesn't seem likely that this is purely a > size-of-file issue. I'd bet on there being something that clang > doesn't do well about the (very stylized) C code being generated > within the grammar productions. > > We actually noticed this or a closely-related problem before [1] > and briefly discussed the possibility of rearranging the generated > code to make it less indigestible to clang. But there was no concrete > idea about what to do specifically, and the thread slid off the radar > screen. One interest aspect might be that preproc.c ends up with ~33% more states than gram.c gram.c: #define YYLAST 116587 preproc.c: #define YYLAST 155495 Greetings, Andres Freund
Re: Improve eviction algorithm in ReorderBuffer
On Sat, Dec 16, 2023 at 1:36 AM Euler Taveira wrote: > > On Fri, Dec 15, 2023, at 9:11 AM, Masahiko Sawada wrote: > > > I assume you mean to add ReorderBufferTXN entries to the binaryheap > and then build it by comparing their sizes (i.e. txn->size). But > ReorderBufferTXN entries are removed and deallocated once the > transaction finished. How can we efficiently remove these entries from > binaryheap? I guess it would be O(n) to find the entry among the > unordered entries, where n is the number of transactions in the > reorderbuffer. > > > O(log n) for both functions: binaryheap_remove_first() and > binaryheap_remove_node(). Right. The binaryheap_binaryheap_remove_first() removes the topmost entry in O(log n), but the ReorderBufferTXN being removed is not necessarily the topmost entry, since we remove the entry when the transaction completes (committed or aborted). The binaryheap_remove_node() removes the entry at the given Nth in O(log n), but I'm not sure how we can know the indexes of each entry. I think we can remember the index of newly added entry after calling binaryheap_add_unordered() but once we call binaryheap_build() the index is out-of-date. So I think that in the worst case we would need to check all entries in order to remove an arbitrary entry in binaryheap. It's O(n). I might be missing something though. > I didn't read your patch but do you really need to > free entries one by one? If not, binaryheap_free(). The patch doesn't touch on how to free entries. ReorderBufferTXN entries are freed one by one after each of which completes (committed or aborted). Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
[DOC] Introducing Quick Start Guide to PL/pgSQL and PL/Python Documentation
Hi I have made some documentation enhancements for PL/pgSQL and PL/Python sections. The changes include the addition of a Quick Start Guide to facilitate a smoother onboarding experience for users. Patch File Name: 0001-plpyhton-plpgsql-docu-changes.patch Patch Description: This patch introduces a Quick Start Guide to the documentation for PL/pgSQL and PL/Python. The Quick Start Guide provides users with a step-by-step walkthrough of setting up and using these procedural languages. The goal is to improve user accessibility and streamline the learning process. Changes Made: 1. Added a new section titled "Quick Start Guide" to both PL/pgSQL and PL/Python documentation. 2. Included step-by-step instructions for users to get started with these procedural languages. 3. Provided explanations, code snippets, and examples to illustrate key concepts. Discussion Points: I am seeking your feedback on the following aspects: - Clarity and completeness of the Quick Start Guide - Any additional information or examples that could enhance the guide - Suggestions for improving the overall documentation structure Your insights and suggestions are highly valuable, and I appreciate your time in reviewing this documentation enhancement. -- Best regards, Ishaan Adarsh 0001-plpyhton-plpgsql-docu-changes.patch Description: Binary data
Re: GUC names in messages
On Thu, Dec 14, 2023 at 09:38:40AM +0100, Peter Eisentraut wrote: > After these discussions, I think this rule change was not a good idea. It > effectively enforces these kinds of inconsistencies. For example, if you > ever refactored > > "DateStyle is wrong" > > to > > "%s is wrong" > > you'd need to adjust the quotes, and thus user-visible behavior, for > entirely internal reasons. This is not good. So, what are you suggesting? Should the encouraged rule be removed from the docs? Or do you object to some of the changes done in the latest patch series v5? FWIW, I am a bit meh with v5-0001, because I don't see the benefits. On the contrary v5-0003 is useful, because it reduces a bit the number of strings to translate. That's always good to take. I don't have a problem with v5-0002, either, where we'd begin using the name of the GUC as stored in the static tables rather than the name provided in the SET query, particularly for the reason that it makes the GUC name a bit more consistent even when using double-quotes around the parameter name in the query, where the error messages would not force a lower-case conversion. The patch would, AFAIU, change HEAD from that: =# SET "intervalstylE" to popo; ERROR: 22023: invalid value for parameter "intervalstylE": "popo" To that: =# SET "intervalstylE" to popo; ERROR: 22023: invalid value for parameter "IntervalStyle": "popo" > And then came the idea to > determine the quoting dynamically, which I think everyone agreed was too > much. So I don't see a way to make this work well. Yeah, with the quotes being language-dependent, any idea I can think of is as good as unreliable and dead. -- Michael signature.asc Description: PGP signature
Re: Remove MSVC scripts from the tree
On Thu, Dec 14, 2023 at 11:43:14AM +0900, NINGWEI CHEN wrote: > Sorry for the delayed response. > We are currently working on transitioning to meson build at hamerkop and > anticipating that this can be accomplished by no later than January. > > If the old build scripts are removed before that, hamerkop will be > temporarily > taken off the master branch, and will rejoin once the adjustment is done. Thanks for the update. Let's move on with that on HEAD then. I've wanted some room to work on improving the set of docs for v17. -- Michael signature.asc Description: PGP signature
