public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 2/3] Allow composite types in bootstrap
7+ messages / 5 participants
[nested] [flat]
* [PATCH 2/3] Allow composite types in bootstrap
@ 2020-11-17 15:28 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: Justin Pryzby @ 2020-11-17 15:28 UTC (permalink / raw)
---
src/backend/bootstrap/bootstrap.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index 9a9fa7fd38..f8a883dad7 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -916,6 +916,7 @@ gettype(char *type)
{
if (Typ != NIL)
{
+ static bool did_reread PG_USED_FOR_ASSERTS_ONLY = false; /* Already reread pg_types */
ListCell *lc;
foreach (lc, Typ)
@@ -927,6 +928,33 @@ gettype(char *type)
return app->am_oid;
}
}
+
+ /*
+ * The type wasn't known; check again to handle composite
+ * types, added since first populating the array.
+ */
+
+ /*
+ * Once all the types are populated and we handled composite
+ * types, shouldn't need to do that again.
+ */
+ Assert(!did_reread);
+ did_reread = true;
+
+ list_free_deep(Typ);
+ Typ = NULL;
+ populate_typ_array();
+
+ /* Need to avoid infinite recursion... */
+ foreach (lc, Typ)
+ {
+ struct typmap *app = lfirst(lc);
+ if (strncmp(NameStr(app->am_typ.typname), type, NAMEDATALEN) == 0)
+ {
+ Ap = app;
+ return app->am_oid;
+ }
+ }
}
else
{
--
2.26.2
--------------8917F6B7186C9FBB138CED7B
Content-Type: text/x-patch; charset=UTF-8;
name="0003-Extended-statistics-on-expressions-20201203.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename="0003-Extended-statistics-on-expressions-20201203.patch"
^ permalink raw reply [nested|flat] 7+ messages in thread
* pgindent vs variable declaration across multiple lines
@ 2023-01-20 01:31 Andres Freund <[email protected]>
0 siblings, 2 replies; 7+ messages in thread
From: Andres Freund @ 2023-01-20 01:31 UTC (permalink / raw)
To: pgsql-hackers
Hi,
There's a few places in the code that try to format a variable definition like this
ReorderBufferChange *next_change =
dlist_container(ReorderBufferChange, node, next);
but pgindent turns that into
ReorderBufferChange *next_change =
dlist_container(ReorderBufferChange, node, next);
even though the same pattern works, and is used fairly widely for assignments
amroutine->amparallelvacuumoptions =
VACUUM_OPTION_PARALLEL_BULKDEL;
Particularly when variable and/or types names are longer, it's sometimes hard
to fit enough into one line to use a different style. E.g., the code I'm
currently hacking on has
RWConflict possibleUnsafeConflict = dlist_container(RWConflictData, inLink, iter.cur);
There's simply no way to make break that across lines that doesn't either
violate the line length limit or makes pgindent do odd things:
too long line:
RWConflict possibleUnsafeConflict = dlist_container(RWConflictData,
inLink,
iter.cur);
pgindent will move start of second line:
RWConflict possibleUnsafeConflict =
dlist_container(RWConflictData, inLink, iter.cur);
I know I can leave the variable initially uninitialized and then do a separate
assignment, but that's not a great fix. And sometimes other initializations
want to access the variable alrady.
Do others dislike this as well?
I assume we'd again have to dive into pg_bsd_indent's code to fix it :(
And even if we were to figure out how, would it be worth the
reindent-all-branches pain? I'd say yes, but...
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: pgindent vs variable declaration across multiple lines
@ 2023-01-20 01:43 Tom Lane <[email protected]>
parent: Andres Freund <[email protected]>
1 sibling, 2 replies; 7+ messages in thread
From: Tom Lane @ 2023-01-20 01:43 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: pgsql-hackers
Andres Freund <[email protected]> writes:
> There's a few places in the code that try to format a variable definition like this
> ReorderBufferChange *next_change =
> dlist_container(ReorderBufferChange, node, next);
> but pgindent turns that into
> ReorderBufferChange *next_change =
> dlist_container(ReorderBufferChange, node, next);
Yeah, that's bugged me too. I suspect that the triggering factor is
use of a typedef name within the assigned expression, but I've not
tried to run it to ground.
> I assume we'd again have to dive into pg_bsd_indent's code to fix it :(
Yeah :-(. That's enough of a rat's nest that I've not really wanted to.
But I'd support applying such a fix if someone can figure it out.
> And even if we were to figure out how, would it be worth the
> reindent-all-branches pain? I'd say yes, but...
What reindent-all-branches pain? We haven't done an all-branches
reindent in the past, even for pgindent fixes that touched far more
code than this would (assuming that the proposed fix doesn't have
other side-effects).
regards, tom lane
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: pgindent vs variable declaration across multiple lines
@ 2023-01-20 01:59 Andres Freund <[email protected]>
parent: Tom Lane <[email protected]>
1 sibling, 1 reply; 7+ messages in thread
From: Andres Freund @ 2023-01-20 01:59 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: pgsql-hackers
Hi,
On 2023-01-19 20:43:44 -0500, Tom Lane wrote:
> Andres Freund <[email protected]> writes:
> > There's a few places in the code that try to format a variable definition like this
>
> > ReorderBufferChange *next_change =
> > dlist_container(ReorderBufferChange, node, next);
>
> > but pgindent turns that into
>
> > ReorderBufferChange *next_change =
> > dlist_container(ReorderBufferChange, node, next);
>
> Yeah, that's bugged me too. I suspect that the triggering factor is
> use of a typedef name within the assigned expression, but I've not
> tried to run it to ground.
It's not that - it happens even with just
int frak =
1;
since it doesn't happen for plain assignments, I think it's somehow related to
code dealing with variable declarations.
> > I assume we'd again have to dive into pg_bsd_indent's code to fix it :(
>
> Yeah :-(. That's enough of a rat's nest that I've not really wanted to.
> But I'd support applying such a fix if someone can figure it out.
It's pretty awful code :(
> > And even if we were to figure out how, would it be worth the
> > reindent-all-branches pain? I'd say yes, but...
>
> What reindent-all-branches pain? We haven't done an all-branches
> reindent in the past, even for pgindent fixes that touched far more
> code than this would (assuming that the proposed fix doesn't have
> other side-effects).
Oh. I thought we had re-indented the other branches when we modified
pg_bsd_intent substantially in the past, to reduce backpatching pain. But I
guess we just discussed that option, but didn't end up pursuing it.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: pgindent vs variable declaration across multiple lines
@ 2023-01-20 02:07 Tom Lane <[email protected]>
parent: Andres Freund <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: Tom Lane @ 2023-01-20 02:07 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: pgsql-hackers
Andres Freund <[email protected]> writes:
> On 2023-01-19 20:43:44 -0500, Tom Lane wrote:
>> What reindent-all-branches pain? We haven't done an all-branches
>> reindent in the past, even for pgindent fixes that touched far more
>> code than this would (assuming that the proposed fix doesn't have
>> other side-effects).
> Oh. I thought we had re-indented the other branches when we modified
> pg_bsd_intent substantially in the past, to reduce backpatching pain. But I
> guess we just discussed that option, but didn't end up pursuing it.
Yeah, we did discuss it, but never did it --- I think the convincing
argument not to was that major reformatting would be very painful
for people maintaining forks, and we shouldn't put them through that
to track minor releases.
regards, tom lane
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: pgindent vs variable declaration across multiple lines
@ 2023-01-20 23:50 Robert Haas <[email protected]>
parent: Andres Freund <[email protected]>
1 sibling, 0 replies; 7+ messages in thread
From: Robert Haas @ 2023-01-20 23:50 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: pgsql-hackers
On Thu, Jan 19, 2023 at 8:31 PM Andres Freund <[email protected]> wrote:
> I know I can leave the variable initially uninitialized and then do a separate
> assignment, but that's not a great fix.
That's what I do.
If you pick names for all of your data types that are very very long
and wordy then you don't feel as bad about this, because you were
gonna need a line break anyway. :-)
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: pgindent vs variable declaration across multiple lines
@ 2023-01-21 18:57 Thomas Munro <[email protected]>
parent: Tom Lane <[email protected]>
1 sibling, 0 replies; 7+ messages in thread
From: Thomas Munro @ 2023-01-21 18:57 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Andres Freund <[email protected]>; pgsql-hackers
On Fri, Jan 20, 2023 at 2:43 PM Tom Lane <[email protected]> wrote:
> Andres Freund <[email protected]> writes:
> > There's a few places in the code that try to format a variable definition like this
>
> > ReorderBufferChange *next_change =
> > dlist_container(ReorderBufferChange, node, next);
>
> > but pgindent turns that into
>
> > ReorderBufferChange *next_change =
> > dlist_container(ReorderBufferChange, node, next);
>
> Yeah, that's bugged me too. I suspect that the triggering factor is
> use of a typedef name within the assigned expression, but I've not
> tried to run it to ground.
>
> > I assume we'd again have to dive into pg_bsd_indent's code to fix it :(
>
> Yeah :-(. That's enough of a rat's nest that I've not really wanted to.
> But I'd support applying such a fix if someone can figure it out.
This may be a clue: the place where declarations are treated
differently seems to be (stangely) in io.c:
ps.ind_stmt = ps.in_stmt & ~ps.in_decl; /* next line should be
* indented if we have not
* completed this stmt and if
* we are not in the middle of
* a declaration */
If you just remove "& ~ps.in_decl" then it does the desired thing for
that new code in predicate.c, but it also interferes with declarations
with commas, ie int i, j; where i and j currently line up, now j just
gets one indentation level. It's probably not the right way to do it
but you can fix that with a last token kluge, something like:
#include "indent_codes.h"
ps.ind_stmt = ps.in_stmt && (!ps.in_decl || ps.last_token != comma);
That improves a lot of code in our tree IMHO but of course there is
other collateral damage...
^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2023-01-21 18:57 UTC | newest]
Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-11-17 15:28 [PATCH 2/3] Allow composite types in bootstrap Justin Pryzby <[email protected]>
2023-01-20 01:31 pgindent vs variable declaration across multiple lines Andres Freund <[email protected]>
2023-01-20 01:43 ` Re: pgindent vs variable declaration across multiple lines Tom Lane <[email protected]>
2023-01-20 01:59 ` Re: pgindent vs variable declaration across multiple lines Andres Freund <[email protected]>
2023-01-20 02:07 ` Re: pgindent vs variable declaration across multiple lines Tom Lane <[email protected]>
2023-01-21 18:57 ` Re: pgindent vs variable declaration across multiple lines Thomas Munro <[email protected]>
2023-01-20 23:50 ` Re: pgindent vs variable declaration across multiple lines Robert Haas <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox