Re: Add LZ4 compression in pg_dump
От | gkokolatos@pm.me |
---|---|
Тема | Re: Add LZ4 compression in pg_dump |
Дата | |
Msg-id | Qzw-4fJWmzIR99t4Wa3IjGtpYIpF6zCOdDhGRPVTDXfmB8RkM0tqBlK2pN5vY1BgyuMB8KlZ2u6apkVv4VXwCtSlevQpTIh8cSoEmtI_MSM=@pm.me обсуждение исходный текст |
Ответ на | Re: Add LZ4 compression in pg_dump (Tomas Vondra <tomas.vondra@enterprisedb.com>) |
Список | pgsql-hackers |
------- Original Message ------- On Friday, January 20th, 2023 at 12:34 AM, Tomas Vondra <tomas.vondra@enterprisedb.com> wrote: > > > On 1/19/23 18:55, Tomas Vondra wrote: > > > Hi, > > > > On 1/19/23 17:42, gkokolatos@pm.me wrote: > > > > > ... > > > > > > Agreed. It was initially submitted as one patch. Then it was requested to be > > > split up in two parts, one to expand the use of the existing API and one to > > > replace with the new interface. Unfortunately the expansion of usage of the > > > existing API requires some tweaking, but that is not a very good reason for > > > the current patch set. I should have done a better job there. > > > > > > Please find v22 attach which combines back 0001 and 0002. It is missing the > > > documentation that was discussed above as I wanted to give a quick feedback. > > > Let me know if you think that the combined version is the one to move forward > > > with. > > > > Thanks, I'll take a look. > > > After taking a look and thinking about it a bit more, I think we should > keep the two parts separate. I think Michael (or whoever proposed) the > split was right, it makes the patches easier to grok. > Excellent. I will attempt a better split this time round. > > While reading the thread, I also noticed this: > > > By the way, I think that this 0002 should drop all the default clauses > > in the switches for the compression method so as we'd catch any > > missing code paths with compiler warnings if a new compression method > > is added in the future. > > > Now I realize why there were "not yet implemented" errors for lz4/zstd > in all the switches, and why after removing them you had to add a > default branch. > > We DON'T want a default branch, because the idea is that after adding a > new compression algorithm, we get warnings about switches not handling > it correctly. > > So I guess we should walk back this change too :-( It's probably easier > to go back to v20 from January 16, and redo the couple remaining things > I commented on. > Sure. > > FWIW I think this is a hint that adding LZ4/ZSTD options, in 5e73a6048, > but without implementation, was not a great idea. It mostly defeats the > idea of getting the compiler warnings - all the places already handle > PG_COMPRESSION_LZ4/PG_COMPRESSION_ZSTD by throwing a pg_fatal. So you'd > have to grep for the options, inspect all the places or something like > that anyway. The warnings would only work for entirely new methods. > > However, I now also realize the compressor API in 0002 replaces all of > this with calls to a generic API callback, so trying to improve this was > pretty silly from me. I can try to do a better job at splitting things up. > > Please, fix the couple remaining details in v20, add the docs for the > callbacks, and I'll try to polish it and get it committed. Excellent. Allow me an attempt to polish and expect a new version soon. Cheers, //Georgios > > > regards > > -- > Tomas Vondra > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company
В списке pgsql-hackers по дате отправления:
Предыдущее
От: "Imseih (AWS), Sami"Дата:
Сообщение: Re: Add index scan progress to pg_stat_progress_vacuum