Обсуждение: Re: Collation & ctype method table, and extension hooks

On 9/27/24 12:30 AM, Jeff Davis wrote:
> The attached patch series refactors the collation and ctype behavior
> into method tables, and provides a way to hook the creation of a
> pg_locale_t so that an extension can create any kind of method table it
> wants.

Great! I had been planning to do this myself so great to see that you 
already did it before me. Will take a look at this work later.

Andreas

On Fri, 2024-10-04 at 15:24 +0200, Andreas Karlsson wrote:
> Great! I had been planning to do this myself so great to see that you
> already did it before me. Will take a look at this work later.

Great! We'll need to test whether there are any regressions in the
regex & pattern matching code due to the indirection.

What would be a good test for that? Just running it over long strings?

Regards,
    Jeff Davis

On 12.06.25 07:49, Jeff Davis wrote:
> On Fri, 2025-02-07 at 11:19 -0800, Jeff Davis wrote:
>>
>> Attached v15. Just a rebase.
> 
> Attached v16.
> 
>> * commit this on the grounds that it's a desirable code improvement
>> and
>> the worst-case regression isn't a major concern; or
> 
> I plan to commit this soon after branching. There's a general consensus
> that enabling multi-lib provider support is a good idea, and turning
> the provider behavior into method tables is a prerequisite for that. I
> doubt the performance issue will be a serious concern and I don't see a
> good way to avoid it.

Patch 0001 and 0002 seem okay to me.

I wish we could take this further and also run the "ctype is c" case 
through the method table.  Right now, there are still a bunch of 
open-coded special cases all over the place, which could be unified.  I 
guess this isn't any worse than before, but maybe this could be a future 
project?

Patch 0003 I don't understand.  It replaces type safety by no type 
safety, and it doesn't have any explanation or comments.  I suppose you 
have further plans in this direction, but until we have seen those and 
have more clarification and explanation, I would hold this back.

Patch 0004 seems ok.  But maybe you could explain this better in the 
commit message, like remove includes from pg_locale.h but instead put 
them in the .c files as needed, and explain why this is possible or 
suitable now.

On Sun, 2025-06-29 at 12:43 +0200, Peter Eisentraut wrote:
> I wish we could take this further and also run the "ctype is c" case
> through the method table.  Right now, there are still a bunch of
> open-coded special cases all over the place, which could be unified. 
> I
> guess this isn't any worse than before, but maybe this could be a
> future
> project?

+1. A few things need to be sorted out, but I don't see any major
problem with that.

> Patch 0003 I don't understand.  It replaces type safety by no type
> safety, and it doesn't have any explanation or comments.  I suppose
> you
> have further plans in this direction, but until we have seen those
> and
> have more clarification and explanation, I would hold this back.

Part of it is simply #include cleanliness, because we can't do v16-0004
if we have the provider-specific details in the union. I don't really
like the idea of including ICU headers (indirectly) so many places.
Another part is that I'd like to abstract the providers more completely
-- I've alluded to that a few times but I haven't made an independent
proposal for that yet. Also, the union doesn't offer a lot of type
safety, so I don't see it as a big loss.

But it's not critical right now either, so I won't push for it.

> Patch 0004 seems ok.  But maybe you could explain this better in the
> commit message, like remove includes from pg_locale.h but instead put
> them in the .c files as needed, and explain why this is possible or
> suitable now.

It goes with v16-0003, so I will hold this back for now as well.

Regards,
    Jeff Davis

This thread was still open in the commitfest and showed up on my dashboard.

My understanding is that v16-0001 and v16-0002 have been committed, and 
per the discussion below, the remaining patches v16-0003 and v16-0004 
have been withdrawn for now.  So I will close this commitfest entry.


On 30.06.25 21:21, Jeff Davis wrote:
> On Sun, 2025-06-29 at 12:43 +0200, Peter Eisentraut wrote:
>> I wish we could take this further and also run the "ctype is c" case
>> through the method table.  Right now, there are still a bunch of
>> open-coded special cases all over the place, which could be unified.
>> I
>> guess this isn't any worse than before, but maybe this could be a
>> future
>> project?
> 
> +1. A few things need to be sorted out, but I don't see any major
> problem with that.
> 
>> Patch 0003 I don't understand.  It replaces type safety by no type
>> safety, and it doesn't have any explanation or comments.  I suppose
>> you
>> have further plans in this direction, but until we have seen those
>> and
>> have more clarification and explanation, I would hold this back.
> 
> Part of it is simply #include cleanliness, because we can't do v16-0004
> if we have the provider-specific details in the union. I don't really
> like the idea of including ICU headers (indirectly) so many places.
> Another part is that I'd like to abstract the providers more completely
> -- I've alluded to that a few times but I haven't made an independent
> proposal for that yet. Also, the union doesn't offer a lot of type
> safety, so I don't see it as a big loss.
> 
> But it's not critical right now either, so I won't push for it.
> 
>> Patch 0004 seems ok.  But maybe you could explain this better in the
>> commit message, like remove includes from pg_locale.h but instead put
>> them in the .c files as needed, and explain why this is possible or
>> suitable now.
> 
> It goes with v16-0003, so I will hold this back for now as well.
> 
> Regards,
>     Jeff Davis
> 
> 
>