>> I've also attached a rebased patch that addresses all the latest feedback.
>> A reworked verison of the test patch is also included, but that's mostly
>> intended for CI purposes and is still not intended for commit (yet).
> And here's an attempt at fixing the alignment problems revealed by cfb
ind 1
test-*# ;
LOG: execute : SELECT $1
;
DETAIL: Parameters: $1 = '1'
?column?
--
1
(1 row)
test=*# close foo_noexist;
LOG: statement: close foo_noexist;
ERROR: cursor "foo_noexist" does not exist
LOG: temporary file: path "base/pgsql_tmp/pgsql_tmp56184.0"
> On Thu, Aug 28, 2025 at 05:53:23PM -0500, Sami Imseih wrote:
> > Just a few things that were discussed earlier, that I incorporated now.
> >
> > 1/ We should be checking that tranche_name is NOT NULL when
> > LWLockNewTrancheId or RequestNamedLWLockTranche is calle
> * I modified the array of tranche names to be a char ** to simplify
> lookups. I also changed how it is first initialized in CreateLWLocks() a
> bit.
That works also.
> * I've left out the tests for now. Those are great for the development
> phase, but I'm not completely sold on committ
> The check has to be done before the strlen() call, if not it segfault:
I don't know what I was thinking there :( silly mistake.
It was also missed in RequestNamedLWLockTranche.
> Most of the places where NAMEDATALEN is mentioned in sgml files also mention
> it's 64 bytes. Should we do the same
Thanks for reviewing!
> === 1
>
> We need to check if tranche_name is NULL and report an error if that's the
> case.
> If not, strlen() would segfault.
Added an error. Good call. The error message follows previously used
convention.
```
+ if (!tranche_name)
+ elog(ERROR, "tr
> > On Mon, Aug 25, 2025 at 04:59:41PM -0500, Sami Imseih wrote:
> > > hmm, can we really avoid a shared lock when reading from shared memory?
> > > considering access for both reads and writes can be concurrent to shared
> > > memory. We are also taking an
fixed the issues mentioned above in v13.
> We probably need to do the sprintf/strcpy before LWLockNewTrancheId().
> Also, I'm thinking we should just use the same tranche for both the DSA and
> the dshash table [0] to evade the DSMR_DSA_TRANCHE_SUFFIX problem, i.e.,
> those tranche names potential
> > That is why I think we should be careful about naming. pg_stat_backend feels
> > very generic, but right now it only shows relation stats. Maybe we call it
> > pg_stat_backend_tables to start? Then if we later add I/O, we could have
> > pg_stat_backend_io, or for conflicts, pg_stat_backend_conf
> On Mon, Aug 25, 2025 at 04:59:41PM -0500, Sami Imseih wrote:
> > hmm, can we really avoid a shared lock when reading from shared memory?
> > considering access for both reads and writes can be concurrent to shared
> > memory. We are also taking an exclusive lock when
> Adding these fields to the backend level stats spread based on the
> backend PID without the knowledge of the relation they're related with
> makes it much less interesting IMO, because we lose a lot of
> granularity value that we have with the pg_statio_* relations, at the
> cost of more bloat,
fit everything into
one view. It also helps us avoid having to rename views in the future.
What do you think?
--
Sami Imseih
Amazon Web Services (AWS)
> On Mon, Aug 25, 2025 at 04:37:21PM -0500, Sami Imseih wrote:
> > When we lookup from shared array only, we need to take a shared lock
> > every lookup. Acquiring that lock is what I am trying to avoid. You
> > are saying it's not worth optimizing that part, correct?
&
> On Fri, Aug 22, 2025 at 03:01:53PM -0500, Sami Imseih wrote:
> > I kept the local array to serve consecutive reads and to avoid having to
> > take a shared lock on shared memory every time GetLWTrancheName is
> > called. A new LWLock to protect this array is required.
>
Here it is.
I was hoping we can even get rid of NamedLWLockTrancheRequests
altogether, but that's not going to be possible because
RequestNamedLWLockTranche happens earlier than
CreateLWLocks, so NamedLWLockTrancheRequests is essentially
tracking the requested lwlocks until we get a chance to crea
> > Another approach is to just change GetNamedLWLockTranche to use
> > NamedLWLockTrancheArray since that is already copied in EXEC_BACKEND, and
> > allow GetNamedLWLockTranche to continue to be used outside of startup. In
> > this case, we will need to add num_lwlocks field to
> > NamedLWLockTran
> Attached is a repro patch. You will need to set
> shared_preload_libraries = 'pg_stat_statements'
> as well.
I forgot to mention. Once patched, you can run a simple
select statement from a new connection to force the crash.
This is because GetNamedLWLockTranche will be called
at pgss_ExecutorEnd
> On Mon, Aug 25, 2025 at 12:58:08PM -0500, Sami Imseih wrote:
> >> If this fails, why doesn't the call in pgss_shmem_startup() also fail? Was
> >> pg_stat_statements not loaded via shared_preload_libraries?
> >
> > because the array is valid in postmas
> On Fri, Aug 22, 2025 at 02:21:55PM -0500, Sami Imseih wrote:
> > While working on [0], I observed that $SUBJECT. I encountered this issue
> > while building test cases for [0], and in which GetNamedLWLockTranche is
> > called outside of startup.
> >
> > [...]
&g
On Fri, Aug 22, 2025 at 3:01 PM Sami Imseih wrote:
>
> >> If there is agreement on setting limits, may I propose
> >> 1024 tranches and NAMEDATALEN. Both seem reasonably sufficient.
>
> > Let's proceed with that approach for now. We can worry about the exac
>> If there is agreement on setting limits, may I propose
>> 1024 tranches and NAMEDATALEN. Both seem reasonably sufficient.
> Let's proceed with that approach for now. We can worry about the exact
> limits once this is closer to commit.
v11 implements the fixed size shared memory as discussed.
_ExecutorEnd
[0]
https://www.postgresql.org/message-id/CAA5RZ0vvED3naph8My8Szv6DL4AxOVK3eTPS0qXsaKi%3DbVdW2A%40mail.gmail.com
[1]
https://www.postgresql.org/docs/current/xfunc-c.html#XFUNC-SHARED-ADDIN-AT-STARTUP
--
Sami Imseih
Amazon Web Services (AWS)
> On Tue, Aug 19, 2025 at 03:52:33PM -0500, Sami Imseih wrote:
> > If we limit the tranche name to NAMEDATALEN and also limit the
> > number of tranches an extension can register, we can put this
> > all in static shared memory (We would still need to have a backend loc
> Nathan Bossart writes:
> > On Tue, Aug 19, 2025 at 02:37:19PM -0400, Andres Freund wrote:
> >> Sure, but we don't need to support a large number of tranches. Just make
> >> it,
> >> idk, 128 entries long. Adding a dynamically allocated dsm to every server
> >> seems like a waste - ever shared m
> I've been staring at the latest patch for a bit, and I'm a bit concerned
> at how much complexity it adds.
The complexity is the fact we have to track a dsa_pointer that points
to an array of dsa_pointers that track the tranche names. We also
do have to copy the array of dsa_pointers into a new
> > "Because this hook is executed by the postmaster and invoked by backends via
> > EXEC_BACKEND, it is essential to ensure that any code intended to run only
> > during postmaster startup is properly protected against repeated execution.
> > This can be enforced by verifying !IsUnderPostmaster be
Thanks for starting this thread! I do think it will be valuable to have
an idea of how many calls were aborted. But I am not so sure about
pg_stat_statements being the correct vehicle for this information.
The pg_stat_statements view deals with stats the are updated when a statement
completes exec
> see v9
Sorry about the quick follow-up I was not happy with one of the
comment updates in
SyncLWLockTrancheNames. I updated it.
Attached is v10,
--
Sami
v10-0001-Implement-a-DSA-for-LWLock-tranche-names.patch
Description: Binary data
v10-0002-Add-tests-for-LWLock-tranche-names-DSA.patch
D
> 1 ===
>
> +} LWLockTrancheNamesShmem;
>
> +} LWLockTrancheNamesStruct;
>
> Add LWLockTrancheNamesShmem and LWLockTrancheNamesStruct to
> src/tools/pgindent/typedefs.list?
done.
> 2 ===
>
> Maybe a comment before the above structs definitions to explain
>> See v7.
I fixed an earlier issue with Windows, which was due to not initializing the
shared memory inside
```
#ifdef EXEC_BACKEND
extern void AttachSharedMemoryStructs(void);
#endif
```
But then I found another one after. LWLockTrancheNames gets forked on Linux,
but of course that will not ha
> So we could do something like:
>
> int i = 0;
> while (i < LWLockTrancheNames.shmem->allocated &&
>!DsaPointerIsValid(shared_ptrs[i]))
> {
> i++;
> }
I found a different approach without tracking an additional counter.
Then sync stops when an invalid pointer is found after at
> On Fri, Aug 15, 2025 at 10:36:47AM -0500, Sami Imseih wrote:
> > But that could potentially be dangerous if code in the startup hook gets
> > re-executed? I guess the doc below is giving a vague warning that one
> > should be careful what they put in that hook.
>
&g
Sorry my last reply got mangled for some reason. Here it is again.
> > Blame shows that this change was introduced in commit 69d903367c,
> > but I could not determine the rationale from the discussion,
> > so it may have been an oversight.
>
> I think the startup hook must run in each backend for
> While working on a related area, I noticed that shmem_startup_hook
> is called twice under EXEC_BACKEND.
>
> The first call occurs in CreateSharedMemoryAndSemaphores() during
> postmaster startup (!IsUnderPostmaster), which is expected.
>
> The second call happens in AttachSharedMemoryStructs() (
rationale from the discussion,
so it may have been an oversight.
Thoughts?
--
Sami Imseih
Amazon Web Services (AWS)
> I haven't followed the latest discussion, but I took a look at the patch.
>
> + It is possible to use a tranche_id that was not
> retrieved
> + using LWLockNewTrancheId, but this is not
> recommended.
> + The ID may clash with an already registered tranche name, or the
> specifi
Hi,
Attached v6 which addresses the feedback from the last review.
1/ Rahila raised a point about the necessity to allocate new dsa pointers
for tranche names when copying ( during resize). That is correct. We can simply
memcpy those dsa pointers at the time of copy. All we need to
do is free the
>From this discussion, and the fact that the tranche name could come from
either local or shared memory, I think we should have tests.
So, I am working on adding tests using INJECTION_POINTS.
Some of the tests I have in mind now are:
1/ Does the shared memory grow correctly,
2/ Is the tranche nam
Thanks for testing!
> Why is it necessary to allocate a new dsa_pointer for tranche names that are
> the same size and then
> free the old one?
> Is there a reason we can't just assign new_ptrs[i] = old_ptrs[i]?
Fair point. I will updated in the next rev. We don't need to free the'
existing tran
Thanks for testing!
> which is what we expect.
>
> But if I look at LWLockTrancheNames.local (for the process that queried
> pg_stat_activity):
>
> (gdb) p LWLockTrancheNames.local[0]
> $1 = 0x0
> (gdb) p LWLockTrancheNames.local[1]
> $2 = 0x7b759a81b038 "BDT_Lck"
>
> It looks like there is an in
> I'll fix this in the next rev.
v5 fixes the above 2 issues found above.
For the issue that was throwing " segment that has been freed",
indeed we should be freeing LWLockTrancheNames.shmem->list_ptr,
list_ptr is a dsa_pointer that stores an array of dsa_pointers, which
then made me realize
Thanks for reviewing!
> Issue 1 --
>
> If I register enough tranches to go to:
>
> + /* Resize if needed */
> + if (LWLockTrancheNames.shmem->count >=
> LWLockTrancheNames.shmem->allocated)
> + {
> + newalloc =
> pg_nextpower2_32(Ma
> > > With a local hash table, I don't think it's necessary to introduce new
> > > code for managing
> > > a DSA based list of tranche names as is done in v3. We can go back to
> > > storing the shared
> > > trance names in dshash.
> > >
> > > What do you think?
> >
> > My first thought is that a p
> On Mon, Aug 04, 2025 at 11:32:19AM -0500, Sami Imseih wrote:
> > With a local hash table, I don't think it's necessary to introduce new
> > code for managing
> > a DSA based list of tranche names as is done in v3. We can go back to
> > storing the shared
>
> > > I think we could add a local backend copy that stays up to date with the
> > > DSA. One idea would be to use an atomic counter to track the number of
> > > entries in the DSA and compare it with a local backend counter whenever
> > > the
> > > tranche name lookup occurs. If the atomic counte
> > I think we could add a local backend copy that stays up to date with the
> > DSA. One idea would be to use an atomic counter to track the number of
> > entries in the DSA and compare it with a local backend counter whenever the
> > tranche name lookup occurs. If the atomic counter is higher (si
> > Also, I suspect that there might be some concerns about the API changes.
> > While it should be a very easy fix, that seems likely to break a lot of
> > extensions.
> > I don't know if it's possible to make this stuff backward
> > compatible, and I also don't know if we really want to, as that'
> > > Unlike the local list of tranche names, appending to and searching the
> > > shared memory list requires an LWLock; in exclusive mode to append, and
> > > shared mode to search.
> >
> > The thing that stood out the most to me is how much more expensive looking
> > up the lock name is. At the
> This means that the configuration parameter (GUC) currently overrides
> any explicit request for a plan type. The documentation does not clearly
> explain the rationale behind this, at least to me.
The documentation [0] mentions " If this default behavior is
unsuitable, you can alter it by
passi
> Patch 0002 was not without turbulences:
Thanks! And for sorting out the misses in the patch.
--
Sami
> So, analysing
> pg_s_s data, it would be beneficial to determine if a generic plan is
> effective or not.
Yes, this is the point of adding these statistics to pg_s_s.
> In practice, with this knowledge, we can access the CachedPlanSource of
> the corresponding PREPARED statement via an extensio
> > The term "NOT_SET" makes me itch a little bit, even if there is an
> > existing parallel with OverridingKind. Perhaps your proposal is OK,
> > still how about "UNKNOWN" instead to use as term for the default?
> +1 to "UNKNOWN".
We currently use both UNKNOWN and NOT_SET in different places.
Ho
> > Attached is my counter-proposal, where I have settled down to four
> > categories of PlannedStmt:
> > - "standard" PlannedStmt, when going through the planner.
> > - internally-generated "fake" PlannedStmt.
> > - custom cache
> > - generic cache
>
> Thanks for the update! I plan on reviewing th
> Attached is my counter-proposal, where I have settled down to four
> categories of PlannedStmt:
> - "standard" PlannedStmt, when going through the planner.
> - internally-generated "fake" PlannedStmt.
> - custom cache
> - generic cache
Thanks for the update! I plan on reviewing this tomorrow.
-
> The current pg_stat_statements change may be a
> good example of the employment of such infrastructure, isn't it?
So, the current set of patches now will help move forward the specific
use-case of this thread. If we need something different that will be
useful for more use-cases, perhaps it's be
> Sami Imseih writes:
> >> Perhaps CachedPlanType is
> >> misnamed, though, would it be more suited to name that as a sort of
> >> "origin" or "source" field concept? We want to know which which
> >> source we have retrieved a plan that
> Perhaps CachedPlanType is
> misnamed, though, would it be more suited to name that as a sort of
> "origin" or "source" field concept? We want to know which which
> source we have retrieved a plan that a PlannedStmt refers to.
Hmm, I’m not sure I see this as an improvement. In my opinion,
Cached
> One option might be to use a local hash table, keyed the same way as the
> shared pgss hash (excluding dbid), to handle cases where a backend has
> more than one active cached plan. Then at ExecutorEnd, the local entry could
> be looked up and passed to pgss_store. Not sure if this is worth the e
> Sami Imseih writes:
> >> That is not to say that I think 719dcf3c4 was a good idea: it looks
> >> rather useless from here. It seems to me that the right place to
> >> accumulate these sorts of stats is in CachedPlanSources, and I don't
> >> see
> Andrei Lepikhov writes:
> > I see you have chosen a variant with a new enum instead of a pointer to
> > a plan cache entry. I wonder if you could write the arguments
> > supporting this choice?
>
> Pointing to a plan cache entry would often mean that the data
> structure as a whole is circular (
> > I know Michael opposed the idea of carrying these structures,
> > at least CachedPlan, to Executor hooks ( or maybe just not QueryDesc?? ).
> > It will be good to see what he think, or if others an opinion about this,
> > about
> > adding a pointer to CachedPlanSource in PlannedStmt vs setting
> Now wondering if it would be better to spend the effort looking at
> pg_hint_plan and seeing how hard it would be to get global hints added
> which are applied to all queries, and then add a way to disable use of
> a named index. (I don't have any experience with that extension other
> than looki
> > with the types of cached plan. We need to be able to differentiate
> > when cached plans are not used, so a simple boolean is not
> > sufficient.
> Sure. But I modestly hope you would add a CachedPlanSource pointer
> solely to the PlannedStmt and restructure it a little as we discussed
> above.
> > It may be more efficient to set the is_generic_plan option at the top
> > plan node (PlannedStmt) and reference it wherever necessary. To identify
> > a cached plan, we may consider pushing the CachedPlan/CachedPlanSource
> > pointer down throughout pg_plan_query and maintaining a reference to
> > This is already an established pattern has been used by other
> > RDBMS's. Having worked with such interface in the past, a combo of
> > ALTER and GUC, I never thought it was awkward and it's quite simple to
> > understand/maintain. But that is subjective.
> >
>
> It's amazing what people are w
> I would like to oppose to the current version a little.
>
> Commits de3a2ea3b264 and 1d477a907e63 introduced elements that are more
> closely related to the execution phase. While the parameter
> es_parallel_workers_to_launch could be considered a planning parameter,
> es_parallel_workers_launche
> >> I was imagining putting the array in one big DSA allocation instead of
> >> carting around a pointer for each tranche name. (Sorry, I realize I am
> >> hand-waving over some of the details.)
> >
> > I understood it like this. Here is a sketch:
> >
> > ```
> > dsa_pointer p;
> >
> > dsa = dsa_
> Note: the size of the change in pg_stat_statements--1.12--1.13.sql
> points that we should seriously consider splitting these attributes
> into multiple sub-functions.
So we don't lose track of this. This should be a follow-up thread. I do
agree something has to be done about the exploding list
> Yes, I think that this is a much better idea to isolate the whole
> concept and let pgss grab these values. We have lived with such
> additions for monitoring in EState a few times already, see for
> example de3a2ea3b264 and 1d477a907e63 that are tainted with my
> fingerprints.
correct, there i
> > "plan cache mode" to be accessible in ExecutorEnd somehow.
> I agree with this - adding references to CachedPlan into the QueryDesc
> looks kludge.
> The most boring aspect of pg_stat_statements for me is the multiple
> statements case: a single incoming query (the only case in the cache of
> a
> > > it will still be extremely risky in
> > > heavy production workloads. In short, we're both walking a bull
> > > through the china shop, but it would seem mine is much more
> > > temperamental than yours.
> >
> > Robert, Could you describe the GUC you would like to see?
> >
> > Also, I'd like
> it will still be extremely risky in
> heavy production workloads. In short, we're both walking a bull
> through the china shop, but it would seem mine is much more
> temperamental than yours.
Robert, Could you describe the GUC you would like to see?
Also, I'd like to ask. what would be the argu
cost_delay
autovacuum_vacuum_cost_limit
autovacuum_vacuum_insert_scale_factor
autovacuum_vacuum_insert_threshold
autovacuum_vacuum_max_threshold
autovacuum_vacuum_scale_factor
autovacuum_vacuum_threshold
--
Sami Imseih
Amazon Web Services (AWS)
> Hmm, I don't propose modifying costs. The focus is on resetting the plan
> cache decision that PostgreSQL has made in automatic mode. During the
> DBMS operation, various factors may cause a generic plan to be
> suboptimal or make it more desirable as well. Discussions from 2010 to
> 2013 indicat
> Unless someone is willing to try and get “The PostgreSQL team’s blessed guide
> to index management”
> into the documentation
I really doubt we can agree on one set of index management guidelines.
If anything, this thread has proven there are many ways to bake this
cake :) and all approaches h
> My concern with #1 is that when we one day do get query hints, we'll
> be left with a bunch of redundant ways to influence planner behaviour.
The GUC is more than just a hint. It can serve as a hint, but it also offers
operational benefits. For example, if I mark an index as invisible and that
> >> this for better tracking. By adding a CachedPlanSource::cplan link, we
> >> can establish a connection to the entire PlanCache entry instead of only
> >> CachedPlan within a queryDesc and, consequently, make it accessible from
> >> the executor. This would give an access to statistics on costs
> > Ugh. This is plugging into an executor-related structure a completely
> > different layer, so that looks like an invasive layer violation to
> > me.. This is passed through ProcessQuery() from a Portal, changing
> > while on it ExplainOnePlan. If we want to get access from a cached
> > plan,
> > Hi,
> >
> > If a dshash table is used to store tranche names and IDs, where would the
> > tranche name for this table
> > be registered?
>
> I guess it could be a new BuiltinTrancheId for this dsa but not sure what
> Nathan
> and Sami have in mind.
Yes, it will be a BuiltinTrancheId for a sha
> this for better tracking. By adding a CachedPlanSource::cplan link, we
> can establish a connection to the entire PlanCache entry instead of only
> CachedPlan within a queryDesc and, consequently, make it accessible from
> the executor. This would give an access to statistics on costs and the
> n
> Ugh. This is plugging into an executor-related structure a completely
> different layer, so that looks like an invasive layer violation to
> me.. This is passed through ProcessQuery() from a Portal, changing
> while on it ExplainOnePlan. If we want to get access from a cached
> plan, wouldn't
On Tue, Jul 15, 2025 at 11:57 AM Nathan Bossart
wrote:
>
> On Tue, Jul 15, 2025 at 11:52:19AM -0500, Sami Imseih wrote:
> >> Another random thought: I worry that the dshash approach might be quite a
> >> bit slower, and IIUC we just need to map an integer to a string. M
> Another random thought: I worry that the dshash approach might be quite a
> bit slower, and IIUC we just need to map an integer to a string. Maybe we
> should just use a DSA for LWLockTrancheNames. IOW we'd leave it as a char**
> but put it in shared memory.
To use DSA just for this purpose, we
> Ah, I missed the problem with postmaster. Could we have the first backend
> that needs to access the table be responsible for creating it and
> populating it with the built-in/requested-at-startup entries?
We can certainly maintain a flag in the shared state that is set once
the first backend l
>> Attached is a proof of concept that does not alter the
>> LWLockRegisterTranche API.
> IMHO we should consider modifying the API, because right now you have to
> call LWLockRegisterTranche() in each backend. Why not accept the name as
> an argument for LWLockNewTrancheId() and only require it t
> > and instead reuse the existing static hash table, which is
> > capped at 128 custom wait events:
> >
> > ```
> > #define WAIT_EVENT_CUSTOM_HASH_MAX_SIZE 128
> > ```
>
> That's probably still high enough, thoughts?
I have no reason to believe that this number could be too low.
I am not aware of
Thanks for the feedback!
> > Attached is a proof of concept that does not alter the
> > LWLockRegisterTranche API. Instead, it detects when a registration is
> > performed by a normal backend and stores the tranche name in shared memory,
> > using a dshash keyed by tranche ID. Tranche name lookup
www.postgresql.org/message-id/aEiTzmndOVPmA6Mm%40nathan
--
Sami Imseih
Amazon Web Services (AWS)
0001-Improve-LWLock-tranche-name-visibility-across-backen.patch
Description: Binary data
>> Perhaps we should think about removing this description, what do you think?
> I think it's a good topic for another patch/thread. Chances are it's not
> the only description that could be updated.
Sure, that could be taken up after this patch.
> That's what I mean. I think it should say
>
>
> Sure, I am not against keeping the function in an existing section, but
> we should remove the description mentioned above for clarity.
to be clear, I am suggesting we just remove the second sentence
in the description. Therefore, instead of:
"The functions shown in Table 9.84 provide server tr
> On Mon, Jun 30, 2025 at 04:08:04PM +0300, Sami Imseih wrote:
> > Correct, I originally proposed the "Transaction ID and Snapshot
> > Information Functions" section, but as stated in [0],
> > pg_get_multixact_members does not fit the description of the section as
&g
On Mon, Jun 30, 2025 at 7:29 AM Ashutosh Bapat
wrote:
>
> On Sat, Jun 28, 2025 at 4:02 AM Nathan Bossart
> wrote:
> >
> > On Thu, Jun 12, 2025 at 03:10:56PM -0500, Nathan Bossart wrote:
> > > Barring comments/objections, I'll plan on committing/back-patching this in
> > > the near future.
> >
>
rebased patch.
Regards,
Sami
v6-0001-Normalize-variable-fetch-sizes-in-a-FETCH-command.patch
Description: Binary data
rebased patch.
Only changes to the tests due to the revert of nested query
tracking in f85f6ab051b
Regards,
Sami
v10-0001-Add-plan_cache-counters-to-pg_stat_statements.patch
Description: Binary data
> Okay, I've done this in the attached patch.
Thanks! v7 LGTM.
--
Sami
On Thu, Jun 12, 2025 at 11:32 AM Álvaro Herrera wrote:
>
> Hello,
>
> I have pushed that now,
thanks!
> and here's a rebase of patch 0003 to add support
> for PARAM_EXTERN. I'm not really sure about this one yet ...
see v11. I added a missing test to show how external param
normalization behav
> Attached patch removing extra comma. Otherwise LGTM.
Thanks! For v4, the final comma in the list is grammatically correct,
and it’s the style we use throughout the docs.
I marked the patch RFC.
--
Sami
I tested v6 and I think GetNamedDSA is a good addition. I did
not find any issues with the code. However, I am still convinced
that GetNamedDSMHash should not append " Hash" to the tranche
name of the dshash [0]. I am ok with " DSA" because the DSA tranche
is created implicitly by the API.
Also,
> It's not clear, without some effort, which lock mode go with which row lock
> in that description.
> For example, "keysh" does not have "for" in it but "fornokeyupd" does. How
> about rephrasing this
> as "The lock modes "keysh", "sh", fornokeyupd", and "forupd" correspond to
> FOR KEY SHARE, F
> IMO, having this GUC to force the use of invisible indexes is quite
> strange. In my view, it detracts from the guarantees that you're meant
> to get from disabling indexes. What if some connection has
> use_invisible_index set to true? The DBA might assume all is well
> after having seen nobody
1 - 100 of 445 matches
Mail list logo
