public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v23 3/3] Avoid some calls to RelationGetRelationName
5+ messages / 5 participants
[nested] [flat]
* [PATCH v23 3/3] Avoid some calls to RelationGetRelationName
@ 2020-02-27 01:22 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 5+ messages in thread
From: Justin Pryzby @ 2020-02-27 01:22 UTC (permalink / raw)
---
src/backend/access/heap/vacuumlazy.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index d10a1bcc96..170b85f7b4 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -611,8 +611,8 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params,
}
appendStringInfo(&buf, msgfmt,
get_database_name(MyDatabaseId),
- get_namespace_name(RelationGetNamespace(onerel)),
- RelationGetRelationName(onerel),
+ vacrelstats->relnamespace,
+ vacrelstats->relname,
vacrelstats->num_index_scans);
appendStringInfo(&buf, _("pages: %u removed, %u remain, %u skipped due to pins, %u skipped frozen\n"),
vacrelstats->pages_removed,
@@ -784,7 +784,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
if (params->nworkers > 0)
ereport(WARNING,
(errmsg("disabling parallel option of vacuum on \"%s\" --- cannot vacuum temporary tables in parallel",
- RelationGetRelationName(onerel))));
+ vacrelstats->relname)));
}
else
lps = begin_parallel_vacuum(RelationGetRelid(onerel), Irel,
@@ -1695,7 +1695,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
if (vacuumed_pages)
ereport(elevel,
(errmsg("\"%s\": removed %.0f row versions in %u pages",
- RelationGetRelationName(onerel),
+ vacrelstats->relname,
tups_vacuumed, vacuumed_pages)));
/*
@@ -1724,7 +1724,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
ereport(elevel,
(errmsg("\"%s\": found %.0f removable, %.0f nonremovable row versions in %u out of %u pages",
- RelationGetRelationName(onerel),
+ vacrelstats->relname,
tups_vacuumed, num_tuples,
vacrelstats->scanned_pages, nblocks),
errdetail_internal("%s", buf.data)));
@@ -1855,7 +1855,7 @@ lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats)
ereport(elevel,
(errmsg("\"%s\": removed %d row versions in %d pages",
- RelationGetRelationName(onerel),
+ vacrelstats->relname,
tupindex, npages),
errdetail_internal("%s", pg_rusage_show(&ru0))));
}
@@ -2383,7 +2383,7 @@ lazy_vacuum_index(Relation indrel, IndexBulkDeleteResult **stats,
ereport(elevel,
(errmsg(msg,
- RelationGetRelationName(indrel),
+ vacrelstats->relname,
dead_tuples->num_tuples),
errdetail_internal("%s", pg_rusage_show(&ru0))));
}
@@ -2531,7 +2531,7 @@ lazy_truncate_heap(Relation onerel, LVRelStats *vacrelstats)
vacrelstats->lock_waiter_detected = true;
ereport(elevel,
(errmsg("\"%s\": stopping truncate due to conflicting lock request",
- RelationGetRelationName(onerel))));
+ vacrelstats->relname)));
return;
}
@@ -2600,7 +2600,7 @@ lazy_truncate_heap(Relation onerel, LVRelStats *vacrelstats)
ereport(elevel,
(errmsg("\"%s\": truncated %u to %u pages",
- RelationGetRelationName(onerel),
+ vacrelstats->relname,
old_rel_pages, new_rel_pages),
errdetail_internal("%s",
pg_rusage_show(&ru0))));
@@ -2665,7 +2665,7 @@ count_nondeletable_pages(Relation onerel, LVRelStats *vacrelstats)
{
ereport(elevel,
(errmsg("\"%s\": suspending truncate due to conflicting lock request",
- RelationGetRelationName(onerel))));
+ vacrelstats->relname)));
vacrelstats->lock_waiter_detected = true;
return blkno;
--
2.17.0
--HcAYCG3uE/tztfnV--
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Add PQsendSyncMessage() to libpq
@ 2023-05-02 14:02 Robert Haas <[email protected]>
2023-05-03 10:03 ` Re: Add PQsendSyncMessage() to libpq Alvaro Herrera <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Robert Haas @ 2023-05-02 14:02 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Anton Kirilov <[email protected]>; pgsql-hackers
On Mon, May 1, 2023 at 8:42 PM Michael Paquier <[email protected]> wrote:
> Another thing that may matter in terms of extensibility? Would a
> boolean argument really be the best design? Could it be better to
> have instead one API with a bits32 and some flags controlling its
> internals?
I wondered that, too. If we never add any more Boolean parameters to
this function then that would end up a waste, but maybe we will and
then it will be genius. Not sure what's best.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Add PQsendSyncMessage() to libpq
2023-05-02 14:02 Re: Add PQsendSyncMessage() to libpq Robert Haas <[email protected]>
@ 2023-05-03 10:03 ` Alvaro Herrera <[email protected]>
2023-05-08 03:14 ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
2023-11-07 09:23 ` Re: Add PQsendSyncMessage() to libpq Jelte Fennema-Nio <[email protected]>
0 siblings, 2 replies; 5+ messages in thread
From: Alvaro Herrera @ 2023-05-03 10:03 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Michael Paquier <[email protected]>; Anton Kirilov <[email protected]>; pgsql-hackers
On 2023-May-02, Robert Haas wrote:
> On Mon, May 1, 2023 at 8:42 PM Michael Paquier <[email protected]> wrote:
> > Another thing that may matter in terms of extensibility? Would a
> > boolean argument really be the best design? Could it be better to
> > have instead one API with a bits32 and some flags controlling its
> > internals?
>
> I wondered that, too. If we never add any more Boolean parameters to
> this function then that would end up a waste, but maybe we will and
> then it will be genius. Not sure what's best.
I agree that adding a flag is the way to go, since it improve chances
that we won't end up with ten different functions in case we decide to
have eight other behaviors. One more function and we're done. And
while I can't think of any use for a future flag, we (I) already didn't
of this one either, so let's not make the same mistake.
We already have 'int' flag masks in PQcopyResult() and
PQsetTraceFlags(). We were using bits32 initially for flag stuff in the
PQtrace facilities, until [1] reminded us that we shouldn't let c.h
creep into app-land, so that was turned into plain 'int'.
[1] https://www.postgresql.org/message-id/TYAPR01MB2990B6C6A32ACF15D97AE94AFEBD0%40TYAPR01MB2990.jpnprd0...
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"No nos atrevemos a muchas cosas porque son difíciles,
pero son difíciles porque no nos atrevemos a hacerlas" (Séneca)
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Add PQsendSyncMessage() to libpq
2023-05-02 14:02 Re: Add PQsendSyncMessage() to libpq Robert Haas <[email protected]>
2023-05-03 10:03 ` Re: Add PQsendSyncMessage() to libpq Alvaro Herrera <[email protected]>
@ 2023-05-08 03:14 ` Michael Paquier <[email protected]>
1 sibling, 0 replies; 5+ messages in thread
From: Michael Paquier @ 2023-05-08 03:14 UTC (permalink / raw)
To: Alvaro Herrera <[email protected]>; +Cc: Robert Haas <[email protected]>; Anton Kirilov <[email protected]>; pgsql-hackers
On Wed, May 03, 2023 at 12:03:57PM +0200, Alvaro Herrera wrote:
> We already have 'int' flag masks in PQcopyResult() and
> PQsetTraceFlags(). We were using bits32 initially for flag stuff in the
> PQtrace facilities, until [1] reminded us that we shouldn't let c.h
> creep into app-land, so that was turned into plain 'int'.
>
> [1] https://www.postgresql.org/message-id/TYAPR01MB2990B6C6A32ACF15D97AE94AFEBD0%40TYAPR01MB2990.jpnprd0...
Indeed. Good point!
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../ZFhpMkFR%[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Add PQsendSyncMessage() to libpq
2023-05-02 14:02 Re: Add PQsendSyncMessage() to libpq Robert Haas <[email protected]>
2023-05-03 10:03 ` Re: Add PQsendSyncMessage() to libpq Alvaro Herrera <[email protected]>
@ 2023-11-07 09:23 ` Jelte Fennema-Nio <[email protected]>
1 sibling, 0 replies; 5+ messages in thread
From: Jelte Fennema-Nio @ 2023-11-07 09:23 UTC (permalink / raw)
To: Alvaro Herrera <[email protected]>; +Cc: Robert Haas <[email protected]>; Michael Paquier <[email protected]>; Anton Kirilov <[email protected]>; pgsql-hackers
On Fri, 28 Apr 2023 at 14:07, Robert Haas <[email protected]> wrote:
> I wonder whether this is the naming that we want. The two names are
> significantly different. Something like PQpipelineSendSync() would be
> more similar.
>
> I also wonder, really even more, whether it would be better to do
> something like PQpipelinePutSync(PGconn *conn, bool flush) with
> PQpipelineSync(conn) just meaning PQpipelinePutSync(conn, true). We're
> basically using the function name as a Boolean parameter to select the
> behavior, which is fine if you only have one parameter and it's a
> Boolean, but it's obviously unworkable if you have say 3 Boolean
> parameters because you don't want 8 different functions, and what if
> you need an integer parameter for some reason?
On Wed, 3 May 2023 at 12:04, Alvaro Herrera <[email protected]> wrote:
> I agree that adding a flag is the way to go, since it improve chances
> that we won't end up with ten different functions in case we decide to
> have eight other behaviors. One more function and we're done. And
> while I can't think of any use for a future flag, we (I) already didn't
> of this one either, so let's not make the same mistake.
On Sat, 29 Apr 2023 at 18:07, Anton Kirilov <[email protected]> wrote:
> The reason is that the function is modeled after PQsendFlushRequest(),
> since it felt closer to what I was trying to achieve, i.e. appending a
> protocol message to the output buffer without doing any actual I/O
> operations.
Sorry for being late to the party, but I think the current API naming
and the flag argument don't fit well with the current libpq API that
we have. I much prefer something similar to the original version of
the patch.
I think this function should be named something with the "PQsend"
prefix since that's the way we name all our public async message
sending functions in libpq. The "Put" word we only use in internal
libpq functions, so I feel it has no place in the external API
surface. My proposal would be to call the function PQsendPipelineSync
(i.e. having the PQsend prefix while still looking similar to the
existing PQpipelineSync).
Also I think the flag argument is completely unnecessary. I understand
the argument that we didn't foresee the need for this non-flushing
behaviour either, and the follow up reasoning that we thus should add
a flag for future things we didn't forsee. But I think it's looking at
the situation from the wrong direction. Instead of looking at it as
adding another version of our current PQpipelineSync API, we should
look at it as an addition to our current list of PQsend functions for
a new packet type. And none of those PQsend functions ever needed a
flag. Which makes sense, because they are the lowest level building
blocks that make sense from a user perspective: They send a message
type over the socket and don't do anything else. And if the assumption
that this is the lowest level building block is wrong, then it will
almost certainly be wrong for all other PQsend functions too. And thus
we'll need a solution that fits for all of them.
Finally, I have one suggestion for a behavioural change: I think the
function should still call pqPipelineFlush, just like all of our other
PQsend functions (except PQsendFlushRequest, but that seems like an
oversight there too).
^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2023-11-07 09:23 UTC | newest]
Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-02-27 01:22 [PATCH v23 3/3] Avoid some calls to RelationGetRelationName Justin Pryzby <[email protected]>
2023-05-02 14:02 Re: Add PQsendSyncMessage() to libpq Robert Haas <[email protected]>
2023-05-03 10:03 ` Re: Add PQsendSyncMessage() to libpq Alvaro Herrera <[email protected]>
2023-05-08 03:14 ` Re: Add PQsendSyncMessage() to libpq Michael Paquier <[email protected]>
2023-11-07 09:23 ` Re: Add PQsendSyncMessage() to libpq Jelte Fennema-Nio <[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