Re: Getting rid of SQLValueFunction

2022-12-30 Thread Michael Paquier
On Fri, Dec 30, 2022 at 10:57:52AM +0900, Ian Lawrence Barwick wrote: > I noticed this commit (f193883f) introduces following regressions: > > postgres=# SELECT current_timestamp(7); > WARNING: TIMESTAMP(7) WITH TIME ZONE precision reduced to maximum > allowed, 6 > ERROR:

Re: Getting rid of SQLValueFunction

2022-12-29 Thread Michael Paquier
On Fri, Dec 30, 2022 at 10:57:52AM +0900, Ian Lawrence Barwick wrote: > I noticed this commit (f193883f) introduces following regressions: > > postgres=# SELECT current_timestamp(7); > WARNING: TIMESTAMP(7) WITH TIME ZONE precision reduced to maximum > allowed, 6 > ERROR:

Re: Getting rid of SQLValueFunction

2022-12-29 Thread Ian Lawrence Barwick
Hi 2022年11月21日(月) 18:39 Michael Paquier : > > On Sun, Nov 20, 2022 at 03:15:34PM -0800, Ted Yu wrote: > > + * timestamp. These require a specific handling with their typmod is given > > + * by the function caller through their SQL keyword. > > > > typo: typmod is given -> typmod given > > > >

Re: Getting rid of SQLValueFunction

2022-11-21 Thread Michael Paquier
On Sun, Nov 20, 2022 at 03:15:34PM -0800, Ted Yu wrote: > + * timestamp. These require a specific handling with their typmod is given > + * by the function caller through their SQL keyword. > > typo: typmod is given -> typmod given > > Other than the above, code looks good to me. Thanks for

Re: Getting rid of SQLValueFunction

2022-11-20 Thread Ted Yu
On Sun, Nov 20, 2022 at 3:12 PM Michael Paquier wrote: > On Sun, Nov 20, 2022 at 08:21:10AM -0800, Ted Yu wrote: > > For get_func_sql_syntax(), the code for cases > > of F_CURRENT_TIME, F_CURRENT_TIMESTAMP, F_LOCALTIME and F_LOCALTIMESTAMP > is > > mostly the same. > > Maybe we can introduce a

Re: Getting rid of SQLValueFunction

2022-11-20 Thread Michael Paquier
On Sun, Nov 20, 2022 at 08:21:10AM -0800, Ted Yu wrote: > For get_func_sql_syntax(), the code for cases > of F_CURRENT_TIME, F_CURRENT_TIMESTAMP, F_LOCALTIME and F_LOCALTIMESTAMP is > mostly the same. > Maybe we can introduce a helper so that code duplication is reduced. It would. Thanks for the

Re: Getting rid of SQLValueFunction

2022-11-20 Thread Ted Yu
On Sat, Nov 19, 2022 at 7:01 PM Michael Paquier wrote: > On Fri, Nov 18, 2022 at 10:23:58AM +0900, Michael Paquier wrote: > > Please note that in order to avoid tweaks when choosing the attribute > > name of function call, this needs a total of 8 new catalog functions > > mapping to the SQL

Re: Getting rid of SQLValueFunction

2022-11-19 Thread Michael Paquier
On Fri, Nov 18, 2022 at 10:23:58AM +0900, Michael Paquier wrote: > Please note that in order to avoid tweaks when choosing the attribute > name of function call, this needs a total of 8 new catalog functions > mapping to the SQL keywords, which is what the test added by 2e0d80c > is about: > -

Re: Getting rid of SQLValueFunction

2022-11-17 Thread Michael Paquier
On Tue, Oct 25, 2022 at 02:20:12PM +0900, Michael Paquier wrote: > Attached is a rebased patch set, as of the conflicts from 2e0d80c. So, this patch set has been sitting in the CF app for a few weeks now, and I would like to apply them to remove a bit of code from the executor. Please note that

Re: Getting rid of SQLValueFunction

2022-10-24 Thread Michael Paquier
On Fri, Oct 21, 2022 at 02:27:07PM +0900, Michael Paquier wrote: > I have looked at that, and the attribute mapping remains compatible > with past versions once the appropriate pg_proc entries are added. > The updated patch set attached does that (with a user() function as > well to keep the code

Re: Getting rid of SQLValueFunction

2022-10-20 Thread Michael Paquier
On Fri, Oct 21, 2022 at 12:34:23PM +0900, Michael Paquier wrote: > A sticky point is that this would need the creation of a pg_proc entry > for "user" which is a generic word, or a shortcut around > FigureColnameInternal(). The code gain overall still looks appealing > in the executor, even if we

Re: Getting rid of SQLValueFunction

2022-10-20 Thread Michael Paquier
On Thu, Oct 20, 2022 at 11:10:22PM -0400, Tom Lane wrote: > The entire point of SQLValueFunction IMO was to hide the underlying > implementation(s). Replacing it with something that leaks > implementation details does not seem like a step forward. Hmm.. Okay, thanks. So this just comes down

Re: Getting rid of SQLValueFunction

2022-10-20 Thread Tom Lane
Michael Paquier writes: > But the patch enforces the attribute name to be the underlying > function name, switching the previous "current_catalog" to > "current_database". The entire point of SQLValueFunction IMO was to hide the underlying implementation(s). Replacing it with something that

Re: Getting rid of SQLValueFunction

2022-10-20 Thread Michael Paquier
On Wed, Oct 19, 2022 at 03:45:48PM +0900, Michael Paquier wrote: > With this in mind, would somebody complain if I commit that? That's a > nice reduction in code, while completing the work done in 40c24bf: > 25 files changed, 338 insertions(+), 477 deletions(-) On second look, there is

Re: Getting rid of SQLValueFunction

2022-10-19 Thread Michael Paquier
(Adding Tom in CC, in case.) On Tue, Oct 18, 2022 at 04:35:33PM -0400, Corey Huinker wrote: > I agree that we shouldn't spend too much effort on a good error message > here, but perhaps we should have the message mention that it is > date/time-related? A person git-grepping for this error message

Re: Getting rid of SQLValueFunction

2022-10-18 Thread Corey Huinker
On Fri, Sep 30, 2022 at 2:04 AM Michael Paquier wrote: > Hi all, > > I have bumped a few days ago on the fact that COERCE_SQL_SYNTAX > (introduced by 40c24bf) and SQLValueFunction are around to do the > exact same thing, as known as enforcing single-function calls with > dedicated SQL keywords.

Getting rid of SQLValueFunction

2022-09-30 Thread Michael Paquier
Hi all, I have bumped a few days ago on the fact that COERCE_SQL_SYNTAX (introduced by 40c24bf) and SQLValueFunction are around to do the exact same thing, as known as enforcing single-function calls with dedicated SQL keywords. For example, keywords like SESSION_USER, CURRENT_DATE, etc. go