> appropriate to introduce a new, more specific error code for this
> purpose?
Makes sense to me. I'm not sure if we should use a new error code or
some other existing one, but conflating other things with serializable
failures seems like a bad plan.
--
Robert Haas
EDB: http://www.enterprisedb.com
at pretty obviously should be fixed.
Done.
*quakes in fear of incoming buildfarm results*
--
Robert Haas
EDB: http://www.enterprisedb.com
ise, it might get broken by some future patch without anybody
noticing.
(For clarity, I'm not attempting to insist on anything here, just
sharing a few thoughts that come to mind.)
--
Robert Haas
EDB: http://www.enterprisedb.com
table
functions that would cause a problem here. I still think it's a bad
direction to go.
--
Robert Haas
EDB: http://www.enterprisedb.com
;s built-in and what's provided by a
user or an extension. But I also don't think it works.
--
Robert Haas
EDB: http://www.enterprisedb.com
s agreement that this
change is an improvement, and it appears to me that we don't have
that. I read Fujii Masao's comments to indicate that he doesn't
necessarily agree with the change and wants it reverted, and I read
Michael Paquier's comments the same way. Unless I'm m
On Tue, Jun 3, 2025 at 1:24 PM Tom Lane wrote:
> Robert Haas writes:
> > I think the proposed patch should be committed and back-patched, after
> > fixing it so that it's pgindent-clean and adding a comment. Does
> > anyone have strong objection to that?
>
> No
mental
> exercises.
I think the proposed patch should be committed and back-patched, after
fixing it so that it's pgindent-clean and adding a comment. Does
anyone have strong objection to that?
--
Robert Haas
EDB: http://www.enterprisedb.com
wledge,
we've never formally classified the former type of problem as a
security vulnerability, although maybe there's an argument that it is
one. We've filed CVEs for problems of the latter sort.
--
Robert Haas
EDB: http://www.enterprisedb.com
of the primary vs. the state of the standby, a
call to pg_sync_replication_slots() may either create a slot or fail
to do so. A call at a slightly earlier or later time might have had a
different result. IIUC, this proposal would make different results due
to minor timing variations less probable.
-
t solution, of course, say if we think
it's sufficiently easier to implement. But considering that Tom and
Noah have both prototyped function trust systems, it seems highly
premature to conclude that there's no way forward along those lines.
--
Robert Haas
EDB: http://www.enterprisedb.com
(in one of several proposed
variations) that can close all of those holes.
--
Robert Haas
EDB: http://www.enterprisedb.com
rs>409GB. While I'm not opposed to a more efficient
solution, it seems reasonable to me to suppose that if you've got
shared_buffers>409GB, you're able to afford having this function use
>1GB.
--
Robert Haas
EDB: http://www.enterprisedb.com
ewriter to the planner to fix various problems discussed on
the thread. But we should decide whether the resulting situation is
acceptable to ship.
To be clear, I like this feature in concept and I don't want it to
crash and burn. But I even more don't want to ship something and then
ha
dby's
> nextXid, causing sync failure.
I still don't understand how this problem arises in the first place.
It seems like you're describing a situation where we need to prevent
the standby from getting ahead of the primary, but that should be
impossible by definition.
--
Robert Haas
EDB: http://www.enterprisedb.com
7;t be interpreted as anything sensible, we should reject
it rather than making up a fake value. However, I agree that it's best
not to do such tightening in the back-branches.
--
Robert Haas
EDB: http://www.enterprisedb.com
rmation on the Internet to explain why this sometimes happens and
sometimes doesn't, and various things I attempted as fixes didn't work
out. There could be something wrong specifically with this machine,
but I also wouldn't be too shocked if this is just randomly broken on
macO
But I also can't shake the feeling that I shouldn't *need* to
understand that stuff to use the feature. Isn't that the whole point?
--
Robert Haas
EDB: http://www.enterprisedb.com
that was what Tom said last September and I just assumed he
was right about the policy. If not, well then that's different.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Thu, May 22, 2025 at 3:36 PM Nathan Bossart wrote:
> +1, I think defaulting to restoring everything in the dump file is much
> less surprising than the alternative.
+1.
--
Robert Haas
EDB: http://www.enterprisedb.com
we would have likely done that a
long time ago.
--
Robert Haas
EDB: http://www.enterprisedb.com
ug" parameter
> and another one - "debuG".
I guess my point is that this works just like other cases where SQL
identifiers are accessible in C code: they're normally lower-case but
they don't have to be if the user quoted them. I don't think it's
w
uery plan after the
check_stack_depth() call and before the rest of the logic in the
function. I've attached a sample patch to show what I have in mind.
--
Robert Haas
EDB: http://www.enterprisedb.com
example-execprocnodefirst-patch.diff
Description: Binary data
select 1;
ERROR: unrecognized EXPLAIN option "VERBOSE"
LINE 1: explain ("VERBOSE") select 1;
^
--
Robert Haas
EDB: http://www.enterprisedb.com
default" item in the open items list under
"Decisions to Recheck Mid-Beta", but that's arguably now. It also sort
of looks like we might have a consensus anyway: Jeff said "I lean
towards making it opt-in for pg_dump and opt-out for pg_upgrade" and I
agree with tha
xpand_virtual_generated_columns.
If Tom doesn't respond right away, I suggest that we need to add an
open item for http://postgr.es/m/602561.1744314...@sss.pgh.pa.us
--
Robert Haas
EDB: http://www.enterprisedb.com
On Wed, May 21, 2025 at 2:31 PM Tom Lane wrote:
> Having said that, what's wrong with inventing some improved function
> names and never removing the old ones?
I don't particularly like the clutter, but if the consensus is that
the clutter doesn't matter, fair enough.
--
ge you to
look through the patch for other, similar places that could benefit
from a fuller explanation. And I hope somebody else shows up to
express interest in this so that your work is not wasted...
--
Robert Haas
EDB: http://www.enterprisedb.com
aged to overlook for all this time.
--
Robert Haas
EDB: http://www.enterprisedb.com
pen before and after the things that we
want the locking to happen after. And maybe that's true. Or maybe it's
close enough to true that it's still better than the status quo where
we're not taking locks at all. But on the other hand, since I can't
think of any compelling reason why it HAS to be true, maybe it isn't.
--
Robert Haas
EDB: http://www.enterprisedb.com
anks for the report.
Since this was Andres's commit, I think it would have been a good idea
to give at least 24 hours, or better yet a couple of days, for him to
comment before committing something.
--
Robert Haas
EDB: http://www.enterprisedb.com
nvent a GUC jsonb_tz_warning = { on | off } that advises you to
use the new functions instead, whenever you use the old ones.
3. After N years, flip the default value from off to on.
4. After M additional years, remove the old functions and the GUC.
5. Still get complaints.
--
Robert Haas
EDB: http://www.enterprisedb.com
check with this design when NoLock is
> > passed, so I think this is a reasonable alternative to that design.
>
> I'd have to see the patch to see whether I liked the end result. But
> I'm guessing that involves a lot of non-mechanical changes in the call
> sites, and
t;
> Given that XMLCast converts values between SQL and XML and vice versa,
> my rationale was that if the target type is not a text type (such as
> TEXTOID, VARCHAROID, or NAMEOID), then the cast operand must be of type
> xml, which makes this default: safe.
> [...]
> But I can see it looks unsafe. Do you have something like this in mind?
> [...]
> default:
> elog(ERROR, "unsupported target data type for XMLCast");
> }
Yes, exactly.
--
Robert Haas
EDB: http://www.enterprisedb.com
in recordDependencyOn()
but assert that the caller has previously acquired one. However, we
could still do the Assert() check with this design when NoLock is
passed, so I think this is a reasonable alternative to that design.
--
Robert Haas
EDB: http://www.enterprisedb.com
The legwork of sorting some of
that kind of stuff out should really happen before making a feature
proposal.
> I'm intrigued, and happy to stand by with an extinguisher. The road to
> great ideas is paved with bad ideas.
Thanks. That proposal is a task for another day, but I appreciate the sentiment.
--
Robert Haas
EDB: http://www.enterprisedb.com
ould make it much less wide in normal cases without making it much
longer. This has been percolating in my brain for a few years now and
I have the vague intention of proposing it at some point, but not
until I'm good and ready to be flamed to a well-done crisp, because
I'm quite sure there will be more than one opinion on the merits.)
--
Robert Haas
EDB: http://www.enterprisedb.com
don't have a better idea right
now, though.
If there are any notes that were taken during that unconference
session, please point me in the right direction; I was in another
session at that time but would read any available notes with interest.
--
Robert Haas
EDB: http://www.enterprisedb.com
in places
like pg_dumpall output, and maybe we should do that here ... kinda
just in case? But I'm not altogether sure that's a sufficient
justification, and at any rate I think we need to be clear on whether
that *is* the justification or whether there's something more concrete
that we'r
ong time (leading to race
conditions) or acquiring two locks on the same object with different
lock modes where we should really only acquire one. I'm all in favor
of solving this problem using heavyweight locks, but I think that
implicitly acquiring them as a side effect of recording de
omplex. Why not start simple and if someone
wants to do the work to add something more complicated, that is fine?
--
Robert Haas
EDB: http://www.enterprisedb.com
. I
don't know if that idea will work out but it certainly seems like a
good thing to try.
--
Robert Haas
EDB: http://www.enterprisedb.com
x27;s a data type for which DatumGetTextP will
produce a non-garbage return value? Maybe there's an answer to that
question, but there's no comment spelling it out; or maybe it's
actually just broken.
--
Robert Haas
EDB: http://www.enterprisedb.com
very
place that uses incremental sort decide whether to use an incremental
sort or a regular sort -- we should try to get to a place where it's
safe to use an incremental sort when it's possible without worrying
that it might actually be slower.
Or maybe that's not possible for some re
y that we can detect violations of this rule
automatically? I recall that we were recently discussing with Richard
Guo a proposed patch that would have had a similar problem, so it's
evidently not that hard for a committer to either fail to understand
what the rule is or fail to realize that they are violating it.
--
Robert Haas
EDB: http://www.enterprisedb.com
rations, but that way doesn't seem like
# it's going to scale well to multiple sources of mutability.
But I'm not sure I understand why it matters that there are multiple
sources of mutability here. Maybe I'm missing a piece of the puzzle
here.
--
Robert Haas
EDB: http://www.enterprisedb.com
dard? And, assuming yes, does the
SQL standard specify that permission checks should work as you
describe here, or is that something we decided?
--
Robert Haas
EDB: http://www.enterprisedb.com
oin removal code, and I'm not
sure about that either.
I'm just studying this for the first time so apologies if this review
is not quite up to standard.
Thanks,
--
Robert Haas
EDB: http://www.enterprisedb.com
On Mon, May 5, 2025 at 8:35 AM Robert Haas wrote:
> So it seems like the only real fix is to do as Tom proposes:
>
> # But don't we need to add
> # createrole_self_grant to the set of GUCs that pg_dump[all]
> # resets in the emitted SQL?
Tom, are you hoping that I'm goi
et it do that.
But we don't need to brand that as metadata; and we don't need a
method for other parts of the system to ask how much metadata exists.
At least, I don't think we do.
--
Robert Haas
EDB: http://www.enterprisedb.com
lobal and giving it a less generic name (e.g.
LogMemoryContextInProgress). However, perhaps others will disagree.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Thu, May 1, 2025 at 2:22 PM Robert Haas wrote:
> > The other approach would be to do what we do for the role options and just
> > specify everything explicitly in the dump. The GUC is only a default
> > specifier so let's not leave room for defaults in the dump file.
ould, if it results in GRANTs that
> weren't there before).
Hmm. That might have been a better design. :-(
--
Robert Haas
EDB: http://www.enterprisedb.com
he SQL standard
and we didn't want to do that. And I don't know how we would have
gotten out from under that problem without a behavior-changing GUC,
and I didn't know how to get out from under this one without it,
either.
That's not to say that I feel great about it, though, because I don't.
--
Robert Haas
EDB: http://www.enterprisedb.com
roach would be to do what we do for the role options and just
> specify everything explicitly in the dump. The GUC is only a default
> specifier so let's not leave room for defaults in the dump file.
+1 for considering that option, although I am not sure which way is better.
--
Robert Haas
EDB: http://www.enterprisedb.com
n't have this
instruction, so restrict the patch to ARM64.
Geoffrey Blake
Discussion:
https://postgr.es/m/78338f29-9d7f-4dc8-bd71-e9674ce71...@amazon.com
I think you should make some kind of argument about why the previous
conclusion was wrong, or why something's changed
rther errors
at that point is extremely limited, and this code is definitely
complex enough that it could error out.
--
Robert Haas
EDB: http://www.enterprisedb.com
imply return immediately if the flag is already set
> like this:
I think that something like this could work, but you would need more
than this. Otherwise, if the function errors out, the flag would
remain permanently set.
--
Robert Haas
EDB: http://www.enterprisedb.com
a valid resource owner, not the
job of the called code to ignore the resource owner when it's
unusable. I suspect that there are many other parts of the system that
rely on the ResourceOwner machinery which likewise assume that the
ResourceOwner that they are passed is valid.
--
Robert H
e to push
> it.
Thanks for the pointer. I will reply to that thread.
--
Robert Haas
EDB: http://www.enterprisedb.com
bove:
select * from pg_get_process_memory_contexts(19321, false, 1);
Then you get:
2025-04-30 15:14:33.878 EDT [19321] ERROR: ResourceOwnerEnlarge
called after release started
2025-04-30 15:14:33.878 EDT [19321] FATAL: terminating connection
because protocol synchronization was lost
I kind of doubt whether that's the only problem here, but it's *a* problem.
--
Robert Haas
EDB: http://www.enterprisedb.com
UPTS() around
ProcessLogMemoryContextInterrupt(), but I think that's probably not
quite good enough because it would make the backend impervious to
pg_terminate_backend() while it's dumping memory contexts, and that
could be a long time if the write blocks.
--
Robert Haas
EDB: http://www.enterprisedb.com
first chunk of the TOAST
value (~2k) seems like it'd probably work well for most cases.
So, again, if you want us to take seriously the idea of dedicating 3
bytes per Datum to something, you need to give us a really good reason
for so doing. The fact a 24-bit metadata length can describe a
metadata header of up to 2^24 bits isn't a reason, good or bad. It's
just math.
--
Robert Haas
EDB: http://www.enterprisedb.com
w the header length. Even if
generic code that works with all types of compression needs to be able
to obtain the header length on a per-compression-type basis, there can
be some kind of callback or table for that, rather than storing it in
every single datum.
--
Robert Haas
EDB: http://www.enterprisedb.com
#x27;t make that change without carefully
considering the implications for the callers.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Wed, Apr 23, 2025 at 11:59 AM Robert Haas wrote:
> heap_toast_insert_or_update care about HeapTupleHasExternal(), which
> seems like it might be a key point.
Care about HeapTupleHasVarWidth, rather.
--
Robert Haas
EDB: http://www.enterprisedb.com
I at least feel like DSA is a
pretty high-level system that I wouldn't want to count on being able
to access from a fairly-arbitrary point in the code, which is why I'm
quite astonished to hear Andres basically saying "don't worry, it's
all fine!". B
at Datum inside a container object (row, range,
array, whatever) detoasting is forced. If there's a clean and
inexpensive way to implement that, then you could avoid having
heap_toast_insert_or_update care about HeapTupleHasExternal(), which
seems like it might be a key point.
--
Robert Haas
EDB: http://www.enterprisedb.com
not have that
many compression methods ever, but you could have a large number of
dictionaries eventually.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Fri, Jan 3, 2025 at 8:44 AM Robert Haas wrote:
> > Actually, I noticed that we are failing to honor that in the places
> > where we inject "*SELECT*" and "*SELECT* %d" names, because that
> > code puts those names into RTE->alias not only RTE->eref
to store
the resulting data and then returned to the interrupted DSA operation,
would you expect the code to cope with that? I do not believe we have
anywhere enough guarantees about reentrancy for that to be safe.
--
Robert Haas
EDB: http://www.enterprisedb.com
inside the ReadStream? e.g. a
ParallelReadStream is really one ReadStream per participant, each with
a separate lock. Mostly you only touch your own, but if necessary you
can poke your fingers into other people's ReadStreams.
--
Robert Haas
EDB: http://www.enterprisedb.com
s to a certain set
of NUMA nodes; perhaps then they wouldn't want to use memory from
other nodes.
--
Robert Haas
EDB: http://www.enterprisedb.com
.
I also don't mind being wrong, of course. But I think it's better to
bet on making this like other things and then change strategy if that
doesn't work out, rather than starting out by making this different
from other things.
--
Robert Haas
EDB: http://www.enterprisedb.com
how CFI could be reachable from there
today, but even if it definitely isn't, I don't see why someone would
hesitate to add one in the future.
--
Robert Haas
EDB: http://www.enterprisedb.com
> "Estimates: capacity=N distinct keys=N lookups=N hit ratio=N.N%"
Is lookups=N here the estimated number of lookups i.e. what we think
nloops will end up being?
--
Robert Haas
EDB: http://www.enterprisedb.com
eems safer to bet on the pfree being a good idea than on the reverse,
because record_in() can be called lots of times in a single
transaction.
--
Robert Haas
EDB: http://www.enterprisedb.com
be the OID
> + * If processing a user-defined tablespace, the tsoid should be the OID
Yeah, I agree with both of these changes. Feel free to commit, unless
you want me to do it.
--
Robert Haas
EDB: http://www.enterprisedb.com
ead, does that
solve this whole problem, or is there more to it?
--
Robert Haas
EDB: http://www.enterprisedb.com
than a more "cooked" number like the estimated hit
ratio that was proposed in v1?
--
Robert Haas
EDB: http://www.enterprisedb.com
arrive at the rude discovery that 0dca5d68d is about 50% slower
> than 0dca5d68d^, because the old implementation builds a plan for fx()
> only once and then re-uses it throughout the query.
I agree that we should do something about this. I haven't reviewed
your patches but the approach
e, but I'm uncertain
myself what we ought to be doing here. I just wanted to raise the
issue.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Mon, Apr 7, 2025 at 11:59 AM Tom Lane wrote:
> Robert Haas writes:
> > The only thing that makes me a little bit sad is that we don't seem to
> > have added this to pg_stat_activity.
>
> Hmm, that seems like it'd be a completely separate discussion.
Yes,
that it was actually being done at parse time, and this complaint is
that it is scribbling on a parse-time data structure.
--
Robert Haas
EDB: http://www.enterprisedb.com
places that do GetBufferDescriptor(buffer - 1) and
similar. Alternating between 0-based indexing and 1-based indexing
like this seems rather error-prone somehow. :-(
--
Robert Haas
EDB: http://www.enterprisedb.com
e? I find
> the table_open/table_close pattern is quite common in the current
> code. In addition to get_relation_info(), I've also seen it in
> get_relation_constraints(), get_relation_data_width(), and others.
You're also right about this.
--
Robert Haas
EDB: http://www.enterprisedb.com
;s best to run pgindent before submitting.
I'd probably write the increments as ++ rather than += 1 but I'm not
sure if everyone would agree.
--
Robert Haas
EDB: http://www.enterprisedb.com
work ... or we're going to be committing to an API that we don't
really want to support.
To be clear, I'm not saying you're wrong, just that it's going to be
hard to change anything without making somebody unhappy.
--
Robert Haas
EDB: http://www.enterprisedb.com
hat
it's safe to assume that the local memory-context tree is in a
consistent state when CHECK_FOR_INTERRUPTS() is called. If there is
some existing discussion of this that I should read, please point me
in the right direction; I didn't see anything in a quick look through
the commit.
T
I don't think we should go back to the bad old days where we litigated
every case of marking something PGDLLIMPORT or not unless we have an
extremely good reason for so doing.
--
Robert Haas
EDB: http://www.enterprisedb.com
luded further down in the
email.
I don't know enough to opine on the questions about full vs. default,
or sequential scans.
--
Robert Haas
EDB: http://www.enterprisedb.com
le who have never even heard of it. Specify some time and data at
> UTC and everyone will understand.
+1.
--
Robert Haas
EDB: http://www.enterprisedb.com
the same time they
collect the pg_stat_activity data and so on; if you just add a column
to pg_stat_activity then the information just shows up as part of
routine data gathering.
--
Robert Haas
EDB: http://www.enterprisedb.com
ny
'data beyond EOF' messages are exactly that -- and it's not like the
user is going to guess that 'data beyond EOF' might mean that such a
thing occurred.
--
Robert Haas
EDB: http://www.enterprisedb.com
don't get matched with someone. On
the other hand, please do apply if you're interested: it's entirely
possible that some new committers will agree to participate, in which
case we have more spots, and in any case, somebody who might not have
agreed to mentor anyone else might ag
just put all their checks in one
> place rather than having to register a bunch of handlers.
Do you want to propose a patch?
--
Robert Haas
EDB: http://www.enterprisedb.com
i.e. you can
> never set conislocal=false in such a case).
Hmm. I think this is different from attislocal/attinhcount.
--
Robert Haas
EDB: http://www.enterprisedb.com
sed my pg_overexplain patch and attach that here.
--
Robert Haas
EDB: http://www.enterprisedb.com
v8-0001-pg_overexplain-Additional-EXPLAIN-options-for-deb.patch
Description: Binary data
On Tue, Mar 18, 2025 at 12:36 AM Masahiko Sawada wrote:
> > Cool. The commit message refers to 003_char_signedness, but the test
> > name is 005, not 003.
>
> Thank you for reviewing the patch. I've pushed the patch after fixing it.
Thanks for taking care of it (and so
on't want to rush into something
that we might later regret. I'm going to spend a bit more time
studying your patch next.
--
Robert Haas
EDB: http://www.enterprisedb.com
1 - 100 of 2038 matches
Mail list logo
