On Wed, Mar 27, 2024, 11:46 Robert Haas wrote:
> On Wed, Mar 27, 2024 at 1:12 PM Isaac Morland
> wrote:
> > On Wed, 27 Mar 2024 at 13:05, Greg Sabino Mullane
> wrote:
> >>> The purpose of the setting is to prevent
> accidental modifications via ALTER
> SYSTEM in environments where
> >> The
On Mon, Mar 18, 2024 at 10:27 AM Robert Haas wrote:
> Right, we're adding this because of environments like Kubernetes,
> which isn't a version, but it is a platform, or at least a deployment
> mode, which is why I thought of that section. I think for now we
> should just file this under "Other
On Mon, Mar 18, 2024 at 7:12 AM Jelte Fennema-Nio wrote:
>
> On Mon, 18 Mar 2024 at 13:57, Robert Haas wrote:
> > I would have been somewhat inclined to find an existing section
> > of postgresql.auto.conf for this parameter, perhaps "platform and
> > version compatibility".
>
> I tried to find
On Thu, Mar 14, 2024 at 1:38 PM Robert Haas wrote:
> On Thu, Mar 14, 2024 at 4:08 PM Tom Lane wrote:
> > The patch-of-record contains no such wording.
>
> I plan to fix that, if nobody else beats me to it.
>
> > And if this isn't a
> > security feature, then what is it? If you have to say to
The following review has been posted through the commitfest application:
make installcheck-world: tested, failed
Implements feature: tested, passed
Spec compliant: not tested
Documentation:tested, passed
I'm not sure if this actually still needs review, but it's
This is not the right mailing list for your question. Try the
pgadmin-support [1] mailing list. You may also want to include more details
in your question, because it's not really possible to tell what's going
wrong from your description.
[1]: https://www.postgresql.org/list/pgadmin-support/
Have you seen the other recent patch regarding this? [1] The mailing
list thread was active pretty recently. The submission is marked as
Needs Review. I haven't looked at either patch, but the proposals are
very similar as I understand it.
[1]: https://commitfest.postgresql.org/45/4345/
On Fri, Dec 1, 2023 at 11:32 AM Joe Conway wrote:
> 1. Is supporting JSON array format sufficient, or does it need to
> support some other options? How flexible does the support scheme need to be?
"JSON Lines" is a semi-standard format [1] that's basically just
newline-separated JSON values. (In
On Tue, Oct 17, 2023, 09:22 Robert Haas wrote:
> On Tue, Oct 17, 2023 at 12:18 PM Tom Lane wrote:
> > Robert Haas writes:
> > Is that actually possible? I had the idea that "git push" is an
> > atomic operation, ie 100% or nothing. Is it only atomic per-branch?
>
> I believe so.
Git push
Great, thanks for the guidance!
I've also tried the patch and I see the same results as Jakub, which
make sense to me. I did have issues getting it to apply, though: `git
am` complains about a conflict, though patch itself was able to apply
it.
Hello,
My colleague has been working on submitting a patch [1] to the Ruby
Rails framework to address some of the problems discussed in [2].
Regardless of whether that change lands, the change in Rails would be
useful since people will be running Postgres versions without this
patch for a while.
On Tue, Sep 19, 2023, 20:23 Maciek Sakrejda wrote:
> I wonder if something like CURRENT (i.e., the search path at function
> creation time) might be a useful keyword addition. I can see some uses
> (more forgiving than SYSTEM but not as loose as SESSION), but I don't
> know if it w
On Tue, Sep 19, 2023 at 5:56 PM Jeff Davis wrote:
>...
> On Tue, 2023-09-19 at 11:41 -0400, Robert Haas wrote:
> > That leads to a second idea, which is having it continue
> > to be a GUC but only affect directly-entered SQL, with all
> > indirectly-entered SQL either being stored as a node tree
On Mon, May 1, 2023, 18:08 Robert Haas wrote:
> I am saying that, while wraparound is perhaps not a perfect term
> for what's happening, it is not, in my opinion, a bad term either.
I don't want to put words into Peter's mouth, but I think that he's arguing
that the term "wraparound" suggests
For what it's worth, I think having the anchors be always-visible when
CSS disabled is a feature. The content is still perfectly readable,
and the core feature from this patch is available. Introducing
JavaScript to lose that functionality seems like a step backwards.
By the way, the latest patch
On Thu, Feb 23, 2023, 09:55 Nikolay Samokhvalov
wrote:
> On Thu, Feb 23, 2023 at 9:05 AM Maciek Sakrejda
> wrote:
>
>> I think Heikki's solution is probably more practical since (1) ..
>
>
> Note that these ideas target two *different* problems:
> - what was the
+1 on solving the general problem of "I forgot to set \timing--how
long did this run?". I could have used that more than once in the
past, and I'm sure it will come up again.
I think Heikki's solution is probably more practical since (1) even if
we add the prompt parameter originally proposed, I
On Tue, Feb 14, 2023 at 11:08 AM Andres Freund wrote:
> One thing I started to wonder about since is whether we should remove the io_
> prefix from io_object, io_context. The prefixes make sense on the C level, but
> it's not clear to me that that's also the case on the table level.
Yeah, +1.
I could have used such an aggregate in the past, so +1.
This is maybe getting into nit-picking, but perhaps it should be
documented as returning an "arbitrary" value instead of a
"non-deterministic" one? Technically the value is deterministic:
there's a concrete algorithm specifying how it's
On Fri, Jan 27, 2023, 18:58 Andres Freund wrote:
> Hi,
>
> On 2023-01-27 16:15:08 +1300, Thomas Munro wrote:
> > It would be pg_current_xact_id() that would have to pay the cost of
> > the WAL flush, not pg_xact_status() itself, but yeah that's what the
> > patch does (with some optimisations).
On Tue, Jan 17, 2023 at 9:22 AM Melanie Plageman
wrote:
> On Mon, Jan 16, 2023 at 4:42 PM Maciek Sakrejda wrote:
> > I missed a couple of versions, but I think the docs are clearer now.
> > I'm torn on losing some of the detail, but overall I do think it's a
> > good t
On Fri, Jan 13, 2023 at 10:38 AM Melanie Plageman
wrote:
>
> Attached is v47.
I missed a couple of versions, but I think the docs are clearer now.
I'm torn on losing some of the detail, but overall I do think it's a
good trade-off. Moving some details out to after the table does keep
the bulk of
On Fri, Jul 15, 2022 at 11:21 AM Maciek Sakrejda wrote:
> On Fri, Jul 1, 2022 at 10:26 AM Andres Freund wrote:
> > On 2022-07-01 01:23:01 -0700, Lukas Fittl wrote:
> >...
> > > Known WIP problems with this patch version:
> > >
> > > * There appears to be
Sure. My C is pretty limited, but I think it's just the attached? I
patterned the usage on the way this is done in CreateRole. It passes
check-world here.
From c7cae5d3e8d179505f26851f1241436a3748f9a8 Mon Sep 17 00:00:00 2001
From: Maciek Sakrejda
Date: Sat, 10 Dec 2022 22:51:02 -0800
Subject
While browsing through varsup.c, I noticed this comment on GetNewObjectId:
* Hence, this routine should generally not be used directly. The only direct
* callers should be GetNewOidWithIndex() and GetNewRelFileNumber() in
* catalog/catalog.c.
But AddRoleMems in user.c appears to also call the
In the pg_stat_statements docs, there are several column descriptions like
Total number of ... by the statement
You added an additional sentence to some describing the equivalent
pg_stat_io values, but you only added a period to the previous
sentence for shared_blks_read (for other columns,
time had you leveraged the freelist. I think the
> decision about which tradeoff to make is quite contentious, though.
Thanks for the explanation--that makes sense.
> On Mon, Nov 7, 2022 at 1:26 PM Maciek Sakrejda wrote:
> > Alternately, what do you think about pulling equivalencies to existing
I noticed an odd behavior today in pg_stat_statements query
normalization for queries called from SQL-language functions. If I
have three functions that call an essentially identical query (the
functions are only marked SECURITY DEFINER to prevent inlining):
maciek=# create or replace function
On Thu, Nov 3, 2022 at 10:00 AM Melanie Plageman
wrote:
>
> I decided not to call it pg_statio because all of the other stats views
> have an underscore after stat and I thought it was an opportunity to be
> consistent with them.
Oh, got it. Makes sense.
> > I'm reviewing the rendered docs now,
On Wed, Oct 26, 2022 at 10:55 AM Melanie Plageman
wrote:
+ The pg_statio_ and
+ pg_stat_io views are primarily useful to determine
+ the effectiveness of the buffer cache. When the number of actual disk reads
Totally nitpicking, but this reads a little funny to me. Previously
the trailing
On Sat, Oct 29, 2022 at 10:40 AM Justin Pryzby wrote:
>
> On Fri, Sep 02, 2022 at 05:24:58PM -0500, Justin Pryzby wrote:
> > It caused no issue when I changed:
> >
> > /* Check that it's acceptable for the indicated
> > parameter */
> > if
On Thu, Oct 20, 2022 at 10:31 AM Andres Freund wrote:
> - "repossession" is a very unintuitive name for me. If we want something like
> it, can't we just name it reuse_failed or such?
+1, I think "repossessed" is awkward. I think "reuse_failed" works,
but no strong opinions on an alternate
On Wed, Oct 19, 2022 at 12:27 PM Melanie Plageman
wrote:
>
> v34 is attached.
> I think the column names need discussion. Also, the docs need more work
> (I added a lot of new content there). I could use feedback on the column
> names and definitions and review/rephrasing ideas for the docs
>
On Thu, Oct 13, 2022 at 10:29 AM Melanie Plageman
wrote:
> I think that it makes sense to count both the initial buffers added to
> the ring and subsequent shared buffers added to the ring (either when
> the current strategy buffer is pinned or in use or when a bulkread
> rejects dirty strategy
Thanks for working on this! Like Lukas, I'm excited to see more
visibility into important parts of the system like this.
On Mon, Oct 10, 2022 at 11:49 AM Melanie Plageman
wrote:
>
> I've gone ahead and implemented option 1 (commented below).
No strong opinion on 1 versus 2, but I guess at least
Thanks for picking this back up, Justin.
>I've started to think that we should really WARN whenever a (set of) GUC is set
>in a manner that the server will fail to start - not just for shared libraries.
+0.5. I think it's a reasonable change, but I've never broken my
server with anything other
I ran that original test case with and without the patch. Here are the
numbers I'm seeing:
master (best of three):
postgres=# SELECT count(*) FROM lotsarows;
Time: 582.423 ms
postgres=# EXPLAIN (ANALYZE, TIMING OFF) SELECT count(*) FROM lotsarows;
Time: 616.102 ms
postgres=# EXPLAIN (ANALYZE,
On Thu, Feb 24, 2022, 16:52 Kyotaro Horiguchi
wrote:
> At Tue, 21 Dec 2021 08:47:27 +0100, Brar Piening wrote in
> > On 20.12.2021 at 16:09, Robert Haas wrote:
> > > As a data point, this is something I have also wanted to do, from time
> > > to time. I am generally of the opinion that any
Andrew made a good case above for avoiding LOG:
>I do think we should be wary of any name starting with "LOG", though.
>Long experience tells us that's something that confuses users when it
refers to the WAL.
On Wed, Feb 9, 2022 at 5:58 PM Justin Pryzby wrote:
> FYI, it has said "while..." and hasn't said "guc" since the 2nd revision of
> the
> patch.
The v3-0001 attached above had "while... for GUC..."--sorry I wasn't clear.
In v4, the message looks fine to me for shared_preload_libraries
(except
I'm chiming in a little late here, but as someone who worked on a
system to basically work around the lack of unprivileged CREATE ROLE
for a cloud provider (I worked on the Heroku Data team for several
years), I thought it might be useful to offer my perspective. This is,
of course, not the only
On Wed, Feb 2, 2022 at 7:39 AM David G. Johnston
wrote:
> I would at least consider having the UX go something like:
>
> postgres=# ALTER SYSTEM SET shared_preload_libraries = not_such_library;
> ERROR: that library is not present>.
> HINT: to bypass the error please add FORCE before SET
>
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: not tested
Documentation:not tested
I tried the latest version of the patch, and it works as discussed.
On Sat, Jan 8, 2022 at 2:07 PM Justin Pryzby wrote:
> Unfortunately, the output for dlopen() is not portable, which (I think) means
> most of what I wrote can't be made to work.. Since it doesn't work to call
> dlopen() when using SET, I tried using just stat(). But that also fails on
>
On Thu, Dec 30, 2021 at 12:21 AM Bharath Rupireddy
wrote:
> Overall the idea looks good to me. A warning on ALTER SYSTEM SET seems
> reasonable than nothing. ERROR isn't the way to go as it limits the
> users of setting the extensions in shared_preload_libraries first,
> installing them later. Is
Thanks for working on this! I tried it out and it worked for me. I
reviewed the patch and didn't see any problems, but I'm not much of a
C programmer.
On Tue, Dec 28, 2021 at 9:45 AM Justin Pryzby wrote:
> 0002 adds context when failing to start.
>
> 2021-12-27 17:01:12.996 CST
On Fri, Dec 10, 2021 at 10:10 AM Godfrin, Philippe E
wrote:
> I may have missed something in this stream, but is this a system controlled
> by Patroni?
In my case, no: it's a pretty vanilla PGDG install on Ubuntu 20.04.
Thanks for the context, though.
Thanks,
Maciek
On Thu, Dec 9, 2021 at 7:42 AM Bharath Rupireddy
wrote:
> How about ALTER SYSTEM SET shared_preload_libraries command itself
> checking for availability of the specified libraries (after fetching
> library names from the parsed string value) with stat() and then
> report an error if any of the
On Wed, Dec 1, 2021 at 5:15 AM Tom Lane wrote:
>
> Justin Pryzby writes:
> > +1 to document it, but it seems like the worse problem is allowing the
> > admin to
> > write a configuration which causes the server to fail to start, without
> > having
> > issued a warning.
>
> > I think you could
Hello,
(I hope it's okay to ask general internals questions here; if this list is
strictly for development, I'll keep my questions on -general but since I'm
asking about internal behavior, this seemed more appropriate.)
I was playing around with inspecting TOAST tables in order to understand
the
On Thu, May 13, 2021 at 8:38 AM Julien Rouhaud wrote:
>
> On Thu, May 13, 2021 at 08:32:50AM -0700, Maciek Sakrejda wrote:
> >
> > The warning makes it clear that there's something wrong, but not how
> > to fix it
>
> I'm confused, are we talking about the
On Thu, May 13, 2021 at 7:42 AM Bruce Momjian wrote:
>
> On Thu, May 13, 2021 at 12:03:42PM +0800, Julien Rouhaud wrote:
> > On Wed, May 12, 2021 at 11:33:32PM -0400, Bruce Momjian wrote:
> > I don't know what to say. So here is a summary of the complaints that I'm
> > aware of:
> >
> > -
> >
On Wed, May 12, 2021 at 9:58 PM Julien Rouhaud wrote:
> Le jeu. 13 mai 2021 à 12:52, Maciek Sakrejda a écrit :
>>
>> For what it's worth, I don't think the actual changing of an extra
>> setting is that big a burden: it's the figuring out that you need to
>> ch
On Wed, May 12, 2021 at 9:03 PM Julien Rouhaud wrote:
>
> On Wed, May 12, 2021 at 11:33:32PM -0400, Bruce Momjian wrote:
> > How do they know to set shared_preload_libraries then? We change the
> > user API all the time, especially in GUCs, and even rename them, but for
> > some reason we don't
On Mon, May 10, 2021 at 7:43 AM Julien Rouhaud wrote:
> On Mon, May 10, 2021 at 04:36:16PM +0200, Pavel Stehule wrote:
> > I expected just an empty column query_id and workable extension. This
> > doesn't look well.
> >
> > More, it increases the (little bit) complexity of migration to Postgres
olled up into
the parallel leader, and that is propagated as expected to the Gather.
Sorry for the confusion.
On Wed, Jun 24, 2020 at 3:18 AM Maciek Sakrejda wrote:
>So we should be seeing an average, not a sum, right?
And here I missed that the documentation specifies rows and actual
time a
On Tue, Jun 23, 2020 at 7:55 PM Amit Kapila wrote:
> > I don't see any other reason for
> > looping over the NL node itself in this plan. The Gather itself
> > doesn't do any real looping, right?
>
> It is right that Gather doesn't do looping but Parallel Seq Scan node does so.
Sorry, I still
On Tue, Jun 23, 2020 at 2:57 AM Amit Kapila wrote:
> I don't think this is an odd situation because in this case, child
> nodes like "Nested Loop" and "Parallel Seq Scan" has a value of
> 'loops' as 3. So, to get the correct stats at those nodes, you need
> to divide it by 3 whereas, at Gather
Hello,
I had some questions about the behavior of some accounting in parallel
EXPLAIN plans. Take the following plan:
```
Gather (cost=1000.43..750173.74 rows=2 width=235) (actual
time=1665.122..1665.122 rows=0 loops=1)
Workers Planned: 2
Workers Launched: 2
Buffers: shared hit=27683
On Mon, Jan 27, 2020 at 11:01 AM Robert Haas wrote:
> On Fri, Nov 15, 2019 at 8:05 PM Maciek Sakrejda wrote:
> > On Fri, Nov 15, 2019 at 5:49 AM Robert Haas wrote:
> > > Personally, I don't care very much about backward-compatibility, or
> > > about how hard it is
Thanks for the thorough review. I obviously missed some critical
issues. I recognized some of the other maintainability problems, but
did not have a sense of how to best avoid them, being unfamiliar with
the code.
For what it's worth, this version makes sense to me.
Okay. Does not getting as many workers as it asks for include
sometimes getting zero, completely changing the actual output? If so,
I'll submit a new version of the patch removing all tests--I was
hoping to improve coverage, but I guess this is not the way to start.
If not, can we keep the json
Great, thank you. I noticed in the CF patch tester app, the build
fails on Windows [1]. Investigating, I realized I had failed to fully
strip volatile EXPLAIN info (namely buffers) in TEXT mode due to a
bad regexp_replace. I've fixed this in the attached patch (which I
also rebased against current
On Wed, Jan 22, 2020 at 9:37 AM Georgios Kokolatos wrote:
> My bad, I should have been more clear. I meant that it is preferable to use
> the C99 standard which calls for declaring variables in the scope that you
> need them.
Ah, I see. I think I got that from ExplainPrintSettings. I've
Thanks! I'll fix the brace issues. Re: the other items:
> + int num_workers = planstate->worker_instrument->num_workers;
> + int n;
> + worker_strs = (StringInfo *) palloc0(num_workers *
> sizeof(StringInfo));
> + for (n = 0; n < num_workers; n++) {
>
> I think C99 would
TEXT format was tricky due to its inconsistencies, but I think I have
something working reasonably well. I added a simple test for TEXT
format output as well, using a similar approach as the JSON format
test, and liberally regexp_replacing away any volatile output. I
suppose in theory we could do
Sounds good, I'll try that format. Any idea how to test YAML? With the
JSON format, I was able to rely on Postgres' own JSON-manipulating
functions to strip or canonicalize fields that can vary across
executions--I can't really do that with YAML. Or should I run EXPLAIN
with COSTS OFF, TIMING OFF,
Thanks for the review! I looked at and rebased the patch
on current master, ac5bdf6.
I introduced a new test file because this bug is specifically about
EXPLAIN output (as opposed to query execution or planning
functionality), and it didn't seem like a test would fit in any of the
other files. I
Done! Thanks!
I wanted to follow up on this patch since I received no feedback. What
should my next steps be (besides rebasing, though I want to confirm there's
interest before I do that)?
On Thu, Oct 24, 2019 at 6:48 PM Andres Freund wrote:
> Unfortunately I think the fix isn't all that trivial, due to the way we
> output the per-worker information at the end of ExplainNode(), by just
> dumping things into a string. It seems to me that a step in the right
> direction would be for
On Fri, Nov 15, 2019 at 5:49 AM Robert Haas wrote:
> Personally, I don't care very much about backward-compatibility, or
> about how hard it is for tools to parse. I want it to be possible, but
> if it takes a little extra effort, so be it.
I think these are two separate issues. I agree on
On Mon, Oct 28, 2019 at 5:02 PM Andres Freund wrote:
> What I dislike about that is that it basically again is introducing
"again"? Am I missing some history here? I'd love to read up on this
if there are mistakes to learn from.
> something that requires either pattern matching on key names
>But that's pretty crappy, because it means that the 'shape' of the output
>depends on the jit_details option.
Yeah, that would be hard to work with. What about adding it as a sibling group?
"Filter": "(lineitem.l_shipdate <= '1998-09-18 00:00:00'::timestamp
without time zone)",
"Filter JIT": {
Hello,
I originally reported this in pgsql-bugs [0], but there wasn't much
feedback there, so moving the discussion here. When using JSON, YAML, or
XML-format EXPLAIN on a plan that uses a parallelized sort, the Sort nodes
list two different entries for "Workers", one for the sort-related info,
76 matches
Mail list logo
