Re: Add llvm version into the version string
On 9/24/24 09:52, Andres Freund wrote: Hi, On 2024-09-24 13:53:49 +0100, Alastair Turner wrote: Since the build and runtime versions may differ for some things like llvm, libxml2 and the interpreters behind some of the PLs, it may be valuable to expand the view and show two values - a build time (or configure time) value and a runtime value for these. +1 Somewhat orthogonal: I've wondered before whether it'd be useful to have a view showing the file path and perhaps the soversion of libraries dynamically loaded into postgres. That's currently hard to figure out over a connection (which is all a lot of our users have access to). +1 -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: Clock-skew management in logical replication
On 9/21/24 01:31, shihao zhong wrote: Nisha Moond writes: Thoughts? Looking forward to hearing others' opinions! Had a productive conversation with Amit Kaplia today about time skew in distributed systems, and wanted to share some thoughts. Essentially, we're grappling with the classic distributed snapshot problem. In a multi-active environment, where multiple nodes can independently process transactions, it becomes crucial to determine the visibility of these transactions across the system. Time skew, where different machines have different timestamps make it a hard problem. How can we ensure consistent transaction ordering and visibility when time itself is unreliable? As you mentioned, there are several ways to tackle the time skew problem in distributed systems. These approaches generally fall into three main categories: 1. Centralized Timestamps (Timestamp Oracle) 2. Atomic Clocks (True Time) 3. Hybrid Logical Clocks 4 Local Clocks I recommend .. implement a pluggable time access method. This allows users to integrate with different time services as needed. Huge +1 In the mid-term, implementing the HLC approach would provide highly consistent snapshot reads. This offers a significant advantage for many use cases. agreed Long-term, we should consider integrating with a distributed time service like AWS Time Sync Service. This ensures high accuracy and scalability for demanding applications. I think the pluggable access method should make this possible, no? -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: CI, macports, darwin version problems
On 9/12/24 02:05, Thomas Munro wrote: On Tue, Sep 10, 2024 at 3:04 PM Thomas Munro wrote: On Mon, Sep 9, 2024 at 1:37 PM Joe Conway wrote: Seems the mounted drive got unmounted somehow ¯\_(ツ)_/¯ Please check it out and let me know if it is working properly now. Looks good, thanks! ... but it's broken again. The external volume somehow got unmounted again :-( I have rebooted it and restarted the process now. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: CI, macports, darwin version problems
On 9/8/24 16:55, Thomas Munro wrote:
On Sat, Aug 3, 2024 at 12:07 AM Joe Conway wrote:
I tried making this run like a service using launchctl, but that was
giving the permissions errors. I finally gave up trying to figure it out
and just accepted that I need to manually start the script whenever I
reboot the mac.
It seems to be unhappy recently:
Persistent worker failed to start the task: tart isolation failed:
failed to create VM cloned from
"ghcr.io/cirruslabs/macos-runner:sonoma": tart command returned
non-zero exit code: "tart/VMStorageOCI.swift:5: Fatal error: 'try!'
expression unexpectedly raised an error: Error
Domain=NSCocoaErrorDomain Code=512 \"The file “ci-run” couldn’t be
saved in the folder “Users”.\" UserInfo={NSFilePath=/Users/ci-run,
NSUnderlyingError=0x619f0720 {Error Domain=NSPOSIXErrorDomain
Code=20 \"Not a directory\"}}"
Seems the mounted drive got unmounted somehow ¯\_(ツ)_/¯
Please check it out and let me know if it is working properly now.
--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Re: allowing extensions to control planner behavior
On 8/27/24 11:45, Robert Haas wrote: On Mon, Aug 26, 2024 at 3:28 PM Robert Haas wrote: Well, I agree that this doesn't address everything you might want to do, ... I will very happily propose more things to address the other problems that I know about ... In that vein, here's a new patch set where I've added a second patch that allows extensions to control choice of index. It's 3 lines of new code, plus 7 lines of comments and whitespace. Feeling inspired, I also included a contrib module, initial_vowels_are_evil, to demonstrate how this can be used by an extension that wants to disable certain indexes but not others. This is obviously quite silly and we might (or might not) want a more serious example in contrib, but it demonstrates how easy this can be with just a tiny bit of core infrastructure: robert.haas=# load 'initial_vowels_are_evil'; LOAD robert.haas=# explain select count(*) from pgbench_accounts; QUERY PLAN - Aggregate (cost=2854.29..2854.30 rows=1 width=8) -> Index Only Scan using pgbench_accounts_pkey on pgbench_accounts (cost=0.29..2604.29 rows=10 width=0) (2 rows) robert.haas=# alter index pgbench_accounts_pkey rename to evil_pgbench_accounts_pkey; ALTER INDEX robert.haas=# explain select count(*) from pgbench_accounts; QUERY PLAN -- Aggregate (cost=2890.00..2890.01 rows=1 width=8) -> Seq Scan on pgbench_accounts (cost=0.00..2640.00 rows=10 width=0) (2 rows) robert.haas=# Nice! On the one hand, excluding indexes by initial vowels is definitely silly. On the other, I can see how it might be useful for an extension to exclude indexes based on a regex match of the index name or something similar, at least for testing. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: how to log into commitfest.postgresql.org and begin review patch
On 8/26/24 03:58, Daniel Gustafsson wrote: On 26 Aug 2024, at 05:21, 王春桂 wrote: hello,all, I am a newcomer who recently joined the PostgreSQL community. I logged into the community using my GitHub account and wanted to start familiarizing myself with community work by reviewing patches. However, I am currently facing an issue. When I log into commitfest.postgresql.org, I encounter the following message. "The site you are trying to log in to (commitfest.postgresql.org) requires a cool-off period between account creation and logging in. You have not passed the cool off period yet." anyone advise me on how to resolve this, or how long this cool off period lasts? please! There is a cool-off period on new accounts since we were seeing SPAM from freshly created accounts. Posting here means that admins can expedite the period for activating your account. Cooling off period expedited. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: Cutting support for OpenSSL 1.0.1 and 1.0.2 in 17~?
On 8/21/24 09:01, Peter Eisentraut wrote: On 07.08.24 15:49, Daniel Gustafsson wrote: On 5 Aug 2024, at 15:36, Joe Conway wrote: It would not shock me to see complaints from others after we rip out support for 1.0.2, but maybe not ¯\_(ツ)_/¯ I think it's highly likely that we will see complaints for any support we deprecate. OpenSSL 1.0.2 will however still be supported for another 5 years with v17 (which is ~9years past its EOL date) so I don't feel too bad about it. Is anything -- other than this inquiry -- preventing this patch set from getting committed? The overwhelming consensus seemed to be "just do it", so FWIW consider my reservations withdrawn ;-) -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: gitmaster server problem?
On 8/19/24 08:06, Jelte Fennema-Nio wrote: On Mon, 19 Aug 2024 at 14:05, Jelte Fennema-Nio wrote: On Mon, 19 Aug 2024 at 14:04, Robert Haas wrote: > I'm still unable to access any https://git.postgresql.org/ URL. Yeah it's broken for me again too. In case the actual error is helpful (although I doubt it): Error 503 Backend fetch failed Backend fetch failed Guru Meditation: XID: 1030914118 I am not seeing any errors here. nagios monitoring is showing an ipv6 error though. Not the same issue as yesterday. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: gitmaster server problem?
On 8/19/24 05:04, Jelte Fennema-Nio wrote: On Sat, 17 Aug 2024 at 22:05, Joe Conway wrote: Just to close this out -- problem found and fixed about an hour ago. Sorry for the interruption. Whatever the problem was it seems to have returned What specifically? I am not seeing anything at the moment. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: gitmaster server problem?
On 8/17/24 13:59, Matthias van de Meent wrote: On Sat, 17 Aug 2024 at 18:41, Bruce Momjian wrote: About 20 minutes ago I starting getting gitmaster pull errors: $ git pull ssh://g...@gitmaster.postgresql.org/postgresql.git ssh: connect to host gitmaster.postgresql.org port 22: Connection timed out fatal: Could not read from remote repository. Please make sure you have the correct access rights and the repository exists. [...] Can anyone confirm or report on the cause? Additional infra that doesn't seem to work right now: mailing list archives [0] seem to fail at a 503 produced by Varnish Cache server: Error 503 Backend fetch failed. Maybe more of infra is down, or otherwise under maintenance? Kind regards, Matthias van de Meent [0] https://www.postgresql.org/list/pgsql-hackers/2024-08/ Just to close this out -- problem found and fixed about an hour ago. Sorry for the interruption. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: Remaining dependency on setlocale()
On 8/7/24 13:16, Jeff Davis wrote: On Wed, 2024-08-07 at 19:07 +1200, Thomas Munro wrote: How far can we get by using more _l() functions? There are a ton of calls to, for example, isspace(), used mostly for parsing. I wouldn't expect a lot of differences in behavior from locale to locale, like might be the case with iswspace(), but behavior can be different at least in theory. So I guess we're stuck with setlocale()/uselocale() for a while, unless we're able to move most of those call sites over to an ascii-only variant. FWIW I see all of these in glibc: isalnum_l, isalpha_l, isascii_l, isblank_l, iscntrl_l, isdigit_l, isgraph_l, islower_l, isprint_l, ispunct_l, isspace_l, isupper_l, isxdigit_l -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: Remaining dependency on setlocale()
On 8/7/24 03:07, Thomas Munro wrote:
How far can we get by using more _l() functions? For example, [1]
shows a use of strftime() that I think can be converted to
strftime_l() so that it doesn't depend on setlocale(). Since POSIX
doesn't specify every obvious _l function, we might need to provide
any missing wrappers that save/restore thread-locally with
uselocale(). Windows doesn't have uselocale(), but it generally
doesn't need such wrappers because it does have most of the obvious
_l() functions.
Most of the strtoX functions have an _l variant, but one to watch is
atoi, which is defined with a hardcoded call to strtol, at least with glibc:
8<--
/* Convert a string to an int. */
int
atoi (const char *nptr)
{
return (int) strtol (nptr, (char **) NULL, 10);
}
8<--
I guess in many/most places we use atoi we don't care, but maybe it
matters for some?
--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Re: Cutting support for OpenSSL 1.0.1 and 1.0.2 in 17~?
On 8/5/24 09:14, Tom Lane wrote: Joe Conway writes: I know I am way late to this thread, and I have only tried a cursory skim of it given the length, but have we made any kind of announcement (packagers at least?) that we intend to not support Postgres 18 with ssl on RHEL 7.9 and derivatives? Yes, RHEL 7 just passed EOL, but there is commercial extended support available until July 2028[1] which means many people will continue to use it. PG v16 will be in-support until November 2028, so it's not like we are leaving RHEL 7 completely in the lurch. I doubt that the sort of people who are still running an EOL OS are looking to put a bleeding-edge database on it, so this seems sufficient to me. ok As for notifying packagers --- Red Hat themselves will certainly not be trying to put new major versions of anything on RHEL 7, and Devrim has stopped packaging newer PG for RHEL 7 altogether, so who among them is going to care? Perhaps no one on packagers. It would not shock me to see complaints from others after we rip out support for 1.0.2, but maybe not ¯\_(ツ)_/¯ -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: Cutting support for OpenSSL 1.0.1 and 1.0.2 in 17~?
On 7/15/24 17:42, Daniel Gustafsson wrote: On 14 Jul 2024, at 14:03, Peter Eisentraut wrote: On 12.07.24 21:42, Daniel Gustafsson wrote: On 11 Jul 2024, at 23:22, Peter Eisentraut wrote: The 0001 patch removes the functions pgtls_init_library() and pgtls_init() but keeps the declarations in libpq-int.h. This should be fixed. Ah, nice catch. Done in the attached rebase. This patch version looks good to me. Thanks for review, I will go ahead with this once back from vacation at the tail end of July when I can properly handle the BF. Small comments on the commit message of 0002: Typo "checkig". Also, maybe the commit message title can be qualified a little, since we're not really doing "Remove pg_strong_random initialization." but something like "Remove unnecessary ..."? Good points, will address before pushing. I know I am way late to this thread, and I have only tried a cursory skim of it given the length, but have we made any kind of announcement (packagers at least?) that we intend to not support Postgres 18 with ssl on RHEL 7.9 and derivatives? Yes, RHEL 7 just passed EOL, but there is commercial extended support available until July 2028[1] which means many people will continue to use it. Joe [1] https://www.redhat.com/en/blog/announcing-4-years-extended-life-cycle-support-els-red-hat-enterprise-linux-7 -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: can we mark upper/lower/textlike functions leakproof?
On 8/2/24 11:07, Tom Lane wrote: Joe Conway writes: Hmmm, and then have "leakproof_mode" = strict/lax/off where 'strict' is current behavior, 'lax' allows the 'maybe's to get pushed down, and 'off' ignores the leakproof attribute entirely and pushes down anything that merits being pushed? So in other words, we might as well just remove RLS. Perhaps deciding where to draw the line for 'maybe' is too hard, but I don't understand how you can say that in a general sense. 'strict' mode would provide the same guarantees as today. And even 'off' has utility for cases where (1) no logins are allowed except for the app (which is quite common in production environments) and no database errors are propagated to the end client (again pretty standard best practice); or (2) non-production environments, e.g. for testing or debugging; or (3) use cases that utilize RLS as a notationally convenient filtering mechanism and are not bothered by some leakage in the worst case. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: can we mark upper/lower/textlike functions leakproof?
On 8/2/24 09:48, Jacob Champion wrote: On Thu, Aug 1, 2024 at 6:03 PM Robert Haas wrote: On Thu, Aug 1, 2024 at 4:45 PM Jacob Champion wrote: > Would it provide enough value for effort to explicitly mark leaky > procedures as such? Maybe that could shrink the grey area enough to be > protective? You mean like proleakproof = true/false/maybe? Yeah, exactly. Hmmm, and then have "leakproof_mode" = strict/lax/off where 'strict' is current behavior, 'lax' allows the 'maybe's to get pushed down, and 'off' ignores the leakproof attribute entirely and pushes down anything that merits being pushed? -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: can we mark upper/lower/textlike functions leakproof?
On 8/1/24 07:57, Robert Haas wrote: On Wed, Jul 31, 2024 at 4:42 PM Joe Conway wrote: You are assuming that everyone allows direct logins with the ability to create procedures. Plenty don't. Well, if you can send queries, then you can do the same thing, driven by client-side logic. Sure. Of course you should be monitoring your production servers for anomalous workloads, no? "Gee, why is Joe running the same query millions of times that keeps throwing errors? Maybe we should go see what Joe is up to" If you can't directly send queries in any form, then I guess things are different. Right, and there are plenty of those. I have even worked with at least one rather large one on behalf of your employer some years ago. But I don't really understand what kind of system you have in mind. Well I did say I was being hand wavy ;-) It has been a long time since I thought deeply about this. I will try to come back with something more concrete if no one beats me to it. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: can we mark upper/lower/textlike functions leakproof?
On 8/1/24 07:17, Laurenz Albe wrote: On Wed, 2024-07-31 at 14:43 -0400, Joe Conway wrote: I still maintain that there is a whole host of users that would accept the risk of side channel attacks via existence of an error or not, if they could only be sure nothing sensitive leaks directly into the logs or to the clients. We should give them that choice. I think that you are right. thanks But what do you tell the users who would not accept that risk? Document that the option should not be used if that is the case ¯\_(ツ)_/¯ -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: can we mark upper/lower/textlike functions leakproof?
On 7/31/24 16:10, Robert Haas wrote: On Wed, Jul 31, 2024 at 2:43 PM Joe Conway wrote: I still maintain that there is a whole host of users that would accept the risk of side channel attacks via existence of an error or not, if they could only be sure nothing sensitive leaks directly into the logs or to the clients. We should give them that choice. I'm not sure what design you have in mind. A lot of possible designs seem to end up like this: 1. You can't directly select the invisible value. 2. But you can write a plpgsql procedure that tries a bunch of things in a loop and catches errors and uses which things error and which things don't to figure out and return the invisible value. And I would argue that's not really that useful. Especially if that plpgsql procedure can extract the hidden values in like 1ms/row. You are assuming that everyone allows direct logins with the ability to create procedures. Plenty don't. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: can we mark upper/lower/textlike functions leakproof?
On 7/31/24 14:03, Tom Lane wrote: Robert Haas writes: If there are some inputs that cause upper() and lower() to fail and others that do not, the functions aren't leakproof, because an attacker can extract information about values that they can't see by feeding those values into these functions and seeing whether they get a failure or not. [ rather exhaustive analysis redacted ] So in summary, I think upper() is ... pretty close to leakproof. But if ICU sometimes fails on certain strings, then it isn't. And if the multi-byte libc path can be made to fail reliably either with really long strings or with certain choices of the LC_CTYPE locale, then it isn't. The problem here is that marking these functions leakproof is a promise about a *whole bunch* of code, much of it not under our control; worse, there's no reason to think all that code is stable. A large fraction of it didn't even exist a few versions ago. Even if we could convince ourselves that the possible issues Robert mentions aren't real at the moment, I think marking these leakproof is mighty risky. It's unlikely we'd remember to revisit the marking the next time someone drops a bunch of new code in here. I still maintain that there is a whole host of users that would accept the risk of side channel attacks via existence of an error or not, if they could only be sure nothing sensitive leaks directly into the logs or to the clients. We should give them that choice. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: can we mark upper/lower/textlike functions leakproof?
On 7/31/24 05:47, Andrew Dunstan wrote: On 2024-07-30 Tu 6:51 PM, David Rowley wrote: On Wed, 31 Jul 2024 at 09:35, Andrew Dunstan wrote: Fast forward to now. The customer has found no observable ill effects of marking these functions leakproof. The would like to know if there is any reason why we can't mark them leakproof, so that they don't have to do this in every database of every cluster they use. Thoughts? According to [1], it's just not been done yet due to concerns about risk to reward ratios. Nobody mentioned any reason why it couldn't be, but there were some fears that future code changes could yield new failure paths. David [1]https://postgr.es/m/02BDFCCF-BDBB-4658-9717-4D95F9A91561%40thebuild.com Hmm, somehow I missed that thread in searching, and clearly I'd forgotten it. Still, I'm not terribly convinced by arguments along the lines you're suggesting. "Sufficient unto the day is the evil thereof." Maybe we need a test to make sure we don't make changes along those lines, although I have no idea what such a test would look like. I think I have expressed this opinion before (which was shot down), and I will grant that it is hand-wavy, but I will give it another try. In my opinion, for this use case and others, it should be possible to redact the values substituted into log messages based on some criteria. One of those criteria could be "I am in a leakproof call right now". In fact in a similar fashion, an extension ought to be able to mutate the log message based on the entire string, e.g. when "ALTER ROLE...PASSWORD..." is spotted I would like to be able to redact everything after "PASSWORD". Yes it might render the error message unhelpful, but I know of users that would accept that tradeoff in order to get better performance and security on their production workloads. Or in some cases (e.g. PASSWORD) just better security. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: CI, macports, darwin version problems
On 7/23/24 10:44, Joe Conway wrote: I guess if all else fails I will have to get the mac mini with more built in storage in order to accommodate sonoma. I *think* I finally have it in a good place. I replaced the nvme enclosure that I bought the other day (which had a 10G interface speed) with a new one (which has 40G rated speed). The entire ~/.tart directory is a symlink to /Volumes/extnvme. The last two runs completed successfully and at about the same speed as the PGX macmini does. Let me know if you see any issues. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: Convert sepgsql tests to TAP
On 7/24/24 12:36, Tom Lane wrote: Peter Eisentraut writes: In my experience, the tests (both the old and the proposed new) only work on Red Hat-like platforms. I had also tried on Debian but decided that it won't work. Yeah, Red Hat is pretty much the only vendor that has pushed SELinux far enough to be usable by non-wizards. I'm not surprised if there are outright bugs in other distros' versions of it, as AFAIK nobody else turns it on by default. I tried some years ago to get it working on my Debian-derived Linux Mint desktop and gave up. I think SELinux is a really good tool on RHEL variants, but I don't think many people use it on anything else. As Tom says, perhaps there are a few wizards out there though... -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: Built-in CTYPE provider
On 7/24/24 11:19, Noah Misch wrote: On Wed, Jul 17, 2024 at 03:03:26PM -0700, Noah Misch wrote: On Wed, Jul 17, 2024 at 08:48:46AM -0700, Jeff Davis wrote: > you have something in mind, please propose it. However, this feature > followed the right policies at the time of commit, so there would need > to be a strong consensus to accept such a change. If I'm counting the votes right, you and Tom have voted that the feature's current state is okay, and I and Laurenz have voted that it's not okay. I still hope more people will vote, to avoid dealing with the tie. Daniel, Peter, and Jeremy, you're all listed as reviewers on commit f69319f. Are you willing to vote one way or the other on the question in https://postgr.es/m/20240706195129...@rfd.leadboat.com? The last vote arrived 6 days ago. So far, we have votes from Jeff, Noah, Tom, Daniel, and Laurenz. I'll keep the voting open for another 24 hours from now or 36 hours after the last vote, whichever comes last. If that schedule is too compressed for anyone, do share. It isn't entirely clear to me exactly what we are voting on. * If someone votes +1 (current state is ok) -- that is pretty clear. * But if someone votes -1 (current state is not ok?) what does that mean in practice? - a revert? - we hold shipping 17 until we get consensus (via some plan or mitigation or whatever)? - something else? In any case, I am a hard -1 against reverting. +0.5 on "current state is ok", and +1 on "current state is ok with agreement on what to do in 18" -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: [18] Policy on IMMUTABLE functions and Unicode updates
On 7/23/24 15:26, Tom Lane wrote: Robert Haas writes: Also, Noah has pointed out that C.UTF-8 introduces some forward-compatibility hazards of its own, at least with respect to ctype semantics. I don't have a clear view of what ought to be done about that, but if we just replace a dependency on an unstable set of libc definitions with a dependency on an equally unstable set of PostgreSQL definitions, we're not really winning. No, I think we *are* winning, because the updates are not "equally unstable": with pg_c_utf8, we control when changes happen. We can align them with major releases and release-note the differences. With libc-based collations, we have zero control and not much notification. +1 Do we need to version the new ctype provider? It would be a version for the underlying Unicode definitions, not the provider as such, but perhaps yes. I don't know to what extent doing so would satisfy Noah's concern; but if it would do so I'd be happy with that answer. I came to the same conclusion. I think someone mentioned somewhere on this thread that other databases support multiple Unicode versions. I think we need to figure out how to do that too. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: CI, macports, darwin version problems
On 7/23/24 06:31, Thomas Munro wrote: On Tue, Jul 23, 2024 at 7:37 AM Andres Freund wrote: [2] https://cirrus-ci.com/task/5190473306865664 "Error: “disk.img” couldn’t be copied to “3FA983DD-3078-4B28-A969-BCF86F8C9585” because there isn’t enough space." Could it be copying the whole image every time, in some way that would get copy-on-write on the same file system, but having to copy physically here? That is, instead of using some kind of chain of overlay disk image files as seen elsewhere, is this Tart thing relying on file system COW for that? Perhaps that is happening here[1] but I don't immediately know how to find out where that Swift standard library call turns into system calls... [1] https://github.com/cirruslabs/tart/blob/main/Sources/tart/VMDirectory.swift#L119 I tried moving ~/.tart/tmp to the external drive as well, but that failed -- I *think* because tart is trying to do some kind of hardlink between the files in ~/.tart/tmp and ~/.tart/vms. So I move that back and at least the ventura runs are working again. I also noticed that when I set up the external drive, I somehow automatically configured time machine to run (it was not done intentionally), and it seemed that the backups were consuming space on the primary drive . Did I mention I really hate messing with macos ;-). Any idea how to disable time machine entirely? The settings app provides next to zero configuration of the thing. Anyway, maybe with the time machine stuff removed the there is enough space? I guess if all else fails I will have to get the mac mini with more built in storage in order to accommodate sonoma. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: CI, macports, darwin version problems
On 7/21/24 17:26, Thomas Munro wrote: On Mon, Jul 22, 2024 at 8:34 AM Joe Conway wrote: Hmmm, maybe nevermind? I rebooted the mac mini and now it seems to be working. Maybe someone can confirm. There ought to be plenty of space available for sonoma and ventura at the same time now. Thanks for doing that. Initial results are that it's running the tests much more slowly. Example: https://cirrus-ci.com/task/5607066713194496 I wonder if there is a way to use the external drive for caching images and so on, but the faster (?) internal drive for work... Maybe -- I moved the symlink to include only the "cache" part of the tree under ~/.tart. 8<-- macmini:.tart ci-run$ cd ~ macmini:~ ci-run$ ll .tart total 0 drwxr-xr-x 5 ci-run staff 160 Jul 22 08:42 . drwxr-x---+ 25 ci-run staff 800 Jul 22 08:41 .. lrwxr-xr-x 1 ci-run staff 35 Jul 22 08:42 cache -> /Volumes/extnvme/ci-run/.tart/cache drwxr-xr-x 3 ci-run staff 96 Jul 22 08:43 tmp drwxr-xr-x 2 ci-run staff 64 Jul 22 08:39 vms 8<-- Previously I had the entire "~/.tart" directory tree on the external drive. Please check again. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: CI, macports, darwin version problems
On 7/21/24 16:15, Joe Conway wrote: On 7/18/24 10:33, Joe Conway wrote: On 7/18/24 10:23, Nazir Bilal Yavuz wrote: On Thu, 18 Jul 2024 at 17:01, Joe Conway wrote: So perhaps I am back to needing more storage... You might not need more storage. Thomas knows better, but AFAIU, CFBot will pull only sonoma images after the patch in this thread gets merged. And your storage seems enough for storing it. I figured I would go ahead and buy it. Basically $250 total for a 2TB WD_BLACK NVMe plus a mac mini expansion enclosure. Should be delivered Sunday. I installed and mounted the new volume, moved "~/.tart" to /Volumes/extnvme and created a symlink, and the restarted the ci process, but now I am getting continuous errors streaming to the log: 8<-- macmini:~ ci-run$ tail -n1 log/cirrus-worker.log time="2024-07-21T16:09:29-04:00" level=error msg="failed to poll upstream https://grpc.cirrus-ci.com:443: rpc error: code = NotFound desc = Can't find worker by session token!" 8<-- Any ideas? Hmmm, maybe nevermind? I rebooted the mac mini and now it seems to be working. Maybe someone can confirm. There ought to be plenty of space available for sonoma and ventura at the same time now. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: CI, macports, darwin version problems
On 7/18/24 10:33, Joe Conway wrote: On 7/18/24 10:23, Nazir Bilal Yavuz wrote: On Thu, 18 Jul 2024 at 17:01, Joe Conway wrote: So perhaps I am back to needing more storage... You might not need more storage. Thomas knows better, but AFAIU, CFBot will pull only sonoma images after the patch in this thread gets merged. And your storage seems enough for storing it. I figured I would go ahead and buy it. Basically $250 total for a 2TB WD_BLACK NVMe plus a mac mini expansion enclosure. Should be delivered Sunday. I installed and mounted the new volume, moved "~/.tart" to /Volumes/extnvme and created a symlink, and the restarted the ci process, but now I am getting continuous errors streaming to the log: 8<-- macmini:~ ci-run$ ll /Users/ci-run/.tart lrwxr-xr-x 1 ci-run staff 29 Jul 21 15:53 /Users/ci-run/.tart -> /Volumes/extnvme/ci-run/.tart macmini:~ ci-run$ df -h /Volumes/extnvme Filesystem Size Used Avail Capacity iused ifree %iused Mounted on /dev/disk5s2 1.8Ti 76Gi 1.7Ti 5% 105 187345322800% /Volumes/extnvme macmini:~ ci-run$ tail -n1 log/cirrus-worker.log time="2024-07-21T16:09:29-04:00" level=error msg="failed to poll upstream https://grpc.cirrus-ci.com:443: rpc error: code = NotFound desc = Can't find worker by session token!" 8<-- Any ideas? -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: CI, macports, darwin version problems
On 7/18/24 10:23, Nazir Bilal Yavuz wrote: On Thu, 18 Jul 2024 at 17:01, Joe Conway wrote: So perhaps I am back to needing more storage... You might not need more storage. Thomas knows better, but AFAIU, CFBot will pull only sonoma images after the patch in this thread gets merged. And your storage seems enough for storing it. I figured I would go ahead and buy it. Basically $250 total for a 2TB WD_BLACK NVMe plus a mac mini expansion enclosure. Should be delivered Sunday. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: CI, macports, darwin version problems
On 7/18/24 08:55, Nazir Bilal Yavuz wrote: Hi, On Thu, 18 Jul 2024 at 15:00, Joe Conway wrote: On 7/18/24 07:55, Joe Conway wrote: > On 7/18/24 04:12, Nazir Bilal Yavuz wrote: >> Could it be pulling the ''macos-runner:sonoma' image on every run? > > Or perhaps since this was the first run it simply needed to pull the > image for the first time? It was not the first run, Thomas rerun it a couple of times but all of them were in the same build. So, I thought that CI may set some settings to pull the image while starting the build, so it re-downloads the image for all the tasks in the same build. But that looks wrong because of what you said below. > > The scheduling timing (21:24) looks a lot like what I observed when I > did the test for the time to download. Unfortunately I did not time the > test though. Actually it does look like the image is gone now based on the free space on the volume, so maybe it is pulling every run and cleaning up rather than caching for some reason? Filesystem Size Used Avail Capacity /dev/disk3s5228Gi 39Gi 161Gi20% That is interesting. Only one thing comes to my mind. It seems that the 'tart prune' command runs automatically to reclaim space when there is no space left and thinks it can reclaim the space by removing some things [1]. So, it could be that somehow 'tart prune' ran automatically and deleted the sonoma image. I think you can check if this is the case. You can check these locations [2] from ci-user to see when ventura images are created. If they have been created less than 1 day ago, I think the current space is not enough to pull both ventura and sonoma images. I think you nailed it (this will wrap badly): 8<- macmini:~ ci-run$ ll ~/.tart/cache/OCIs/ghcr.io/cirruslabs/* /Users/ci-run/.tart/cache/OCIs/ghcr.io/cirruslabs/macos-runner: total 0 drwxr-xr-x 2 ci-run staff 64 Jul 17 23:53 . drwxr-xr-x 5 ci-run staff 160 Jul 17 17:16 .. /Users/ci-run/.tart/cache/OCIs/ghcr.io/cirruslabs/macos-sonoma-base: total 0 drwxr-xr-x 2 ci-run staff 64 Jul 17 13:18 . drwxr-xr-x 5 ci-run staff 160 Jul 17 17:16 .. /Users/ci-run/.tart/cache/OCIs/ghcr.io/cirruslabs/macos-ventura-base: total 0 drwxr-xr-x 4 ci-run staff 128 Jul 17 23:53 . drwxr-xr-x 5 ci-run staff 160 Jul 17 17:16 .. lrwxr-xr-x 1 ci-run staff 140 Jul 17 23:53 latest -> /Users/ci-run/.tart/cache/OCIs/ghcr.io/cirruslabs/macos-ventura-base/sha256:bddfa1e2b6f6ec41b5db844b06a6784a2bffe0b071965470efebd95ea3355b4f drwxr-xr-x 5 ci-run staff 160 Jul 17 23:53 sha256:bddfa1e2b6f6ec41b5db844b06a6784a2bffe0b071965470efebd95ea3355b4f 8<----- So perhaps I am back to needing more storage... -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: CI, macports, darwin version problems
On 7/18/24 07:55, Joe Conway wrote: On 7/18/24 04:12, Nazir Bilal Yavuz wrote: Could it be pulling the ''macos-runner:sonoma' image on every run? Or perhaps since this was the first run it simply needed to pull the image for the first time? The scheduling timing (21:24) looks a lot like what I observed when I did the test for the time to download. Unfortunately I did not time the test though. Actually it does look like the image is gone now based on the free space on the volume, so maybe it is pulling every run and cleaning up rather than caching for some reason? Filesystem Size Used Avail Capacity /dev/disk3s5228Gi 39Gi 161Gi 20% -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: CI, macports, darwin version problems
On 7/18/24 04:12, Nazir Bilal Yavuz wrote: Hi, On Thu, 18 Jul 2024 at 07:40, Thomas Munro wrote: On Thu, Jul 18, 2024 at 9:58 AM Joe Conway wrote: > On 7/17/24 16:41, Andres Freund wrote: > > Does "tart pull ghcr.io/cirruslabs/macos-runner:sonoma" as the CI user > > succeed? > > Yes, with about 25 GB to spare. Thanks. Now it works! But for some reason it spends several minutes in the "scheduling" stage before it starts. Are there any logs that might give a clue what it was doing, for example for this run? https://cirrus-ci.com/task/5963784852865024 I only see this in the log: time="2024-07-17T23:13:56-04:00" level=info msg="started task 5963784852865024" time="2024-07-17T23:42:24-04:00" level=info msg="task 5963784852865024 completed" Could it be pulling the ''macos-runner:sonoma' image on every run? Or perhaps since this was the first run it simply needed to pull the image for the first time? The scheduling timing (21:24) looks a lot like what I observed when I did the test for the time to download. Unfortunately I did not time the test though. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: CI, macports, darwin version problems
On 7/17/24 16:41, Andres Freund wrote: Hi, On 2024-07-17 13:20:06 -0400, Joe Conway wrote: > > Or maybe simpler -- how do people typically add storage to a mac mini? I > > don't mind buying an external disk or whatever. > > That I do not know, not a mac person at all... Well maybe unneeded? Does "tart pull ghcr.io/cirruslabs/macos-runner:sonoma" as the CI user succeed? Yes, with about 25 GB to spare. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: CI, macports, darwin version problems
On 7/17/24 13:01, Andres Freund wrote: On 2024-07-16 12:12:37 -0400, Joe Conway wrote: > It's possible you have some old images stored as your user, check > "tart list" for both users. Hmm, this is not the easiest ever to parse for me... Unfortunately due to the wrapping it's not easy to read here either... I don't think it quite indicates 6 - the ones with :latest are just aliases for the one with the hash, I believe. makes sense macmini:~ ci-run$ tart list Source Name Disk Size State local ventura-base-test 50 20 stopped ocighcr.io/cirruslabs/macos-ventura-base:latest 50 21 stopped oci ghcr.io/cirruslabs/macos-ventura-base@sha256:bddfa1e2b6f6ec41b5db844b06a6784a2bffe0b071965470efebd95ea3355b4f 50 21 stopped macmini:~ jconway$ tart list I'd delete all of the ones stored for jconway - that's just redundant. done tart delete ghcr.io/cirruslabs/macos-ventura-base:latest and done tart list for both users shows nothing now. Or maybe simpler -- how do people typically add storage to a mac mini? I don't mind buying an external disk or whatever. That I do not know, not a mac person at all... Well maybe unneeded? -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: RFC: pg_stat_logmsg
On 7/16/24 19:08, Michael Paquier wrote: On Wed, Jul 17, 2024 at 12:14:36AM +0200, Tomas Vondra wrote: I noticed this patch hasn't moved since September 2023, so I wonder what's the main blocker / what is needed to move this? + /* Location of permanent stats file (valid when database is shut down) */ + #define PGLM_DUMP_FILEPGSTAT_STAT_PERMANENT_DIRECTORY "/pg_stat_logmsg.stat Perhaps this does not count as a valid reason, but does it really make sense to implement things this way, knowing that this could be changed to rely on a potential pluggable pgstats? I mean this one I've proposed: https://www.postgresql.org/message-id/Zmqm9j5EO0I4W8dx%40paquier.xyz Yep, see my adjacent reply to Tomas. One potential implementation is to stick that to be fixed-numbered, because there is a maximum cap to the number of entries proposed by the patch, while keeping the whole in memory. + logmsg_store(ErrorData *edata, Size *logmsg_offset, +int *logmsg_len, int *gc_count) The patch shares a lot of perks with pg_stat_statements that don't scale well. I'm wondering if it is a good idea to duplicate these properties in a second, different, module, like the external file can could be written out on a periodic basis depending on the workload. I am not saying that the other thread is a magic solution for everything (not looked yet at how this would plug with the cap in entries that pg_stat_statements wants), just one option on the table. As for the code, I wonder if the instability of line numbers could be a problem - these can change (a little bit) between minor releases, so after an upgrade we'll read the dump file with line numbers from the old release, and then start adding entries with new line numbers. Do we need to handle this in some way? Indeed. Perhaps a PostgreSQL version number assigned to each entry to know from which binary an entry comes from? This would cost a couple of extra bytes for each entry still that would be the best information possible to match that with a correct code tree. If it comes to that, even getting down to a commit SHA1 could be useful to provide the lowest level of granularity. Another thing would be to give up on the line number, stick to the uniqueness in the stats entries with the errcode and the file name, but that won't help for things like tablecmds.c. I think including version in the key makes most sense. Also do we even have a mechanism to grab the commit sha in running code? -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: RFC: pg_stat_logmsg
On 7/16/24 18:14, Tomas Vondra wrote: I noticed this patch hasn't moved since September 2023, so I wonder what's the main blocker / what is needed to move this? Mainly me finding time I'm afraid. As for the feature, I've never done a fleet-wide analysis, so if this is one of the main use cases, I'm not really sure I can judge if this is a good tool for that. It seems like it might be a convenient way to do that, but does that require we add the module to contrib? I had an offlist chat with Andres about this IIRC and he suggested he thought it ought to be just built in to the backend as part of the statistics subsystem. Lately though I have been toying with the idea of keeping it as an extension and basing it off Michael Paquier's work for Pluggable cumulative statistics. As for the code, I wonder if the instability of line numbers could be a problem - these can change (a little bit) between minor releases, so after an upgrade we'll read the dump file with line numbers from the old release, and then start adding entries with new line numbers. Do we need to handle this in some way? Hmm, yeah, I had been planning to include postgres version as part of the output, but maybe it would need to be part of the key. This might be partially solved by eviction of entries from the old release - we apply decay, so after a while their usage will be 0. But what if there's no pressure for space, we'll not actually evict them. And it'll be confusing to have a mix of old/new line numbers. Agreed. I am going to try hard to get back to this sooner rather than later, but realistically that might be in time for the September commitfest rather than during this one. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: [18] Policy on IMMUTABLE functions and Unicode updates
On 7/16/24 16:16, Tom Lane wrote: Joe Conway writes: So you are proposing we add STATIC to VOLATILE/STABLE/IMMUTABLE (in the third position before IMMUTABLE), give it IMMUTABLE semantics, mark builtin functions that deserve it, and document with suitable caution statements? What is the point of that, exactly? I'll agree that the user documentation could use some improvement in how it describes the volatility levels, but I do not see how it will reduce anybody's confusion to invent multiple aliases for what's effectively the same volatility level. Nor do I see a use-case for actually having multiple versions of "immutable". Once you've decided you can put something into an index, quibbling over just how immutable it is doesn't really change anything. To put this another way: the existing volatility levels were basically reverse-engineered from the ways that the planner could meaningfully treat a function: it's dangerous, it is safe enough to use in an index condition (which changes the number of times the query will evaluate it), or it's safe to constant-fold in advance of execution. Unless there's a fourth planner behavior that's worth having, we don't need a fourth level. Possibly you could argue that "safe to put in an index" is a different level from "safe to constant-fold", but I don't really agree with that. Fair enough, but then I think we should change the documentation to not say "forever". -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: [18] Policy on IMMUTABLE functions and Unicode updates
On 7/16/24 15:33, David G. Johnston wrote:
On Tue, Jul 16, 2024 at 11:57 AM Joe Conway <mailto:m...@joeconway.com>> wrote:
> There are two alternative philosophies:
>
> A. By choosing to use a Unicode-based function, the user has opted in
> to the Unicode stability guarantees[2], and it's fine to update
Unicode
> occasionally in new major versions as long as we are transparent with
> the user.
>
> B. IMMUTABLE implies some very strict definition of stability, and we
> should never again update Unicode because it changes the results of
> IMMUTABLE functions.
>
> We've been following (A), and that's the defacto policy today[3][4].
> Noah and Laurenz argued[5] that the policy starting in version 18
> should be (B). Given that it's a policy decision that affects
more than
> just the builtin collation provider, I'd like to discuss it more
> broadly outside of that subthread.
On the general topic, we have these definitions in the fine manual:
8<-
A VOLATILE function can do anything, ... A query using a volatile
function will re-evaluate the function at every row where its value is
needed.
A STABLE function cannot modify the database and is guaranteed to
return
the same results given the same arguments for all rows within a single
statement...
An IMMUTABLE function cannot modify the database and is guaranteed to
return the same results given the same arguments forever.
8<-
As Jeff points out, the IMMUTABLE definition has never really been
true.
Even the STABLE is not quite right, as there are at least some STABLE
functions that will return the same value for multiple statements if
they are within a transaction block (e.g. "now()" -- TBH I don't
remember offhand if that is true for all stable functions).
Under-specification here doesn't make the meaning of stable incorrect.
We don't have anything that guarantees stability at the transaction
scope because I don't think it can be guaranteed there without
considering whether said transaction is read-committed, repeatable read,
or serializable. The function itself can promise more but the marker
seems correctly scoped for how the system uses it in statement optimization.
The way it is described is still surprising and can bite you if you are
not familiar with the nuances. In particular I have seen now() used in
transaction blocks surprise more than one person over the years.
and allow those to be
used like we do IMMUTABLE except with appropriate warning labels. E.g.
something ("STABLE_VERSION"?) to mean "forever within a major version
lifetime" and something ("STABLE_SYSTEM?") to mean "as long as you
don't
upgrade your OS".
I'd be content cutting "forever" down to "within a given server
configuration". Then just note that immutable functions can depend
implicitly on external server characteristics and so when moving data
between servers re-evaluation of immutable functions may be necessary.
Not so bad for indexes. A bit more problematic for generated values.
Yeah I forgot about the configuration controlled ones.
I'm not against adding metadata options here but for internal functions
comments and documentation can work. For user-defined functions I have
my doubts on how trustworthy they would end up being.
People lie all the time for user-defined functions, usually specifically
when they need IMMUTABLE semantics and are willing to live with the risk
and/or apply their own controls to ensure no changes in output.
For the original question, I suggest continuing behaving per "A" and
work on making it more clear to users what that means in terms of server
upgrades.
If we do add metadata to reflect our reality I'd settle on a generic
"STATIC" marker that can be used on those functions the rely on real
world state, whether we are directly calling into the system (e.g.,
hashing) or have chosen to provide the state access management ourselves
(e.g., unicode).
So you are proposing we add STATIC to VOLATILE/STABLE/IMMUTABLE (in the
third position before IMMUTABLE), give it IMMUTABLE semantics, mark
builtin functions that deserve it, and document with suitable caution
statements?
I guess can live with just one additional level of granularity.
--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Re: [18] Policy on IMMUTABLE functions and Unicode updates
On 7/16/24 13:42, Jeff Davis wrote:
The IMMUTABLE marker for functions is quite simple on the surface, but
could be interpreted a few different ways, and there's some historical
baggage that makes it complicated.
There are a number of ways in which IMMUTABLE functions can change
behavior:
1. Updating or moving to a different OS affects all collations that use
the libc provider (other than "C" and "POSIX", which don't actually use
libc). LOWER(), INITCAP(), UPPER() and pattern matching are also
affected.
2. Updating ICU affects the collations that use the ICU provider.
ICU_UNICODE_VERSION(), LOWER(), INITCAP(), UPPER() and pattern matching
are also affected.
3. Moving to a different database encoding may affect collations that
use the "C" or "POSIX" locales in the libc provider (NB: those locales
don't actually use libc).
4. A PG Unicode update may change the results of functions that depend
on Unicode. For instance, NORMALIZE(), UNICODE_ASSIGNED(), and
UNICODE_VERSION(). Or, if using the new builtin provider's "C.UTF-8"
locale in version 17, LOWER(), INITCAP(), UPPER(), and pattern matching
(NB: collation itself is not affected -- always code point order).
5. If a well-defined IMMUTABLE function produces the wrong results, we
may fix the bug in the next major release.
6. The GUC extra_float_digits can change the results of floating point
text output.
7. A UDF may be improperly marked IMMUTABLE. A particularly common
variant is a UDF without search_path specified, which is probably not
truly IMMUTABLE.
(more I'm sure, please add to list...)
#1 and #2 have been discussed much more than the rest, but I think it's
worthwhile to enumerate the other problems even if the impact is a lot
lower.
Noah seemed particularly concerned[1] about #4, so I'll start off by
discussing that. Here's a brief history (slightly confusing because the
PG and Unicode versions are similar numbers):
PG13: Unicode 13.0 and NORMALIZE() is first exposed as a SQL function
PG15: Unicode updated to 14.0
PG16: Unicode updated to 15.0
PG17: Unicode updated to 15.1, UNICODE_ASSIGNED(), UNICODE_VERSION()
and builtin "C.UTF-8" locale are introduced
To repeat, these Unicode updates do not affect collation itself, they
affect affect NORMALIZE(), UNICODE_VERSION(), and UNICODE_ASSIGNED().
If using the builtin "C.UTF-8" locale, they also affect LOWER(),
INITCAP(), UPPER(), and pattern matching. (NB: the builtin collation
provider hasn't yet gone through any Unicode update.)
There are two alternative philosophies:
A. By choosing to use a Unicode-based function, the user has opted in
to the Unicode stability guarantees[2], and it's fine to update Unicode
occasionally in new major versions as long as we are transparent with
the user.
B. IMMUTABLE implies some very strict definition of stability, and we
should never again update Unicode because it changes the results of
IMMUTABLE functions.
We've been following (A), and that's the defacto policy today[3][4].
Noah and Laurenz argued[5] that the policy starting in version 18
should be (B). Given that it's a policy decision that affects more than
just the builtin collation provider, I'd like to discuss it more
broadly outside of that subthread.
On the general topic, we have these definitions in the fine manual:
8<-
A VOLATILE function can do anything, ... A query using a volatile
function will re-evaluate the function at every row where its value is
needed.
A STABLE function cannot modify the database and is guaranteed to return
the same results given the same arguments for all rows within a single
statement...
An IMMUTABLE function cannot modify the database and is guaranteed to
return the same results given the same arguments forever.
8<-
As Jeff points out, the IMMUTABLE definition has never really been true.
Even the STABLE is not quite right, as there are at least some STABLE
functions that will return the same value for multiple statements if
they are within a transaction block (e.g. "now()" -- TBH I don't
remember offhand if that is true for all stable functions).
In any case, there is quite a gap between "forever" and "single
statement". Perhaps we need to have more volatility categories, with
guarantees that lie somewhere between the two, and allow those to be
used like we do IMMUTABLE except with appropriate warning labels. E.g.
something ("STABLE_VERSION"?) to mean "forever within a major version
lifetime" and something ("STABLE_SYSTEM?") to mean "as long as you don't
upgrade your OS".
--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Re: CI, macports, darwin version problems
On 7/16/24 11:44, Andres Freund wrote: hi, On 2024-07-16 09:38:21 -0400, Joe Conway wrote: On 7/16/24 08:28, Joe Conway wrote: > On 7/16/24 00:34, Thomas Munro wrote: > > temporarily disabled that machine from the pool and click the re-run > > button, and it failed[2] on jc-m2-1: "Error: The operation couldn’t be > > completed. No space left on device" after a long period during which > > it was presumably trying to download that image. I could try this > > experiment again if Joe could see a way to free up some disk space. > > Hmmm, sorry, will take a look now I am not super strong on Macs in general, but cannot see anything full: /dev/disk3s5228Gi 102Gi 111Gi48% 365768 11651432400% /System/Volumes/Data Unfortunately the 'base disk' for sonoma is 144GB large... It might be worth trying to pull it separately from a CI job, under your control. As the CI user (it'll be downloaded redundantly if you do it as your user!), you can do: tart pull ghcr.io/cirruslabs/macos-runner:sonoma It's possible you have some old images stored as your user, check "tart list" for both users. Hmm, this is not the easiest ever to parse for me... macmini:~ ci-run$ tart list Source Name Disk Size State local ventura-base-test 50 20 stopped ocighcr.io/cirruslabs/macos-ventura-base:latest 50 21 stopped oci ghcr.io/cirruslabs/macos-ventura-base@sha256:bddfa1e2b6f6ec41b5db844b06a6784a2bffe0b071965470efebd95ea3355b4f 50 21 stopped macmini:~ jconway$ tart list Source Name Disk Size State local ventura-test 50 20 stopped ocighcr.io/cirruslabs/macos-ventura-base:latest 50 50 stopped oci ghcr.io/cirruslabs/macos-ventura-base@sha256:a4d4861123427a23ad3dc53a6a1d4d20d6bc1a0df82bd1495cc53217075c0a8c 50 50 stopped So does that mean I have 6 copies of the ventura image? How do I get rid of them? Or maybe simpler -- how do people typically add storage to a mac mini? I don't mind buying an external disk or whatever. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: CI, macports, darwin version problems
On 7/16/24 08:28, Joe Conway wrote: On 7/16/24 00:34, Thomas Munro wrote: temporarily disabled that machine from the pool and click the re-run button, and it failed[2] on jc-m2-1: "Error: The operation couldn’t be completed. No space left on device" after a long period during which it was presumably trying to download that image. I could try this experiment again if Joe could see a way to free up some disk space. Hmmm, sorry, will take a look now I am not super strong on Macs in general, but cannot see anything full: df -h Filesystem Size Used Avail Capacity iused ifree %iused Mounted on /dev/disk3s1s1 228Gi 8.7Gi 111Gi 8% 356839 11651432400% / devfs 199Ki 199Ki0Bi 100% 690 0 100% /dev /dev/disk3s6228Gi 20Ki 111Gi 1% 0 11651432400% /System/Volumes/VM /dev/disk3s2228Gi 5.0Gi 111Gi 5%1257 11651432400% /System/Volumes/Preboot /dev/disk3s4228Gi 28Mi 111Gi 1% 47 11651432400% /System/Volumes/Update /dev/disk1s2500Mi 6.0Mi 483Mi 2% 149414800% /System/Volumes/xarts /dev/disk1s1500Mi 6.2Mi 483Mi 2% 2949414800% /System/Volumes/iSCPreboot /dev/disk1s3500Mi 492Ki 483Mi 1% 5549414800% /System/Volumes/Hardware /dev/disk3s5228Gi 102Gi 111Gi48% 365768 11651432400% /System/Volumes/Data map auto_home 0Bi0Bi0Bi 100% 0 0 100% /System/Volumes/Data/home As far as I can tell, the 100% usage for /dev and /System/Volumes/Data/home are irrelevant. ¯\_(ツ)_/¯ I ran an update to the latest Ventura and rebooted as part of that. Can you check again? -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: CI, macports, darwin version problems
On 7/16/24 00:34, Thomas Munro wrote: temporarily disabled that machine from the pool and click the re-run button, and it failed[2] on jc-m2-1: "Error: The operation couldn’t be completed. No space left on device" after a long period during which it was presumably trying to download that image. I could try this experiment again if Joe could see a way to free up some disk space. Hmmm, sorry, will take a look now -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: Commitfest manager for July 2024
On 7/8/24 11:38, Joe Conway wrote: On 7/3/24 12:51, Andrey M. Borodin wrote: On 3 Jul 2024, at 01:08, Corey Huinker wrote: I'll give it a shot. Great, thank you! Do you have extended access to CF? Like activity log and mass-mail functions? If no I think someone from PG_INFRA can grant you necessary access. I can do that, although given that Corey and I are colleagues, it might be better if someone else on pginfra did. Or at least if a few other hackers tell me to "just do it"... Based on off-list (sysadmin telegram channel) confirmation from Magnus and Dave Page -- done -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: Commitfest manager for July 2024
On 7/3/24 12:51, Andrey M. Borodin wrote: On 3 Jul 2024, at 01:08, Corey Huinker wrote: I'll give it a shot. Great, thank you! Do you have extended access to CF? Like activity log and mass-mail functions? If no I think someone from PG_INFRA can grant you necessary access. I can do that, although given that Corey and I are colleagues, it might be better if someone else on pginfra did. Or at least if a few other hackers tell me to "just do it"... -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: CI, macports, darwin version problems
On 7/2/24 17:39, Thomas Munro wrote: One difference that jumps out is that the successful v3 run has label worker:jc-m2-1 (Mac hosted by Joe), and the failure has worker:pgx-m2-1 (Mac hosted by Christophe P). Is this a software version issue, ie need newer Tart to use that image, or could be a difficulty fetching the image? CCing our Mac Mini pool attendants. How can I help? Do you need to know versions of some of the stuff on my mac mini? -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: question regarding policy for patches to out-of-support branches
On 6/6/24 14:12, Tom Lane wrote: Robert Haas writes: On Thu, Jun 6, 2024 at 4:25 AM Hannu Krosing wrote: Not absolutely sure, but would at least adding a page to PostgreSQL Wiki about this make sense ? I feel like we need to do something. Tom says this is a policy, and he's made that comment before about other things, but the fact that they're not memorialized anywhere is a huge problem, IMHO. I didn't say it wasn't ;-) ISTM we have two basic choices: wiki page, or new SGML docs section. In the short term I'd lean to a wiki page. It'd be reasonable for https://wiki.postgresql.org/wiki/Committing_checklist to link to it (and maybe the existing section there about release freezes would be more apropos on a "Project policies" page? Not sure.) To get a sense of how much of a problem we have, I grepped the git history for comments mentioning project policies. Ignoring ones that are really talking about very localized issues, what I found is attached. It seems like it's little enough that a single wiki page with subsections ought to be enough. I'm not super handy with editing the wiki, plus I'm only firing on one cylinder today (seem to have acquired a head cold at pgconf), so maybe somebody else would like to draft something? I added them here with minimal copy editing an no attempt to organize or sort into groups: https://wiki.postgresql.org/wiki/Committing_checklist#Policies If someone has thoughts on how to improve I am happy to make more changes. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
question regarding policy for patches to out-of-support branches
I was having a discussion regarding out-of-support branches and effort to keep them building, but could not for the life of me find any actual documented policy (although I distinctly remember that we do something...). I did find fleeting references, for example: 8<--- commit c705646b751e08d584f6eeb098f1ed002aa7b11c Author: Tom Lane Date: 2022-09-21 13:52:38 -0400 Per project policy, this is a candidate for back-patching into out-of-support branches: it suppresses annoying compiler warnings but changes no behavior. Hence, back-patch all the way to 9.2. 8<--- and on its related thread: 8<--- However, I think that that would *not* be fit material for back-patching into out-of-support branches, since our policy for them is "no behavioral changes". 8<--- Is the policy written down somewhere, or is it only project lore? In either case, what is the actual policy? Thanks, -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: Optimizing COPY with SIMD
On 6/2/24 15:17, Neil Conway wrote: Inspired by David Rowley's work [1] Welcome back! -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: commitfest.postgresql.org is no longer fit for purpose
On 5/24/24 15:45, Tom Lane wrote: I was *not* proposing doing a regular review, unless of course somebody really wants to do that. What I am thinking about is suggesting how to make progress on patches that are stuck, or in some cases delivering the bad news that this patch seems unlikely to ever get accepted and it's time to cut our losses. (Patches that seem to be moving along in good order probably don't need any attention in this process, beyond determining that that's the case.) That's why I think we need some senior people doing this, as their opinions are more likely to be taken seriously. Maybe do a FOSDEM-style dev meeting with triage review at PG.EU would at least move us forward? Granted it is less early and perhaps less often than the thread seems to indicate, but has been tossed around before and seems doable. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: commitfest.postgresql.org is no longer fit for purpose
On 5/19/24 05:37, Dmitry Dolgov wrote: * How to deal with review scalability bottleneck. An evergreen question. PostgreSQL is getting more popular and, as stated in diverse research whitepapers, the amount of contribution grows as a power law, where the number of reviewers grows at best sub-linearly (limited by the velocity of knowledge sharing). I agree with Andrey on this, the only way I see to handle this is to scale CF management efforts. The number of items tracked are surely growing, but I am not sure I would call it exponential -- see attached -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: commitfest.postgresql.org is no longer fit for purpose
On 5/17/24 11:58, Robert Haas wrote: I realize that many people here are (rightly!) concerned with burdening patch authors with more steps that they have to follow. But the current system is serving new patch authors very poorly. If they get attention, it's much more likely to be because somebody saw their email and wrote back than it is to be because somebody went through the CommitFest and found their entry and was like "oh, I should review this". Honestly, if we get to a situation where a patch author is sad because they have to click a link every 2 months to say "yeah, I'm still here, please review my patch," we've already lost the game. That person isn't sad because we asked them to click a link. They're sad it's already been N * 2 months and nothing has happened. +many -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: commitfest.postgresql.org is no longer fit for purpose
On 5/17/24 09:08, Peter Eisentraut wrote: On 17.05.24 14:42, Joe Conway wrote: Namely, the week before commitfest I don't actually know if I will have the time during that month, but I will make sure my patch is in the commitfest just in case I get a few clear days to work on it. Because if it isn't there, I can't take advantage of those "found" hours. A solution to both of these issues (yours and mine) would be to allow things to be added *during* the CF month. What is the point of having a "freeze" before every CF anyway? Especially if they start out clean. If something is ready for review on day 8 of the CF, why not let it be added for review? Maybe this all indicates that the idea of bimonthly commitfests has run its course. The original idea might have been, if we have like 50 patches, we can process all of them within a month. We're clearly not doing that anymore. How would the development process look like if we just had one commitfest per dev cycle that runs from July 1st to March 31st? What's old is new again? ;-) I agree with you. Before commitfests were a thing, we had no structure at all as I recall. The dates for the dev cycle were more fluid as I recall, and we had no CF app to track things. Maybe retaining the structure but going back to the continuous development would be just the thing, with the CF app tracking just the currently (as of this week/month/???) active stuff. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: commitfest.postgresql.org is no longer fit for purpose
On 5/17/24 08:31, Jelte Fennema-Nio wrote: On Fri, 17 May 2024 at 14:19, Joe Conway wrote: On 5/16/24 22:26, Robert Haas wrote: > For example, imagine that the CommitFest is FORCIBLY empty > until a week before it starts. You can still register patches in the > system generally, but that just means they get CI runs, not that > they're scheduled to be reviewed. A week before the CommitFest, > everyone who has a patch registered in the system that still applies > gets an email saying "click here if you think this patch should be > reviewed in the upcoming CommitFest -- if you don't care about the > patch any more or it needs more work before other people review it, > don't click here". Then, the CommitFest ends up containing only the > things where the patch author clicked there during that week. 100% agree. This is in line with what I suggested on an adjacent part of the thread. Such a proposal would basically mean that no-one that cares about their patches getting reviews can go on holiday and leave work behind during the week before a commit fest. That seems quite undesirable to me. Well, I'm sure I'll get flamed for this suggestion, be here goes anyway... I wrote: Namely, the week before commitfest I don't actually know if I will have the time during that month, but I will make sure my patch is in the commitfest just in case I get a few clear days to work on it. Because if it isn't there, I can't take advantage of those "found" hours. A solution to both of these issues (yours and mine) would be to allow things to be added *during* the CF month. What is the point of having a "freeze" before every CF anyway? Especially if they start out clean. If something is ready for review on day 8 of the CF, why not let it be added for review? -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: commitfest.postgresql.org is no longer fit for purpose
On 5/16/24 22:26, Robert Haas wrote: For example, imagine that the CommitFest is FORCIBLY empty until a week before it starts. You can still register patches in the system generally, but that just means they get CI runs, not that they're scheduled to be reviewed. A week before the CommitFest, everyone who has a patch registered in the system that still applies gets an email saying "click here if you think this patch should be reviewed in the upcoming CommitFest -- if you don't care about the patch any more or it needs more work before other people review it, don't click here". Then, the CommitFest ends up containing only the things where the patch author clicked there during that week. 100% agree. This is in line with what I suggested on an adjacent part of the thread. The point is - we need a much better signal to noise ratio here. I bet the number of patches in the CommitFest that actually need review is something like 25% of the total. The rest are things that are just parked there by a committer, or that the author doesn't care about right now, or that are already being actively discussed, or where there's not a clear way forward. I think there is another case that no one talks about, but I'm sure exists, and that I am not the only one guilty of thinking this way. Namely, the week before commitfest I don't actually know if I will have the time during that month, but I will make sure my patch is in the commitfest just in case I get a few clear days to work on it. Because if it isn't there, I can't take advantage of those "found" hours. We could create new statuses for all of those states - "Parked", "In Hibernation," "Under Discussion," and "Unclear" - but I think that's missing the point. What we really want is to not see that stuff in the first place. It's a CommitFest, not once-upon-a-time-I-wrote-a-patch-Fest. +1 -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: commitfest.postgresql.org is no longer fit for purpose
On 5/16/24 17:36, Jacob Champion wrote: On Thu, May 16, 2024 at 2:29 PM Joe Conway wrote: If no one, including the author (new or otherwise) is interested in shepherding a particular patch, what chance does it have of ever getting committed? That's a very different thing from what I think will actually happen, which is - new author posts patch - community member says "use commitfest!" Here is where we should point them at something that explains the care and feeding requirements to successfully grow a patch into a commit. - new author registers patch - no one reviews it - patch gets automatically booted Part of the care and feeding instructions should be a warning regarding what happens if you are unsuccessful in the first CF and still want to see it through. - community member says "register it again!" - new author says ಠ_ಠ As long as this is not a surprise ending, I don't see the issue. Like Tom said upthread, the issue isn't really that new authors are somehow uninterested in their own patches. First, some of them objectively are uninterested in doing more than dropping a patch over the wall and never looking back. But admittedly that is not too often. Second, I don't think a once every two months effort in order to register continuing interest is too much to ask. And third, if we did something like Magnus' suggestion about a CF parking lot, the process would be even more simple. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: commitfest.postgresql.org is no longer fit for purpose
On 5/16/24 17:24, Jacob Champion wrote: On Thu, May 16, 2024 at 2:06 PM Joe Conway wrote: Maybe the word "care" was a poor choice, but forcing authors to think about and decide if they have the "time to shepherd a patch" for the *next CF* is exactly the point. If they don't, why clutter the CF with it. Because the community regularly encourages new patch contributors to park their stuff in it, without first asking them to sign on the dotted line and commit to the next X months of their free time. If that's not appropriate, then I think we should decide what those contributors need to do instead, rather than making a new bar for them to clear. If no one, including the author (new or otherwise) is interested in shepherding a particular patch, what chance does it have of ever getting committed? IMHO the probability is indistinguishable from zero anyway. Perhaps we should be more explicit to new contributors that they need to either own their patch through the process, or convince someone to do it for them. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: commitfest.postgresql.org is no longer fit for purpose
On 5/16/24 16:48, Magnus Hagander wrote: On Thu, May 16, 2024 at 10:46 PM Melanie Plageman I was reflecting on why a general purpose patch tracker sounded appealing to me, and I realized that, at least at this time of year, I have a few patches that really count as "waiting on author" that I know I need to do additional work on before they need more review but which aren't currently my top priority. I should probably simply withdraw and re-register them. My justification was that I'll lose them if I don't keep them in the commitfest app. But, I could just, you know, save them somewhere myself instead of polluting the commitfest app with them. I don't know if others are in this situation. Anyway, I'm definitely currently guilty of parking. One thing I think we've talked about before (but not done) is to basically have a CF called "parking lot", where you can park patches that aren't active in a commitfest but you also don't want to be dead. It would probably also be doable to have the cf bot run patches in that commitfest as well as the current one, if that's what people are using it for there. +1 -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: commitfest.postgresql.org is no longer fit for purpose
On 5/16/24 16:57, Jacob Champion wrote: On Thu, May 16, 2024 at 1:31 PM Joe Conway wrote: Maybe we should just make it a policy that *nothing* gets moved forward from commitfest-to-commitfest and therefore the author needs to care enough to register for the next one? I think that's going to severely disadvantage anyone who doesn't do this as their day job. Maybe I'm bristling a bit too much at the wording, but not having time to shepherd a patch is not the same as not caring. Maybe the word "care" was a poor choice, but forcing authors to think about and decide if they have the "time to shepherd a patch" for the *next CF* is exactly the point. If they don't, why clutter the CF with it. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: commitfest.postgresql.org is no longer fit for purpose
On 5/16/24 15:47, Tom Lane wrote: Daniel Gustafsson writes: On 16 May 2024, at 20:30, Robert Haas wrote: The original intent of CommitFests, and of commitfest.postgresql.org by extension, was to provide a place where patches could be registered to indicate that they needed to be reviewed, thus enabling patch authors and patch reviewers to find each other in a reasonably efficient way. I don't think it's working any more. But which part is broken though, the app, our commitfest process and workflow and the its intent, or our assumption that we follow said process and workflow which may or may not be backed by evidence? IMHO, from being CMF many times, there is a fair bit of the latter, which excacerbates the problem. This is harder to fix with more or better software though. Yeah. I think that Robert put his finger on a big part of the problem, which is that punting a patch to the next CF is a lot easier than rejecting it, particularly for less-senior CFMs who may not feel they have the authority to say no (or at least doubt that the patch author would accept it). Maybe we should just make it a policy that *nothing* gets moved forward from commitfest-to-commitfest and therefore the author needs to care enough to register for the next one? I spent a good deal of time going through the CommitFest this week And you deserve a big Thank You for that. + many +1 agreed -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: PostgreSQL 17 Beta 1 release announcement draft
On 5/16/24 08:05, Joe Conway wrote: On 5/15/24 21:45, Jonathan S. Katz wrote: Please provide feedback no later than Wed 2024-05-22 18:00 UTC. As the beta release takes some extra effort, I want to ensure all changes are in with time to spare before release day. "`EXPLAIN` can now show how much time is spent for I/O block reads and writes" Is that really EXPLAIN, or rather EXPLAIN ANALYZE? -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: First draft of PG 17 release notes
On 5/15/24 23:48, Andres Freund wrote: On 2024-05-15 10:38:20 +0200, Alvaro Herrera wrote: I disagree with this. IMO the impact of the Sawada/Naylor change is likely to be enormous for people with large tables and large numbers of tuples to clean up (I know we've had a number of customers in this situation, I can't imagine any Postgres service provider that doesn't). The fact that maintenance_work_mem is no longer capped at 1GB is very important and I think we should mention that explicitly in the release notes, as setting it higher could make a big difference in vacuum run times. +many. We're having this debate every release. I think the ongoing reticence to note performance improvements in the release notes is hurting Postgres. For one, performance improvements are one of the prime reason users upgrade. Without them being noted anywhere more dense than the commit log, it's very hard to figure out what improved for users. A halfway widely applicable performance improvement is far more impactful than many of the feature changes we do list in the release notes. many++ For another, it's also very frustrating for developers that focus on performance. The reticence to note their work, while noting other, far smaller, things in the release notes, pretty much tells us that our work isn't valued. agreed -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: PostgreSQL 17 Beta 1 release announcement draft
On 5/15/24 21:45, Jonathan S. Katz wrote: Please provide feedback no later than Wed 2024-05-22 18:00 UTC. As the beta release takes some extra effort, I want to ensure all changes are in with time to spare before release day. "You can find information about all of the features and changes found in PostgreSQL 17" Sounds repetitive; maybe: "Information about all of the features and changes in PostgreSQL 17 can be found in the [release notes]" "more explicitly SIMD instructions" I think ought to be "more explicit SIMD instructions" "PostgreSQL 17 includes a built-in collation provider that provides similar semantics to the `C` collation provided by libc." I think that needs to mention UTF-8 encoding somehow, and "provided by libc" is not really true; maybe: "PostgreSQL 17 includes a built-in collation provider that provides similar sorting semantics to the `C` collation except with UTF-8 encoding rather than SQL_ASCII." -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: First draft of PG 17 release notes
On 5/11/24 09:57, Jelte Fennema-Nio wrote: On Fri, 10 May 2024 at 23:31, Tom Lane wrote: Bruce Momjian writes: > I looked at both of these. In both cases I didn't see why the user > would need to know these changes were made: I agree that the buffering change is not likely interesting, but the fact that you can now control-C out of a psql "\c" command is user-visible. People might have internalized the fact that it didn't work, or created complicated workarounds. The buffering change improved performance up to ~40% in some of the benchmarks. The case it improves mostly is COPY of large rows and streaming a base backup. That sounds user-visible enough to me to warrant an entry imho. +1 -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: 'trusted'/'untrusted' PL in DoD/DISA PostgreSQL STIGs
On 5/5/24 13:53, Chapman Flack wrote: The four STIGs suggest the same email address [5] for comments or proposed revisions. I could send these comments there myself, but I thought it likely that others in the community have already been involved in the development of those documents and might have better connections. Those docs were developed by the respective companies (Crunchy and EDB) in cooperation with DISA. The community has nothing to do with them. I suggest you contact the two companies with corrections and suggestions. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: pg_trgm comparison bug on cross-architecture replication due to different char implementation
On 5/3/24 11:44, Peter Eisentraut wrote: On 03.05.24 16:13, Tom Lane wrote: Peter Eisentraut writes: On 30.04.24 19:29, Tom Lane wrote: Also, the bigger picture here is the seeming assumption that "if we change pg_trgm then it will be safe to replicate from x86 to arm". I don't believe that that's a good idea and I'm unwilling to promise that it will work, regardless of what we do about char signedness. That being the case, I don't want to invest a lot of effort in the signedness issue. Option (1) is clearly a small change with little if any risk of future breakage. But note that option 1 would prevent some replication that is currently working. The point of this thread though is that it's working only for small values of "work". People are rightfully unhappy if it seems to work and then later they get bitten by compatibility problems. Treating char signedness as a machine property in pg_control would signal that we don't intend to make it work, and would ensure that even the most minimal testing would find out that it doesn't work. If we do not do that, it seems to me we have to buy into making it work. That would mean dealing with the consequences of an incompatible change in pg_trgm indexes, and then going through the same dance again the next time(s) similar problems are found. Yes, that is understood. But anecdotally, replicating between x86-64 arm64 is occasionally used for upgrades or migrations. In practice, this appears to have mostly worked. If we now discover that it won't work with certain index extension modules, it's usable for most users. Even if we say, you have to reindex everything afterwards, it's probably still useful for these scenarios. +1 I have heard similar anecdotes, and the reported experience goes even further -- many such upgrade/migration uses, with exceedingly rare reported failures. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: New GUC autovacuum_max_threshold ?
On 4/26/24 09:31, Robert Haas wrote: On Fri, Apr 26, 2024 at 9:22 AM Joe Conway wrote: Although I don't think 50 is necessarily too small. In my view, having autovac run very quickly, even if more frequently, provides an overall better user experience. Can you elaborate on why you think that? I mean, to me, that's almost equivalent to removing autovacuum_vacuum_scale_factor entirely, because only for very small tables will that calculation produce a value lower than 500k. If I understood Nathan's proposed calc, for small tables you would still get (thresh + sf * numtuples). Once that number exceeds the new limit parameter, then the latter would kick in. So small tables would retain the current behavior and large enough tables would be clamped. We might need to try to figure out some test cases here. My intuition is that this is going to vacuum large tables insanely aggressively. It depends on workload to be sure. Just because a table is large, it doesn't mean that dead rows are generated that fast. Admittedly it has been quite a while since I looked at all this that closely, but if A/V runs on some large busy table for a few milliseconds once every few minutes, that is far less disruptive than A/V running for tens of seconds once every few hours or for minutes ones every few days -- or whatever. The key thing to me is the "few milliseconds" runtime. The short duration means that no one notices an impact, and the longer duration almost guarantees that an impact will be felt. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: New GUC autovacuum_max_threshold ?
On 4/26/24 04:43, Michael Banck wrote: So this proposal (probably along with a higher default threshold than 50, but IMO less than what Robert and Nathan suggested) sounds like a stop forward to me. DBAs can set the threshold lower if they want, or maybe we can just turn it off by default if we cannot agree on a sane default, but I think this (using the simplified formula from Nathan) is a good approach that takes some pain away from autovacuum tuning and reserves that for the really difficult cases. +1 to the above Although I don't think 50 is necessarily too small. In my view, having autovac run very quickly, even if more frequently, provides an overall better user experience. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: Table AM Interface Enhancements
On 4/10/24 09:19, Robert Haas wrote: When you commit a patch and another committer writes a post-commit review saying that your patch has so many serious problems that he gave up on reviewing before enumerating all of them, that's a really bad sign. That should be a strong signal to you to step back and take a close look at whether you really understand the area of the code that you're touching well enough to be doing whatever it is that you're doing. If I got a review like that, I would have reverted the patch instantly, given up for the release cycle, possibly given up on the patch permanently, and most definitely not tried again to commit unless I was absolutely certain that I'd learned a lot in the meantime *and* had the agreement of the committer who wrote that review (or maybe some other committer who was acknowledged as an expert in that area of the code). It's not Andres's job to make sure my patches are not broken. It's my job. That applies to the patches I write, and the patches written by other people that I commit. If I commit something and it turns out that it is broken, that's my bad. If I commit something and it turns out that it does not have consensus, that is also my bad. It is not the fault of the other people for not helping me get my patches to a state where they are up to project standard. It is my fault, and my fault alone, for committing something that was not ready. Now that does not mean that it isn't frustrating when I can't get the help I need. It is extremely frustrating. But the solution is not to commit anyway and then blame the other people for not providing feedback. +many -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: Security lessons from liblzma
On 4/8/24 22:57, Bruce Momjian wrote: On Sat, Mar 30, 2024 at 04:50:26PM -0400, Robert Haas wrote: An awful lot of what we do operates on the principle that we know the people who are involved and trust them, and I'm glad we do trust them, but the world is full of people who trusted somebody too much and regretted it afterwards. The fact that we have many committers rather than a single maintainer probably reduces risk at least as far as the source repository is concerned, because there are more people paying attention to potentially notice something that isn't as it should be. One unwritten requirement for committers is that we are able to communicate with them securely. If we cannot do that, they potentially could be forced by others, e.g., governments, to add code to our repositories. Unfortunately, there is on good way for them to communicate with us securely once they are unable to communicate with us securely. I suppose some special word could be used to communicate that status --- that is how it was done in non-electronic communication in the past. I don't know how that really helps. If one of our committers is under duress, they probably cannot risk outing themselves anyway. The best defense, IMHO, is the fact that our source code is open and can be reviewed freely. The trick is to get folks to do the review. I know, for example, at $past_employer we had a requirement to get someone on our staff to review every single commit in order to maintain certain certifications. Of course there is no guarantee that such reviews would catch everything, but maybe we could establish post commit reviews by contributors in a more rigorous way? Granted, getting more qualified volunteers is not a trivial problem... -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: PostgreSQL 17 Release Management Team & Feature Freeze
On 4/8/24 11:05, Tom Lane wrote: Pavel Borisov writes: IMO the fact that people struggle to work on patches, and make them better, etc. is an immense blessing for the Postgres community. Is the peak of commits really a big problem provided we have 6 months before actual release? I doubt March patches tend to be worse than the November ones. Yes, it's a problem, and yes the average quality of last-minute patches is visibly worse than that of patches committed in a less hasty fashion. We have been through this every year for the last couple decades, seems like, and we keep re-learning that lesson the hard way. I'm just distressed at our utter failure to learn from experience. I don't dispute that we could do better, and this is just a simplistic look based on "number of commits per day", but the attached does put it in perspective to some extent. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: Security lessons from liblzma
On 3/31/24 11:49, Tom Lane wrote: Joe Conway writes: I am saying maybe those patches should be eliminated in favor of our tree including build options that would produce the same result. I don't really see how that can be expected to work sanely. It turns the responsibility for platform-specific build issues on its head, and it doesn't work at all for issues discovered after we make a release. The normal understanding of how you can vet a distro's package is that you look at the package contents (the SRPM in Red Hat world and whatever the equivalent concept is elsewhere), check that the contained tarball matches upstream and that the patches and build instructions look sane, and then build it locally and check for a match to the distro's binary package. Even if we could overcome the obstacles to putting the patch files into the upstream tarball, we're surely not going to include the build instructions, so we'd not have moved the needle very far in terms of whether the packager could do something malicious. True enough I guess. But it has always bothered me how many patches get applied to the upstream tarballs by the OS package builders. Some of them, e.g. glibc on RHEL 7, include more than 1000 patches that you would have to manually vet if you cared enough and had the skills. Last time I looked at the openssl package sources it was similar in volume and complexity. They might as well be called forks if everyone were being honest about it... I know our PGDG packages are no big deal compared to that, fortunately. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: Security lessons from liblzma
On 3/30/24 21:52, Bruce Momjian wrote: On Sat, Mar 30, 2024 at 07:54:00PM -0400, Joe Conway wrote: Virtually every RPM source, including ours, contains out of tree patches that get applied on top of the release tarball. At least for the PGDG packages, it would be nice to integrate them into our git repo as build options or whatever so that the packages could be built without any patches applied to it. Add a tarball that is signed and traceable back to the git tag, and we would be in a much better place than we are now. How would someone access the out-of-tree patches? I think Debian includes the patches in its source tarball. I am saying maybe those patches should be eliminated in favor of our tree including build options that would produce the same result. For example, these patches are applied to our release tarball files when the RPM is being built for pg16 on RHEL 9: - https://git.postgresql.org/gitweb/?p=pgrpms.git;a=blob;f=rpm/redhat/main/non-common/postgresql-16/main/postgresql-16-rpm-pgsql.patch;h=d9b6d12b7517407ac81352fa325ec91b05587641;hb=HEAD https://git.postgresql.org/gitweb/?p=pgrpms.git;a=blob;f=rpm/redhat/main/non-common/postgresql-16/main/postgresql-16-var-run-socket.patch;h=f2528efaf8f4681754b20283463eff3e14eedd39;hb=HEAD https://git.postgresql.org/gitweb/?p=pgrpms.git;a=blob;f=rpm/redhat/main/non-common/postgresql-16/main/postgresql-16-conf.patch;h=da28ed793232316dd81fdcbbe59a6479b054a364;hb=HEAD https://git.postgresql.org/gitweb/?p=pgrpms.git;a=blob;f=rpm/redhat/main/non-common/postgresql-16/main/postgresql-16-perl-rpath.patch;h=748c42f0ec2c9730af3143e90e5b205c136f40d9;hb=HEAD - Nothing too crazy, but wouldn't it be better if no patches were required at all? Ideally we should have reproducible builds so that starting with our tarball (which is traceable back to the git release tag) one can easily obtain the same binary as what the RPMs/DEBs deliver. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: Security lessons from liblzma
On 3/30/24 19:54, Joe Conway wrote: On 2024-03-30 16:50:26 -0400, Robert Haas wrote: or what Tom does when he builds the release tarballs. Tom follows this, at least last time I checked: https://wiki.postgresql.org/wiki/Release_process Reading through that, I wonder if this part is true anymore: In principle this could be done anywhere, but again there's a concern about reproducibility, since the results may vary depending on installed bison, flex, docbook, etc versions. Current practice is to always do this as pgsql on borka.postgresql.org, so it can only be done by people who have a login there. In detail: Maybe if we split out the docs from the release tarball, we could also add the script (mk-release) to our git repo? Some other aspects of that wiki page look out of date too. Perhaps it needs an overall update? Maybe Tom and/or Magnus could weigh in here. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: Security lessons from liblzma
On 3/30/24 17:12, Andres Freund wrote: Hi, On 2024-03-30 16:50:26 -0400, Robert Haas wrote: We might also want to move toward signing commits and tags. One of the meson maintainers was recommending that on-list not long ago. I don't know how valuable the commit signing really is, but I strongly agree that we should sign both tags and tarballs. +1 We should think about weaknesses that might occur during the packaging process, too. If someone who alleges that their packaging PG is really packaging PG w/badstuff123.patch, how would we catch that? I don't think we realistically can. The environment, configure and compiler options will influence things too much to do any sort of automatic verification. But that shouldn't stop us from ensuring that at least the packages distributed via *.postgresql.org are reproducibly built. Another good avenue for introducing an attack would be to propose some distro specific changes to the packaging teams - there's a lot fewer eyes there. I think it might be worth working with some of our packagers to integrate more of their changes into our tree. Huge +1 to that. I have thought many times, and even said to Devrim, a huge number of people trust him to not be evil. Virtually every RPM source, including ours, contains out of tree patches that get applied on top of the release tarball. At least for the PGDG packages, it would be nice to integrate them into our git repo as build options or whatever so that the packages could be built without any patches applied to it. Add a tarball that is signed and traceable back to the git tag, and we would be in a much better place than we are now. I can't for example verify what the infrastructure team is doing, Not sure what you feel like you should be able to follow -- anything specific? or what Tom does when he builds the release tarballs. Tom follows this, at least last time I checked: https://wiki.postgresql.org/wiki/Release_process This one however, I think we could improve upon. Making sure the tarball generation is completely binary reproducible and providing means of checking that would surely help. This will be a lot easier if we, as dicussed elsewhere, I believe, split out the generated docs into a separately downloadable archive. We already stopped including other generated files recently. again, big +1 We shouldn't make the mistake of assuming that bad things can't happen to us. +1 and again with the +1 ;-) -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: Popcount optimization using AVX512
On 3/25/24 11:12, Tom Lane wrote: "Amonson, Paul D" writes: I am re-posting the patches as CI for Mac failed (CI error not code/test error). The patches are the same as last time. Just for a note --- the cfbot will re-test existing patches every so often without needing a bump. The current cycle period seems to be about two days. Just an FYI -- there seems to be an issue with all three of the macos cfbot runners (mine included). I spent time over the weekend working with Thomas Munro (added to CC list) trying different fixes to no avail. Help from macos CI wizards would be gratefully accepted... -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: Possibility to disable `ALTER SYSTEM`
On 3/19/24 07:49, Andrew Dunstan wrote: On Tue, Mar 19, 2024 at 5:26 AM Heikki Linnakangas <mailto:hlinn...@iki.fi>> wrote: I want to remind everyone of this from Gabriele's first message that started this thread: > At the moment, a possible workaround is that `ALTER SYSTEM` can be blocked > by making the postgresql.auto.conf read only, but the returned message is > misleading and that’s certainly bad user experience (which is very > important in a cloud native environment): > > > ``` > postgres=# ALTER SYSTEM SET wal_level TO minimal; > ERROR: could not open file "postgresql.auto.conf": Permission denied > ``` I think making the config file read-only is a fine solution. If you don't want postgres to mess with the config files, forbid it with the permission system. Problems with pg_rewind, pg_basebackup were mentioned with that approach. I think if you want the config files to be managed outside PostgreSQL, by kubernetes, patroni or whatever, it would be good for them to be read-only to the postgres user anyway, even if we had a mechanism to disable ALTER SYSTEM. So it would be good to fix the problems with those tools anyway. The error message is not great, I agree with that. Can we improve it? Maybe just add a HINT like this: postgres=# ALTER SYSTEM SET wal_level TO minimal; ERROR: could not open file "postgresql.auto.conf" for writing: Permission denied HINT: Configuration might be managed outside PostgreSQL Perhaps we could make that even better with a GUC though. I propose a GUC called 'configuration_managed_externally = true / false". If you set it to true, we prevent ALTER SYSTEM and make the error message more definitive: postgres=# ALTER SYSTEM SET wal_level TO minimal; ERROR: configuration is managed externally As a bonus, if that GUC is set, we could even check at server startup that all the configuration files are not writable by the postgres user, and print a warning or refuse to start up if they are. (Another way to read this proposal is to rename the GUC that's been discussed in this thread to 'configuration_managed_externally'. That makes it look less like a security feature, and describes the intended use case.) I agree with pretty much all of this. +1 me too. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: Improving contrib/tablefunc's error reporting
On 3/9/24 15:39, Tom Lane wrote: Joe Conway writes: On 3/9/24 13:07, Tom Lane wrote: BTW, while I didn't touch it here, it seems fairly bogus that connectby() checks both type OID and typmod for its output columns while crosstab() only checks type OID. I think crosstab() is in the wrong and needs to be checking typmod. That might be fit material for a separate patch though. Something like the attached what you had in mind? (applies on top of your two patches) Yeah, exactly. As far as the comment change goes: - * - attribute [1] of the sql tuple is the category; no need to check it - - * attribute [2] of the sql tuple should match attributes [1] to [natts] + * attribute [1] of the sql tuple is the category; no need to check it + * attribute [2] of the sql tuple should match attributes [1] to [natts - 1] * of the return tuple I suspect that this block looked better when originally committed but didn't survive contact with pgindent. You should check whether your version will; if not, some dashes on the /* line will help. Thanks for the review and heads up. I fiddled with it a bit to make it pgindent clean. I saw you commit your patches so I just pushed mine. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: Improving contrib/tablefunc's error reporting
On 3/9/24 13:07, Tom Lane wrote:
Joe Conway writes:
On 3/5/24 17:04, Tom Lane wrote:
After reading the thread at [1], I could not escape the feeling
that contrib/tablefunc's error reporting is very confusing.
The changes all look good to me and indeed more consistent with the docs.
Do you want me to push these? If so, development tip only, or backpatch?
I can push that. I was just thinking HEAD, we aren't big on changing
error reporting in back branches.
BTW, while I didn't touch it here, it seems fairly bogus that
connectby() checks both type OID and typmod for its output
columns while crosstab() only checks type OID. I think
crosstab() is in the wrong and needs to be checking typmod.
That might be fit material for a separate patch though.
I can take a look at this. Presumably this would not be for backpatching.
I'm not sure whether that could produce results bad enough to be
called a bug or not. But I too would lean towards not back-patching,
in view of the lack of field complaints.
Something like the attached what you had in mind? (applies on top of
your two patches)
--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Make tablefunc crosstab() check typmod too
tablefunc connectby() checks both type OID and typmod for its output
columns while crosstab() only checks type OID. Fix that by makeing
the crosstab() check look more like the connectby() check.
--- tablefunc.c.v0002 2024-03-09 14:38:29.285393890 -0500
+++ tablefunc.c 2024-03-09 14:43:47.021399855 -0500
@@ -1527,10 +1527,10 @@
compatCrosstabTupleDescs(TupleDesc ret_tupdesc, TupleDesc sql_tupdesc)
{
int i;
- Form_pg_attribute ret_attr;
Oid ret_atttypid;
- Form_pg_attribute sql_attr;
Oid sql_atttypid;
+ int32 ret_atttypmod;
+ int32 sql_atttypmod;
if (ret_tupdesc->natts < 2)
ereport(ERROR,
@@ -1539,34 +1539,39 @@
errdetail("Return row must have at least two columns.")));
Assert(sql_tupdesc->natts == 3); /* already checked by caller */
- /* check the rowid types match */
+ /* check the row_name types match */
ret_atttypid = TupleDescAttr(ret_tupdesc, 0)->atttypid;
sql_atttypid = TupleDescAttr(sql_tupdesc, 0)->atttypid;
- if (ret_atttypid != sql_atttypid)
+ ret_atttypmod = TupleDescAttr(ret_tupdesc, 0)->atttypmod;
+ sql_atttypmod = TupleDescAttr(sql_tupdesc, 0)->atttypmod;
+ if (ret_atttypid != sql_atttypid ||
+ (ret_atttypmod >= 0 && ret_atttypmod != sql_atttypmod))
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg("invalid crosstab return type"),
errdetail("Source row_name datatype %s does not match return row_name datatype %s.",
- format_type_be(sql_atttypid),
- format_type_be(ret_atttypid;
+ format_type_with_typemod(sql_atttypid, sql_atttypmod),
+ format_type_with_typemod(ret_atttypid, ret_atttypmod;
/*
- * - attribute [1] of the sql tuple is the category; no need to check it -
- * attribute [2] of the sql tuple should match attributes [1] to [natts]
+ * attribute [1] of the sql tuple is the category; no need to check it
+ * attribute [2] of the sql tuple should match attributes [1] to [natts - 1]
* of the return tuple
*/
- sql_attr = TupleDescAttr(sql_tupdesc, 2);
+ sql_atttypid = TupleDescAttr(sql_tupdesc, 2)->atttypid;
+ sql_atttypmod = TupleDescAttr(sql_tupdesc, 2)->atttypmod;
for (i = 1; i < ret_tupdesc->natts; i++)
{
- ret_attr = TupleDescAttr(ret_tupdesc, i);
-
- if (ret_attr->atttypid != sql_attr->atttypid)
+ ret_atttypid = TupleDescAttr(ret_tupdesc, i)->atttypid;
+ ret_atttypmod = TupleDescAttr(ret_tupdesc, i)->atttypmod;
+ if (ret_atttypid != sql_atttypid ||
+ (ret_atttypmod >= 0 && ret_atttypmod != sql_atttypmod))
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg("invalid crosstab return type"),
errdetail("Source value datatype %s does not match return value datatype %s in column %d.",
- format_type_be(sql_attr->atttypid),
- format_type_be(ret_attr->atttypid),
+ format_type_with_typemod(sql_atttypid, sql_atttypmod),
+ format_type_with_typemod(ret_atttypid, ret_atttypmod),
i + 1)));
}
Re: Improving contrib/tablefunc's error reporting
On 3/5/24 17:04, Tom Lane wrote: After reading the thread at [1], I could not escape the feeling that contrib/tablefunc's error reporting is very confusing. Looking into the source code, I soon found that it is also very inconsistent, with similar error reports being phrased quite differently. The terminology for column names doesn't match the SGML docs either. And there are some places that are so confused about whether they are complaining about the calling query or the called query that the output is flat-out backwards. So at that point my nascent OCD wouldn't let me rest without fixing it. Here's a quick patch series to do that. For review purposes, I split this into two patches. 0001 simply adds some more test cases to reach currently-unexercised error reports. Then 0002 makes my proposed code changes and shows how the existing error messages change. I'm not necessarily wedded to the phrasings I used here, in case anyone has better ideas. The changes all look good to me and indeed more consistent with the docs. Do you want me to push these? If so, development tip only, or backpatch? BTW, while I didn't touch it here, it seems fairly bogus that connectby() checks both type OID and typmod for its output columns while crosstab() only checks type OID. I think crosstab() is in the wrong and needs to be checking typmod. That might be fit material for a separate patch though. I can take a look at this. Presumably this would not be for backpatching. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: Emitting JSON to file using COPY TO
On 3/8/24 12:28, Andrey M. Borodin wrote: Hello everyone! Thanks for working on this, really nice feature! On 9 Jan 2024, at 01:40, Joe Conway wrote: Thanks -- will have a look Joe, recently folks proposed a lot of patches in this thread that seem like diverted from original way of implementation. As an author of CF entry [0] can you please comment on which patch version needs review? I don't know if I agree with the proposed changes, but I have also been waiting to see how the parallel discussion regarding COPY extensibility shakes out. And there were a couple of issues found that need to be tracked down. Additionally I have had time/availability challenges recently. Overall, chances seem slim that this will make it into 17, but I have not quite given up hope yet either. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: Improving contrib/tablefunc's error reporting
On 3/5/24 17:04, Tom Lane wrote: After reading the thread at [1], I could not escape the feeling that contrib/tablefunc's error reporting is very confusing. Looking into the source code, I soon found that it is also very inconsistent, with similar error reports being phrased quite differently. The terminology for column names doesn't match the SGML docs either. And there are some places that are so confused about whether they are complaining about the calling query or the called query that the output is flat-out backwards. So at that point my nascent OCD wouldn't let me rest without fixing it. Here's a quick patch series to do that. For review purposes, I split this into two patches. 0001 simply adds some more test cases to reach currently-unexercised error reports. Then 0002 makes my proposed code changes and shows how the existing error messages change. I'm not necessarily wedded to the phrasings I used here, in case anyone has better ideas. BTW, while I didn't touch it here, it seems fairly bogus that connectby() checks both type OID and typmod for its output columns while crosstab() only checks type OID. I think crosstab() is in the wrong and needs to be checking typmod. That might be fit material for a separate patch though. Been a long time since I gave contrib/tablefunc any love I guess ;-) I will have a look at your patches, and the other issue you mention, but it might be a day or three before I can give it some quality time. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
PostgreSQL Contributors Updates
All, The PostgreSQL Contributor Page (https://www.postgresql.org/community/contributors/) includes people who have made substantial, long-term contributions of time and effort to the PostgreSQL project. The PostgreSQL Contributors Team recognizes the following people for their contributions. New PostgreSQL Contributors: * Bertrand Drouvot * Gabriele Bartolini * Richard Guo New PostgreSQL Major Contributors: * Alexander Lakhin * Daniel Gustafsson * Dean Rasheed * John Naylor * Melanie Plageman * Nathan Bossart Thank you and congratulations to all! Thanks, On behalf of the PostgreSQL Contributors Team -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: An improved README experience for PostgreSQL
On 2/28/24 14:36, Tom Lane wrote: Nathan Bossart writes: On Wed, Feb 28, 2024 at 01:08:03PM -0600, Nathan Bossart wrote: -PostgreSQL Database Management System -= +# PostgreSQL Database Management System This change can be omitted, which makes the conversion even simpler. That's a pretty convincing proof-of-concept. Let's just do this, and then make sure to keep the file legible as plain text. +1 -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: An improved README experience for PostgreSQL
On 2/28/24 12:25, Daniel Gustafsson wrote: On 28 Feb 2024, at 18:02, Nathan Bossart wrote: On Wed, Feb 28, 2024 at 02:46:11PM +0100, Daniel Gustafsson wrote: On 26 Feb 2024, at 21:30, Tom Lane wrote: Nathan Bossart writes: I think this would be nice. If the Markdown version is reasonably readable as plain-text, maybe we could avoid maintaining two READMEs files, too. But overall, +1 to modernizing the README a bit. Per past track record, we change the top-level README only once every three years or so, so I doubt it'd be too painful to maintain two versions of it. It wont be, and we kind of already have two since there is another similar README displayed at https://www.postgresql.org/ftp/. That being said, a majority of those reading the README will likely be new developers accustomed to Markdown (or doing so via interfaces such as Github) so going to Markdown might not be a bad idea. We can also render a plain text version with pandoc for release builds should we want to. Sorry, my suggestion wasn't meant to imply that I have any strong concerns about maintaining two README files. If we can automate generating one or the other, that'd be great, but I don't see that as a prerequisite to adding a Markdown version. Agreed, and I didn't say we should do it but rather that we can do it based on the toolchain we already have. Personally I think just having a Markdown version is enough, it's become the de facto standard for such documentation for good reasons. +1 Markdown is pretty readable as text, I'm not sure why we need both. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: PGC_SIGHUP shared_buffers?
On 2/19/24 13:13, Andres Freund wrote: On 2024-02-19 09:19:16 -0500, Joe Conway wrote: Couldn't we scale the rounding, e.g. allow small allocations as we do now, but above some number always round? E.g. maybe >= 2GB round to the nearest 256MB, >= 4GB round to the nearest 512MB, >= 8GB round to the nearest 1GB, etc? That'd make the translation considerably more expensive. Which is important, given how common an operation this is. Perhaps it is not practical, doesn't help, or maybe I misunderstand, but my intent was that the rounding be done/enforced when setting the GUC value which surely cannot be that often. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: PGC_SIGHUP shared_buffers?
On 2/18/24 15:35, Andres Freund wrote: On 2024-02-18 17:06:09 +0530, Robert Haas wrote: How many people set shared_buffers to something that's not a whole number of GB these days? I'd say the vast majority of postgres instances in production run with less than 1GB of s_b. Just because numbers wise the majority of instances are running on small VMs and/or many PG instances are running on one larger machine. There are a lot of instances where the total available memory is less than 2GB. I mean I bet it happens, but in practice if you rounded to the nearest GB, or even the nearest 2GB, I bet almost nobody would really care. I think it's fine to be opinionated here and hold the line at a relatively large granule, even though in theory people could want something else. I don't believe that at all unfortunately. Couldn't we scale the rounding, e.g. allow small allocations as we do now, but above some number always round? E.g. maybe >= 2GB round to the nearest 256MB, >= 4GB round to the nearest 512MB, >= 8GB round to the nearest 1GB, etc? -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: Replace current implementations in crypt() and gen_salt() to OpenSSL
On 2/16/24 04:16, Daniel Gustafsson wrote:
On 15 Feb 2024, at 16:49, Peter Eisentraut wrote:
1. All the block ciphers currently supported by crypt() and gen_salt() are not
FIPS-compliant.
2. The crypt() and gen_salt() methods built on top of them (modes of operation,
kind of) are not FIPS-compliant.
I wonder if it's worth trying to make pgcrypto disallow non-FIPS compliant
ciphers when the compiled against OpenSSL is running with FIPS mode enabled, or
raise a WARNING when used? It seems rather unlikely that someone running
OpenSSL with FIPS=yes want to use our DES cipher without there being an error
or misconfiguration somewhere.
Something like the below untested pseudocode.
diff --git a/contrib/pgcrypto/pgcrypto.c b/contrib/pgcrypto/pgcrypto.c
index 96447c5757..3d4391ebe1 100644
--- a/contrib/pgcrypto/pgcrypto.c
+++ b/contrib/pgcrypto/pgcrypto.c
@@ -187,6 +187,14 @@ pg_crypt(PG_FUNCTION_ARGS)
*resbuf;
text *res;
+#if defined FIPS_mode
+ if (FIPS_mode())
+#else
+ if
(EVP_default_properties_is_fips_enabled(OSSL_LIB_CTX_get0_global_default()))
+#endif
+ ereport(ERROR,
+ (errmsg("not available when using OpenSSL in FIPS
mode")));
Makes sense +1
--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Re: pgjdbc is not working with PKCS8 certificates with password
On 2/7/24 06:42, just madhu wrote:
On further investigation,
/With certificate generated as below. JDBC connection is successful./
openssl pkcs8 -topk8 -inform PEM -in client.key -outform DER -out
client.pk8 -passout pass:foobar / -v1 PBE-MD5-DES
But a connection from pgAdmin (connection failed:
\SSLCerts\pk8_pass\client_pass_PBE.pk8": no start line) and psql(psql:
error: could not load private key file "client_pass_PBE.pk8":
unsupported) is failing
Is there a common way in which certificate with passwords can be
created for both libpq and jdbc ?
You may want to check with the pgjdbc project on github rather than (or
in addition to?) here; see:
https://github.com/pgjdbc/pgjdbc/issues
Joe
On Wed, Feb 7, 2024 at 3:17 PM just madhu <mailto:justvma...@gmail.com>> wrote:
Hi ,
postgresql-42.7.1.jar
Trying to use establish a connection using PKCS8 certificate created
with password.
/openssl pkcs8 -topk8 -inform PEM -in client.key -outform DER -out
client.pk8 -passout pass:foobar
/
I set the properties as below:
/.../
/sslProperties.setProperty("sslkey", "client.pk8");
sslProperties.setProperty("sslpassword","foobar");/
/.../
/Connection connection = DriverManager.getConnection(jdbcUrl,
sslProperties);
/
//
/This is failing with the error:/
/org.postgresql.util.PSQLException: SSL error: Connection reset
at org.postgresql.ssl.MakeSSL.convert(MakeSSL.java:43)
at
org.postgresql.core.v3.ConnectionFactoryImpl.enableSSL(ConnectionFactoryImpl.java:584)
at
org.postgresql.core.v3.ConnectionFactoryImpl.tryConnect(ConnectionFactoryImpl.java:168)
/
/.../
Regards,
Madhu
--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Re: [17] CREATE SUBSCRIPTION ... SERVER
On 1/12/24 20:17, Jeff Davis wrote: On Fri, 2024-01-05 at 16:11 +0530, Ashutosh Bapat wrote: I don't think we need to add a test for every FDW. E.g. adding a test in file_fdw would be pointless. But postgres_fdw is special. The test could further create a foreign table ftab_foo on subscriber referencing foo on publisher and then compare the data from foo and ftab_foo to make sure that the replication is happening. This will serve as a good starting point for replicated tables setup in a sharded cluster. Attached updated patch set with added TAP test for postgres_fdw, which uses a postgres_fdw server as the source for subscription connection information. I think 0004 needs a bit more work, so I'm leaving it off for now, but I'll bring it back in the next patch set. I took a quick scan through the patch. The only thing that jumped out at me was that it seems like it might make sense to use quote_literal_cstr() rather than defining your own appendEscapedValue() function? -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: System username in pg_stat_activity
On 1/10/24 08:59, Magnus Hagander wrote: On Wed, Jan 10, 2024 at 2:56 PM Bertrand Drouvot I think it depends what we want the new field to reflect. If it is the exact same thing as the SYSTEM_USER then I think it has to be text (as the SYSTEM_USER is made of "auth_method:identity"). Now if we want it to be "only" the identity part of it, then the `name` datatype would be fine. I'd vote for the exact same thing as the SYSTEM_USER (means auth_method:identity). I definitely think it should be the same. If it's not exactly the same, then it should be *two* columns, one with auth method and one with the name. And thinking more about it maybe that's cleaner, because that makes it easier to do things like filter based on auth method? +1 for the overall feature and +1 for two columns > + > + > + authname name > + > + > + The authentication method and identity (if any) that the user > + used to log in. It contains the same value as > +returns in the backend. > + > + I'm fine with auth_method:identity. > +S.authname, What about using system_user as the field name? (because if we keep auth_method:identity it's not really the authname anyway). I was worried system_user or sysuser would both be confusing with the fact that we have usesysid -- which would reference a *different* sys... I think if it is exactly "system_user" it would be pretty clearly a match for SYSTEM_USER -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: Emitting JSON to file using COPY TO
On 1/8/24 14:36, Dean Rasheed wrote: On Thu, 7 Dec 2023 at 01:10, Joe Conway wrote: The attached should fix the CopyOut response to say one column. Playing around with this, I found a couple of cases that generate an error: COPY (SELECT 1 UNION ALL SELECT 2) TO stdout WITH (format json); COPY (VALUES (1), (2)) TO stdout WITH (format json); both of those generate the following: ERROR: record type has not been registered Thanks -- will have a look -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: Password leakage avoidance
On 1/6/24 15:10, Tom Lane wrote:
Joe Conway writes:
The only code specific comments were Tom's above, which have been
addressed. If there are no serious objections I plan to commit this
relatively soon.
I had not actually read this patchset before, but now I have, and
I have a few minor suggestions:
Many thanks!
* The API comment for PQchangePassword should specify that encryption
is done according to the server's password_encryption setting, and
probably the SGML docs should too. You shouldn't have to read the
code to discover that.
Check
* I don't especially care for the useless initializations of
encrypted_password, fmtpw, and fmtuser. In all those cases the
initial NULL is immediately replaced by a valid value. Probably
the compiler will figure out that the initializations are useless,
but human readers have to do so as well. Moreover, I think this
style is more bug-prone not less so, because if you ever change
the logic in a way that causes some code paths to fail to set
the variables, you won't get use-of-possibly-uninitialized-value
warnings from the compiler.
* Perhaps move the declaration of "buf" to the inner block where
it's actually used?
Makes sense -- fixed
* This could be shortened to just "return res":
+ if (!res)
+ return NULL;
+ else
+ return res;
Heh, apparently I needed more coffee at this point :-)
* I'd make the SGML documentation a bit more explicit about the
return value, say
+ Returns a PGresult pointer representing
+ the result of the ALTER USER command, or
+ a null pointer if the routine failed before issuing any command.
Fixed.
I also ran pgindent. I was kind of surprised/unhappy when it made me
change this (which aligned the two var names):
8<
PQExpBufferDatabuf;
PGresult*res;
8<
to this (which leaves the var names unaligned):
8<
PQExpBufferDatabuf;
PGresult*res;
8<----
Anyway, the resulting adjustments attached.
--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt
index 850734a..28b861f 100644
*** a/src/interfaces/libpq/exports.txt
--- b/src/interfaces/libpq/exports.txt
*** PQclosePrepared 188
*** 191,193
--- 191,194
PQclosePortal 189
PQsendClosePrepared 190
PQsendClosePortal 191
+ PQchangePassword 192
diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index 912aa14..dcb7c9f 100644
*** a/src/interfaces/libpq/fe-auth.c
--- b/src/interfaces/libpq/fe-auth.c
*** PQencryptPasswordConn(PGconn *conn, cons
*** 1372,1374
--- 1372,1455
return crypt_pwd;
}
+
+ /*
+ * PQchangePassword -- exported routine to change a password
+ *
+ * This is intended to be used by client applications that wish to
+ * change the password for a user. The password is not sent in
+ * cleartext because it is encrypted on the client side. This is
+ * good because it ensures the cleartext password is never known by
+ * the server, and therefore won't end up in logs, pg_stat displays,
+ * etc. The password encryption is performed by PQencryptPasswordConn(),
+ * which is passed a NULL for the algorithm argument. Hence encryption
+ * is done according to the server's password_encryption
+ * setting. We export the function so that clients won't be dependent
+ * on the implementation specific details with respect to how the
+ * server changes passwords.
+ *
+ * Arguments are a connection object, the SQL name of the target user,
+ * and the cleartext password.
+ *
+ * Return value is the PGresult of the executed ALTER USER statement
+ * or NULL if we never get there. The caller is responsible to PQclear()
+ * the returned PGresult.
+ *
+ * PQresultStatus() should be called to check the return value for errors,
+ * and PQerrorMessage() used to get more information about such errors.
+ */
+ PGresult *
+ PQchangePassword(PGconn *conn, const char *user, const char *passwd)
+ {
+ char *encrypted_password = PQencryptPasswordConn(conn, passwd,
+ user, NULL);
+
+ if (!encrypted_password)
+ {
+ /* PQencryptPasswordConn() already registered the error */
+ return NULL;
+ }
+ else
+ {
+ char *fmtpw = PQescapeLiteral(conn, encrypted_password,
+ strlen(encrypted_password));
+
+ /* no longer needed, so clean up now */
+ PQfreemem(encrypted_password);
+
+ if (!fmtpw)
+ {
+ /* PQescapeLiteral() already registered the error */
+ return NULL;
+ }
+ else
+ {
+ char *fmtuser = PQescapeIdentifier(conn, user, strlen(user));
+
+ if (!fmtuser)
+ {
+
Re: Password leakage avoidance
On 1/6/24 13:16, Sehrope Sarkuni wrote: On Sat, Jan 6, 2024 at 12:39 PM Joe Conway <mailto:m...@joeconway.com>> wrote: The only code specific comments were Tom's above, which have been addressed. If there are no serious objections I plan to commit this relatively soon. One more thing that we do in pgjdbc is to zero out the input password args so that they don't remain in memory even after being freed. It's kind of odd in Java as it makes the input interface a char[] and we have to convert them to garbage collected Strings internally (which kind of defeats the purpose of the exercise). But in libpq could be done via something like: memset(pw1, 0, strlen(pw1)); memset(pw2, 0, strlen(pw2)); That part is in psql not libpq There was some debate on our end of where to do that and we settled on doing it inside the encoding functions to ensure it always happens. So the input password char[] always gets wiped regardless of how the encoding functions are invoked. Even if it's not added to the password encoding functions (as that kind of changes the after effects if anything was relying on the password still having the password), I think it'd be good to add it to the command.c stuff that has the two copies of the password prior to freeing them. While that change might or might not be worthwhile, I see it as independent of this patch. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: Password leakage avoidance
On 12/24/23 10:14, Joe Conway wrote:
On 12/23/23 11:00, Tom Lane wrote:
Joe Conway writes:
The attached patch set moves the guts of \password from psql into the
libpq client side -- PQchangePassword() (patch 0001).
Haven't really read the patch, just looked at the docs, but here's
a bit of bikeshedding:
Thanks!
* This seems way too eager to promote the use of md5. Surely the
default ought to be SCRAM, full stop. I question whether we even
need an algorithm parameter. Perhaps it's a good idea for
future-proofing, but we could also plan that the function would
make its own decisions based on noting the server's version.
(libpq is far more likely to be au courant about what to do than
the calling application, IMO.)
* Parameter order seems a bit odd: to me it'd be natural to write
user before password.
* Docs claim return type is char *, but then say bool (which is
also what the code says). We do not use bool in libpq's API;
the admittedly-hoary convention is to use int with 1/0 values.
Rather than quibble about that, though, I wonder if we should
make the result be the PGresult from the command, which the
caller would be responsible to free. That would document error
conditions much more flexibly. On the downside, client-side
errors would be harder to report, particularly OOM, but I think
we have solutions for that in existing functions.
* The whole business of nonblock mode seems fairly messy here,
and I wonder whether it's worth the trouble to support. If we
do want to claim it works then it ought to be tested.
All of these (except for the doc "char *" cut-n-pasteo) were due to
trying to stay close to the same interface as PQencryptPasswordConn().
But I agree with your assessment and the attached patch series addresses
all of them.
The original use of PQencryptPasswordConn() in psql passed a NULL for
the algorithm, so I dropped that argument entirely. I also swapped user
and password arguments because as you pointed out that is more natural.
This version returns PGresult. As far as special handling for
client-side errors like OOM, I don't see anything beyond returning a
NULL to signify fatal error, e,g,:
8<--
PGresult *
PQmakeEmptyPGresult(PGconn *conn, ExecStatusType status)
{
PGresult *result;
result = (PGresult *) malloc(sizeof(PGresult));
if (!result)
return NULL;
8<--
That is the approach I took.
One thing I have not done but, considered, is adding an additional
optional parameter to allow "VALID UNTIL" to be set. Seems like it would
be useful to be able to set an expiration when setting a new password.
No strong opinion about that.
Thanks -- hopefully others will weigh in on that.
I just read through the thread and my conclusion is that, specifically
related to this patch set (i.e. notwithstanding suggestions for related
features), there is consensus in favor of adding this feature.
The only code specific comments were Tom's above, which have been
addressed. If there are no serious objections I plan to commit this
relatively soon.
--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt
index 850734a..28b861f 100644
*** a/src/interfaces/libpq/exports.txt
--- b/src/interfaces/libpq/exports.txt
*** PQclosePrepared 188
*** 191,193
--- 191,194
PQclosePortal 189
PQsendClosePrepared 190
PQsendClosePortal 191
+ PQchangePassword 192
diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index 912aa14..a6897f8 100644
*** a/src/interfaces/libpq/fe-auth.c
--- b/src/interfaces/libpq/fe-auth.c
*** PQencryptPasswordConn(PGconn *conn, cons
*** 1372,1374
--- 1372,1459
return crypt_pwd;
}
+
+ /*
+ * PQchangePassword -- exported routine to change a password
+ *
+ * This is intended to be used by client applications that wish to
+ * change the password for a user. The password is not sent in
+ * cleartext because it is encrypted on the client side. This is
+ * good because it ensures the cleartext password is never known by
+ * the server, and therefore won't end up in logs, pg_stat displays,
+ * etc. We export the function so that clients won't be dependent
+ * on the implementation specific details with respect to how the
+ * server changes passwords.
+ *
+ * Arguments are a connection object, the SQL name of the target user,
+ * and the cleartext password.
+ *
+ * Return value is the PGresult of the executed ALTER USER statement
+ * or NULL if we never get there. The caller is responsible to PQclear()
+ * the returned PGresult.
+ *
+ * PQresultStatus() should be called to check the return value for errors,
+ * and PQerrorMessage() used
Re: Password leakage avoidance
On 1/2/24 07:23, Sehrope Sarkuni wrote: 1. There's two sets of defaults, the client program's default and the server's default. Need to pick one for each implemented function. They don't need to be the same across the board. Is there a concrete recommendation here? 2. Password encoding should be split out and made available as its own functions. Not just as part of a wider "create _or_ alter a user's password" function attached to a connection. It already is split out in libpq[1], unless I don't understand you correctly. [1] https://www.postgresql.org/docs/current/libpq-misc.html#LIBPQ-PQENCRYPTPASSWORDCONN We went a step further and added an intermediate function that generates the "ALTER USER ... PASSWORD" SQL. I don't think we want that in libpq, but in any case separate patch/discussion IMHO. 3. We only added an "ALTER USER ... PASSWORD" function, not anything to create a user. There's way too many options for that and keeping this targeted at just assigning passwords makes it much easier to test. +1 Also separate patch/discussion, but I don't think the CREATE version is needed. 4. RE:defaults, the PGJDBC approach is that the encoding-only function uses the driver's default (SCRAM). The "alterUserPassword(...)" uses the server's default (again usually SCRAM for modern installs but could be something else). It's kind of odd that they're different but the use cases are different as well. Since PQencryptPasswordConn() already exists, and psql's "\password" used it with its defaults, I don't think we want to change the behavior. The patch as written behaves in a backward compatible way. 5. Our SCRAM specific function allows for customizing the algo iteration and salt parameters. That topic came up on hackers previously[1]. Our high level "alterUserPassword(...)" function does not have those options but it is available as part of our PasswordUtil SCRAM API for advanced users who want to leverage it. The higher level functions have defaults for iteration counts (4096) and salt size (16-bytes). Again separate patch/discussion, IMHO. 6. Getting the verbiage right for the PGJDBC version was kind of annoying as we wanted to match the server's wording, e.g. "password_encryption", but it's clearly hashing, not encryption. We settled on "password encoding" for describing the overall task with the comments referencing the server's usage of the term "password_encryption". Found a similar topic[2] on changing that recently as well but looks like that's not going anywhere. Really this is irrelevant to this discussion, because the new function is called PQchangePassword(). The function PQencryptPasswordConn() has been around for a while and the horse is out of the gate on that one. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: BUG #17946: LC_MONETARY & DO LANGUAGE plperl - BUG
On 1/5/24 12:56, Robert Haas wrote: On Sun, Aug 27, 2023 at 4:25 PM Heikki Linnakangas wrote: I think you got that it backwards. 'perl_locale_obj' is set to the perl interpreter's locale, whenever we are *outside* the interpreter. This thread has had no update for more than 4 months, so I'm marking the CF entry RwF for now. It can always be reopened, if Joe or Tristan or Heikki or someone else picks it up again. It is definitely a bug, so I do plan to get back to it at some point, hopefully sooner rather than later... -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: SET ROLE x NO RESET
On 12/30/23 17:19, Michał Kłeczek wrote: On 30 Dec 2023, at 17:16, Eric Hanson wrote: What do you think of adding a NO RESET option to the SET ROLE command? What I proposed some time ago is SET ROLE … GUARDED BY ‘password’, so that you could later: RESET ROLE WITH ‘password' I like that too, but see it as a separate feature. FWIW that is also supported by the set_user extension referenced elsewhere on this thread. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: Build versionless .so for Android
On 12/31/23 01:24, Michael Paquier wrote: On Sun, Dec 31, 2023 at 06:08:04AM +0100, Matthias Kuhn wrote: I was wondering if there are a) any comments on the approach and if I should be handed in for a commitfest (currently waiting for the cooldown period after account activation, I am not sure how long that is) Such discussions are adapted in a commit fest entry. No idea if there is a cooldown period after account creation before being able to touch the CF app contents, though. FWIW I have expedited the cooldown period, so Matthias can log in right away. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: SET ROLE x NO RESET
On 12/30/23 11:16, Eric Hanson wrote: Hi, What do you think of adding a NO RESET option to the SET ROLE command? Right now Postgres can enforce data security with roles and RLS, but role-per-end-user doesn't really scale: Db connections are per-role, so a connection pooler can't share connections across users. We can work around this with policies that use session variables and checks against current_user, but it seems like role-per end user would be more beautiful. If SET ROLE had a NO RESET option, you could connect through a connection pooler as a privileged user, but downgrade to the user's role for the duration of the session. +1 I agree this would be useful. In the meantime, in case it helps, see https://github.com/pgaudit/set_user Specifically set_session_auth(text): - When set_session_auth(text) is called, the effective session and current user is switched to the rolename supplied, irrevocably. Unlike set_user() or set_user_u(), it does not affect logging nor allowed statements. If set_user.exit_on_error is "on" (the default), and any error occurs during execution, a FATAL error is thrown and the backend session exits. ----- -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
