Re: Add support for AT LOCAL
Andres Freund writes: > On 2023-10-19 10:38:14 -0400, Robert Haas wrote: >> To be honest, I'm not entirely sure that even AIX gcc support is >> delivering enough value per unit work to justify keeping it around. >> But the xlc situation is worse. > Agreed with both. If it were just a platform that didn't need special casing > in a bunch of places, it'd be one thing, but it's linkage model is so odd that > it makes no sense to keep AIX support around. But I'll take what I can get... The other thread recently referred to: https://www.postgresql.org/message-id/flat/20220702183354.a6uhja35wta7agew%40alap3.anarazel.de was mostly about how AIX's choice that alignof(double) < alignof(int64) breaks a whole bunch of assumptions in our code. AFAICS we've done nothing to resolve that, and nobody really wants to deal with it, and there's no good reason to think that fixing it would improve portability to any other platform. So maybe there's an adequate case for just nuking AIX support altogether? I can't recall the last time I saw a report from an actual AIX end user. regards, tom lane
Re: Add support for AT LOCAL
Hi, On 2023-10-19 10:38:14 -0400, Robert Haas wrote: > On Wed, Oct 18, 2023 at 7:33 PM Noah Misch wrote: > > I feel the gravity and longevity of xlc bugs has been out of proportion with > > the compiler's contribution to PostgreSQL. I would find it reasonable to > > revoke xlc support in v17+, leaving AIX gcc support in place. > > +1 for this proposal. I just think this is getting silly. We're saying > that we only have access to 1 or 2 AIX machines, and most of us have > access to none, and the compiler has serious code generation bugs that > are present in both a release 11 years old and also a release current > release, meaning they went unfixed for 10 years, and we can't report > bugs or get them fixed when we find them, and the use of this > particular compiler in the buildfarm isn't finding any issues that > matter anywhere else. +1. > To be honest, I'm not entirely sure that even AIX gcc support is > delivering enough value per unit work to justify keeping it around. > But the xlc situation is worse. Agreed with both. If it were just a platform that didn't need special casing in a bunch of places, it'd be one thing, but it's linkage model is so odd that it makes no sense to keep AIX support around. But I'll take what I can get... Greetings, Andres Freund
Re: Add support for AT LOCAL
Robert Haas writes: > On Wed, Oct 18, 2023 at 7:33 PM Noah Misch wrote: >> I feel the gravity and longevity of xlc bugs has been out of proportion with >> the compiler's contribution to PostgreSQL. I would find it reasonable to >> revoke xlc support in v17+, leaving AIX gcc support in place. > +1 for this proposal. WFM, too. regards, tom lane
Re: Add support for AT LOCAL
On Wed, Oct 18, 2023 at 7:33 PM Noah Misch wrote: > I feel the gravity and longevity of xlc bugs has been out of proportion with > the compiler's contribution to PostgreSQL. I would find it reasonable to > revoke xlc support in v17+, leaving AIX gcc support in place. +1 for this proposal. I just think this is getting silly. We're saying that we only have access to 1 or 2 AIX machines, and most of us have access to none, and the compiler has serious code generation bugs that are present in both a release 11 years old and also a release current release, meaning they went unfixed for 10 years, and we can't report bugs or get them fixed when we find them, and the use of this particular compiler in the buildfarm isn't finding any issues that matter anywhere else. To be honest, I'm not entirely sure that even AIX gcc support is delivering enough value per unit work to justify keeping it around. But the xlc situation is worse. -- Robert Haas EDB: http://www.enterprisedb.com
Re: Add support for AT LOCAL
On Wed, Oct 18, 2023 at 04:45:46PM -0400, Tom Lane wrote: > > On Wed, Oct 18, 2023 at 12:02 PM Tom Lane wrote: > >> Probably. Independent of that, it's fair to ask why we're still > >> testing against xlc 12.1 and not the considerably-more-recent xlclang, > >> or at least xlc 16.1. (I also wonder why we're still testing AIX 7.1 > >> rather than an OS version that's not EOL.) > The machine belongs to OSU (via the gcc compile farm), and I see > that they have another one that's POWER8 and is running AIX 7.3 [1]. > So in principle the buildfarm animals could just be moved over > to that one. > > Perhaps Noah has some particular attachment to 7.1, but now that that's > EOL it seems like we shouldn't be paying so much attention to it. > My guess is that it's still there in the compile farm because the gcc > people think it's still useful to have access to POWER7 hardware; but > I doubt there's enough difference for our purposes to be worth dealing > with a dead OS and ancient compiler. No particular attachment. From 2019 to 2023-08, hoverfly tested xlc16 on AIX 7.2; its run ended when cfarm119's owner replaced cfarm119 with an AIX 7.3, ibm-clang v17.1.1 machine. Since 2015, hornet and mandrill have tested xlc12 on AIX 7.1. That said, given your finding that later xlc versions have the same code generation bug, the choice of version is a side issue. A migration to ibm-clang wouldn't have prevented this week's xlc-prompted commits. I feel the gravity and longevity of xlc bugs has been out of proportion with the compiler's contribution to PostgreSQL. I would find it reasonable to revoke xlc support in v17+, leaving AIX gcc support in place. The main contribution of AIX has been to find the bug behind commit a1b8aa1. That benefited from the AIX kernel, not from any particular compiler. hornet and mandrill would continue to test v16-. By the way, I once tried to report an xlc bug. Their system was tailored to accept bugs from paid support customers only. I submitted it via some sales inquiry form, just in case, but never heard back.
Re: Add support for AT LOCAL
Robert Haas writes: > On Wed, Oct 18, 2023 at 12:02 PM Tom Lane wrote: >> Probably. Independent of that, it's fair to ask why we're still >> testing against xlc 12.1 and not the considerably-more-recent xlclang, >> or at least xlc 16.1. (I also wonder why we're still testing AIX 7.1 >> rather than an OS version that's not EOL.) > Well, according to Wikipedia, AIX 7.3 (released in 2021) requires > POWER8. AIX 7.2 (released 2015) only requires POWER7, and according to > the buildfarm page, this machine is POWER7. So it could possibly be > upgraded from 7.1 to 7.2, supposing that it is indeed compatible with > that release and that Noah's willing to do it and that there's not an > exorbitant fee and so on, but that still leaves you running an OS > version that is almost certainly closer to EOL than it is to the > original release date. Anything newer would require buying new > hardware, or so I guess. The machine belongs to OSU (via the gcc compile farm), and I see that they have another one that's POWER8 and is running AIX 7.3 [1]. So in principle the buildfarm animals could just be moved over to that one. Perhaps Noah has some particular attachment to 7.1, but now that that's EOL it seems like we shouldn't be paying so much attention to it. My guess is that it's still there in the compile farm because the gcc people think it's still useful to have access to POWER7 hardware; but I doubt there's enough difference for our purposes to be worth dealing with a dead OS and ancient compiler. regards, tom lane [1] https://portal.cfarm.net/machines/list/
Re: Add support for AT LOCAL
On Wed, Oct 18, 2023 at 12:02 PM Tom Lane wrote: > Probably. Independent of that, it's fair to ask why we're still > testing against xlc 12.1 and not the considerably-more-recent xlclang, > or at least xlc 16.1. (I also wonder why we're still testing AIX 7.1 > rather than an OS version that's not EOL.) Well, according to Wikipedia, AIX 7.3 (released in 2021) requires POWER8. AIX 7.2 (released 2015) only requires POWER7, and according to the buildfarm page, this machine is POWER7. So it could possibly be upgraded from 7.1 to 7.2, supposing that it is indeed compatible with that release and that Noah's willing to do it and that there's not an exorbitant fee and so on, but that still leaves you running an OS version that is almost certainly closer to EOL than it is to the original release date. Anything newer would require buying new hardware, or so I guess. Put otherwise, I think the reason we're testing on this AIX rather than anything else is probably that there is exactly 1 person associated with the project who has >0 pieces of hardware that can run AIX, and that person has one, so we're testing on that one. That might be a reason to question whether that particular strain of hardware has a bright future, at least in terms of PostgreSQL support. -- Robert Haas EDB: http://www.enterprisedb.com
Re: Add support for AT LOCAL
Robert Haas writes: > On Tue, Oct 17, 2023 at 7:35 PM Tom Lane wrote: >> Discounting the Windows animals, it looks like the xlc animals are >> our only remaining ones that use anything except gcc or clang. > After some research I determined that the release date for xlc 12.1 > seems to be June 1, 2012. At that time, clang 3.1 was current and just > after, GCC release version 4.7.1 was released. The oldest version of > clang that I find in the buildfarm is 3.9, and the oldest version of > gcc I find in the buildfarm is 4.6.3. So, somewhat to my surprise, xlc > is not the oldest compiler that we're still supporting in the > buildfarm. But it is very old, and it seems like that gcc and clang > are going to continue to gain ground against gcc and other proprietary > compilers for some time to come. Probably. Independent of that, it's fair to ask why we're still testing against xlc 12.1 and not the considerably-more-recent xlclang, or at least xlc 16.1. (I also wonder why we're still testing AIX 7.1 rather than an OS version that's not EOL.) > What does concern me is finding and coding > around compiler bugs. 19fa977311b9da9c6c84f0108600e78213751a38 is just > ridiculous, IMHO. I would agree, except for the downthread discovery that the bug is still present in current xlc and xlclang. Short of blowing off AIX altogether, it seems like we need to do something about it. regards, tom lane
Re: Add support for AT LOCAL
On Tue, Oct 17, 2023 at 7:35 PM Tom Lane wrote: > Thomas Munro writes: > > On Wed, Oct 18, 2023 at 11:54 AM Tom Lane wrote: > >> Should we be testing against xlclang instead? > > > I hesitated to suggest it because it's not my animal/time we're > > talking about but it seems to make more sense. It appears to be IBM's > > answer to the nothing-builds-with-this-thing phenomenon, since it > > accepts a lot of GCCisms via Clang's adoption of them. From a quick > > glance at [1], it lacks the atomics builtins but we have our own > > assembler magic for POWER. So maybe it'd all just work™. > > Discounting the Windows animals, it looks like the xlc animals are > our only remaining ones that use anything except gcc or clang. > That feels uncomfortably like a compiler monoculture to me, so > I can understand the reasoning for keeping hornet/mandrill going. > Still, maybe we should just accept the fact that gcc/clang have > outcompeted everything else in the C compiler universe. It's > getting hard to imagine that anyone would bring out some new product > that didn't try to be bug-compatible with gcc, for precisely the > reason you mention. After some research I determined that the release date for xlc 12.1 seems to be June 1, 2012. At that time, clang 3.1 was current and just after, GCC release version 4.7.1 was released. The oldest version of clang that I find in the buildfarm is 3.9, and the oldest version of gcc I find in the buildfarm is 4.6.3. So, somewhat to my surprise, xlc is not the oldest compiler that we're still supporting in the buildfarm. But it is very old, and it seems like that gcc and clang are going to continue to gain ground against gcc and other proprietary compilers for some time to come. I think it's reasonable to ask ourselves whether we really want to go to the trouble of maintaining something that is likely to get so little real-world usage. To be honest, I'm not particularly concerned about the need to adjust compiler and linker options from time to time, even though I know that probably annoys Andres. What does concern me is finding and coding around compiler bugs. 19fa977311b9da9c6c84f0108600e78213751a38 is just ridiculous, IMHO. If an end-of-life compiler for an end-of-life operating system has bugs that mean that C code that's doing nothing more than a bit of arithmetic isn't compiling properly, it's time to pull the plug. Nor is this the first example of working around a bug that only manifests in ancient xlc. I think that, when there was more real diversity in the software ecosystem, testing on a lot of platforms was a good way of finding out whether you'd done something that was in general correct or just something that happened to work on the machine you had in front of you. But hornet and mandrill are not telling us about things we've done that are incorrect in general yet happen to work on gcc and clang. What they seem to be telling us about, in this case and some others, are things that are CORRECT in general yet happen NOT to work on ancient xlc. And that's an important difference, because if we were finding real mistakes for which other platforms were not punishing us, then we could hope that fixing those mistakes would improve compatibility with other, equally niche platforms, potentially including future platforms that haven't come along yet. As it is, it's hard to believe that any work we put into this is going to have any benefit on any system other than ancient AIX. If there are other niche systems out there that have a similar number of bugs, they'll probably be *different* bugs. Sources for release dates: https://www.ibm.com/support/pages/fix-list-xl-c-aix https://releases.llvm.org/ https://gcc.gnu.org/releases.html -- Robert Haas EDB: http://www.enterprisedb.com
Re: Add support for AT LOCAL
Thomas Munro writes: > On Wed, Oct 18, 2023 at 11:54 AM Tom Lane wrote: >> Should we be testing against xlclang instead? > I hesitated to suggest it because it's not my animal/time we're > talking about but it seems to make more sense. It appears to be IBM's > answer to the nothing-builds-with-this-thing phenomenon, since it > accepts a lot of GCCisms via Clang's adoption of them. From a quick > glance at [1], it lacks the atomics builtins but we have our own > assembler magic for POWER. So maybe it'd all just work™. FWIW, I tried a test build with xlclang 16.1 on cfarm111, and it does seem like it Just Works, modulo a couple of oddities: * fails to compile, due to references to struct in6_addr, unless is included first. Most of our references to tcp.h already do that, but not libpq-be.h and fe-protocol3.c. I'm a bit at a loss why we've not seen this with the existing BF animals on this machine, because AFAICS they're all using the same /usr/include tree. * configure recognizes this as gcc but not Clang, which may or may not be fine: ... checking whether we are using the GNU C compiler... yes ... checking whether xlclang is Clang... no ... This doesn't seem to break anything, but it struck me as odd. configure seems to pick a sane set of compiler options anyway. Interestingly, xlclang shows the same failure with the pre-19fa97731 versions of timetz_zone/timetz_izone as plain xlc does. I guess this is not so astonishing since they presumably share the same codegen backend. But maybe somebody ought to file a bug with IBM? regards, tom lane
Re: Add support for AT LOCAL
On Tue, Oct 17, 2023 at 12:45:28PM -0400, Tom Lane wrote:
> Whoops, no: for negative starting values we'd need truncate-towards-
> minus-infinity division whereas C99 specifies truncate-towards-zero.
> However, the attached does pass for me on cfarm111 as well as my
> usual dev machine.
I guess that the following trick could be used for the negative case,
with one modulo followed by one extra addition:
if (result->time < INT64CONST(0))
{
result->time %= USECS_PER_DAY;
result->time += USECS_PER_DAY;
}
> Presumably this is a pre-existing bug that also appears in back
> branches. But in the interests of science I propose that we
> back-patch only the test case and see which machine(s) fail it
> before back-patching the code change.
Sure, as you see fit.
--
Michael
signature.asc
Description: PGP signature
Re: Add support for AT LOCAL
Thomas Munro writes: > On Wed, Oct 18, 2023 at 11:54 AM Tom Lane wrote: >> Should we be testing against xlclang instead? > I hesitated to suggest it because it's not my animal/time we're > talking about but it seems to make more sense. It appears to be IBM's > answer to the nothing-builds-with-this-thing phenomenon, since it > accepts a lot of GCCisms via Clang's adoption of them. From a quick > glance at [1], it lacks the atomics builtins but we have our own > assembler magic for POWER. So maybe it'd all just work™. Discounting the Windows animals, it looks like the xlc animals are our only remaining ones that use anything except gcc or clang. That feels uncomfortably like a compiler monoculture to me, so I can understand the reasoning for keeping hornet/mandrill going. Still, maybe we should just accept the fact that gcc/clang have outcompeted everything else in the C compiler universe. It's getting hard to imagine that anyone would bring out some new product that didn't try to be bug-compatible with gcc, for precisely the reason you mention. regards, tom lane
Re: Add support for AT LOCAL
On Wed, Oct 18, 2023 at 11:54 AM Tom Lane wrote: > Thomas Munro writes: > > > Given that IBM describes xlc as "legacy" (replaced by xlclang, but > > still supported for some unspecified period of time for the benefit of > > people who need C++ ABI compatibility with old code), I wonder how > > long we plan to support it... > > Should we be testing against xlclang instead? I hesitated to suggest it because it's not my animal/time we're talking about but it seems to make more sense. It appears to be IBM's answer to the nothing-builds-with-this-thing phenomenon, since it accepts a lot of GCCisms via Clang's adoption of them. From a quick glance at [1], it lacks the atomics builtins but we have our own assembler magic for POWER. So maybe it'd all just work™. [1] https://www.ibm.com/docs/en/xl-c-and-cpp-aix/16.1?topic=migration-checklist-when-moving-from-xl-based-front-end-clang-based-front-end
Re: Add support for AT LOCAL
Thomas Munro writes: > Given that IBM describes xlc as "legacy" (replaced by xlclang, but > still supported for some unspecified period of time for the benefit of > people who need C++ ABI compatibility with old code), I wonder how > long we plan to support it... Should we be testing against xlclang instead? regards, tom lane
Re: Add support for AT LOCAL
Hmm, I guess I must have missed some important flag or environment variable when trying to reproduce it, sorry. Given that IBM describes xlc as "legacy" (replaced by xlclang, but still supported for some unspecified period of time for the benefit of people who need C++ ABI compatibility with old code), I wonder how long we plan to support it... Anecdotally, from a time 1-2 decades ago when I used AIX daily, I can report that vast amounts of open source stuff couldn't build with xlc, so gcc was used for pretty much anything that didn't have a C++ ABI requirement. I kinda wonder if a single person in the entire world appreciates that we support this.
Re: Add support for AT LOCAL
I wrote:
> Yeah, the same thing occurred to me in the shower this morning, and it
> does seem to work! We can replace both loops with a %= operator, at
> least if we're willing to assume C99 division semantics, which seems
> pretty safe in 2023.
Whoops, no: for negative starting values we'd need truncate-towards-
minus-infinity division whereas C99 specifies truncate-towards-zero.
However, the attached does pass for me on cfarm111 as well as my
usual dev machine.
Presumably this is a pre-existing bug that also appears in back
branches. But in the interests of science I propose that we
back-patch only the test case and see which machine(s) fail it
before back-patching the code change.
regards, tom lane
diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index c4da10d47a..56c7746c11 100644
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -3083,10 +3083,11 @@ timetz_zone(PG_FUNCTION_ARGS)
result = (TimeTzADT *) palloc(sizeof(TimeTzADT));
result->time = t->time + (t->zone - tz) * USECS_PER_SEC;
+ /* C99 modulo has the wrong sign convention for negative input */
while (result->time < INT64CONST(0))
result->time += USECS_PER_DAY;
- while (result->time >= USECS_PER_DAY)
- result->time -= USECS_PER_DAY;
+ if (result->time >= USECS_PER_DAY)
+ result->time %= USECS_PER_DAY;
result->zone = tz;
@@ -3116,10 +3117,11 @@ timetz_izone(PG_FUNCTION_ARGS)
result = (TimeTzADT *) palloc(sizeof(TimeTzADT));
result->time = time->time + (time->zone - tz) * USECS_PER_SEC;
+ /* C99 modulo has the wrong sign convention for negative input */
while (result->time < INT64CONST(0))
result->time += USECS_PER_DAY;
- while (result->time >= USECS_PER_DAY)
- result->time -= USECS_PER_DAY;
+ if (result->time >= USECS_PER_DAY)
+ result->time %= USECS_PER_DAY;
result->zone = tz;
diff --git a/src/test/regress/expected/timetz.out b/src/test/regress/expected/timetz.out
index 3f8e005ce7..cbab6cfe5d 100644
--- a/src/test/regress/expected/timetz.out
+++ b/src/test/regress/expected/timetz.out
@@ -304,4 +304,25 @@ TABLE timetz_local_view;
23:59:59.99-07 | 06:59:59.99+00 | 06:59:59.99+00 | 06:59:59.99+00 | 06:59:59.99+00
(12 rows)
+SELECT f1 AS dat,
+ f1 AT TIME ZONE 'UTC+10' AS dat_at_tz,
+ f1 AT TIME ZONE INTERVAL '-10:00' AS dat_at_int
+ FROM TIMETZ_TBL
+ ORDER BY f1;
+ dat | dat_at_tz| dat_at_int
+++
+ 00:01:00-07| 21:01:00-10| 21:01:00-10
+ 01:00:00-07| 22:00:00-10| 22:00:00-10
+ 02:03:00-07| 23:03:00-10| 23:03:00-10
+ 08:08:00-04| 02:08:00-10| 02:08:00-10
+ 07:07:00-08| 05:07:00-10| 05:07:00-10
+ 11:59:00-07| 08:59:00-10| 08:59:00-10
+ 12:00:00-07| 09:00:00-10| 09:00:00-10
+ 12:01:00-07| 09:01:00-10| 09:01:00-10
+ 15:36:39-04| 09:36:39-10| 09:36:39-10
+ 15:36:39-05| 10:36:39-10| 10:36:39-10
+ 23:59:00-07| 20:59:00-10| 20:59:00-10
+ 23:59:59.99-07 | 20:59:59.99-10 | 20:59:59.99-10
+(12 rows)
+
ROLLBACK;
diff --git a/src/test/regress/sql/timetz.sql b/src/test/regress/sql/timetz.sql
index 33f7f8aafb..d797f478f0 100644
--- a/src/test/regress/sql/timetz.sql
+++ b/src/test/regress/sql/timetz.sql
@@ -100,4 +100,9 @@ CREATE VIEW timetz_local_view AS
ORDER BY f1;
SELECT pg_get_viewdef('timetz_local_view', true);
TABLE timetz_local_view;
+SELECT f1 AS dat,
+ f1 AT TIME ZONE 'UTC+10' AS dat_at_tz,
+ f1 AT TIME ZONE INTERVAL '-10:00' AS dat_at_int
+ FROM TIMETZ_TBL
+ ORDER BY f1;
ROLLBACK;
Re: Add support for AT LOCAL
Michael Paquier writes: > On Tue, Oct 17, 2023 at 01:40:18AM -0400, Tom Lane wrote: >> makes the failure go away. Unfortunately, I've not yet found another >> way to make it go away :-(. My upthread idea of using a local variable >> instead of result->time is no help, and some other random code >> alterations didn't change the results either. > That may be a long shot, but even a modulo? Yeah, the same thing occurred to me in the shower this morning, and it does seem to work! We can replace both loops with a %= operator, at least if we're willing to assume C99 division semantics, which seems pretty safe in 2023. Your idea of doing a range check to skip the division in typical cases is a refinement I'd not thought of, but it seems like a good idea for performance. (I see that the negative-starting-point case isn't covered in the current regression tests, so maybe we better add a test for that.) regards, tom lane
Re: Add support for AT LOCAL
On Tue, Oct 17, 2023 at 01:40:18AM -0400, Tom Lane wrote: > makes the failure go away. Unfortunately, I've not yet found another > way to make it go away :-(. My upthread idea of using a local variable > instead of result->time is no help, and some other random code > alterations didn't change the results either. That may be a long shot, but even a modulo? Say in these two code paths: - while (result->time >= USECS_PER_DAY) - result->time -= USECS_PER_DAY; + if (result->time >= USECS_PER_DAY) + result->time %= USECS_PER_DAY; > Not sure where we go from here. While I don't have much hesitation > about blowing off xlc_r 12.1, it would be sad if their latest > toolchain doesn't work either. (I didn't try permuting the code > while using the newer compiler, though.) We've spent a lot of time on that. I'm OK to just give up, trim the values of the view with a qual, and call it a day. -- Michael signature.asc Description: PGP signature
Re: Add support for AT LOCAL
Noah Misch writes: > On Mon, Oct 16, 2023 at 01:54:23AM -0400, Tom Lane wrote: >> Ugh. So if the failure is robust enough to persist across >> several major xlc versions, why couldn't Thomas reproduce it? > Beats me. hornet wipes its starting environment down to OBJECT_MODE=32_64 > PERL5LIB=/home/nm/sw/cpan64/lib/perl5 SPECIES=xlc64 PATH=/usr/bin, then > applies all the environment settings seen in buildfarm logs. I was able to reproduce the failure on cfarm111 after adopting these settings from hornet's configuration: export OBJECT_MODE=64 export CC='xlc_r -D_LARGE_FILES=1 ' export CFLAGS='-O2 -qmaxmem=33554432 -qsuppress=1500-010:1506-995 -qsuppress=1506-010:1506-416:1506-450:1506-480:1506-481:1506-492:1506-944:1506-1264 -qinfo=all:nocnd:noeff:noext:nogot:noini:noord:nopar:noppc:norea:nouni:nouse -qsuppress=1506-374:1506-419:1506-434:1506-438:1506-451:1506-452:1506-453:1506-495:1506-786' and doing ./configure --enable-cassert --enable-debug --without-icu --prefix=/home/tgl/testversion etc etc. It is absolutely, gold-platedly, a compiler bug, because inserting a debug printf into the loop while (result->time >= USECS_PER_DAY) result->time -= USECS_PER_DAY; makes the failure go away. Unfortunately, I've not yet found another way to make it go away :-(. My upthread idea of using a local variable instead of result->time is no help, and some other random code alterations didn't change the results either. Not sure where we go from here. While I don't have much hesitation about blowing off xlc_r 12.1, it would be sad if their latest toolchain doesn't work either. (I didn't try permuting the code while using the newer compiler, though.) regards, tom lane
Re: Add support for AT LOCAL
Michael Paquier writes: > On Mon, Oct 16, 2023 at 09:54:41AM -0400, Tom Lane wrote: >> I'm mighty tempted though to (a) add coverage for timetz_izone >> to HEAD, and (b) backpatch the new tests, sans the AT LOCAL case, >> to the back branches (maybe not v11). > I see that you've already done (a) with 2f04720307. I'd be curious to > see what happens for (b), as well, once (a) is processed on hornet > once.. Sure enough, timetz_izone has exactly the same behavior [1]. I'd kind of decided that back-patching wouldn't be worth the trouble; do you foresee that it'd teach us anything new? regards, tom lane [1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=hornet=2023-10-17%2000%3A07%3A36
Re: Add support for AT LOCAL
On Mon, Oct 16, 2023 at 09:54:41AM -0400, Tom Lane wrote: > I'm mighty tempted though to (a) add coverage for timetz_izone > to HEAD, and (b) backpatch the new tests, sans the AT LOCAL case, > to the back branches (maybe not v11). I see that you've already done (a) with 2f04720307. I'd be curious to see what happens for (b), as well, once (a) is processed on hornet once.. -- Michael signature.asc Description: PGP signature
Re: Add support for AT LOCAL
Michael Paquier writes:
> Perhaps that's a stupid question.. But a server running under this
> environment fails the two following queries even for older branches,
> right?
> select timezone('UTC', '23:59:59.99-07'::timetz);
> select timezone('UTC', '23:59:00-07'::timetz);
One would expect, since the AT LOCAL syntax is just sugar for that.
I'm mighty tempted though to (a) add coverage for timetz_izone
to HEAD, and (b) backpatch the new tests, sans the AT LOCAL case,
to the back branches (maybe not v11).
regards, tom lane
Re: Add support for AT LOCAL
On Sun, Oct 15, 2023 at 11:05:10PM -0700, Noah Misch wrote:
> On Mon, Oct 16, 2023 at 01:54:23AM -0400, Tom Lane wrote:
>> Ugh. So if the failure is robust enough to persist across
>> several major xlc versions, why couldn't Thomas reproduce it?
>
> Beats me. hornet wipes its starting environment down to OBJECT_MODE=32_64
> PERL5LIB=/home/nm/sw/cpan64/lib/perl5 SPECIES=xlc64 PATH=/usr/bin, then
> applies all the environment settings seen in buildfarm logs.
Perhaps that's a stupid question.. But a server running under this
environment fails the two following queries even for older branches,
right?
select timezone('UTC', '23:59:59.99-07'::timetz);
select timezone('UTC', '23:59:00-07'::timetz);
--
Michael
signature.asc
Description: PGP signature
Re: Add support for AT LOCAL
On Mon, Oct 16, 2023 at 01:54:23AM -0400, Tom Lane wrote: > Noah Misch writes: > > On Sun, Oct 15, 2023 at 09:58:04PM -0700, Noah Misch wrote: > >> Works for me. I've started a test run with the xlc version change. > > > It failed similarly: > > > + 23:59:00-07| 4294966103:4294967295:00+00 | > > 4294966103:4294967295:00+00 | 4294966103:4294967295:00+00 > > + 23:59:59.99-07 | 4294966103:00:00.01+00 | 4294966103:00:00.01+00 > > | 4294966103:00:00.01+00 > > Ugh. So if the failure is robust enough to persist across > several major xlc versions, why couldn't Thomas reproduce it? Beats me. hornet wipes its starting environment down to OBJECT_MODE=32_64 PERL5LIB=/home/nm/sw/cpan64/lib/perl5 SPECIES=xlc64 PATH=/usr/bin, then applies all the environment settings seen in buildfarm logs.
Re: Add support for AT LOCAL
Noah Misch writes: > On Sun, Oct 15, 2023 at 09:58:04PM -0700, Noah Misch wrote: >> Works for me. I've started a test run with the xlc version change. > It failed similarly: > + 23:59:00-07| 4294966103:4294967295:00+00 | 4294966103:4294967295:00+00 > | 4294966103:4294967295:00+00 > + 23:59:59.99-07 | 4294966103:00:00.01+00 | 4294966103:00:00.01+00 > | 4294966103:00:00.01+00 Ugh. So if the failure is robust enough to persist across several major xlc versions, why couldn't Thomas reproduce it? regards, tom lane
Re: Add support for AT LOCAL
On Sun, Oct 15, 2023 at 09:58:04PM -0700, Noah Misch wrote: > On Sun, Oct 15, 2023 at 11:30:17PM -0400, Tom Lane wrote: > > Thomas Munro writes: > > > On Mon, Oct 16, 2023 at 4:02 PM Tom Lane wrote: > > >> I'm having a hard time not believing that this is a compiler bug. > > >> Looking back at 8d2a01ae12cd and its speculation that xlc is overly > > >> liberal about reordering code around sequence points ... I wonder > > >> if it'd help to do this calculation in a local variable, and only > > >> assign the final value to result->time ? But we have to reproduce > > >> the problem first. > > > > > If that can be shown I would vote for switching to /opt/IBM/xlc/16.1.0 > > > and not changing a single bit of PostgreSQL. > > > > If switching to 16.1 removes the failure, I'd agree. It's hard > > to believe that any significant number of users still care about > > building PG with xlc 12. > > Works for me. I've started a test run with the xlc version change. It failed similarly: + 23:59:00-07| 4294966103:4294967295:00+00 | 4294966103:4294967295:00+00 | 4294966103:4294967295:00+00 + 23:59:59.99-07 | 4294966103:00:00.01+00 | 4294966103:00:00.01+00 | 4294966103:00:00.01+00
Re: Add support for AT LOCAL
On Sun, Oct 15, 2023 at 11:30:17PM -0400, Tom Lane wrote: > Thomas Munro writes: > > On Mon, Oct 16, 2023 at 4:02 PM Tom Lane wrote: > >> I'm having a hard time not believing that this is a compiler bug. > >> Looking back at 8d2a01ae12cd and its speculation that xlc is overly > >> liberal about reordering code around sequence points ... I wonder > >> if it'd help to do this calculation in a local variable, and only > >> assign the final value to result->time ? But we have to reproduce > >> the problem first. > > > If that can be shown I would vote for switching to /opt/IBM/xlc/16.1.0 > > and not changing a single bit of PostgreSQL. > > If switching to 16.1 removes the failure, I'd agree. It's hard > to believe that any significant number of users still care about > building PG with xlc 12. Works for me. I've started a test run with the xlc version change.
Re: Add support for AT LOCAL
On Sun, Oct 15, 2023 at 11:30:17PM -0400, Tom Lane wrote: > Thomas Munro writes: >> If that can be shown I would vote for switching to /opt/IBM/xlc/16.1.0 >> and not changing a single bit of PostgreSQL. > > If switching to 16.1 removes the failure, I'd agree. It's hard > to believe that any significant number of users still care about > building PG with xlc 12. FWIW, I really wish that we were less conservative here and just drop that rather than waste resources in debugging things. Now, I'm also OK to put this one aside and put a WHERE clause to timetz_local_view to only fetch one value, as the test has the same value as long as we check that AT LOCAL converts the result to UTC for the three expression patterns tested. -- Michael signature.asc Description: PGP signature
Re: Add support for AT LOCAL
Thomas Munro writes: > On Mon, Oct 16, 2023 at 4:02 PM Tom Lane wrote: >> I'm having a hard time not believing that this is a compiler bug. >> Looking back at 8d2a01ae12cd and its speculation that xlc is overly >> liberal about reordering code around sequence points ... I wonder >> if it'd help to do this calculation in a local variable, and only >> assign the final value to result->time ? But we have to reproduce >> the problem first. > If that can be shown I would vote for switching to /opt/IBM/xlc/16.1.0 > and not changing a single bit of PostgreSQL. If switching to 16.1 removes the failure, I'd agree. It's hard to believe that any significant number of users still care about building PG with xlc 12. regards, tom lane
Re: Add support for AT LOCAL
On Mon, Oct 16, 2023 at 4:02 PM Tom Lane wrote: > I'm having a hard time not believing that this is a compiler bug. > Looking back at 8d2a01ae12cd and its speculation that xlc is overly > liberal about reordering code around sequence points ... I wonder > if it'd help to do this calculation in a local variable, and only > assign the final value to result->time ? But we have to reproduce > the problem first. If that can be shown I would vote for switching to /opt/IBM/xlc/16.1.0 and not changing a single bit of PostgreSQL.
Re: Add support for AT LOCAL
Thomas Munro writes: > On Mon, Oct 16, 2023 at 2:58 PM Michael Paquier wrote: >> Another theory would be one of these weird compiler optimization issue >> from xlc? In recent history, there was 8d2a01ae12cd. > Yeah, there are more like that too. xlc 12.1 is dead (like the OS > version it shipped with). New versions are available on cfarm if we > care about this target. But I am conscious of the cosmic law that if > you blame the compiler too soon you can cause the bug to move into > your code... I'm having a hard time not believing that this is a compiler bug. Looking back at 8d2a01ae12cd and its speculation that xlc is overly liberal about reordering code around sequence points ... I wonder if it'd help to do this calculation in a local variable, and only assign the final value to result->time ? But we have to reproduce the problem first. regards, tom lane
Re: Add support for AT LOCAL
On Mon, Oct 16, 2023 at 2:58 PM Michael Paquier wrote: > Another theory would be one of these weird compiler optimization issue > from xlc? In recent history, there was 8d2a01ae12cd. Yeah, there are more like that too. xlc 12.1 is dead (like the OS version it shipped with). New versions are available on cfarm if we care about this target. But I am conscious of the cosmic law that if you blame the compiler too soon you can cause the bug to move into your code...
Re: Add support for AT LOCAL
On Mon, Oct 16, 2023 at 11:50:08AM +1300, Thomas Munro wrote: > On Mon, Oct 16, 2023 at 11:24 AM Thomas Munro wrote: >> On Mon, Oct 16, 2023 at 10:57 AM Tom Lane wrote: >>> I'm tempted to wonder if this helps: >>> >>> - result->time = t->time + (t->zone - tz) * USECS_PER_SEC; >>> + result->time = t->time + (int64) (t->zone - tz) * USECS_PER_SEC; All that should use TZNAME_FIXED_OFFSET as timezone type, and I don't really see why this would overflow.. Perhaps a more aggressive (int64) ((t->zone - (int64) tz) * USECS_PER_SEC) would help? >> I wanted to be able to try this and any other theories and managed to >> build the tip of master on cfarm111 with the same CC and CFLAGS as >> Noah used, but the problem didn't reproduce! Hmm, I didn't enable any >> extra options, so now I'm wondering if something in some random header >> somewhere is involved here... trying again with more stuff turned >> on... > > Oh, I can't use any of the handrolled packages in ~nm due to > permissions. I tried enabling perl from /opt/freeware (perl is my > usual first guess for who is !@#$ing with the system headers), but the > test passes. Another theory would be one of these weird compiler optimization issue from xlc? In recent history, there was 8d2a01ae12cd. -- Michael signature.asc Description: PGP signature
Re: Add support for AT LOCAL
On Sun, Oct 15, 2023 at 06:47:04PM -0400, Tom Lane wrote: > Another possibly interesting factoid: it appears that before > 97957fdba, we had zero regression test coverage of timetz_zone --- > and we still have none of timetz_izone, which contains essentially > the same code. So if there is a problem here, whether it's ours or > the compiler's, it's not hard to see why we didn't notice. Right. This one is just a lucky, or say unlucky find. I didn't notice that this path was entirely missing coverage, planting an assertion in the middle of timetz_zone() passes check-world. -- Michael signature.asc Description: PGP signature
Re: Add support for AT LOCAL
On Mon, Oct 16, 2023 at 11:24 AM Thomas Munro wrote: > On Mon, Oct 16, 2023 at 10:57 AM Tom Lane wrote: > > I'm tempted to wonder if this helps: > > > > - result->time = t->time + (t->zone - tz) * USECS_PER_SEC; > > + result->time = t->time + (int64) (t->zone - tz) * USECS_PER_SEC; > > I wanted to be able to try this and any other theories and managed to > build the tip of master on cfarm111 with the same CC and CFLAGS as > Noah used, but the problem didn't reproduce! Hmm, I didn't enable any > extra options, so now I'm wondering if something in some random header > somewhere is involved here... trying again with more stuff turned > on... Oh, I can't use any of the handrolled packages in ~nm due to permissions. I tried enabling perl from /opt/freeware (perl is my usual first guess for who is !@#$ing with the system headers), but the test passes.
Re: Add support for AT LOCAL
Another possibly interesting factoid: it appears that before 97957fdba, we had zero regression test coverage of timetz_zone --- and we still have none of timetz_izone, which contains essentially the same code. So if there is a problem here, whether it's ours or the compiler's, it's not hard to see why we didn't notice. regards, tom lane
Re: Add support for AT LOCAL
On Mon, Oct 16, 2023 at 10:57 AM Tom Lane wrote: > I'm tempted to wonder if this helps: > > - result->time = t->time + (t->zone - tz) * USECS_PER_SEC; > + result->time = t->time + (int64) (t->zone - tz) * USECS_PER_SEC; I wanted to be able to try this and any other theories and managed to build the tip of master on cfarm111 with the same CC and CFLAGS as Noah used, but the problem didn't reproduce! Hmm, I didn't enable any extra options, so now I'm wondering if something in some random header somewhere is involved here... trying again with more stuff turned on...
Re: Add support for AT LOCAL
Thomas Munro writes: > One of the AIX animals gave a strange result here: > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=hornet=2023-10-15%2011%3A40%3A01 > If you ignore the diffs due to change in column width, the interesting > change seems to be: > - 23:59:00-07| 06:59:00+00| 06:59:00+00| 06:59:00+00 > - 23:59:59.99-07 | 06:59:59.99+00 | 06:59:59.99+00 | 06:59:59.99+00 > + 23:59:00-07| 4294966103:4294967295:00+00 | > 4294966103:4294967295:00+00 | 4294966103:4294967295:00+00 > + 23:59:59.99-07 | 4294966103:00:00.01+00 | > 4294966103:00:00.01+00 | 4294966103:00:00.01+00 > But the other AIX animal 'sungazer' was OK with it. They're both on > the same AIX7.1 host IIRC, both 64 bit builds, but the former is using > xlc and the latter gcc. I don't immediately see what would cause that > underflow on that old compiler but not elsewhere. I have a shell > there (cfarm111) if someone has an idea... Hmm. Seems like the error has to be creeping in during this part of timetz_zone(): result->time = t->time + (t->zone - tz) * USECS_PER_SEC; while (result->time < INT64CONST(0)) result->time += USECS_PER_DAY; while (result->time >= USECS_PER_DAY) result->time -= USECS_PER_DAY; According to my machine, the initial computation of result->time (for the '23:59:59.99-07' input) yields 1115, and then we iterate the second loop once to get 251, which is the right answer. If I force a second iteration to get -6120001, I get # select '23:59:59.99-07'::timetz at local; timezone 4294967279:00:00.01+00 (1 row) which doesn't quite match hornet's result but it seems suggestively close. Another line of thought is that while the time fields are int64, t->zone and tz are only int32. Multiplying by the INT64CONST USECS_PER_SEC ought to be enough to make the compiler widen the subtraction result to int64, but maybe that's screwing up? I'm tempted to wonder if this helps: - result->time = t->time + (t->zone - tz) * USECS_PER_SEC; + result->time = t->time + (int64) (t->zone - tz) * USECS_PER_SEC; Forcing the wrong thing to happen there doesn't produce a match to hornet's result either, so I don't have a lot of hope for that theory, but it seems like the explanation has to be somewhere here. regards, tom lane
Re: Add support for AT LOCAL
One of the AIX animals gave a strange result here: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=hornet=2023-10-15%2011%3A40%3A01 If you ignore the diffs due to change in column width, the interesting change seems to be: - 23:59:00-07| 06:59:00+00| 06:59:00+00| 06:59:00+00 - 23:59:59.99-07 | 06:59:59.99+00 | 06:59:59.99+00 | 06:59:59.99+00 + 23:59:00-07| 4294966103:4294967295:00+00 | 4294966103:4294967295:00+00 | 4294966103:4294967295:00+00 + 23:59:59.99-07 | 4294966103:00:00.01+00 | 4294966103:00:00.01+00 | 4294966103:00:00.01+00 But the other AIX animal 'sungazer' was OK with it. They're both on the same AIX7.1 host IIRC, both 64 bit builds, but the former is using xlc and the latter gcc. I don't immediately see what would cause that underflow on that old compiler but not elsewhere. I have a shell there (cfarm111) if someone has an idea...
Re: Add support for AT LOCAL
On Fri, Oct 13, 2023 at 07:03:20AM +0200, Vik Fearing wrote: > Thank you, Michael君! No pb, ヴィックさん。 -- Michael signature.asc Description: PGP signature
Re: Add support for AT LOCAL
On 10/13/23 05:07, Michael Paquier wrote: On Fri, Oct 13, 2023 at 02:20:59AM +0200, Vik Fearing wrote: On 10/10/23 05:34, Michael Paquier wrote: I am attaching a v5 that addresses the documentation bits, could you look at the business with date.c? Here is a v6 Thanks for the new version. which hopefully addresses all of your concerns. Mostly ;) The first thing I did was to extract the doc bits about timezone(zone, time) for AT TIME ZONE from v6 and applied it independently. I have then looked at the rest and it looked mostly OK to me, including the extra description you have added for the fifth example in the docs. I have tweaked a few things: the regression tests to make the views a bit more appealing to the eye, an indentation to not have koel complain and did a catalog bump. Then applied it. Thank you, Michael君! -- Vik Fearing
Re: Add support for AT LOCAL
On Fri, Oct 13, 2023 at 02:20:59AM +0200, Vik Fearing wrote: > On 10/10/23 05:34, Michael Paquier wrote: > > I am attaching a v5 that addresses the documentation bits, could you > > look at the business with date.c? > > Here is a v6 Thanks for the new version. > which hopefully addresses all of your concerns. Mostly ;) The first thing I did was to extract the doc bits about timezone(zone, time) for AT TIME ZONE from v6 and applied it independently. I have then looked at the rest and it looked mostly OK to me, including the extra description you have added for the fifth example in the docs. I have tweaked a few things: the regression tests to make the views a bit more appealing to the eye, an indentation to not have koel complain and did a catalog bump. Then applied it. -- Michael signature.asc Description: PGP signature
Re: Add support for AT LOCAL
On 10/10/23 05:34, Michael Paquier wrote:
I am attaching a v5 that addresses the documentation bits, could you
look at the business with date.c?
Here is a v6 which hopefully addresses all of your concerns.
--
Vik Fearing
From 042ce9b581ca3b17afbf229d209ca59addb6c9a2 Mon Sep 17 00:00:00 2001
From: Vik Fearing
Date: Wed, 4 Oct 2023 15:46:38 +0100
Subject: [PATCH v6] Add support for AT LOCAL
When converting a timestamp to/from with/without time zone, the SQL
Standard specifies an AT LOCAL variant of AT TIME ZONE which uses the
session's time zone.
---
doc/src/sgml/func.sgml| 103 +-
src/backend/parser/gram.y | 7 ++
src/backend/utils/adt/date.c | 14 +++
src/backend/utils/adt/ruleutils.c | 10 +++
src/backend/utils/adt/timestamp.c | 20 +
src/include/catalog/pg_proc.dat | 9 ++
src/test/regress/expected/timestamptz.out | 47 ++
src/test/regress/expected/timetz.out | 39
src/test/regress/sql/timestamptz.sql | 21 +
src/test/regress/sql/timetz.sql | 17
10 files changed, 284 insertions(+), 3 deletions(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index f1ad64c3d6..ce62cb37b5 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -10604,42 +10604,46 @@ SELECT date_bin('15 minutes', TIMESTAMP '2020-02-11 15:44:17', TIMESTAMP '2001-0
that date_bin can truncate to an arbitrary interval.
The stride interval must be greater than zero and
cannot contain units of month or larger.
- AT TIME ZONE
+ AT TIME ZONE and AT LOCAL
time zone
conversion
AT TIME ZONE
+
+AT LOCAL
+
+
The AT TIME ZONE operator converts time
stamp without time zone to/from
time stamp with time zone, and
time with time zone values to different time
zones. shows its
variants.
- AT TIME ZONE Variants
+ AT TIME ZONE and AT LOCAL Variants
Operator
Description
@@ -10658,93 +10662,186 @@ SELECT date_bin('15 minutes', TIMESTAMP '2020-02-11 15:44:17', TIMESTAMP '2001-0
Converts given time stamp without time zone to
time stamp with time zone, assuming the given
value is in the named time zone.
timestamp '2001-02-16 20:38:40' at time zone 'America/Denver'
2001-02-17 03:38:40+00
+
+
+ timestamp without time zone AT LOCAL
+ timestamp with time zone
+
+
+ Converts given time stamp without time zone to
+ time stamp with the session's
+ TimeZone value as time zone.
+
+
+ timestamp '2001-02-16 20:38:40' at local
+ 2001-02-17 03:38:40+00
+
+
+
timestamp with time zone AT TIME ZONE zone
timestamp without time zone
Converts given time stamp with time zone to
time stamp without time zone, as the time would
appear in that zone.
timestamp with time zone '2001-02-16 20:38:40-05' at time zone 'America/Denver'
2001-02-16 18:38:40
+
+
+ timestamp with time zone AT LOCAL
+ timestamp without time zone
+
+
+ Converts given time stamp with time zone to
+ time stamp without time zone, as the time would
+ appear with the session's TimeZone value as time zone.
+
+
+ timestamp with time zone '2001-02-16 20:38:40-05' at local
+ 2001-02-16 18:38:40
+
+
+
time with time zone AT TIME ZONE zone
time with time zone
Converts given time with time zone to a new time
zone. Since no date is supplied, this uses the currently active UTC
offset for the named destination zone.
time with time zone '05:34:17-05' at time zone 'UTC'
10:34:17+00
+
+
+
+ time with time zone AT LOCAL
+ time with time zone
+
+
+ Converts given time with time zone to a new time
+ zone. Since no date is supplied, this uses the currently active UTC
+ offset for the session's TimeZone value.
+
+
+ Assuming the session's TimeZone is set to UTC:
+
+
+ time with time zone '05:34:17-05' at local
+ 10:34:17+00
+
+
In these expressions, the desired time zone zone can be
specified either as a text value (e.g., 'America/Los_Angeles')
or as an interval (e.g., INTERVAL '-08:00').
In the text case
Re: Add support for AT LOCAL
On Sat, Oct 07, 2023 at 02:35:06AM +0100, Vik Fearing wrote:
> I realized that my regression tests are not exercising what I originally
> intended them to after this change. They are supposed to show that calling
> the function explicitly or using AT LOCAL is correctly reproduced by
> ruleutils.c.
Yes, I don't really see the point in adding more tests for the
deparsing of AT TIME ZONE in this context. I would not expect one to
call directly timezone() with the grammar in place, but I have no
objections either to do that in the view for the regression tests as
you are suggesting in v4. The minimal set of changes to test is to
make sure that both paths (ts->tstz and tstz->tz) are exercised, and
that's what you do.
Anyway, upon review, I have a few issues with this patch. First is
the documentation that I find too light:
- There is no description for AT LOCAL and what kind of result one
gets back depending on the input given, while AT TIME ZONE has more
details about the arguments that can be used and the results
obtained.
- The function timezone(zone, timestamp) is mentioned, and I think
that we should do the same with timezone(timestamp) for AT LOCAL.
Another thing that I have been surprised with is the following, which
is inconsistent with AT TIME ZONE because we are lacking one system
function:
=# select time with time zone '05:34:17-05' at local;
ERROR: 42883: function pg_catalog.timezone(time with time zone) does not exist
I think that we should include that to have a full set of operations
supported, similar to AT TIME ZONE (see \df+ timezone). It looks like
this would need one extra timetz_at_local(), which would require a bit
more refactoring in date.c so as an equivalent of timetz_zone() could
feed on the current session's TimeZone instead. I guess that you
could just have an timetz_zone_internal() that optionally takes a
timezone provided by the user or gets the current session's Timezone
(session_timezone). Am I missing something?
I am attaching a v5 that addresses the documentation bits, could you
look at the business with date.c?
--
Michael
From e136d38967a863452762e1ee6e28d9ab40056adf Mon Sep 17 00:00:00 2001
From: Michael Paquier
Date: Tue, 10 Oct 2023 13:28:46 +0900
Subject: [PATCH v5] Add support for AT LOCAL with timestamps
When converting a timestamp to/from with/without time zone, the SQL
Standard specifies an AT LOCAL variant of AT TIME ZONE which uses the
session's time zone.
Note: bump catalog version
---
src/include/catalog/pg_proc.dat | 6 +++
src/backend/parser/gram.y | 7 +++
src/backend/utils/adt/ruleutils.c | 9
src/backend/utils/adt/timestamp.c | 20
src/test/regress/expected/timestamptz.out | 47 +
src/test/regress/sql/timestamptz.sql | 21
doc/src/sgml/func.sgml| 62 +--
7 files changed, 169 insertions(+), 3 deletions(-)
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index f0b7b9cbd8..21645b63a4 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -2319,6 +2319,12 @@
{ oid => '1159', descr => 'adjust timestamp to new time zone',
proname => 'timezone', prorettype => 'timestamp',
proargtypes => 'text timestamptz', prosrc => 'timestamptz_zone' },
+{ oid => '9159', descr => 'adjust timestamp to local time zone',
+ proname => 'timezone', prorettype => 'timestamp',
+ proargtypes => 'timestamptz', prosrc => 'timestamptz_at_local' },
+{ oid => '9160', descr => 'adjust timestamp to local time zone',
+ proname => 'timezone', prorettype => 'timestamptz',
+ proargtypes => 'timestamp', prosrc => 'timestamp_at_local' },
{ oid => '1160', descr => 'I/O',
proname => 'interval_in', provolatile => 's', prorettype => 'interval',
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index e56cbe77cb..50ed504e5a 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -14508,6 +14508,13 @@ a_expr: c_expr { $$ = $1; }
COERCE_SQL_SYNTAX,
@2);
}
+ | a_expr AT LOCAL %prec AT
+{
+ $$ = (Node *) makeFuncCall(SystemFuncName("timezone"),
+ list_make1($1),
+ COERCE_SQL_SYNTAX,
+ -1);
+}
/*
* These operators must be called out explicitly in order to make use
* of bison's automatic operator-precedence handling. All other
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 442205382e..9d1b0b13b1 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -10347,6 +10347,15 @@ get_func_sql_syntax(FuncExpr *expr, deparse_context *context)
appendStringInfoChar(buf, ')');
return true;
+ case F_TIMEZONE_TIMESTAMP:
+ case F_TIMEZONE_TIMESTAMPTZ:
+ /* AT LOCAL */
+ appe
Re: Add support for AT LOCAL
On 10/6/23 07:05, Michael Paquier wrote:
I haven't yet finished my review of the patch, still looking at it.
I realized that my regression tests are not exercising what I originally
intended them to after this change. They are supposed to show that
calling the function explicitly or using AT LOCAL is correctly
reproduced by ruleutils.c.
The attached v4 changes the regression tests (and nothing else).
--
Vik Fearing
From 03273214f0320e347a0b012763dc82cd91ae6775 Mon Sep 17 00:00:00 2001
From: Vik Fearing
Date: Wed, 4 Oct 2023 15:46:38 +0100
Subject: [PATCH v4] Add support for AT LOCAL
When converting a timestamp to/from with/without time zone, the SQL
Standard specifies an AT LOCAL variant of AT TIME ZONE which uses the
session's time zone.
---
doc/src/sgml/func.sgml| 13 +++
src/backend/parser/gram.y | 7
src/backend/utils/adt/ruleutils.c | 9 +
src/backend/utils/adt/timestamp.c | 20 ++
src/include/catalog/pg_proc.dat | 6 +++
src/test/regress/expected/timestamptz.out | 47 +++
src/test/regress/sql/timestamptz.sql | 21 ++
7 files changed, 123 insertions(+)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index f1ad64c3d6..6bc61705a9 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -10619,12 +10619,16 @@ SELECT date_bin('15 minutes', TIMESTAMP '2020-02-11 15:44:17', TIMESTAMP '2001-0
AT TIME ZONE
+
+AT LOCAL
+
+
The AT TIME ZONE operator converts time
stamp without time zone to/from
time stamp with time zone, and
time with time zone values to different time
zones. shows its
@@ -10707,24 +10711,33 @@ SELECT date_bin('15 minutes', TIMESTAMP '2020-02-11 15:44:17', TIMESTAMP '2001-0
In the text case, a time zone name can be specified in any of the ways
described in .
The interval case is only useful for zones that have fixed offsets from
UTC, so it is not very common in practice.
+
+The syntax AT LOCAL may be used as shorthand for AT TIME ZONE
+local, where local is the
+session's TimeZone value.
+
+
Examples (assuming the current setting
is America/Los_Angeles):
SELECT TIMESTAMP '2001-02-16 20:38:40' AT TIME ZONE 'America/Denver';
Result: 2001-02-16 19:38:40-08
SELECT TIMESTAMP WITH TIME ZONE '2001-02-16 20:38:40-05' AT TIME ZONE 'America/Denver';
Result: 2001-02-16 18:38:40
SELECT TIMESTAMP '2001-02-16 20:38:40' AT TIME ZONE 'Asia/Tokyo' AT TIME ZONE 'America/Chicago';
Result: 2001-02-16 05:38:40
+
+SELECT TIMESTAMP WITH TIME ZONE '2001-02-16 20:38:40-05' AT LOCAL;
+Result: 2001-02-16 17:38:40
The first example adds a time zone to a value that lacks it, and
displays the value using the current TimeZone
setting. The second example shifts the time stamp with time zone value
to the specified time zone, and returns the value without a time zone.
This allows storage and display of values different from the current
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index e56cbe77cb..50ed504e5a 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -14505,12 +14505,19 @@ a_expr: c_expr { $$ = $1; }
{
$$ = (Node *) makeFuncCall(SystemFuncName("timezone"),
list_make2($5, $1),
COERCE_SQL_SYNTAX,
@2);
}
+ | a_expr AT LOCAL %prec AT
+{
+ $$ = (Node *) makeFuncCall(SystemFuncName("timezone"),
+ list_make1($1),
+ COERCE_SQL_SYNTAX,
+ -1);
+}
/*
* These operators must be called out explicitly in order to make use
* of bison's automatic operator-precedence handling. All other
* operator names are handled by the generic productions using "Op",
* below; and all those operators will have the same precedence.
*
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 442205382e..9d1b0b13b1 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -10344,12 +10344,21 @@ get_func_sql_syntax(FuncExpr *expr, deparse_context *context)
appendStringInfoString(buf, " AT TIME ZONE ");
get_rule_expr_paren((Node *) linitial(expr->args), context, false,
(Node *) expr);
appendStringInfoChar(buf, ')');
return true;
+ case F_TIMEZONE_TIMESTAMP:
+ case F_TIMEZONE_TIMESTAMPTZ:
+ /* AT LOCAL */
+ appendStringInfoChar(buf, '(');
+ get_rule_expr_paren((Node *) linitial(expr->args), context, false,
+(Node *) expr);
+ appendStringInfoString(buf, " AT LOCAL)");
+ return true;
+
case F_OVERLAPS_TIMESTAMPTZ_INTERVAL_TIMESTAMPTZ_INTERVAL:
case F_OVERLAPS_TIMESTAMPTZ_INTERVAL_TIMESTAMPTZ_TIMESTAMPTZ:
case F_OVERLAPS_TIMESTAMPTZ_TIMESTAMPTZ_TIMESTAMPTZ_INTERVAL:
case F_
Re: Add support for AT LOCAL
On Wed, Oct 04, 2023 at 03:49:03PM +0100, Vik Fearing wrote: > Okay. Here is a v3 using that approach. You have not posted any numbers to show if there's a difference in performance, so I have run a simple test: PREPARE test AS SELECT TIMESTAMP '1978-07-07 19:38' AT LOCAL; DO $$ BEGIN FOR i IN 1..100 LOOP EXECUTE 'EXECUTE test'; END LOOP; END $$; On a medium-ish benchmark machine I have (16 vCPUs, 32GB of memory, -O2, no asserts), this DO block takes in average 4.3s to run with v2, versus 3.6s with v3. So yes, that's faster. I haven't yet finished my review of the patch, still looking at it. -- Michael signature.asc Description: PGP signature
Re: Add support for AT LOCAL
On 9/29/23 09:27, Michael Paquier wrote:
On Sat, Sep 23, 2023 at 12:54:01AM +0200, Vik Fearing wrote:
On 9/22/23 23:46, cary huang wrote:
I think this feature can be a useful addition in dealing with time
zones. I have applied and tried out the patch, The feature works as
described and seems promising. The problem with compilation failure
was probably reported on CirrusCI when compiled on different
platforms. I have run the latest patch on my own Cirrus CI environment
and everything checked out fine.
Thank you for reviewing!
+| a_expr AT LOCAL%prec AT
+{
+/* Use the value of the session's time zone */
+FuncCall *tz =
makeFuncCall(SystemFuncName("current_setting"),
+
list_make1(makeStringConst("TimeZone", -1)),
+COERCE_SQL_SYNTAX,
+-1);
+$$ = (Node *) makeFuncCall(SystemFuncName("timezone"),
+ list_make2(tz, $1),
+ COERCE_SQL_SYNTAX,
+ @2);
As the deparsing code introduced by this patch is showing, this leads
to a lot of extra complexity. And, actually, this can be quite
expensive as well with these two layers of functions. Note also that
in comparison to SQLValueFunctions, COERCE_SQL_SYNTAX does less
inlining. So here comes my question: why doesn't this stuff just use
one underlying function to do this job?
Okay. Here is a v3 using that approach.
--
Vik Fearing
From e68305dafd2d4f9439cbd341e8f04745d8a945ed Mon Sep 17 00:00:00 2001
From: Vik Fearing
Date: Wed, 4 Oct 2023 15:46:38 +0100
Subject: [PATCH v3] Add support for AT LOCAL
When converting a timestamp to/from with/without time zone, the SQL
Standard specifies an AT LOCAL variant of AT TIME ZONE which uses the
session's time zone.
---
doc/src/sgml/func.sgml| 13 +++
src/backend/parser/gram.y | 7
src/backend/utils/adt/ruleutils.c | 9 +
src/backend/utils/adt/timestamp.c | 20 ++
src/include/catalog/pg_proc.dat | 6 +++
src/test/regress/expected/timestamptz.out | 47 +++
src/test/regress/sql/timestamptz.sql | 21 ++
7 files changed, 123 insertions(+)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index f1ad64c3d6..6bc61705a9 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -10619,12 +10619,16 @@ SELECT date_bin('15 minutes', TIMESTAMP '2020-02-11 15:44:17', TIMESTAMP '2001-0
AT TIME ZONE
+
+AT LOCAL
+
+
The AT TIME ZONE operator converts time
stamp without time zone to/from
time stamp with time zone, and
time with time zone values to different time
zones. shows its
@@ -10707,24 +10711,33 @@ SELECT date_bin('15 minutes', TIMESTAMP '2020-02-11 15:44:17', TIMESTAMP '2001-0
In the text case, a time zone name can be specified in any of the ways
described in .
The interval case is only useful for zones that have fixed offsets from
UTC, so it is not very common in practice.
+
+The syntax AT LOCAL may be used as shorthand for AT TIME ZONE
+local, where local is the
+session's TimeZone value.
+
+
Examples (assuming the current setting
is America/Los_Angeles):
SELECT TIMESTAMP '2001-02-16 20:38:40' AT TIME ZONE 'America/Denver';
Result: 2001-02-16 19:38:40-08
SELECT TIMESTAMP WITH TIME ZONE '2001-02-16 20:38:40-05' AT TIME ZONE 'America/Denver';
Result: 2001-02-16 18:38:40
SELECT TIMESTAMP '2001-02-16 20:38:40' AT TIME ZONE 'Asia/Tokyo' AT TIME ZONE 'America/Chicago';
Result: 2001-02-16 05:38:40
+
+SELECT TIMESTAMP WITH TIME ZONE '2001-02-16 20:38:40-05' AT LOCAL;
+Result: 2001-02-16 17:38:40
The first example adds a time zone to a value that lacks it, and
displays the value using the current TimeZone
setting. The second example shifts the time stamp with time zone value
to the specified time zone, and returns the value without a time zone.
This allows storage and display of values different from the current
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index e56cbe77cb..50ed504e5a 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -14505,12 +14505,19 @@ a_expr: c_expr { $$ = $1; }
{
$$ = (Node *) makeFuncCall(SystemFuncName("timezone"),
list_make2($5, $1),
COERCE_SQL_SYNTAX,
@2);
}
+ | a_expr AT LOCAL %prec AT
+{
+ $$ = (Node *) makeFuncCall(SystemFuncName("timezone"),
+ list_make1($1),
+ COERCE_SQL_SYNTAX,
+ -1);
+}
/*
Re: Add support for AT LOCAL
On Tue, Oct 03, 2023 at 02:10:48AM +0200, Vik Fearing wrote: > On 9/29/23 09:27, Michael Paquier wrote: >> As the deparsing code introduced by this patch is showing, this leads >> to a lot of extra complexity. And, actually, this can be quite >> expensive as well with these two layers of functions. Note also that >> in comparison to SQLValueFunctions, COERCE_SQL_SYNTAX does less >> inlining. So here comes my question: why doesn't this stuff just use >> one underlying function to do this job? > > I guess I don't understand the question. Why do you think a single function > that repeats what these functions do would be preferable? I am not sure how > doing it a different way would be better. Leaving aside the ruleutils.c changes introduced by the patch that are quite confusing, having one function in the executor stack is going to be more efficient than two (aka less ExecInitFunc), and this syntax could be used in SQL queries where the operations is repeated a lot. This patch introduces two COERCE_SQL_SYNTAX, meaning that we would do twice the ACL check, twice the function hook, etc, so this could lead to significant differences. I think that we should be careful with the approach taken, and do benchmarks to choose an efficient approach from the start. See for example: https://www.postgresql.org/message-id/zghbge45jkw8f...@paquier.xyz -- Michael signature.asc Description: PGP signature
Re: Add support for AT LOCAL
On 9/29/23 09:27, Michael Paquier wrote:
On Sat, Sep 23, 2023 at 12:54:01AM +0200, Vik Fearing wrote:
On 9/22/23 23:46, cary huang wrote:
I think this feature can be a useful addition in dealing with time
zones. I have applied and tried out the patch, The feature works as
described and seems promising. The problem with compilation failure
was probably reported on CirrusCI when compiled on different
platforms. I have run the latest patch on my own Cirrus CI environment
and everything checked out fine.
Thank you for reviewing!
+| a_expr AT LOCAL%prec AT
+{
+/* Use the value of the session's time zone */
+FuncCall *tz =
makeFuncCall(SystemFuncName("current_setting"),
+
list_make1(makeStringConst("TimeZone", -1)),
+COERCE_SQL_SYNTAX,
+-1);
+$$ = (Node *) makeFuncCall(SystemFuncName("timezone"),
+ list_make2(tz, $1),
+ COERCE_SQL_SYNTAX,
+ @2);
As the deparsing code introduced by this patch is showing, this leads
to a lot of extra complexity. And, actually, this can be quite
expensive as well with these two layers of functions. Note also that
in comparison to SQLValueFunctions, COERCE_SQL_SYNTAX does less
inlining. So here comes my question: why doesn't this stuff just use
one underlying function to do this job?
I guess I don't understand the question. Why do you think a single
function that repeats what these functions do would be preferable? I am
not sure how doing it a different way would be better.
--
Vik Fearing
Re: Add support for AT LOCAL
On Sat, Sep 23, 2023 at 12:54:01AM +0200, Vik Fearing wrote:
> On 9/22/23 23:46, cary huang wrote:
>> I think this feature can be a useful addition in dealing with time
>> zones. I have applied and tried out the patch, The feature works as
>> described and seems promising. The problem with compilation failure
>> was probably reported on CirrusCI when compiled on different
>> platforms. I have run the latest patch on my own Cirrus CI environment
>> and everything checked out fine.
>
> Thank you for reviewing!
+| a_expr AT LOCAL%prec AT
+{
+/* Use the value of the session's time zone */
+FuncCall *tz =
makeFuncCall(SystemFuncName("current_setting"),
+
list_make1(makeStringConst("TimeZone", -1)),
+COERCE_SQL_SYNTAX,
+-1);
+$$ = (Node *) makeFuncCall(SystemFuncName("timezone"),
+ list_make2(tz, $1),
+ COERCE_SQL_SYNTAX,
+ @2);
As the deparsing code introduced by this patch is showing, this leads
to a lot of extra complexity. And, actually, this can be quite
expensive as well with these two layers of functions. Note also that
in comparison to SQLValueFunctions, COERCE_SQL_SYNTAX does less
inlining. So here comes my question: why doesn't this stuff just use
one underlying function to do this job?
--
Michael
signature.asc
Description: PGP signature
Re: Add support for AT LOCAL
On 9/22/23 23:46, cary huang wrote: The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation:tested, passed Hello I think this feature can be a useful addition in dealing with time zones. I have applied and tried out the patch, The feature works as described and seems promising. The problem with compilation failure was probably reported on CirrusCI when compiled on different platforms. I have run the latest patch on my own Cirrus CI environment and everything checked out fine. Thank you Thank you for reviewing! -- Vik Fearing
Re: Add support for AT LOCAL
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation:tested, passed Hello I think this feature can be a useful addition in dealing with time zones. I have applied and tried out the patch, The feature works as described and seems promising. The problem with compilation failure was probably reported on CirrusCI when compiled on different platforms. I have run the latest patch on my own Cirrus CI environment and everything checked out fine. Thank you Cary Huang -- Highgo Software Canada www.highgo.ca
Re: Add support for AT LOCAL
On 7/3/23 15:42, Daniel Gustafsson wrote:
On 6 Jun 2023, at 05:13, Vik Fearing wrote:
Patch against 3f1180 attached.
This patch fails to compile, the declaration of variables in the switch block
needs to be scoped within a { } block.
Interesting. It compiles for me.
I've fixed this trivial error in the
attached v2 and also reflowed the comments which now no longer fit.
Thank you.
--
Vik Fearing
Re: Add support for AT LOCAL
> On 6 Jun 2023, at 05:13, Vik Fearing wrote:
> Patch against 3f1180 attached.
This patch fails to compile, the declaration of variables in the switch block
needs to be scoped within a { } block. I've fixed this trivial error in the
attached v2 and also reflowed the comments which now no longer fit.
--
Daniel Gustafsson
v2-0001-Add-support-for-AT-LOCAL.patch
Description: Binary data
Re: Add support for AT LOCAL
On 6/12/23 17:37, Alvaro Herrera wrote: On 2023-Jun-06, Laurenz Albe wrote: At a quick glance, it looks like you resolve "timezone" at the time the query is parsed. Shouldn't the resolution happen at query execution time? Sounds like it -- consider the case where the timestamp value is a partition key and one of the partition boundaries falls in between two timezone offsets for some particular ts value; then you use a prepared query to read from a view defined with AT LOCAL. Partition pruning would need to compute partitions to read from at runtime, not plan time. Can you show me an example of that happening with my patch? -- Vik Fearing
Re: Add support for AT LOCAL
On 2023-Jun-06, Laurenz Albe wrote: > At a quick glance, it looks like you resolve "timezone" at the time > the query is parsed. Shouldn't the resolution happen at query > execution time? Sounds like it -- consider the case where the timestamp value is a partition key and one of the partition boundaries falls in between two timezone offsets for some particular ts value; then you use a prepared query to read from a view defined with AT LOCAL. Partition pruning would need to compute partitions to read from at runtime, not plan time. -- Álvaro HerreraBreisgau, Deutschland — https://www.EnterpriseDB.com/
Re: Add support for AT LOCAL
On Tue, 2023-06-06 at 04:24 -0400, Vik Fearing wrote: > > At a quick glance, it looks like you resolve "timezone" at the time > > the query is parsed. Shouldn't the resolution happen at query > > execution time? > > current_setting(text) is stable, and my tests show that it is calculated > at execution time. Ah, ok, then sorry for the noise. I misread the code then. Yours, Laurenz Albe
Re: Add support for AT LOCAL
On 6/6/23 03:56, Laurenz Albe wrote: On Mon, 2023-06-05 at 23:13 -0400, Vik Fearing wrote: The Standard defines time zone conversion as follows: ::= [ ] ::= AT ::= LOCAL | TIME ZONE While looking at something else, I noticed we do not support AT LOCAL. The local time zone is defined as that of *the session*, not the server, which can make this quite interesting in views where the view will automatically adjust to the session's time zone. Patch against 3f1180 attached. +1 on the idea; it should be faily trivial, if not very useful. Thanks. At a quick glance, it looks like you resolve "timezone" at the time the query is parsed. Shouldn't the resolution happen at query execution time? current_setting(text) is stable, and my tests show that it is calculated at execution time. postgres=# prepare x as values (now() at local); PREPARE postgres=# set timezone to 'UTC'; SET postgres=# execute x; column1 2023-06-06 08:23:02.088634 (1 row) postgres=# set timezone to 'Asia/Pyongyang'; SET postgres=# execute x; column1 2023-06-06 17:23:14.837219 (1 row) Am I missing something? -- Vik Fearing
Re: Add support for AT LOCAL
On Mon, 2023-06-05 at 23:13 -0400, Vik Fearing wrote: > The Standard defines time zone conversion as follows: > > ::= > [ ] > > ::= > AT > > ::= > LOCAL > | TIME ZONE > > > While looking at something else, I noticed we do not support AT LOCAL. > The local time zone is defined as that of *the session*, not the server, > which can make this quite interesting in views where the view will > automatically adjust to the session's time zone. > > Patch against 3f1180 attached. +1 on the idea; it should be faily trivial, if not very useful. At a quick glance, it looks like you resolve "timezone" at the time the query is parsed. Shouldn't the resolution happen at query execution time? Yours, Laurenz Albe
Add support for AT LOCAL
The Standard defines time zone conversion as follows:
::=
[ ]
::=
AT
::=
LOCAL
| TIME ZONE
While looking at something else, I noticed we do not support AT LOCAL.
The local time zone is defined as that of *the session*, not the server,
which can make this quite interesting in views where the view will
automatically adjust to the session's time zone.
Patch against 3f1180 attached.
--
Vik FearingFrom b8317f3070c11df1e2ad791bd8d823aaae66dbe4 Mon Sep 17 00:00:00 2001
From: Vik Fearing
Date: Mon, 5 Jun 2023 19:42:42 -0400
Subject: [PATCH v1] Add support for AT LOCAL
When converting a timestamp to/from with/without time zone, the SQL
Standard specifies an AT LOCAL variant of AT TIME ZONE which uses the
session's time zone.
---
doc/src/sgml/func.sgml| 13 +++
src/backend/parser/gram.y | 12 ++
src/backend/utils/adt/ruleutils.c | 43 ++---
src/test/regress/expected/timestamptz.out | 47 +++
src/test/regress/sql/timestamptz.sql | 21 ++
5 files changed, 131 insertions(+), 5 deletions(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 5a47ce4343..6d07f063e0 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -10553,14 +10553,18 @@ SELECT date_bin('15 minutes', TIMESTAMP '2020-02-11 15:44:17', TIMESTAMP '2001-0
conversion
AT TIME ZONE
+
+AT LOCAL
+
+
The AT TIME ZONE operator converts time
stamp without time zone to/from
time stamp with time zone, and
time with time zone values to different time
zones. shows its
variants.
@@ -10641,26 +10645,35 @@ SELECT date_bin('15 minutes', TIMESTAMP '2020-02-11 15:44:17', TIMESTAMP '2001-0
or as an interval (e.g., INTERVAL '-08:00').
In the text case, a time zone name can be specified in any of the ways
described in .
The interval case is only useful for zones that have fixed offsets from
UTC, so it is not very common in practice.
+
+The syntax AT LOCAL may be used as shorthand for AT TIME ZONE
+local, where local is the
+session's TimeZone value.
+
+
Examples (assuming the current setting
is America/Los_Angeles):
SELECT TIMESTAMP '2001-02-16 20:38:40' AT TIME ZONE 'America/Denver';
Result: 2001-02-16 19:38:40-08
SELECT TIMESTAMP WITH TIME ZONE '2001-02-16 20:38:40-05' AT TIME ZONE 'America/Denver';
Result: 2001-02-16 18:38:40
SELECT TIMESTAMP '2001-02-16 20:38:40' AT TIME ZONE 'Asia/Tokyo' AT TIME ZONE 'America/Chicago';
Result: 2001-02-16 05:38:40
+
+SELECT TIMESTAMP WITH TIME ZONE '2001-02-16 20:38:40-05' AT LOCAL;
+Result: 2001-02-16 17:38:40
The first example adds a time zone to a value that lacks it, and
displays the value using the current TimeZone
setting. The second example shifts the time stamp with time zone value
to the specified time zone, and returns the value without a time zone.
This allows storage and display of values different from the current
TimeZone setting. The third example converts
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 39ab7eac0d..2b27904970 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -14414,14 +14414,26 @@ a_expr: c_expr { $$ = $1; }
| a_expr AT TIME ZONE a_expr %prec AT
{
$$ = (Node *) makeFuncCall(SystemFuncName("timezone"),
list_make2($5, $1),
COERCE_SQL_SYNTAX,
@2);
}
+ | a_expr AT LOCAL %prec AT
+{
+ /* Use the value of the session's time zone */
+ FuncCall *tz = makeFuncCall(SystemFuncName("current_setting"),
+list_make1(makeStringConst("TimeZone", -1)),
+COERCE_SQL_SYNTAX,
+-1);
+ $$ = (Node *) makeFuncCall(SystemFuncName("timezone"),
+ list_make2(tz, $1),
+ COERCE_SQL_SYNTAX,
+ @2);
+}
/*
* These operators must be called out explicitly in order to make use
* of bison's automatic operator-precedence handling. All other
* operator names are handled by the generic productions using "Op",
* below; and all those operators will have the same precedence.
*
* If you add more explicitly-known operators, be sure to add them
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index d3a973d86b..1fca65a7f2 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -10313,20 +10313,53 @@ get_func_sql_syntax(FuncExpr *expr, deparse_context *context)
case F_TIMEZONE_INTERVAL_TIMESTAMP:
case F_TIMEZONE_INTERVAL_TIMESTAMPTZ:
case F_TIMEZONE_INTERVAL_TIMETZ:
case F_TIMEZONE_TEXT_TIMESTAMP:
case F_TIMEZONE_TEXT_TIMESTAMPTZ:
case F_TIMEZONE_TEXT_TIMETZ:
/* AT TIME ZONE ... note reversed argument order */
+ Node *t
