On Tue, 22 Apr 2025 at 22:25, Tom Lane wrote:
>
> Pavel Borisov writes:
> > On Tue, 22 Apr 2025 at 21:13, Tom Lane wrote:
> >> BTW, you forgot to update expected/float4-misrounded-input.out.
>
> > Added in v3. Thanks for a mention!
>
> v3 LGTM, pushed.
Thank you!
Regards,
Pavel
Hi, Tom!
On Tue, 22 Apr 2025 at 21:13, Tom Lane wrote:
>
> Pavel Borisov writes:
> > On Tue, 22 Apr 2025 at 20:22, Alexander Korotkov
> > wrote:
> >> On Tue, Apr 22, 2025 at 7:20 PM Tom Lane wrote:
> >>> Alexander Korotkov writes:
> >>>>
Hi!
On Tue, 22 Apr 2025 at 20:22, Alexander Korotkov wrote:
>
> On Tue, Apr 22, 2025 at 7:20 PM Tom Lane wrote:
> > Alexander Korotkov writes:
> > > I'd like to add that float4.out not only assumes that insert-ordering is
> > > preserved (this could be more-or-less portable between table AMs).
On Tue, 22 Apr 2025 at 18:40, Pavel Borisov wrote:
>
> Hi, Tom!
>
> On Tue, 22 Apr 2025 at 18:23, Tom Lane wrote:
> >
> > Pavel Borisov writes:
> > > It's not a big problem, but propose a simple fix for the tests. It
> > > just adds ORDE
Hi, Tom!
On Tue, 22 Apr 2025 at 18:23, Tom Lane wrote:
>
> Pavel Borisov writes:
> > It's not a big problem, but propose a simple fix for the tests. It
> > just adds ORDER BY 1 to all relevant float4 and floa8 queries.
>
> You do realize that this problem is hardly
ckpatching.
Regards,
Pavel Borisov,
Supabase.
v1-0001-Fortify-float4-and-float8-tests-by-ordering-test-.patch
Description: Binary data
he first position.
+EXPLAIN (COSTS OFF)
+SELECT * FROM tenk1 WHERE unique1 < 1 OR unique1 < 3 OR hundred < 2;
I propose small changes for comments:
s/To have this property,/To do so,/g
s/in place, but all the/in place, and all the/g
s/some groups, only/some groups,/g
s/Resort/Re-sort/п
The patch overall looks good to me.
Regards,
Pavel Borisov
Supabase
t
the OR order is warranted [1]. So changing OR order was (and is) ok
and any users query tricks about OR order may work and may not work.
[1] https://www.postgresql.org/docs/17/sql-expressions.html#SYNTAX-EXPRESS-EVAL
Regards,
Pavel Borisov
Supabase
s no modification of atomic
variable, so we want to see all modifications from the concurrent
operations, but will not enforce them to see any "our" change. [2]
Maybe I'm too optimistic here and having a full memory barrier is
better.
Thank you very much for considering this issue! I think the right
operation of atomic primitives is both very very important and also
hard to check thoroughly. I think the test that could reproduce the
reordering of atomic and non-atomic operations could be very useful in
regression set.
[1]
https://en.cppreference.com/w/c/atomic/memory_order#Sequentially-consistent_ordering
[2] https://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html
Best regards,
Pavel Borisov
Supabase
Oops, I send wrong patch as a fix.
The right one is attached.
Pavel
On Mon, 17 Feb 2025 at 13:40, Pavel Borisov wrote:
>
> Hi, Kirill!
> Per your report, I revised the comment to fix typos. Also some little
> changes in grammar.
>
> Kind regards,
> Pavel Borisov
v2-0001
o I'd rather see
a header like "Replace WALBufMappingLock by lockless algorithm" or
"Initialize WAL buffers concurrently without using WALBufMappingLock"
or something like that.
Kind regards,
Pavel Borisov
Supabase
Hi, Kirill!
Per your report, I revised the comment to fix typos. Also some little
changes in grammar.
Kind regards,
Pavel Borisov
0001-Fix-typo-and-grammar-in-comment-introduced-by-6a2275.patch
Description: Binary data
On Mon, 17 Feb 2025 at 13:20, Pavel Borisov wrote:
>
> Hi, Victor!
>
> On Mon, 17 Feb 2025 at 12:47, Victor Yegorov wrote:
> >
> > Hey.
> >
> > I find “Get rid of WALBufMappingLock" commit message misleading, 'cos Lock
> > it's bein
Hi, Alexander!
On Fri, 14 Feb 2025 at 00:59, Alexander Korotkov wrote:
>
> Hi, Pavel!
>
> On Thu, Feb 13, 2025 at 6:39 PM Pavel Borisov wrote:
> > On Thu, 13 Feb 2025 at 14:08, Alexander Korotkov
> > wrote:
> > >
> > > On Thu, Feb 13, 2
to..."
- Comment inside typedef struct XLogCtlData:
/* 1st byte ptr-s + XLOG_BLCKSZ (and condvar * for) */
need to be returned back
/* 1st byte ptr-s + XLOG_BLCKSZ */
- Commented out code for cleanup in the final patch:
//ConditionVariableBroadcast(&XLogCtl->xlblocks[nextidx].condvar);
- in AdvanceXLInsertBuffer()
npages initialised to 0 but it is not increased anymore
Block under
> if (XLOG_DEBUG && npages > 0)
became unreachable
(InvalidXLogRecPtr + 1) is essentially 0+1 and IMO this semantically
calls for adding #define FirstValidXLogRecPtr 1
Typo in a commit message: %s/usign/using/g
For patch 0002:
I think Yura's explanation from above in this thread need to get place
in a commit message and in a comment to this:
> int attempts = 2;
Comments around:
"try another lock next time" could be modified to reflect that we do
repeat twice
Kind regards,
Pavel Borisov
Supabase
ewing!
Thanks to Michel and everyone working on the patch!
Regards,
Pavel Borisov
Hi, Tom!
On Mon, 3 Feb 2025 at 21:53, Tom Lane wrote:
>
> Pavel Borisov writes:
> > Minor notes on the patches:
>
> > If dump_* functions could use the newly added walker, the code would
> > look better. I suppose the main complication is that dump_* functions
> &g
ost and execution time.
>
> I've also adjusted another test query as proposed by Andrei.
>
> I'm going to push this patch is there is no more notes.
>
> Links.
> 1.
> https://www.postgresql.org/message-id/fc1017ca-877b-4f86-b491-154cf123eedd%40gmail.com
>
>
> Okay.I agree with your code and have no more notes
Hi, Alexander!
I've looked at patchset v48 and it looks good to me.
Regards,
Pavel Borisov
Supabase
sbsref of
expr->expr_simple_expr that already has separate names.
Transferring target param as int paramid looks completely ok. But we
have unconditional checking Assert(paramid == expr->target_param + 1),
so it looks as a redundant split as of now. Do we plan a true split
and removal of this assert in the future?
Thanks for creating and working on this patch!
Regards,
Pavel Borisov
Supabase
;
> regards, tom lane
I started looking at the patchset.
Recently it got conflicts with changes to yyparse (473a575e05979b4db).
Could you rebase it and also do naming changes proposed by Andrew
Borodin, which I definitely agree with?
Regards,
Pavel Borisov
comment for match_clause_to_indexcol() I think
needs change. This could be as a separate commit, not regarding
current patch v46-0001.
> * NOTE: returns NULL if clause is an OR or AND clause; it is the
> * responsibility of higher-level routines to co
I think the patch can be pushed with possible additions to regression
test and comments.
Regards,
Pavel Borisov
Supabase
On Tue, 10 Dec 2024 at 19:13, Tom Lane wrote:
> Pavel Borisov writes:
> > I see Aleksander worked on this patch (with many other hackers) and
> marked
> > it as rejected. And now Evgeniy, a new developer wants to continue and
> > rebase this patch and also the patches
ds,
>> Evgeny Voropaev,
>> TantorLabs, LLC.
>>
>>
>
>
I see Aleksander worked on this patch (with many other hackers) and marked
it as rejected. And now Evgeniy, a new developer wants to continue and
rebase this patch and also the patches in another 64-xid thread
disregarding the fact that the patch is rejected. It's not a crime. It's
rejected, true.
Regards,
Pavel Borisov
Hi, Heikki!
On Wed, 23 Oct 2024 at 21:00, Heikki Linnakangas wrote:
> On 23/10/2024 12:18, Pavel Borisov wrote:
> > Hi, Hackers!
> >
> > Current comments on the usage of WL_POSTMASTER_DEATH state that it
> > should be used for scenarios of finishing other than imm
n the walsender and in libpq).
So I propose change the comments on WL_POSTMASTER_DEATH stating that it
could be used for both cases: for processing and setting return values if
that's needed, and for immediate exit otherwise.
Regards,
Pavel Borisov
Supabase
v1-0001-Refine-comment
t;
> > OK, thank you!
> >
> > The attached revision fixes EXTRA_INSTALL in
> > src/test/modules/typcache/Makefile. Spotted off-list by Arthur
> > Zakirov.
>
> I've re-checked that regression tests pass with
> -DCLOBBER_CACHE_ALWAYS. Also did some grammar corrections for
> comments and commit message. I'm going to push this if no objections.
>
Thank you for working on this patch!
Looked through the patchset once more.
Patch 0001 (minor): "in the last" -> "after everything else" or "after
other TypeCacheEntry contents"
Patch 0002 looks ready to me.
Regards,
Pavel Borisov
Supabase
On Thu, 12 Sept 2024 at 16:09, Pavel Borisov wrote:
> Hi, Maxim!
>
> Previously we accessed offsets in shared MultiXactState without locks as
> 32-bit read is always atomic. But I'm not sure it's so when offset become
> 64-bit.
> E.g. GetNewMultiXactId():
>
o the same as well.
Regards,
Pavel Borisov
Supabase
2_345
> syntax? This is not implemented in the attached patch and arguably
> could be discussed separately when and if we merge it.
>
I think 12345678912345 is good enough. Underscore dividers make reading
little bit easier but look weird overall. I can't remember other places
where we output long numbers with dividers.
Regards,
Pavel Borisov
Supabase
On Thu, 5 Sept 2024 at 20:49, Tom Lane wrote:
> Karina Litskevich writes:
> > On Thu, Sep 5, 2024 at 6:07 PM Karina Litskevich
> > wrote:
> >> In v3 of the patch I grouped all the *_junk rules together and included
> >> the suggested comment with a little added something.
>
> > Oops, I forgot t
warning, rule cannot be matched
FWIW output of the whole string in the error message doesnt' look nice to
me, but other places of code do this anyway e.g:
select ('1'||repeat('p',100))::integer;
This may be worth fixing.
Regards,
Pavel Borisov
Supabase
Hi, Alexander!
On Wed, 21 Aug 2024 at 19:29, Alexander Korotkov
wrote:
> Hi, Pavel!
>
>
> On Wed, Aug 21, 2024 at 4:28 PM Pavel Borisov
> wrote:
> > I've looked at patch v8.
> >
> > 1.
> > In function check_insert_rel_type_cache() th
eHash,
+ &typentry->typrelid,
+ HASH_FIND, &found);
+ Assert(found);
+#endif
+}
3. I think check_delete_rel_type_cache and check_insert_rel_type_cache are
better to be renamed to be more clear, though I don't have exact proposals
yet,
4. I haven't looked into comments, though I'd recommend oid -> OID
replacement in the comments.
Thank you for working on this patchset!
Regards,
Pavel Borisov
Supabase
Hi, Alexander!
On Wed, 21 Aug 2024 at 15:55, Alexander Korotkov
wrote:
> Hi, Pavel!
>
> On Wed, Aug 21, 2024 at 1:48 PM Pavel Borisov
> wrote:
> > On Mon, 19 Aug 2024 at 02:24, Alexander Korotkov
> wrote:
> >>
> >> On Sat, Aug 10, 2024 at 6:57 PM D
Put the Oid -> Store the OID
so caller might use it -> for the caller to use it.
(Maybe also caller -> table create function)
Regards,
Pavel Borisov
Supabase
Hi, Alexander!
On Wed, 19 Jun 2024 at 05:27, Alexander Korotkov
wrote:
> On Tue, Jun 18, 2024 at 4:14 PM Pavel Borisov
> wrote:
> >> Controls if the query planner will produce a plan which will provide
> GROUP BY keys presorted in the order of keys of a child
> node of
>
> Controls if the query planner will produce a plan which will provide
> GROUP BY keys presorted in the order of keys of a child
> node of the plan, such as an index scan. When disabled, the query planner
> will produce a plan with GROUP BY keys only reordered to
> match
> the ORDER BY clause, if
e of the plan, such as an index scan. When disabled, the query planner
will produce a plan with GROUP BY keys only reordered to
match
the ORDER BY clause, if any. When enabled, the planner
will try to produce a more efficient plan. The default value is on.
Regards,
Pavel Borisov
Supabase
ed value of ec_sortref of the pathkey's
EquivalenceClass -> sets the value of the pathkey's EquivalenceClass unless
it's already initialized
wasn't set yet -> hasn't been set yet
0002: additional assert checking only. Looks right.
0003: pure renaming, looks good.
0004: Restores pre 0452b461bc state to preprocess_groupclause with removed
partial_match fallback. Looks right. I haven't checked the comments
provided they are restored from pre 0452b461bc state.
0005: Looks right.
Regards,
Pavel Borisov
Supabase
x27;m happy that the test now
passes and confirms that amcheck feature works as intended.
Kind regards,
Pavel Borisov
Hi, Mark!
On Fri, 17 May 2024 at 23:10, Mark Dilger
wrote:
>
>
> > On May 17, 2024, at 11:51 AM, Pavel Borisov
> wrote:
> >
> > Amcheck with checkunique option does check uniqueness violation between
> pages. But it doesn't warranty detection of cross page u
cation and amcheck is not a tool that provides any
warranty when checking an index.
I'm not against docs modification in any way that clarifies its exact usage
and limitations.
Kind regards,
Pavel Borisov
[1]
https://www.postgresql.org/message-id/CAH2-Wz%3DttG__BTZ-r5ccopBRb5evjg%3DzsF_o_3C5h4zRBA_LjQ%40mail.gmail.com
ible bug. I've revised the commit message to reflect this.
> >
> > So, the picture for the patches is the following now.
> > 0001 – optimization, but rather simple and giving huge effect
> > 0002 – refactoring
> > 0003 – fix for the bug
> > 0004 – better error reporting
>
> I think the thread contains enough motivation on why 0002, 0003 and
> 0004 are material for post-FF. They are fixes and refactoring for
> new-in-v17 feature. I'm going to push them if no objections.
>
> Regarding 0001, I'd like to ask Tom and Mark if they find convincing
> that given that optimization is small, simple and giving huge effect,
> it could be pushed post-FF? Otherwise, this could wait for v18.
>
In my view, patches 0002-0004 are worth pushing.
0001 is ready in my view. But I see no problem pushing it into v18
regarding that this optimization could be not eligible for post-FF. I don't
know the criteria for this just let's be safe about it.
Regards,
Pavel Borisov
Check that detection, that the new partition has the same name as
one of
158 +-- the merged partitions, works correctly for temporary partitions
Test for split with comment for merge. Maybe better something like:
"Split partition of a temporary table when one of the partitions after
split has the same name as the partition being split"
0002:
analgous -> analogous (maybe better using "like" instead of "analogous to")
heirarchy -> hierarchy
alter_table.sgml:
Maybe in documentation it's better not to provide reasoning, just state how
it works:
for consistency with CREATE TABLE PARTITION OF ->
similar to CREATE TABLE PARTITION OF
Regards,
Pavel Borisov
A correction of a typo in previous message:
non-leaf pages iteration cycles (under P_ISLEAF(topaque)) -> non-leaf pages
iteration cycles (under !P_ISLEAF(topaque))
On Mon, 13 May 2024 at 16:19, Pavel Borisov wrote:
>
>
> On Mon, 13 May 2024 at 15:55, Pavel Borisov
> wrote:
>
On Mon, 13 May 2024 at 15:55, Pavel Borisov wrote:
> Hi, Alexander!
>
> On Mon, 13 May 2024 at 05:42, Alexander Korotkov
> wrote:
>
>> On Mon, May 13, 2024 at 12:23 AM Alexander Korotkov
>> wrote:
>> > On Sat, May 11, 2024 at 4:13 AM Mark Dilger
>> &g
for cross-page
unique check was not beautiful, and Peter pointed out this. In my opinion
the patch 0003 is a pure code refactoring.
As for the cross-page check regression/TAP testing, this test had problems
since the btree page layout is not fixed (especially it's different on
32-bit arch). I
On Fri, 10 May 2024, 22:42 Pavel Borisov, wrote:
> Hi, Mark!
>
>
> On Fri, 10 May 2024, 21:35 Mark Dilger,
> wrote:
>
>>
>>
>> > On May 10, 2024, at 5:10 AM, Pavel Borisov
>> wrote:
>> >
>> > Hi, Alexander!
>> >
>>
Hi, Mark!
On Fri, 10 May 2024, 21:35 Mark Dilger,
wrote:
>
>
> > On May 10, 2024, at 5:10 AM, Pavel Borisov
> wrote:
> >
> > Hi, Alexander!
> >
> > On Fri, 10 May 2024 at 12:39, Alexander Korotkov
> wrote:
> > On Fri, May 10, 2024 at 3:43 AM T
Hi, Alexander!
On Fri, 10 May 2024 at 12:39, Alexander Korotkov
wrote:
> On Fri, May 10, 2024 at 3:43 AM Tom Lane wrote:
> > Alexander Korotkov writes:
> > > The revised patchset is attached. I applied cosmetical changes. I'm
> > > going to push it if no objections.
> >
> > Is this really su
But they were requested in the thread, and make sense
in my opinion.
I really don't know what's the policy of applying code improvements other
than bugfixes post feature-freeze. IMO they are safe to be appiled to v17,
but they also could be added later.
Regards,
Pavel Borisov
Supabase
>
est
> > PostgreSQL committers.
> >
> > Please join us in wishing them much success and few reverts!
>
Congratulations! Well deserved!
Pavel Borisov
-
1) : NULL;
overridden by
5278 datum = list_nth(spec->upperdatums, abs(cmpval) -
1);
and
5290 datum = list_nth(spec->upperdatums, abs(cmpval) -
1);
Otherwise - good.
0004:
I suggest also getting rid of thee-noun compound words like:
salesperson_name. Maybe salesperson -> clerk? Or maybe use the same terms
like in pgbench: branches, tellers, accounts, balance.
0005: Good
0006: Patch is right
In comments:
+ New partitions will have the same table access method,
+ same column names and types as the partitioned table to which they
belong.
(I'd suggest to remove second "same")
Tests are passed. I suppose that it's better to add similar tests for
SPLIT/MERGE PARTITION(S) to those covering ATTACH/DETACH PARTITION (e.g.:
subscription/t/013_partition.pl and regression tests)
Overall, great work! Thanks!
Regards,
Pavel Borisov,
Supabase.
Hi, Karina!
On Thu, 25 Apr 2024 at 17:44, Karina Litskevich
wrote:
> Hi, hackers!
>
> On Thu, Apr 25, 2024 at 4:00 PM Pavel Borisov
> wrote:
>
>> 0005: Rename checkunique parameter to more user friendly as proposed by
>> Peter Eisentraut and Alexander Korotkov
&
0003: Don't load rightpage into BtreeCheckState (code refactoring) as
proposed by Peter Geoghegan
Loading of right page for cross-page unique constraint check in the same
way as in bt_right_page_check_scankey()
0004: Report error when next page to a leaf is not a leaf as proposed by
Peter Ge
I did notice (I meant to point out) that I have concerns about this
> part of the new uniqueness check code:
> "
> if (P_IGNORE(topaque) || !P_ISLEAF(topaque))
> break;
> "
My concern here is with the !P_ISLEAF(topaque) test -- it shouldn't be
> required
I agree. But I didn't see the need to
On Tue, 16 Apr 2024 at 14:52, Alexander Korotkov
wrote:
> On Mon, Apr 15, 2024 at 11:17 PM Andres Freund wrote:
> > On 2024-04-15 16:02:00 -0400, Robert Haas wrote:
> > > Do you want that patch applied, not applied, or applied with some set
> of
> > > modifications?
> >
> > I think we should app
On Mon, 15 Apr 2024 at 22:09, Robert Haas wrote:
> On Mon, Apr 15, 2024 at 12:37 PM Pavel Borisov
> wrote:
> > In my understanding, the downside of 041b96802ef is bringing
> read_stream* things from being heap-only-related up to the level of
> acquire_sample_rows() that is
On Mon, 15 Apr 2024 at 19:36, Robert Haas wrote:
> On Sat, Apr 13, 2024 at 5:28 AM Alexander Korotkov
> wrote:
> > Yes, I think so. Table AM API deals with TIDs and block numbers, but
> > doesn't force on what they actually mean. For example, in ZedStore
> > [1], data is stored on per-column B
Hi, Alexander!
On Wed, 10 Apr 2024 at 16:20, Alexander Korotkov
wrote:
> On Mon, Apr 8, 2024 at 9:54 PM Robert Haas wrote:
> > On Mon, Apr 8, 2024 at 12:33 PM Alexander Korotkov
> wrote:
> > > Yes, it was my mistake. I got rushing trying to fit this to FF, even
> doing significant changes just
On Mon, 8 Apr 2024 at 19:48, Matthias van de Meent <
boekewurm+postg...@gmail.com> wrote:
> On Mon, 8 Apr 2024 at 17:21, Tomas Vondra
> wrote:
> >
> >
> >
> > On 4/8/24 16:59, Tom Lane wrote:
> > > Heikki Linnakangas writes:
> > >> On 08/04/2024 16:43, Tom Lane wrote:
> > >>> I was just about to
different, so are the ways they feel motivation and inspiration.
This could be easily broken with bureaucratic decisions some of them, like
proposed counting of lines of code vs period of time look even little bit
repressive.
Let's remain an open community, support inspiration in each other, and
don't build an over-regulated corporation. I feel that Postgres will win if
people feel less limited by formal rules. Personally, I believe RMT has
enough experience and authority to stabilize and interact with authors if
questions arise.
Regards,
Pavel Borisov
On Mon, 8 Apr 2024 at 16:27, John Naylor wrote:
> On Sun, Apr 7, 2024 at 9:08 AM John Naylor
> wrote:
> >
> > I've attached a mostly-polished update on runtime embeddable values,
> > storing up to 3 offsets in the child pointer (1 on 32-bit platforms).
> > As discussed, this includes a macro to
mon/tidstore.c
../pgsql/src/backend/access/common/tidstore.c:48:3: error: anonymous
structs are a C11 extension [-Werror,-Wc11-extensions]
struct
^
1 error generated.
Regards,
Pavel Borisov
Supabase
Hi, Alexander
On Mon, 8 Apr 2024 at 13:59, Pavel Borisov wrote:
>
>
> On Mon, 8 Apr 2024 at 13:34, Alexander Korotkov
> wrote:
>
>> On Mon, Apr 8, 2024 at 10:18 AM Pavel Borisov
>> wrote:
>> > On Mon, 8 Apr 2024 at 03:25, Alexander Korotkov
>> wrote:
On Mon, 8 Apr 2024 at 13:34, Alexander Korotkov
wrote:
> On Mon, Apr 8, 2024 at 10:18 AM Pavel Borisov
> wrote:
> > On Mon, 8 Apr 2024 at 03:25, Alexander Korotkov
> wrote:
> >>
> >> Hi,
> >>
> >> On Mon, Apr 8, 2024 at 12:40 AM Andres F
ock based AMs, the new interface basically requires
> > reimplementing all of analyze.c.
> .
> Non-lock base AM needs to just provide an alternative implementation
> for what acquire_sample_rows() does. This seems like reasonable
> effort for me, and surely not reimplementing all of analyze.c.
>
I agree.
Regards,
Pavel Borisov
Supabase
Hi, Alexander!
On Sun, 7 Apr 2024 at 12:34, Pavel Borisov wrote:
> Hi, Alexander!
>
> On Sun, 7 Apr 2024 at 07:33, Alexander Korotkov
> wrote:
>
>> Hi, Pavel!
>>
>> On Fri, Apr 5, 2024 at 6:58 PM Pavel Borisov
>> wrote:
>> > On Tue, 2 Apr 2024
Hi, Alexander!
On Sun, 7 Apr 2024 at 07:33, Alexander Korotkov
wrote:
> Hi, Pavel!
>
> On Fri, Apr 5, 2024 at 6:58 PM Pavel Borisov
> wrote:
> > On Tue, 2 Apr 2024 at 19:17, Jeff Davis wrote:
> >>
> >> On Tue, 2024-04-02 at 11:49 +0300, Alexander Korotkov
Hi, hackers!
On Tue, 2 Apr 2024 at 19:17, Jeff Davis wrote:
> On Tue, 2024-04-02 at 11:49 +0300, Alexander Korotkov wrote:
> > I don't like the idea that every custom table AM reltoptions should
> > begin with StdRdOptions. I would rather introduce the new data
> > structure with table options,
Hi, Alexander!
On Wed, 3 Apr 2024 at 22:18, Alexander Korotkov
wrote:
> On Wed, Apr 3, 2024 at 7:55 PM Alvaro Herrera
> wrote:
> >
> > On 2024-Apr-03, Alexander Korotkov wrote:
> >
> > > Regarding the shmem data structure for LSN waiters. I didn't pick
> > > LWLock or ConditionVariable, becaus
correction: so now code is not broken
>
Relation OldTable, Relation NewTable order. It coincides to what is
expected by the function, no now code is not broken. The only wrong thing
is naming of arguments in declaration of this function in tableam.h I think
this is a minor oversight from original commit d25f519107b
Provided all the abov
>
> I think for better code look this could be removed:
> >vlock:
> > CHECK_FOR_INTERRUPTS();
> together with CHECK_FOR_INTERRUPTS(); in heapam_tuple_insert_with_arbiter
> placed in the beginning of while loop.
>
To clarify things, this I wrote only about CHECK_FOR_INTERRUPTS
I've looked at patch 0003.
Generally, it does a similar thing as 0001 - it exposes a more generalized
method tuple_insert_with_arbiter that encapsulates
tuple_insert_speculative/tuple_complete_speculative and at the same time
allows extensibility of this i.e. different implementation for custom ta
er_status_interval in seconds
tcp_keepalives_idle in seconds
commit_delay in microseconds
deadlock_timeout in milliseconds
max_standby_archive_delay in milliseconds
vacuum_cost_delay in milliseconds
autovacuum_vacuum_cost_delay in milliseconds
etc..
I haven't counted precisely, but I feel that milliseconds are the most
often used in both guc's and functions. So I'd propose using milliseconds
for the patch as it was proposed originally.
Regards,
Pavel Borisov
Supabase.
te to describe better
who should do what.
I think, with rebase and correction in the comments/commit message patch
0006 is ready to be committed.
Regards,
Pavel Borisov.
#include
#include "utils/sampling.h"
#include "utils/spccache.h"
On Wed, Mar 27, 2024 at 2:52 PM Pavel Borisov
> wrote:
> >> The revised rest of the patchset is attached.
> >> 0001 (was 0006) – I prefer the definition of AcquireSampleRowsFunc to
>
>
> This seems not needed, it's already inited to InvalidOid before.
> +else
> +accessMethod = default_table_access_method;
>
> + accessMethodId = InvalidOid;
>
> This code came from 374c7a22904. I don't insist on this simplification in
> a patch 0002.
>
A correction of the code quote for th
+accessMethod = default_table_access_method;
+ accessMethodId = InvalidOid;
This code came from 374c7a22904. I don't insist on this simplification in a
patch 0002.
Overall both patches look good to me.
Regards,
Pavel Borisov.
Hi, Anton!
Looks like an oversight when refactoring BTScanOpaqueData.firstPage into
using function argument in 06b10f80ba4.
@@ -2487,14 +2486,13 @@ _bt_endpoint(IndexScanDesc scan, ScanDirection dir)
104
105 /* remember which buffer we have pinned */
106 so->currPos.buf = buf;
107 - so->
On Fri, 22 Mar 2024 at 08:51, Pavel Borisov wrote:
> Hi, Alexander!
>
> Thank you for working on this patchset and pushing some of these patches!
>
> I tried to write comments for tts_minimal_is_current_xact_tuple()
> and tts_minimal_getsomeattrs() for them to be the sa
thread. (tts_minimal_getsysattr is not introduced by the current patchset,
but anyway)
Meanwhile I found that (never appearing) error message
for tts_minimal_is_current_xact_tuple needs to be corrected. Please see the
patch in the attachment.
Regards,
Pavel Borisov
Add-comments-on
Hi, Alexander!
On Wed, 20 Mar 2024 at 09:22, Pavel Borisov wrote:
> Hi, Alexander!
>
> For 0007:
>
> Code inside
>
> +heapam_reloptions(char relkind, Datum reloptions, bool validate)
> +{
> + if (relkind == RELKIND_RELATION ||
> + relkind == RELKIND_T
rc/include/access/tableam.h:extern bytea
*index_reloptions(amoptions_function amoptions, Datum reloptions,
Otherwise the patch looks good and doing what it's proposed to do.
Regards,
Pavel Borisov.
shouldn't get here, but provide a user-friendly message if we do.
+ */
also applies to tts_minimal_is_current_xact_tuple()
I'd propose changes for clarity of this comment:
%s/a storage tuples/storage tuples/g
%s/generally//g
Otherwise patch 0005 also looks good to me.
I'm pla
st patches are unchanged.
Regards,
Pavel Borisov
amount of WAL records and seeing how it works. I think it's regarding
this patchset only and could remove or substantiate the main questions
about the current patchset.
[0]
https://www.postgresql.org/message-id/CACG=ezZe1NQSCnfHOr78AtAZxJZeCvxrts0ygrxYwe=pyyj...@mail.gmail.com
Kind regards,
Pavel Borisov.
would be only in some
specific artificial workload. Did you do some measurements? Do we have
several percent performance-wise?
Kind regards,
Pavel Borisov
Alexander,
On Tue, 26 Dec 2023 at 23:35, Alexander Korotkov
wrote:
> Pavel,
>
> On Mon, Dec 25, 2023 at 8:32 PM Pavel Borisov
> wrote:
> > I've reviewed both patches:
> > 0001 - is a pure refactoring replacing argument transfer from via struct
> member to t
page (according to the scan direction).
Maybe, in this case, it would be more clear like: "...(for backwards scan
it will be the first item on a page)"
Otherwise the patch 0002 looks like a good fix for the bug to be pushed.
Kind regards,
Pavel Borisov
>
>
> Additionally changes in 0007 looks dependent from 0005. Does replacement
> of slot inside ExecInsert, that is already used in the code below the call
> of
>
> >/* insert the tuple normally */
> >- table_tuple_insert(resultRelationDesc, slot,
> >- estate->es_output_cid,
> >- 0, NULL);
>
>
Hi, Alexander!
I've reviewed patch 0004. It's clear enough and I think does what it's
supposed.
One thing, in function signature
+bool (*tuple_is_current) (Relation rel, TupleTableSlot *slot);
there is a Relation agrument, which is unused in both existing heapam
method. Also it's unused in OrioleD
patch, since MOX patch is unlikely to be committed and now
>> for test purpose only.
>>
>
If the patch is RwF the CF entry is finished and can't be enabled, rather
the patch needs to be submitted in a new entry, which I have just done.
https://commitfest.postgresql.org/46/4703/
Please feel free to submit your review.
Kind regards,
Pavel Borisov,
Supabase
On Mon, 4 Dec 2023 at 10:34, John Naylor wrote:
> On Thu, Nov 30, 2023 at 4:37 PM Alexander Korotkov
> wrote:
> >
> > On Thu, Nov 30, 2023 at 10:29 AM Pavel Borisov
> wrote:
> > > Agree. The fix is attached.
> >
> > What an oversight.
> > Tha
On Thu, 30 Nov 2023 at 08:03, Tom Lane wrote:
> Alexander Lakhin writes:
> > And a warning:
> > $ CC=gcc-12 CFLAGS="-Wall -Wextra -Wno-unused-parameter
> -Wno-sign-compare -Wno-clobbered
> > -Wno-missing-field-initializers" ./configure -q && make -s
> > slru.c:63:1: warning: ‘inline’ is not at b
there is underlying [free] function and simply falls
> through otherwise,
> also, take into account that it could be located in the uninterruptible
> part of the code.
>
> On the whole topic I have to
>
> On Wed, Nov 29, 2023 at 4:56 PM Pavel Borisov
> wrote:
>
>> Hi,
Hi, Alexander!
> I'm planning to review some of the other patches from the current patchset
> soon.
>
I've looked into the patch 0003.
The patch looks in good shape and is uncontroversial to me. Making memory
structures to be dynamically allocated is simple enough and it allows to
store complex d
On Tue, 28 Nov 2023 at 14:37, Heikki Linnakangas wrote:
> On 28/11/2023 12:14, Pavel Borisov wrote:
> > On Tue, 28 Nov 2023 at 13:13, Heikki Linnakangas
> wrote:
> >>
> >> On 27/11/2023 01:43, Alexander Korotkov wrote:
> >>> v61 looks good to me. I&
ail.com
Kind regards,
Pavel Borisov
1 - 100 of 363 matches
Mail list logo
