e end of the June - feel free to register anyway.
> If you do not have a ticket to StHighload - we have some speaker entrance
> tickets.
> At the moment we have 4 potential patch authors ready to present.
>
> Please contact me with any questions regarding the event. Thanks!
Great initiative, thanks!
--
Best regards,
Aleksander Alekseev
config.sgml bits
> which are improvements of their own.
Thanks, Michael.
I propose my original v1 patch for correcting the --help output of
'postgres' too. I agree with the above comments that corresponding
changes in v4 became somewhat unwieldy.
--
Best regards,
Aleksander Alekseev
v
Hi,
> Thanks for all your great input. Here is the updated patch.
Here is the patch v4 with fixed typo ("geoq"). Per off-list feedback
from Alvaro - thanks!
--
Best regards,
Aleksander Alekseev
v4-0001-Clarify-error-message-about-specifying-config-fil.patch
Description: Binary data
s with hyphens should generally be
> preferred in documentation.
Thanks for all your great input. Here is the updated patch.
--
Best regards,
Aleksander Alekseev
From 1d55400adba93381d8a08249c95e4e16fb9a5945 Mon Sep 17 00:00:00 2001
From: "David G. Johnston"
Date: Fri, 2 Feb 202
ast enum items.
Thanks, fixed.
--
Best regards,
Aleksander Alekseev
v2-0001-Use-macro-to-define-the-number-of-enum-values.patch
Description: Binary data
s sense, it can be resurrected (probably with a better
> implementation).
>
> Any objections to fixing this in 17 by removing it? (cc:ing Michael from the
> RMT)
+1 Something that is not documented or used by anyone (apparently) and
is broken should just be removed.
--
Best regards,
Aleksander Alekseev
integers rather than by 32-bit ones" where the authors are:
Maxim Orlov, Aleksander Alekseev, Alexander Korotkov, Teodor Sigaev,
Nikita Glukhov, Pavel Borisov, Yura Sokolov.
--
Best regards,
Aleksander Alekseev
ss and few reverts!
Completely deserved. Congrats!
--
Best regards,
Aleksander Alekseev
also very happy for someone with a
> > committer bit to just fix this).
>
> Indeed, there is no @(box,box) or ~(box,box) in the \dAo output. These
> operators were removed by 2f70fdb0644c back in 2020.
>
> I will submit a patch for the documentation shortly. Thanks for reporting.
Here is the patch.
--
Best regards,
Aleksander Alekseev
v1-0001-Remove-mention-of-and-operators.patch
Description: Binary data
ox) or ~(box,box) in the \dAo output. These
operators were removed by 2f70fdb0644c back in 2020.
I will submit a patch for the documentation shortly. Thanks for reporting.
--
Best regards,
Aleksander Alekseev
Hi,
> > Fair point. PFA the alternative version of the patch.
> >
>
> Thanks. Committed.
Thanks. I see a few pieces of code that use special FOO_NUMBER enum
values instead of a macro. Should we refactor these pieces
accordingly? PFA another patch.
--
Best regards,
Aleksander
integrity. It is the trigger programmer's
> responsibility to avoid that.
That's perfect!
--
Best regards,
Aleksander Alekseev
eople than use foreign keys. So I suggest something like the
> attached.
I don't mind changing the chapter, but I prefer the wording chosen in
v3. The explanation in v4 is somewhat hard to follow IMO.
--
Best regards,
Aleksander Alekseev
Hi,
> I guess the argument against inserting an enum element at the end is
> that a switch statement on the enum value might generate a compiler
> warning if it didn't have a default clause.
Fair point. PFA the alternative version of the patch.
--
Best regards,
Aleksander Alekseev
Oversight of 0294df2f1f84 [1].
[1]: https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=0294df2f1f84
--
Best regards,
Aleksander Alekseev
v1-0001-Replace-constant-3-with-NUM_MERGE_MATCH_KINDS.patch
Description: Binary data
RETURN_DATUM(PG_GETARG_DATUM(0));
> else
> PG_RETURN_DATUM(PG_GETARG_DATUM(1));
>
> because really there's no point in detoasting at all.
Many thanks. Here is the corrected patch. Now it also includes MIN()
support and tests.
--
Best regards,
Aleksander Alekseev
v2-0001-Support-min-record-and-max-record-aggregates.patch
Description: Binary data
mentation is safe.
--
Best regards,
Aleksander Alekseev
v1-0001-Support-MIN-MAX-record-aggregates-WIP.patch
Description: Binary data
not executed often.
[1]: https://www.postgresql.org/docs/current/functions-string.html
--
Best regards,
Aleksander Alekseev
t regards,
Aleksander Alekseev
ay() in the general case.
[1]:
https://learn.microsoft.com/en-us/windows/win32/api/sysinfoapi/nf-sysinfoapi-getsystemtimepreciseasfiletime
--
Best regards,
Aleksander Alekseev
upporting
NOT VALID constraints is out of scope of this patch.
Except for named nitpicks v4 LGTM.
--
Best regards,
Aleksander Alekseev
nged the status to "Waiting on Author". The patch needs a
rebase since January.
* Allow INSTEAD OF DELETE triggers to modify the tuple for RETURNING
Needs rebase.
--
Best regards,
Aleksander Alekseev
ng to cfbot.
Best regards,
Aleksander Alekseev (wearing co-CFM hat)
Hi,
> Thought I would show off what is possible with this patchset.
>
> [...]
Just wanted to let you know that cfbot doesn't seem to be entirely
happy with the patch [1]. Please consider submitting an updated
version.
Best regards,
Aleksander Alekseev (wearing co-CFM hat)
inherited by fork.
>
> That imposes an overhead for every single postgres backend. So maybe there's
> a better solution.
It looks like the patch rotted a bit, see cfbot. Could you please
submit an updated version?
Best regards,
Aleksander Alekseev (wearing co-CFM hat)
Hi,
> > So maybe we don't need the _extract_variant function?
>
> I think it's the best possible solution. The variant has no value besides
> detecting if a version can be extracted.
+1 to the idea. I doubt that anyone will miss it.
--
Best regards,
Aleksander Alekseev
due to lack
of a better status. Maybe we need an additional "Duplicate" status.
[1]: https://commitfest.postgresql.org/47/4834/
[2]: https://commitfest.postgresql.org/47/4835/
--
Best regards,
Aleksander Alekseev
ng thread [1].
[1]:
https://www.postgresql.org/message-id/CAJ7c6TN9SnYdq%3DkfP-txgo5AaT%2Bt9YU%2BvQHfLBZqOBiHwoipAg%40mail.gmail.com
--
Best regards,
Aleksander Alekseev
e attention from a committer?
* UUID v7
The patch is in good shape. Michael argued that the patch should
be merged when RFC is approved. No action seems to be needed until
then.
* (to be continued)
--
Best regards,
Aleksander Alekseev
;
> [...]
Just wanted to let you know that v20231226 doesn't apply. The patch
needs love from somebody interested in it.
Best regards,
Aleksander Alekseev (wearing a co-CFM hat)
bout-PageUsableSpace.patch
>
> Fixes for FSM to use runtime size:
>
> v3-0028-feature-teach-FSM-about-reserved-page-space.patch
>
> I hope this makes sense for reviewing, I know it's a big job, so breaking
> things up a little more and organizing will hopefully help.
Just wanted to let you know that the patchset seems to need a rebase,
according to cfbot.
Best regards,
Aleksander Alekseev (wearing a co-CFM hat)
h LGTM. I think it could be merged unless there are any open
issues left. I don't think so, but maybe I missed something.
--
Best regards,
Aleksander Alekseev
v20-0001-Implement-UUID-v7.patch
Description: Binary data
ew it too.
--
Best regards,
Aleksander Alekseev
v4-0001-Improve-performance-of-type-cache-cleanup.patch
Description: Binary data
`if(!hash) hash++` or use an
additional boolean argument here.
--
Best regards,
Aleksander Alekseev
v3-0001-Improve-performance-of-type-cache-cleanup.patch
Description: Binary data
n't think we currently have this in the core, but maybe I just missed it.
--
Best regards,
Aleksander Alekseev
ut giving such a guarantee only once
the value of this is arguably low.
--
Best regards,
Aleksander Alekseev
TWIMC see single-install-meson.sh [1]. The
speedup I got on the provided benchmark is about 150 times. cfbot
seems to be happy with the patch.
I would like to tweak the patch a little bit - change some comments,
add some Asserts, etc. Don't you mind?
[1]: https://github.com/afiskon/pgscripts/
--
Best regards,
Aleksander Alekseev
ardless the order of applying:
```
error: patch failed: src/backend/utils/cache/typcache.c:356
error: src/backend/utils/cache/typcache.c: patch does not apply
```
So it's difficult to confirm case 4, not to mention the fact that we
are unable to test the patches on cfbot.
Could you please rebase the patches against the recent master branch
(in any order) and submit the result of `git format-patch` ?
--
Best regards,
Aleksander Alekseev
for the patch. LGTM.
I registered the patch on the nearest open CF [1] and marked it as
RfC. It is a pretty straightforward refactoring.
[1]: https://commitfest.postgresql.org/48/4879/
--
Best regards,
Aleksander Alekseev
traint for
> deferred PK constraint. Moreover, skip that for any PK constraint.
I confirm that the patch fixes the bug. All the tests pass. Looks like
RfC to me.
--
Best regards,
Aleksander Alekseev
akhin
> > * Daniel Gustafsson
> > * Dean Rasheed
> > * John Naylor
> > * Melanie Plageman
> > * Nathan Bossart
> >
> > Thank you and congratulations to all!
>
> +1. Congratulations to all!
Congratulations to all!
--
Best regards,
Aleksander Alekseev
rom pginfra will give you the required admin permissions on the CF
> app)
Thanks for volunteering, Andrey!
If you need any help please let me know.
--
Best regards,
Aleksander Alekseev
hat there is no obvious
way to bind a NULL value, except for something like:
```
create table t (v text);
insert into t values (case when $1 = '' then NULL else $1 end) \bind '' \g
select v, v is null from t;
```
Maybe we should also support something like ... \bind val1 \null val3 \g ?
--
Best regards,
Aleksander Alekseev
e proper way of testing PRNG would be to call setseed() and compare
return values with expected ones. I don't mind testing the proposed
invariants but they should do this after calling setseed(). Currently
the patch places the tests right before the call.
--
Best regards,
Aleksander Alekseev
Index Cond: (a < 1000)
-> Sort
Sort Key: ma0_2.a
-> Seq Scan on ma1 ma0_2
Filter: (a < 1000)
```
The rest of the tests pass.
--
Best regards,
Aleksander Alekseev
ike \whoami for psql. Wouldn't
> it be worth trying that?
IMO it's worth trying submitting the patch, if your time permits it of course.
--
Best regards,
Aleksander Alekseev
fter applying your patch.
I'm not 100% sure whether this is significant or not.
I added your patch to the nearest open commitfest so that we will not lose it:
https://commitfest.postgresql.org/47/4794/
--
Best regards,
Aleksander Alekseev
t; -{
> -pfree(tuple);
> -}
> -
>
> Why does ReorderBufferReturnTupleBuf need to be moved from
> reorderbuffer.c to reorderbuffer.h? It seems not related to this
> refactoring patch so I think we should do it in a separate patch if we
> really want it. I'm not sure it's necessary, though.
OK, fixed.
--
Best regards,
Aleksander Alekseev
v6-0001-Remove-ReorderBufferTupleBuf-structure.patch
Description: Binary data
use some help from a native English speaker for this.
--
Best regards,
Aleksander Alekseev
v14-0001-Implement-UUID-v7.patch
Description: Binary data
py and I don't think we have any open items left. So
changing CF entry status back to RfC.
--
Best regards,
Aleksander Alekseev
ike to see
a list of extensions available in the current database while for
another this is redundant information.
Even if we do this I don't think this should be a PL/pgSQL function
but rather a \whoami command for psql. This solution however will
leave users of DataGrip and similar products unhappy
usability of UUID v7.
Again, personally I don't insist on the 1us precision [1]. Only the
fact that timestamp from the far past generates UUID from the future
bothers me.
[1]:
https://postgr.es/m/CAJ7c6TPCSprWwVNdOB%3D%3DpgKZPqO5q%3DHRgmU7zmYqz9Dz5ffVYw%40mail.gmail.com
--
Best regards,
Aleksander Alekseev
es
will be useful in real-world applications, at least the ones acting
exclusively within Postgres.
This being said, I understand your point of view too. Let's see what
other people think.
--
Best regards,
Aleksander Alekseev
ever, years 2977 BC and 5943 AC are
clearly not equal, thus 2977 BC should be rejected as an invalid value
for UUIDv7.
--
Best regards,
Aleksander Alekseev
ld hold. Otherwise you can calculate crc64(X) or sha256(X)
internally in order to generate an unique ID and claim that it's fine.
Values that violate named invariants should be rejected with an error.
--
Best regards,
Aleksander Alekseev
e "..." (three dots)
similarly to the previous one? Also I think we need at least one
negative test for OF.
Other than that v2 looks OK.
--
Best regards,
Aleksander Alekseev
Best regards,
Aleksander Alekseev
atch and tested it on MacOS and generally concur with
stated above. The only nitpick I have is the apparent lack of negative
tests for to_timestamp(), e.g. when the string doesn't match the
specified format.
--
Best regards,
Aleksander Alekseev
ribute tests and documentation.
--
Best regards,
Aleksander Alekseev
ct
it". A friendly reminder like "hey, this patch was waiting long
enough, maybe someone could take a look" would be more appropriate
IMO. I remember during previous commitfests some CF managers created a
list of patches that could use more attention. That was useful.
I will review the patch, but probably only tomorrow.
--
Best regards,
Aleksander Alekseev
estamps are implicitly casted to
TimestampTz's, so users preferring Timestamps can do this:
```
=# select uuidv7('2024-01-22 12:34:56' :: timestamp);
uuidv7
--
018d3085-de00-77c1-9e7b-7b04ddb9ebb9
```
Cfbot also seems to be happy with the patch so I'm changing the CF
entry status to RfC.
--
Best regards,
Aleksander Alekseev
HEAPTUPLESIZE;
```
Here is the corrected patch.
--
Best regards,
Aleksander Alekseev
v5-0001-Remove-ReorderBufferTupleBuf-structure.patch
Description: Binary data
ame above but
apparently forgot:
> On top of that IMO it doesn't make much sense to keep a one-field
> wrapper structure. Perhaps we should get rid of it entirely and just
> use HeapTupleData instead.
After actually trying the refactoring I agree that the code becomes
cleaner and it
urns $1)
```
... arguably doesn't give much more information to the user comparing
to what we have now:
```
- Filter: (SubPlan 1)
- SubPlan 1
```
... I believe this is the right step toward more detailed EXPLAINs,
and perhaps could be useful for debugging and/or educational purposes.
Also the patch improves code coverage.
--
Best regards,
Aleksander Alekseev
t; rely on it, e.g., 009_twophase.pl or 012_subtransactions.pl (in fact, my
> research of the issue was initiated per a test failure).
I suggest focusing on particular flaky tests then and how to fix them.
--
Best regards,
Aleksander Alekseev
ng happens then the CF entry will be closed ("Returned with
> feedback") at the end of this CF.
I don't think that closing CF entries only due to inactivity is a good
practice, nor something we typically do. When someone will have spare
time this person will (hopefully) review the code.
--
Best regards,
Aleksander Alekseev
n working on it? Typically it is a good idea
to start with an RFC document and discuss it with the community.
--
Best regards,
Aleksander Alekseev
ng we did before) as long as there is
code, it applies, etc. There are a lot of patches and few people
working on them. Inactivity in a given thread doesn't necessarily
indicate lack of interest, more likely lack of resources.
--
Best regards,
Aleksander Alekseev
s :) What if this is a very important data
and node2 was promoted mistakenly, either manually or by a buggy
script.
It's been a while since I seriously played with replication, but if
memory serves, a proper way to switch node1 to a replica mode would be
to use pg_rewind on it first.
--
Best regards,
Aleksander Alekseev
entry.st_auth_identity) instead of
NAMEDATALEN is generally considered a better practice.
Calling MemSet for a CString seems to be an overkill. I suggest
setting lbeentry.st_auth_identity[0] to zero.
Except for these nitpicks v4 LGTM.
--
Best regards,
Aleksander Alekseev
TimestampTz by users who need them and have experience using them.
--
Best regards,
Aleksander Alekseev
nd
> > I'd appreciate any feedback the community has on the approach.
If I read this correctly, basically the patch adds 16 useless bits for
all applications except for ML ones...
Perhaps implementing an alternative storage specifically for ML using
TAM interface would be a better approach?
--
Best regards,
Aleksander Alekseev
row)
```
And the difference between the value in the text and the actual value
is 10 hours as you pointed out.
Also you named the date 1582-10-15 00:00:00 UTC. Maybe you actually
meant 1970-01-01 00:00:00 UTC?
[1]:
https://www.ietf.org/archive/id/draft-peabody-dispatch-new-uuid-format-04.html
--
Best regards,
Aleksander Alekseev
d
whether there was a good reason to choose TimestampTz over Timestamp.
--
Best regards,
Aleksander Alekseev
ally see how to process further.
--
Best regards,
Aleksander Alekseev
the Head. It is giving
> the following error:
Yes it does for a while now. Until we reach any agreement regarding
questions (1) and (2) personally I don't see the point in submitting
rebased patches. We can continue the discussion but mark the CF entry
as RwF for now it will be helpful.
--
Best regards,
Aleksander Alekseev
ct somebody will request this eventually. During the cast to UUID
we will always get the same value for the given Timestamp[Tz], which
probably can be useful in certain applications. It can't be done with
gen_uuid_v7() and its volatility doesn't permit it.
--
Best regards,
Aleksander Alekseev
gt;
> The above analysis is based on the latest master branch.
>
> I'm not sure if my idea is reasonable, I hope you can give me some
> suggestions. Thanks.
Any chance you could provide minimal steps to reproduce the issue on
an empty PG instance, ideally as a script? That's going to be helpful
to reproduce / investigate the issue and also make sure that it's
fixed.
--
Best regards,
Aleksander Alekseev
ree and don't mind affecting the error message per se.
However I see that the actual logic of how WAL is processed is being
changed. If we do this, at very least it requires thorough thinking. I
strongly suspect that the proposed code is wrong and/or not safe
and/or less safe than it is now for the reasons named above.
--
Best regards,
Aleksander Alekseev
th char and/or varchar,
> and that makes for a simpler needs_toast_table callback.
Good ideas. Additionally we could provide a proxy TAM for a heap TAM
which does nothing but logging used TAM methods, its arguments and
return values. This would be a good example and also potentially can
be used as a debugging tool.
--
Best regards,
Aleksander Alekseev
for TAMs is
indeed steep, and I wonder if we could do a better job in this respect
e.g. by providing a simpler example. This being said, I know several
people who learned TAM successfully (so far only for R tasks) which
indicates that its difficulty is adequate.
--
Best regards,
Aleksander Alekseev
or the help message, I’d minimally add:
>
> “You must specify the --config-file (or equivalent -c) or -D invocation …”
Good idea.
PFA the patch. It's short but I think it mitigates the problem.
--
Best regards,
Aleksander Alekseev
v1-0001-Clarify-error-message-about-specifying-config-fil.patch
Description: Binary data
he registered shmem_startup_hook shortly
> > after it attaches to shared memory.
That's much better, thanks.
I think the patch could use another pair of eyes, ideally from a
native English speaker. But if no one will express any objections for
a while I suggest merging it.
--
Best regards,
Aleksander Alekseev
orrect --help output? Should we update the
documentation?
[1]:
https://www.postgresql.org/docs/current/config-setting.html#CONFIG-SETTING-SHELL
--
Best regards,
Aleksander Alekseev
ng function name here
is redundant and complicates reading just a bit. But maybe it's just
me.
Except for these nitpicks the patch looks good.
--
Best regards,
Aleksander Alekseev
r if we should fix this. Again, not necessarily in scope of
this work, but if this is not a difficult task, again, it would be
nice to have.
Other than that v2 looks good.
--
Best regards,
Aleksander Alekseev
hance that PG will interpret this as a normal situation. To my
knowledge this is not what we typically do - typically PG would report
an error and ask a human to figure out what happened. Of course the
possibility of such a scenario is small, but I don't think that as
DBMS developers we can ignore it.
Does anyone agree or maybe I'm making things up?
--
Best regards,
Aleksander Alekseev
ot; everywhere for
consistency.
Since the patch affects pg_proc.dat I believe the commit message
should remind bumping the catalog version.
--
Best regards,
Aleksander Alekseev
ollversion))
ereport(ERROR, ...
```
Should we just silence the warning like this - see attachment? I don't
think createdb() is called that often to worry about slight
performance change, if there is any.
--
Best regards,
Aleksander Alekseev
v1-0001-Silence-compiler-warnings.patch
Description: Binary data
Hi,
> least lists what's expected -- I'm not sure if this is the way to go,
> or if we should somehow list the individual tools that are failing
> here?
Personally I think the patch is OK.
--
Best regards,
Aleksander Alekseev
Best regards,
Aleksander Alekseev
r requires Perl >= 5.14, which is fine [1].
[1]: https://www.postgresql.org/docs/current/install-requirements.html
--
Best regards,
Aleksander Alekseev
t session
and/or a given session ID. In the latter case it can have an effect
only for the new transactions. This however could be implemented
separately from the proposed patchset. I suggest keeping the scope
small.
--
Best regards,
Aleksander Alekseev
rnings and
also building the patch on Windows:
http://cfbot.cputube.org/
--
Best regards,
Aleksander Alekseev
ffers as proposed
elsewhere [1].
[1]:
https://www.postgresql.org/message-id/efaac0be-27e9-4186-b925-79b7c696d...@amazon.com
--
Best regards,
Aleksander Alekseev
_segment_names == true) could be a better
strategy. Maybe it's not but I believe this should be discussed
separately from the patchset under question.
--
Best regards,
Aleksander Alekseev
xible as a result.
Well even assuming this patch will make it to the upstream some day,
which I seriously doubt, it will take somewhere between 2 and 5 years.
Personally I would recommend reconsidering this design.
--
Best regards,
Aleksander Alekseev
ql.org/docs/current/sql-createdomain.html
--
Best regards,
Aleksander Alekseev
limit would be. Regardless of the
chosen number there is a possibility of breaking backward
compatibility for certain users.
For this reason I believe merging the proposed patch would be the
right move at this point. It doesn't make anything worse for the
existing users and solves a potential problem
art treating them as regular tables this will cause a severe
performance degradation. I doubt that such a patch will make it.
I sort of suspect that you are working on a very specific extension
and/or feature for PG fork. Any chance you could give us more details
about the case?
--
Best regards,
Aleksan
1 - 100 of 702 matches
Mail list logo
