Re: Adding SHOW CREATE TABLE
On Sat, May 20, 2023 at 2:33 PM Stephen Frost wrote: > Greetings, > > On Sat, May 20, 2023 at 13:32 David G. Johnston < > david.g.johns...@gmail.com> wrote: > >> On Sat, May 20, 2023 at 10:26 AM Stephen Frost >> wrote: >> >>> > A server function can be conveniently called from any client code. >>> >>> Clearly any client using libpq can conveniently call code which is in >>> libpq. >>> >> >> Clearly there are clients that don't use libpq. JDBC comes to mind. >> > > Indeed … as I mentioned up-thread already. > > Are we saying that we want this to be available server side, and largely > duplicated, specifically to cater to non-libpq users? I’ll put out there, > again, the idea that perhaps we put it into the common library then and > make it available via both libpq and as a server side function ..? > > We also have similar code in postgres_fdw.. ideally, imv anyway, we’d not > end up with three copies of it. > > Thanks, > > Stephen > First, as the person chasing this down, and a JDBC user, I really would prefer pg_get_tabledef() as Laurenz mentioned. Next, I have reviewed all 3 implementations (pg_dump [most appropriate], psql \d (very similar), and the FDW which is "way off", since it actually focuses on "CREATE FOREIGN TABLE" exclusively, and already fails to handle many pieces not required in creating a "real" table, as it creates a "reflection" of table. I am using pg_dump as my source of truth. But I noticed it does not create "TEMPORARY" tables with that syntax. [Leading to a question on mutating the pg_temp_# schema name back to pg_temp. or just stripping it, in favor of the TEMPORARY] I was surprised to see ~ 2,000 lines of code in the FDW and in psql... Whereas pg_dump is shorter because it gets more detailed table information in a structure passed in. I would love to leverage existing code, in the end. But I want to take my time on this, and become intimate with the details. Each of the above 3 approaches have different goals. And I would prefer the lowest risk:reward possible, and the least expensive maintenance. Having it run server side hides a ton of details, and as Tom pointed out, obviates DDL versioning control for other server versions. Thanks for the references to the old discussions. I have queued them up to review. Kirk...
Re: Naming of gss_accept_deleg
On Sat, May 20, 2023 at 11:21:57PM -0400, Tom Lane wrote: > Nathan Bossart writes: >> The buildfarm and cfbot seem unhappy with 9c0a0e2. It looks like there are >> a few remaining uses of gss_accept_deleg to rename. I'm planning to commit >> the attached patch shortly. Done. > I thought the plan was to also rename the libpq "gssdeleg" connection > parameter and so on? I can look into that tomorrow, if nobody beats > me to it. Please do. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
Re: Naming of gss_accept_deleg
On Sat, May 20, 2023 at 11:21:57PM -0400, Tom Lane wrote: > Nathan Bossart writes: > > On Sat, May 20, 2023 at 09:33:44PM -0400, Bruce Momjian wrote: > >> With less then 48 hours to beta 1 packaging, I have made this change and > >> adjusted internal variable to match. > > > The buildfarm and cfbot seem unhappy with 9c0a0e2. It looks like there are > > a few remaining uses of gss_accept_deleg to rename. I'm planning to commit > > the attached patch shortly. > > I thought the plan was to also rename the libpq "gssdeleg" connection > parameter and so on? I can look into that tomorrow, if nobody beats > me to it. Oh, I didn't consider those. I thought we would leave libpq alone since those are often supplied on the command line and there are existing short-style libpq options, e.g., gssencmode, krbsrvname, sslcrl. I am fine with such a change though. -- Bruce Momjian https://momjian.us EDB https://enterprisedb.com Only you can decide what is important to you.
Re: Naming of gss_accept_deleg
Nathan Bossart writes: > On Sat, May 20, 2023 at 09:33:44PM -0400, Bruce Momjian wrote: >> With less then 48 hours to beta 1 packaging, I have made this change and >> adjusted internal variable to match. > The buildfarm and cfbot seem unhappy with 9c0a0e2. It looks like there are > a few remaining uses of gss_accept_deleg to rename. I'm planning to commit > the attached patch shortly. I thought the plan was to also rename the libpq "gssdeleg" connection parameter and so on? I can look into that tomorrow, if nobody beats me to it. regards, tom lane
Re: Naming of gss_accept_deleg
On Sat, May 20, 2023 at 08:17:57PM -0700, Nathan Bossart wrote: > On Sat, May 20, 2023 at 09:33:44PM -0400, Bruce Momjian wrote: > > With less then 48 hours to beta 1 packaging, I have made this change and > > adjusted internal variable to match. > > The buildfarm and cfbot seem unhappy with 9c0a0e2. It looks like there are > a few remaining uses of gss_accept_deleg to rename. I'm planning to commit > the attached patch shortly. Yes, please do. I saw some matches in the tests but was confused since my tests passed. I now realize I wasn't testing those. -- Bruce Momjian https://momjian.us EDB https://enterprisedb.com Only you can decide what is important to you.
Re: Naming of gss_accept_deleg
On Sat, May 20, 2023 at 09:33:44PM -0400, Bruce Momjian wrote:
> With less then 48 hours to beta 1 packaging, I have made this change and
> adjusted internal variable to match.
The buildfarm and cfbot seem unhappy with 9c0a0e2. It looks like there are
a few remaining uses of gss_accept_deleg to rename. I'm planning to commit
the attached patch shortly.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index c8018da04a..b090ec5245 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -101,7 +101,7 @@
# GSSAPI using Kerberos
#krb_server_keyfile = 'FILE:${sysconfdir}/krb5.keytab'
#krb_caseins_users = off
-#gss_accept_deleg = off
+#gss_accept_delegation = off
# - SSL -
diff --git a/src/test/kerberos/t/001_auth.pl b/src/test/kerberos/t/001_auth.pl
index 39c035de32..5aff49a513 100644
--- a/src/test/kerberos/t/001_auth.pl
+++ b/src/test/kerberos/t/001_auth.pl
@@ -496,7 +496,7 @@ test_access(
"connection authorized: user=$username database=$dbname application_name=$application GSS (authenticated=yes, encrypted=yes, deleg_credentials=no, principal=test1\@$realm)"
);
-$node->append_conf('postgresql.conf', qq{gss_accept_deleg=off});
+$node->append_conf('postgresql.conf', qq{gss_accept_delegation=off});
$node->restart;
test_access(
@@ -520,7 +520,7 @@ test_access(
"connection authorized: user=$username database=$dbname application_name=$application GSS (authenticated=yes, encrypted=yes, deleg_credentials=no, principal=test1\@$realm)"
);
-$node->append_conf('postgresql.conf', qq{gss_accept_deleg=on});
+$node->append_conf('postgresql.conf', qq{gss_accept_delegation=on});
$node->restart;
test_access(
LLVM 16 (opaque pointers)
Hi,
Here is a draft version of the long awaited patch to support LLVM 16.
It's mostly mechanical donkeywork, but it took some time: this donkey
found it quite hard to understand the mighty getelementptr
instruction[1] and the code generation well enough to figure out all
the right types, and small mistakes took some debugging effort*. I
now finally have a patch that passes all tests.
Though it's not quite ready yet, I thought I should give this status
update to report that the main task is more or less complete, since
we're starting to get quite a few emails about it (mostly from Fedora
users) and there is an entry for it on the Open Items for 16 wiki
page. Comments/review/testing welcome.
Here are some things I think I need to do next (probably after PGCon):
1. If you use non-matching clang and LLVM versions I think we might
use "clang -no-opaque-pointers" at the wrong times (I've not looked
into that interaction yet).
2. The treatment of function types is a bit inconsistent/messy and
could be tidied up.
3. There are quite a lot of extra function calls that could perhaps be
elided (ie type variables instead of LLVMTypeInt8(), and calls to
LLVMStructGetTypeAtIndex() that are not used in LLVM < 16).
4. Could use some comments.
5. I need to test with very old versions of LLVM and Clang that we
claim to support (I have several years' worth of releases around but
nothing older than 9).
6. I need to go through the types again with a fine tooth comb, and
check the test coverage to look out for eg GEP array arithmetic with
the wrong type/size that isn't being exercised.
*For anyone working with this type of IR generation code and
questioning their sanity, I can pass on some excellent advice I got
from Andres: build LLVM yourself with assertions enabled, as they
catch some classes of silly mistake that otherwise just segfault
inscrutably on execution.
[1] https://llvm.org/docs/GetElementPtr.html
From 996e4bceee21a9f27116623c1fdac8eab0cdf814 Mon Sep 17 00:00:00 2001
From: Thomas Munro
Date: Mon, 9 May 2022 08:27:47 +1200
Subject: [PATCH] jit: Support opaque pointers in LLVM 16.
Remove use of LLVMGetElementType() and provide the type of all pointers
to LLVMBuildXXX() functions when emitting IR, as required by modern LLVM
versions[1].
* For LLVM <= 14, we'll still use the old LLVMBuildXXX() functions.
* For LLVM == 15, we'll continue to do the same, explicitly opting
out of opaque pointer mode.
* For LLVM >= 16, we'll use the new LLVMBuildXXX2() functions that take
the extra type argument.
The difference is hidden behind some new IR emitting wrapper functions
l_load(), l_gep(), l_call() etc. The change is mostly mechanical,
except that at each site the correct type had to be provided.
In some places we needed to do some extra work to get functions types,
including some new wrappers for C++ APIs that are not yet exposed by in
LLVM's C API, and some new "example" functions in llvmjit_types.c
because it's no longer possible to start from the function pointer type
and ask for the function type.
Back-patch to all releases.
[1] https://llvm.org/docs/OpaquePointers.html
diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c
index 04ae3052a8..b83bc04ae3 100644
--- a/src/backend/jit/llvm/llvmjit.c
+++ b/src/backend/jit/llvm/llvmjit.c
@@ -85,8 +85,11 @@ LLVMTypeRef StructExprState;
LLVMTypeRef StructAggState;
LLVMTypeRef StructAggStatePerGroupData;
LLVMTypeRef StructAggStatePerTransData;
+LLVMTypeRef StructPlanState;
LLVMValueRef AttributeTemplate;
+LLVMValueRef ExecEvalSubroutineTemplate;
+LLVMValueRef ExecEvalBoolSubroutineTemplate;
LLVMModuleRef llvm_types_module = NULL;
@@ -382,11 +385,7 @@ llvm_pg_var_type(const char *varname)
if (!v_srcvar)
elog(ERROR, "variable %s not in llvmjit_types.c", varname);
- /* look at the contained type */
- typ = LLVMTypeOf(v_srcvar);
- Assert(typ != NULL && LLVMGetTypeKind(typ) == LLVMPointerTypeKind);
- typ = LLVMGetElementType(typ);
- Assert(typ != NULL);
+ typ = LLVMGlobalGetValueType(v_srcvar);
return typ;
}
@@ -398,12 +397,14 @@ llvm_pg_var_type(const char *varname)
LLVMTypeRef
llvm_pg_var_func_type(const char *varname)
{
- LLVMTypeRef typ = llvm_pg_var_type(varname);
+ LLVMValueRef v_srcvar;
+ LLVMTypeRef typ;
+
+ v_srcvar = LLVMGetNamedFunction(llvm_types_module, varname);
+ if (!v_srcvar)
+ elog(ERROR, "function %s not in llvmjit_types.c", varname);
- /* look at the contained type */
- Assert(LLVMGetTypeKind(typ) == LLVMPointerTypeKind);
- typ = LLVMGetElementType(typ);
- Assert(typ != NULL && LLVMGetTypeKind(typ) == LLVMFunctionTypeKind);
+ typ = LLVMGetFunctionType(v_srcvar);
return typ;
}
@@ -433,7 +434,7 @@ llvm_pg_func(LLVMModuleRef mod, const char *funcname)
v_fn = LLVMAddFunction(mod,
funcname,
- LLVMGetElementType(LLVMTypeOf(v_srcfn)));
+ LLVMGetFunctionType(v_srcfn));
llvm_copy_attributes(v_srcfn, v_fn);
return v_fn;
@@ -529,7 +530,7 @@
Re: Order changes in PG16 since ICU introduction
On Fri, 2023-05-19 at 21:13 +0200, Daniel Verite wrote: > ISTM that if we want to go that route, we need the make the minimum > changes at the user interface level and not any deeper, so that when > (locale="C" OR locale="POSIX") AND the provider has not been > specified, > then the command (initdb and create database) act as if the user had > specified provider=libc. If we special case locale=C, but do nothing for locale=fr_FR, then I'm not sure we've solved the problem. Andrew Gierth raised the issue here, which he called "maximally confusing": https://postgr.es/m/874jp9f5jo@news-spur.riddles.org.uk That's why I feel that we need to make locale apply to whatever the provider is, not just when it happens to be C. > > (3) Support iculocale=C in the ICU provider using the memcmp() > > path. > > > In other words, if provider=icu and iculocale=C, lc_collate_is_c() > > and > > lc_ctpye_is_c() would both return true. > > ICU does not provide a locale that behaves like that, and it doesn't > feel right to pretend it does. It feels like attacking the problem > at the wrong level. I agree that #3 feels slightly wrong, but I think it's still a viable option until we have consensus on something better. > > (4) Create a new "none" provider (which has no locale and always > > memcmp > > semantics), and automatically change the provider to "none" if > > provider=icu and iculocale=C. > > It still uses libc/C for character classification and case changing, > so "no locale" is technically not true. The provider affects callers that have a pg_locale_t, such as the SQL- callable lower() function. For those callers, the "none" provider uses pg_ascii_tolower(), etc., not libc. That's why I called it "none" -- it's using simple internal postgres implementations instead of a provider. For callers that don't have a pg_locale_t, they may call libc functions directly and rely on the server environment. But in those cases, there's no way to set a provider at all, it's just relying on the server environment. There aren't many of these cases, and hopefully we can eliminate the reliance on the server environment over time. If I'm missing something, let me know what cases you have in mind. Regards, Jeff Davis
Re: Naming of gss_accept_deleg
On Fri, May 19, 2023 at 07:58:34AM -0700, Nathan Bossart wrote: > On Fri, May 19, 2023 at 09:42:00AM -0400, Tom Lane wrote: > > Abhijit Menon-Sen writes: > >> I would also prefer a GUC named gss_accept_delegation, but the current > >> name matches the libpq gssdeleg connection parameter and the PGSSDELEG > >> environment variable. Maybe there's something to be said for keeping > >> those three things alike? > > > > +1 for spelling it out in all user-visible names. I do not think > > that that GSS-API C symbol is a good precedent to follow. > > +1 With less then 48 hours to beta 1 packaging, I have made this change and adjusted internal variable to match. -- Bruce Momjian https://momjian.us EDB https://enterprisedb.com Only you can decide what is important to you.
Re: PG 16 draft release notes ready
On Sat, May 20, 2023 at 08:51:21PM -0400, Tom Lane wrote: > Bruce Momjian writes: > > I have added this release note item: > > > Add functions pg_input_is_valid() and pg_input_error_message() to check > > for type conversion errors (Tom Lane) > > pg_input_error_message got renamed to pg_input_error_info later, > cf b8da37b3a (which maybe should be included in the comment > for this entry). Oh, I skipped the original entry so I skipped that one too. I have adjusted the release note item and added the commit to the comment: --> Add functions pg_input_is_valid() and pg_input_error_info() to check for type conversion errors (Tom Lane) -- Bruce Momjian https://momjian.us EDB https://enterprisedb.com Only you can decide what is important to you.
Re: PG 16 draft release notes ready
On Sat, May 20, 2023 at 10:37:58AM +0200, Drouvot, Bertrand wrote: > It's the snapshot of running transactions (aka the xl_running_xacts WAL > record) that is used during the > logical slot creation to determine if the logical decoding find a consistent > state to start with. > > On a primary this WAL record is being emitted during the logical slot > creation, but on a standby > we can't write WAL records (so we are waiting for the primary to emit it). > > Outside of logical slot creation, this WAL record is also emitted during > checkpoint or periodically > by the bgwriter. > > What about? > > This adds the function pg_log_standby_snapshot() to emit the WAL record that > contains the list > of running transactions. > > If the primary is idle, the logical slot creation on a standby can take a > while (waiting for this WAL record > to be replayed to determine if the logical decoding find a consistent state > to start with). > > In that case, this new function can be used on the primary to speed up the > logical slot > creation on the standby. Okay, this helps. I split the entry into two with this text: Allow logical decoding on standbys (Bertrand Drouvot, Andres Freund, Amit Khandekar) Add function pg_log_standby_snapshot() to force creation of a WAL snapshot (Bertrand Drouvot) WAL snapshots are required for logical slot creation so this function speeds their creation on standbys. -- Bruce Momjian https://momjian.us EDB https://enterprisedb.com Only you can decide what is important to you.
Re: PG 16 draft release notes ready
Bruce Momjian writes: > I have added this release note item: > Add functions pg_input_is_valid() and pg_input_error_message() to check > for type conversion errors (Tom Lane) pg_input_error_message got renamed to pg_input_error_info later, cf b8da37b3a (which maybe should be included in the comment for this entry). regards, tom lane
Re: PG 16 draft release notes ready
On Fri, May 19, 2023 at 11:08:18PM -0400, Isaac Morland wrote: > On Fri, 19 May 2023 at 22:59, jian he wrote: > > > Sorry for changing the subject line. > > these two commits seems not mentioned. > > > On a similar topic, should every committed item from the commitfest be > mentioned, or only ones that are significant enough? > > I’m wondering because I had a role in this very small item, yet I don’t see it > listed in the psql section: > > https://commitfest.postgresql.org/42/4133/ > > It’s OK if we don’t mention every single change, I just want to make sure I > understand. I have never considered the presence of an item in the commitfest as an indicator of importance to be in the release notes. The major release notes, for me, is a balance of listing the most visible changes without going into unmanageable detail. -- Bruce Momjian https://momjian.us EDB https://enterprisedb.com Only you can decide what is important to you.
Re: PG 16 draft release notes ready
On Sat, May 20, 2023 at 10:59:20AM +0800, jian he wrote: > > Sorry for changing the subject line. > > these two commits seems not mentioned. > Fix ts_headline() edge cases for empty query and empty search text. > https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h= > 029dea882a7aa34f46732473eed7c917505e6481 I usually don't cover bug fixes for rare cases that used to generate errors. However, the bigger issue is that this commit did not appear in my output of git_changelog because it was backpatched, as indicated in the commit text. > Simplify the implementations of the to_reg* functions. > https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h= The commit for this is: Author: Tom Lane 2022-12-27 [3ea7329c9] Simplify the implementations of the to_reg* functions. Simplify the implementations of the to_reg* functions. Given the soft-input-error feature, we can reduce these functions to be just thin wrappers around a soft-error call of the corresponding datatype input function. This means less code and more certainty that the to_reg* functions match the normal input behavior. --> Notably, it also means that they will accept numeric OID input, --> which they didn't before. It's not clear to me if that omission had more than laziness behind it, but it doesn't seem like something we need to work hard to preserve. Discussion: https://postgr.es/m/3910031.1672095...@sss.pgh.pa.us The change is that to_reg* functions can now accept OIDs, which I didn't notice when I read the commit message. I have added this release note item: Allow to_reg* functions to accept OIDs parameters (Tom Lane) -- Bruce Momjian https://momjian.us EDB https://enterprisedb.com Only you can decide what is important to you.
Re: PG 16 draft release notes ready
On Sat, May 20, 2023 at 12:59:35AM +0800, jian he wrote: > Add function pg_dissect_walfile_name() to report the segment and timeline > values of WAL file names (Bharath Rupireddy) > > https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h= > 13e0d7a603852b8b05c03b45228daabffa0cced2 > the function rename to pg_split_walfile_name. Fixed. I copied the commit that did the rename, but forgot to actually update the release note text to match. > seems didn't mention pg_input_is_valid,pg_input_error_info? > https://www.postgresql.org/docs/devel/functions-info.html# > FUNCTIONS-INFO-VALIDITY-TABLE Good point. I incorrectly interpreted the commit text as part of our test infrastuture and not the addition of two SQL functions: Add test scaffolding for soft error reporting from input functions. pg_input_is_valid() returns boolean, while pg_input_error_message() returns the primary error message if the input is bad, or NULL if the input is OK. The main reason for having two functions is so that we can test both the details-wanted and the no-details-wanted code paths. I have added this release note item: Add functions pg_input_is_valid() and pg_input_error_message() to check for type conversion errors (Tom Lane) -- Bruce Momjian https://momjian.us EDB https://enterprisedb.com Only you can decide what is important to you.
Re: PG 16 draft release notes ready
On Sat, May 20, 2023 at 06:07:08PM -0400, Tom Lane wrote: > Bruce Momjian writes: > > On Fri, May 19, 2023 at 07:05:12PM -0400, Tom Lane wrote: > >> ... "ł" doesn't exist in ISO8859-1. I'd suggest dumbing these > >> down to plain "l". > > > Done. I know we used to be limited to Latin-1 but when my build of HTML > > worked, I thought that had changed. :-( > > Yeah, I think the HTML toolchain is better than it used to be on > this score. But PDF is still limited. Ah, makes sense. I will need to test the PDF build next time. -- Bruce Momjian https://momjian.us EDB https://enterprisedb.com Only you can decide what is important to you.
Re: PG 16 draft release notes ready
Bruce Momjian writes: > On Fri, May 19, 2023 at 07:05:12PM -0400, Tom Lane wrote: >> ... "ł" doesn't exist in ISO8859-1. I'd suggest dumbing these >> down to plain "l". > Done. I know we used to be limited to Latin-1 but when my build of HTML > worked, I thought that had changed. :-( Yeah, I think the HTML toolchain is better than it used to be on this score. But PDF is still limited. regards, tom lane
Re: PG 16 draft release notes ready
On Fri, May 19, 2023 at 07:05:12PM -0400, Tom Lane wrote: > The v16 release notes are problematic in a PDF docs build: > > [WARN] FOUserAgent - Glyph "?" (0x142, lslash) not available in font > "Times-Roman". > > This is evidently from > > Add functions to add, subtract, and generate timestamptz values in a > specified time zone (Przemysław Sztoch, Gurjeet Singh) > > Change date_trunc(unit, timestamptz, time_zone) to be an immutable function > (Przemysław Sztoch) > > since "ł" doesn't exist in ISO8859-1. I'd suggest dumbing these > down to plain "l". Done. I know we used to be limited to Latin-1 but when my build of HTML worked, I thought that had changed. :-( -- Bruce Momjian https://momjian.us EDB https://enterprisedb.com Only you can decide what is important to you.
Re: benchmark results comparing versions 15.2 and 16
On Fri, May 19, 2023 at 4:04 PM Andres Freund wrote: > With "yet to see any significant changes" do you mean that the runs are > comparable with earlier runs, showing the same regression? Or that the > regression vanished? Or ...? > I mean that I might be chasing noise and the mean+stddev for throughput in version 16 pre-beta so far appears to be similar to 15.2. When I ran the insert benchmark a few times, I focused on the cases where 16 pre-beta was worse than 15.2 while ignoring the cases where it was better. Big regressions are easy to document, small ones not so much. Regardless, I am repeating tests from both the insert benchmark and sysbench for version 16 (pre-beta, and soon beta1).
Re: Adding SHOW CREATE TABLE
Greetings, On Sat, May 20, 2023 at 13:32 David G. Johnston wrote: > On Sat, May 20, 2023 at 10:26 AM Stephen Frost wrote: > >> > A server function can be conveniently called from any client code. >> >> Clearly any client using libpq can conveniently call code which is in >> libpq. >> > > Clearly there are clients that don't use libpq. JDBC comes to mind. > Indeed … as I mentioned up-thread already. Are we saying that we want this to be available server side, and largely duplicated, specifically to cater to non-libpq users? I’ll put out there, again, the idea that perhaps we put it into the common library then and make it available via both libpq and as a server side function ..? We also have similar code in postgres_fdw.. ideally, imv anyway, we’d not end up with three copies of it. Thanks, Stephen
Re: Adding SHOW CREATE TABLE
"David G. Johnston" writes: > On Sat, May 20, 2023 at 10:26 AM Stephen Frost wrote: >> Clearly any client using libpq can conveniently call code which is in >> libpq. > Clearly there are clients that don't use libpq. JDBC comes to mind. Yeah. I'm also rather concerned about the bloat factor; it's hardly unlikely that this line of development would double the size of libpq, to add functionality that only a small minority of applications would use. A client-side implementation also has no choice but to cope with multiple server versions, and to figure out what it's going to do about a too-new server. Up to now we broke compatibility between libpq and server maybe once every couple decades, but it'd be once a year for this. regards, tom lane
Re: Adding SHOW CREATE TABLE
On Sat, May 20, 2023 at 10:26 AM Stephen Frost wrote: > > A server function can be conveniently called from any client code. > > Clearly any client using libpq can conveniently call code which is in > libpq. > > Clearly there are clients that don't use libpq. JDBC comes to mind. David J.
Re: Adding SHOW CREATE TABLE
Greetings, * Laurenz Albe (laurenz.a...@cybertec.at) wrote: > On Fri, 2023-05-19 at 13:08 -0400, Andrew Dunstan wrote: > > On 2023-05-18 Th 19:53, Stephen Frost wrote: > > > * Kirk Wolak (wol...@gmail.com) wrote: > > > > My approach for now is to develop this as the \st command. > > > > After reviewing the code/output from the 3 sources (psql, fdw, and > > > > pg_dump). > > > > > > Having this only available via psql seems like the least desirable > > > option as then it wouldn't be available to any other callers.. > > > > > > Having it in libpq, on the other hand, would make it available to psql > > > as well as any other utility, library, or language / driver which uses > > > libpq, including pg_dump.. > > > > I think the ONLY place we should have this is in server side functions. > > +1 ... but it already exists in pg_dump, so I'm unclear why folks seem to be pushing to have a duplicate of it in core? We certainly can't remove it from pg_dump even if we add it to core because pg_dump has to understand the changes between major versions. > A function "pg_get_tabledef" would blend nicely into the following list: > > \df pg_get_*def >List of functions >Schema │ Name │ Result data type │ Argument > data types │ Type > ╪╪══╪═══╪══ > pg_catalog │ pg_get_constraintdef │ text │ oid > │ func > pg_catalog │ pg_get_constraintdef │ text │ oid, > boolean │ func > pg_catalog │ pg_get_functiondef │ text │ oid > │ func > pg_catalog │ pg_get_indexdef│ text │ oid > │ func > pg_catalog │ pg_get_indexdef│ text │ oid, > integer, boolean │ func > pg_catalog │ pg_get_partition_constraintdef │ text │ oid > │ func > pg_catalog │ pg_get_partkeydef │ text │ oid > │ func > pg_catalog │ pg_get_ruledef │ text │ oid > │ func > pg_catalog │ pg_get_ruledef │ text │ oid, > boolean │ func > pg_catalog │ pg_get_statisticsobjdef│ text │ oid > │ func > pg_catalog │ pg_get_triggerdef │ text │ oid > │ func > pg_catalog │ pg_get_triggerdef │ text │ oid, > boolean │ func > pg_catalog │ pg_get_viewdef │ text │ oid > │ func > pg_catalog │ pg_get_viewdef │ text │ oid, > boolean │ func > pg_catalog │ pg_get_viewdef │ text │ oid, > integer │ func > pg_catalog │ pg_get_viewdef │ text │ text > │ func > pg_catalog │ pg_get_viewdef │ text │ text, > boolean │ func > (17 rows) > > > A server function can be conveniently called from any client code. Clearly any client using libpq can conveniently call code which is in libpq. Thanks, Stephen signature.asc Description: PGP signature
Re: ICU locale validation / canonicalization
On Tue, 2023-05-02 at 07:29 -0700, Noah Misch wrote:
> On Thu, Mar 30, 2023 at 08:59:41AM +0200, Peter Eisentraut wrote:
> > On 30.03.23 04:33, Jeff Davis wrote:
> > > Attached is a new version of the final patch, which performs
> > > canonicalization. I'm not 100% sure that it's wanted, but it
> > > still
> > > seems like a good idea to get the locales into a standard format
> > > in the
> > > catalogs, and if a lot more people start using ICU in v16
> > > (because it's
> > > the default), then it would be a good time to do it. But perhaps
> > > there
> > > are risks?
> >
> > I say, let's do it.
>
> The following is not cause for postgresql.git changes at this time,
> but I'm
> sharing it in case it saves someone else the study effort. Commit
> ea1db8a
> ("Canonicalize ICU locale names to language tags.") slowed buildfarm
> member
> hoverfly, but that disappears if I drop debug_parallel_query from its
> config.
> Typical end-to-end duration rose from 2h5m to 2h55m. Most-affected
> were
> installcheck runs, which rose from 11m to 19m. (The "check" stage
> uses
> NO_LOCALE=1, so it changed less.) From profiles, my theory is that
> each of
> the many parallel workers burns notable CPU and I/O opening its ICU
> collator
> for the first time.
I didn't repro the overall test timings (mine is ~1m40s compared to
~11-19m on hoverfly) but I think a microbenchmark on the ICU calls
showed a possible cause.
I ran open in a loop 10M times on the requested locale. The root locale
("und"[1], "root" and "") take about 1.3s to open 10M times; simple
locales like 'en' and 'fr-CA' and 'de-DE' are all a little shower at
3.3s.
Unrecognized locales like "xyz" take about 10 times as long: 13s to
open 10M times, presumably to perform the fallback logic that
ultimately opens the root locale. Not sure if 10X slower in the open
path is enough to explain the overall test slowdown.
My guess is that the ICU locale for these tests is not recognized, or
is some other locale that opens slowly. Can you tell me the actual
daticulocale?
Regards,
Jeff Davis
[1] It appears that "und" is also slow to open in ICU < 64. Hoverfly is
on v58, so it's possible that's the problem if daticulocale=und.
Re: pgbench: using prepared BEGIN statement in a pipeline could cause an error
Hello Alvaro, 21.02.2023 19:32, Alvaro Herrera wrote: On 2023-Feb-20, Alvaro Herrera wrote: Found one last problem: if you do "-f foobar.sql -M prepared" in that order, then the prepare fails because the statement names would not be assigned when the file is parsed. This coding only supported doing "-M prepared -f foobar.sql", which funnily enough is the only one that PostgreSQL/Cluster.pm->pgbench() supports. So I moved the prepared statement name generation to the postprocess step. Pushed to all three branches -- thanks, Nagata-san, for diagnosing the issue. Starting from 038f586d5, the following script: echo " \startpipeline \endpipeline " >test.sql pgbench -n -M prepared -f test.sql leads to the pgbench's segfault: Core was generated by `pgbench -n -M prepared -f test.sql'. Program terminated with signal SIGSEGV, Segmentation fault. warning: Section `.reg-xstate/2327306' in core file too small. #0 0x555a402546b4 in prepareCommandsInPipeline (st=st@entry=0x555a409d62e0) at pgbench.c:3130 3130 st->prepared[st->use_file][st->command] = true; (gdb) bt #0 0x555a402546b4 in prepareCommandsInPipeline (st=st@entry=0x555a409d62e0) at pgbench.c:3130 #1 0x555a40257fca in executeMetaCommand (st=st@entry=0x555a409d62e0, now=now@entry=0x7ffdd46eff58) at pgbench.c:4413 #2 0x555a402585ce in advanceConnectionState (thread=thread@entry=0x555a409d6580, st=st@entry=0x555a409d62e0, agg=agg@entry=0x7ffdd46f0090) at pgbench.c:3807 #3 0x555a40259564 in threadRun (arg=arg@entry=0x555a409d6580) at pgbench.c:7535 #4 0x555a4025ca40 in main (argc=, argv=) at pgbench.c:7253 Best regards, Alexander
Re: Assert failure of the cross-check for nullingrels
Richard Guo writes:
> I tried with v4 patch and find that, as you predicted, it might reject
> all the clones in some cases. Check the query below
> ...
> So the qual 't3.a = t4.a' is missing in this plan shape.
I poked into that more closely and realized that the reason that
clause_is_computable_at() misbehaves is that both clones of the
"t3.a = t4.a" qual have the same clause_relids: (4 5 6) which is
t3, the left join to t3, and t4. This is unsurprising because
the difference in these clones is whether they are expected to be
evaluated above or below outer join 3 (the left join to t2), and
t2 doesn't appear in the qual. (It does appear in "t2.b = t4.b",
which is why there's no similar misbehavior for that qual.)
If they have the same clause_relids, then clearly in its current
form clause_is_computable_at() cannot distinguish them. So what
I now think we should do is have clause_is_computable_at() examine
their required_relids instead. Those will be different, by
construction in deconstruct_distribute_oj_quals(), ensuring that
we select only one of the group of clones.
BTW, while I've not tried it, I suspect your v2 patch also fails
on this example for the same reason: it cannot distinguish the
clones of this qual.
regards, tom lane
diff --git a/src/backend/optimizer/util/restrictinfo.c b/src/backend/optimizer/util/restrictinfo.c
index d2bc096e1c..760d24bebf 100644
--- a/src/backend/optimizer/util/restrictinfo.c
+++ b/src/backend/optimizer/util/restrictinfo.c
@@ -544,13 +544,24 @@ clause_is_computable_at(PlannerInfo *root,
RestrictInfo *rinfo,
Relids eval_relids)
{
- Relids clause_relids = rinfo->clause_relids;
+ Relids clause_relids;
ListCell *lc;
/* Nothing to do if no outer joins have been performed yet. */
if (!bms_overlap(eval_relids, root->outer_join_rels))
return true;
+ /*
+ * For an ordinary qual clause, we consider the actual clause_relids as
+ * explained above. However, it's possible for multiple members of a
+ * group of clone quals to have the same clause_relids, so for clones use
+ * the required_relids instead to ensure we select just one of them.
+ */
+ if (rinfo->has_clone || rinfo->is_clone)
+ clause_relids = rinfo->required_relids;
+ else
+ clause_relids = rinfo->clause_relids;
+
foreach(lc, root->join_info_list)
{
SpecialJoinInfo *sjinfo = (SpecialJoinInfo *) lfirst(lc);
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 8fa2b376f3..1fd75c8f58 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -2500,6 +2500,74 @@ select * from int4_tbl t1
-> Seq Scan on int4_tbl t4
(12 rows)
+explain (costs off)
+select * from int4_tbl t1
+ left join (int4_tbl t2 left join int4_tbl t3 on t2.f1 > 0) on t2.f1 > 1
+ left join int4_tbl t4 on t2.f1 > 2 and t3.f1 > 3
+where t1.f1 = coalesce(t2.f1, 1);
+ QUERY PLAN
+
+ Nested Loop Left Join
+ Join Filter: ((t2.f1 > 2) AND (t3.f1 > 3))
+ -> Nested Loop Left Join
+ Join Filter: (t2.f1 > 0)
+ -> Nested Loop Left Join
+ Filter: (t1.f1 = COALESCE(t2.f1, 1))
+ -> Seq Scan on int4_tbl t1
+ -> Materialize
+ -> Seq Scan on int4_tbl t2
+ Filter: (f1 > 1)
+ -> Seq Scan on int4_tbl t3
+ -> Materialize
+ -> Seq Scan on int4_tbl t4
+(13 rows)
+
+explain (costs off)
+select * from int4_tbl t1
+ left join ((select t2.f1 from int4_tbl t2
+left join int4_tbl t3 on t2.f1 > 0
+where t3.f1 is null) s
+ left join tenk1 t4 on s.f1 > 1)
+on s.f1 = t1.f1;
+ QUERY PLAN
+-
+ Nested Loop Left Join
+ Join Filter: (t2.f1 > 1)
+ -> Hash Right Join
+ Hash Cond: (t2.f1 = t1.f1)
+ -> Nested Loop Left Join
+ Join Filter: (t2.f1 > 0)
+ Filter: (t3.f1 IS NULL)
+ -> Seq Scan on int4_tbl t2
+ -> Materialize
+ -> Seq Scan on int4_tbl t3
+ -> Hash
+ -> Seq Scan on int4_tbl t1
+ -> Seq Scan on tenk1 t4
+(13 rows)
+
+explain (costs off)
+select * from onek t1
+left join onek t2 on t1.unique1 = t2.unique1
+left join onek t3 on t2.unique1 = t3.unique1
+left join onek t4 on t3.unique1 = t4.unique1 and t2.unique2 = t4.unique2;
+ QUERY PLAN
+
+ Hash Left Join
+ Hash Cond: ((t3.unique1 = t4.unique1) AND (t2.unique2 = t4.unique2))
+ -> Hash Left Join
+ Hash Cond: (t2.unique1 = t3.unique1)
+ -> Hash Left Join
+ Hash Cond: (t1.unique1 = t2.unique1)
+
Re: New COPY options: DELIMITER NONE and QUOTE NONE
On 2023-05-20 Sa 02:59, Joel Jacobson wrote:
On Fri, May 19, 2023, at 19:03, Andrew Dunstan wrote:
> I think you've been a bit too cute with the grammar changes, but as
you say this is a POC.
Thanks for feedback.
The approach I took for the new grammar rules was inspired by previous
commits,
such as de7531a971b, which introduced support for 'FORCE QUOTE '*''.
In that
case, a new separate grammar rule was crafted.
Not sure what you mean with it being "too cute", but maybe you think
it's a bit
verbose with another grammar rule and it would be better to integrate
it into
the existing one?
Example:
| DELIMITER opt_as (Sconst | NONE)
{
if ($3 == NONE)
$$ = makeDefElem("delimiter", (Node *)
makeString("\0"), @1);
else
$$ = makeDefElem("delimiter", (Node *)
makeString($3), @1);
}
I would probably go for something like this for "DELIMITER NONE" in a
separate rule:
| DELIMITER NONE
{
$$ = makeDefElem("delimiter_none", (Node *)makeInteger(true), @1);
}
and deal with that element further down the stack. It looks to me at
first glance that your changes would allow "DELIMITER ''" which is
probably not what we want.
Similarly for "QUOTE NONE".
cheers
andrew
--
Andrew Dunstan
EDB:https://www.enterprisedb.com
Re: PG 16 draft release notes ready
Hi, On 5/19/23 2:29 PM, Bruce Momjian wrote: On Fri, May 19, 2023 at 09:49:18AM +0200, Drouvot, Bertrand wrote: Thanks! " This adds the function pg_log_standby_snapshot(). TEXT?: " My proposal: This adds the function pg_log_standby_snapshot() to log details of the current snapshot to WAL. If the primary is idle, the slot creation on a standby can take a while. This function can be used on the primary to speed up the logical slot creation on the standby. Yes, I got this concept from the commit message, but I am unclear on what is actually happening so I can clearly explain it. Slot creation on the standby needs a snapshot, and that is only created when there is activity, or happens periodically, and this forces it to happen, or something? And what snapshot is this? The current session's? It's the snapshot of running transactions (aka the xl_running_xacts WAL record) that is used during the logical slot creation to determine if the logical decoding find a consistent state to start with. On a primary this WAL record is being emitted during the logical slot creation, but on a standby we can't write WAL records (so we are waiting for the primary to emit it). Outside of logical slot creation, this WAL record is also emitted during checkpoint or periodically by the bgwriter. What about? This adds the function pg_log_standby_snapshot() to emit the WAL record that contains the list of running transactions. If the primary is idle, the logical slot creation on a standby can take a while (waiting for this WAL record to be replayed to determine if the logical decoding find a consistent state to start with). In that case, this new function can be used on the primary to speed up the logical slot creation on the standby. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: Should CSV parsing be stricter about mid-field quotes?
On Fri, May 19, 2023, at 18:06, Daniel Verite wrote: > COPY FROM file CSV somewhat differs as your example shows, > but it still mishandle \. when unquoted. For instance, consider this > file to load with COPYt FROM '/tmp/t.csv' WITH CSV > $ cat /tmp/t.csv > line 1 > \. > line 3 > line 4 > > It results in having only "line 1" being imported. Hmm, this is a problem for one of the new use-cases I brought up that would be possible with DELIMITER NONE QUOTE NONE, i.e. to import unstructured log files, where each raw line should be imported "as is" into a single text column. Is there a valid reason why \. is needed for COPY FROM filename? It seems to me it would only be necessary for the COPY FROM STDIN case, since files have a natural end-of-file and a known file size. /Joel
Re: New COPY options: DELIMITER NONE and QUOTE NONE
On Fri, May 19, 2023, at 19:03, Andrew Dunstan wrote:
> I think you've been a bit too cute with the grammar changes, but as you say
> this is a POC.
Thanks for feedback.
The approach I took for the new grammar rules was inspired by previous commits,
such as de7531a971b, which introduced support for 'FORCE QUOTE '*''. In that
case, a new separate grammar rule was crafted.
Not sure what you mean with it being "too cute", but maybe you think it's a bit
verbose with another grammar rule and it would be better to integrate it into
the existing one?
Example:
| DELIMITER opt_as (Sconst | NONE)
{
if ($3 == NONE)
$$ = makeDefElem("delimiter", (Node *)
makeString("\0"), @1);
else
$$ = makeDefElem("delimiter", (Node *) makeString($3),
@1);
}
/Joel
