Обсуждение: Is there a better way to do this?
Hi all I want to implement something like the following: CREATE OR REPLACE FUNCTION AddDays (TIMESTAMP WITHOUT TIME ZONE , INT) RETURNS TIMESTAMP WITHOUT TIME ZONE AS ' DECLARE time ALIAS FOR $1; days ALIAS FOR $2; BEGIN RETURN time+days*24*3600*''1 second''::INTERVAL; END; ' LANGUAGE 'plpgsql'; Basically the function takes two parameters, and add the second one (as days) onto the first one (as timestamp without timezone) I don't really like this implementation. Is there a more concise way to do this? Thanks Wei
On Tue, Aug 28, 2007 at 04:59:46PM -0400, Wei Weng wrote: > Hi all > > I want to implement something like the following: Well, you could always implement it as SQL instead (untested): CREATE OR REPLACE FUNCTION AddDays (TIMESTAMP WITHOUT TIME ZONE , INT) RETURNS TIMESTAMP WITHOUT TIME ZONE AS ' SELECT $1+$2*24*3600*''1 second''::INTERVAL; ' LANGUAGE 'sql'; Not sure if it qualifies as 'more concise' though. Have a nice day, -- Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/ > From each according to his ability. To each according to his ability to litigate.
Вложения
On Aug 28, 2007, at 15:59 , Wei Weng wrote: > I don't really like this implementation. Is there a more concise > way to do this? create or replace function add_days(timestamp, int) returns timestamp language sql as $body$ select $1 + $2 * interval '1 day' $body$; Note that interval '1 day' is not equal to interval '24 hours'. '1 day' can be 23 or 25 hours across daylight saving time boundaries. If you mean 24 hours (which you're getting with your 24 * 3600 * interval '2 second'), you could do create or replace function add_24_hour_intervals(timestamp, int) returns timestamp language sql as $body$ select $1 + $2 * interval '24 hours'; $body$; Do you *really* want timestamp without time zone? It's not like you're gaining any performance or decreasing storage requirements. Unless you have a reason for *not* using with time zone, I'd recommend using timestamp with time zone. Hope this helps. Michael Glaesemann grzm seespotcode net
Wei Weng escreveu: > Hi all > > I want to implement something like the following: > > CREATE OR REPLACE FUNCTION AddDays > (TIMESTAMP WITHOUT TIME ZONE > , INT) > RETURNS TIMESTAMP WITHOUT TIME ZONE AS ' > DECLARE > time ALIAS FOR $1; > days ALIAS FOR $2; > BEGIN > RETURN time+days*24*3600*''1 second''::INTERVAL; > END; > ' LANGUAGE 'plpgsql'; > > Basically the function takes two parameters, and add the second one (as > days) onto the first one (as timestamp without timezone) > I don't really like this implementation. Is there a more concise way to > do this? > CREATE OR REPLACE FUNCTION AddDays (time TIMESTAMP WITHOUT TIME ZONE , days INT) RETURNS TIMESTAMP WITHOUT TIME ZONE AS $$ BEGIN RETURN time + days*'1 day'::INTERVAL; END; $$ LANGUAGE 'plpgsql'; Osvaldo
On 8/28/07, Wei Weng <wweng@kencast.com> wrote: > Is there a more concise way to do this? CREATE OR REPLACE FUNCTION ADDDAYS (TIMESTAMP WITHOUT TIME ZONE, INT) RETURNS TIMESTAMP WITHOUT TIME ZONE AS ' SELECT $1+($2 * ''1 DAY''::INTERVAL) ' LANGUAGE SQL;
Wei Weng wrote: > I want to implement something like the following: > CREATE OR REPLACE FUNCTION AddDays You don't know how many seconds are in a day, so just add the days using SQL. RETURN time + (days || ' days')::INTERVAL; You don't even need to make that a function, just do that you your SQL directly. -- Dante
On Aug 28, 2007, at 16:51 , Michael Glaesemann wrote: > If you mean 24 hours (which you're getting with your 24 * 3600 * > interval '2 second'), you could do Or, 24 * 3600 * interval '1 second', rather Michael Glaesemann grzm seespotcode net
On Aug 28, 2007, at 16:55 , D. Dante Lorenso wrote: > RETURN time + (days || ' days')::INTERVAL; It's bad practice to concatenate like this. Use time + days * interval '1 day' and be done with it. Michael Glaesemann grzm seespotcode net
Michael Glaesemann wrote: > > On Aug 28, 2007, at 16:55 , D. Dante Lorenso wrote: >> RETURN time + (days || ' days')::INTERVAL; > It's bad practice to concatenate like this. Use time + days * interval > '1 day' and be done with it. Why? Is this functionality expected to break in the future or has unexpected side effects? Is it less clear or less efficient? Who declared it bad practice and where can I read that documentation? -- Dante
On Aug 28, 2007, at 17:22 , D. Dante Lorenso wrote: > Michael Glaesemann wrote: >> On Aug 28, 2007, at 16:55 , D. Dante Lorenso wrote: >>> RETURN time + (days || ' days')::INTERVAL; >> It's bad practice to concatenate like this. Use time + days * >> interval '1 day' and be done with it. > > Why? Is this functionality expected to break in the future or has > unexpected side effects? Is it less clear or less efficient? Who > declared it bad practice and where can I read that documentation? It's generally bad practice to interpolate unnecessarily. You're right, in this case you're probably safe from this particular case ever changing. I personally find it less clear (though clarity is often in the eye of the beholder). time + * interval '1 day' is to me a clearer expression of what you're doing: add this multiple of days to the time. (days || ' days')::interval says "Make a string using this value (oh, it's an int? we need to cast it to text) and this string: the result just happens to match the proper input format for an interval, which is fortunate because now we're casting the string to interval". Okay, there's a little editorializing thrown in, but that's what the concatenation says to me. The concatenation is probably less efficient: you're casting an int to text and then the text to interval with the concatenation you're using. I don't know how that compares in terms of cycles to the int * interval math, but efficiency isn't really the reason I would avoid it. I'm sure others could provide more cogent explanations, but those are my initial thoughts. Michael Glaesemann grzm seespotcode net
On Aug 28, 2007, at 17:46 , Michael Glaesemann wrote: > I'm sure others could provide more cogent explanations, but those > are my initial thoughts. Thinking about this a little bit more: pushing interpolation/ concatenation to the furthest extreme you get to using eval-like construct, which is generally not considered something you want to do. Not quite that far you get SQL-injection (to bring the point a little closer to home). Now, granted you don't need to worry about these types of things in the specific example you provided, but these are related to the more general "it's bad practice" statement. Michael Glaesemann grzm seespotcode net
Michael Glaesemann wrote: > On Aug 28, 2007, at 17:22 , D. Dante Lorenso wrote: >> Michael Glaesemann wrote: >>> On Aug 28, 2007, at 16:55 , D. Dante Lorenso wrote: >>>> RETURN time + (days || ' days')::INTERVAL; >>> It's bad practice to concatenate like this. Use time + days * >>> interval '1 day' and be done with it. >> >> Why? Is this functionality expected to break in the future or has >> unexpected side effects? Is it less clear or less efficient? Who >> declared it bad practice and where can I read that documentation? > > It's generally bad practice to interpolate unnecessarily. You're right, > in this case you're probably safe from this particular case ever > changing. I personally find it less clear (though clarity is often in > the eye of the beholder). time + * interval '1 day' is to me a clearer > expression of what you're doing: add this multiple of days to the time. Something in my just doesn't want to trust that: 30 * interval '1 day' == interval '30 days' Intervals are magical things unlike int and text. Doing multiplication on a magical thing is scary, but waiting until the end before applying the magic just *feels* safer. I do like your syntax, though. There are less parentheses. Maybe I can warm up to it ;-) -- Dante
Michael Glaesemann <grzm@seespotcode.net> writes: > Note that interval '1 day' is not equal to interval '24 hours'. '1 > day' can be 23 or 25 hours across daylight saving time boundaries. When you are adding to timestamp without time zone, they *are* interchangeable, since no daylight-savings arithmetic is involved. Michael's right for the case of adding to timestamp with time zone, though. > Do you *really* want timestamp without time zone? I agree with that comment --- you should at least think hard about what you are trying to represent. The "date" type is also worth considering if time-of-day isn't so important to you. regards, tom lane
On Aug 28, 2007, at 19:30 , Tom Lane wrote: > Michael Glaesemann <grzm@seespotcode.net> writes: >> Note that interval '1 day' is not equal to interval '24 hours'. '1 >> day' can be 23 or 25 hours across daylight saving time boundaries. > > When you are adding to timestamp without time zone, they *are* > interchangeable, since no daylight-savings arithmetic is involved. > Michael's right for the case of adding to timestamp with time zone, > though. Ah, right. Thanks for pointing that out. Michael Glaesemann grzm seespotcode net
Michael Glaesemann <grzm@seespotcode.net> writes: > The concatenation is probably less efficient: you're casting an int > to text and then the text to interval with the concatenation you're > using. I don't know how that compares in terms of cycles to the int * > interval math, but efficiency isn't really the reason I would avoid it. It's a *lot* less efficient: number times interval-constant is basically three floating-point multiplies, whereas as you said, the other way involves formatting the number as text, performing a string concatenation, and then parsing the result to see whether it's legal input for an interval and if so what its value is. Here's a simple experiment to compare the costs: regression=# explain analyze select x from generate_series(1,100000) x; QUERY PLAN ------------------------------------------------------------------------------------------------------------------------------ Function Scan on generate_series x (cost=0.00..12.50 rows=1000 width=4) (actual time=670.205..1337.351 rows=100000 loops=1) Total runtime: 1713.093 ms (2 rows) regression=# explain analyze select x * '1 day'::interval from generate_series(1,100000) x; QUERY PLAN ------------------------------------------------------------------------------------------------------------------------------ Function Scan on generate_series x (cost=0.00..17.50 rows=1000 width=4) (actual time=664.579..1841.494 rows=100000 loops=1) Total runtime: 2222.444 ms (2 rows) regression=# explain analyze select (x || ' days')::interval from generate_series(1,100000) x; QUERY PLAN ------------------------------------------------------------------------------------------------------------------------------ Function Scan on generate_series x (cost=0.00..25.00 rows=1000 width=4) (actual time=741.236..3015.771 rows=100000 loops=1) Total runtime: 3402.385 ms (2 rows) Subtracting off the overhead estimated by the first query, we arrive at 196k conversions/second using multiply vs 60k conversions/sec using concatenation. This is on a rather old and slow server, but the ratio probably holds up on other hardware. Allowing for a lot of noise in the measurements (I didn't bother trying to average several measurements), I'd say 2X to 4X slower is a good estimate. As Michael says, the speed argument is really kinda minor compared to the other ones, but it's real enough. regards, tom lane
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 08/28/07 20:06, Tom Lane wrote: [snip] > > As Michael says, the speed argument is really kinda minor compared > to the other ones, but it's real enough. Every little bit counts, though. For example, if it's part of an otherwise computationally-intensive operation that you're performing on 20 million rows, you'll definitely see the difference. - -- Ron Johnson, Jr. Jefferson LA USA Give a man a fish, and he eats for a day. Hit him with a fish, and he goes away for good! -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) iD8DBQFG1OP4S9HxQb37XmcRAvxUAJ0TFcC06TgAsOv8aou3BC1pJpbADwCgkQyy Ib8H44C7d8mvvWfsHbWjFVE= =3/7d -----END PGP SIGNATURE-----
On Tue, Aug 28, 2007 at 04:59:46PM -0400, Wei Weng wrote: > Hi all > > I want to implement something like the following: > > CREATE OR REPLACE FUNCTION AddDays > (TIMESTAMP WITHOUT TIME ZONE > , INT) > RETURNS TIMESTAMP WITHOUT TIME ZONE AS ' > DECLARE > time ALIAS FOR $1; > days ALIAS FOR $2; > BEGIN > RETURN time+days*24*3600*''1 second''::INTERVAL; > END; > ' LANGUAGE 'plpgsql'; This seems like a lot of extra work. SELECT now() + 5 * INTERVAL '1 day'; does a similar trick, and is quite clear as to what it does :) Cheers, David. -- David Fetter <david@fetter.org> http://fetter.org/ phone: +1 415 235 3778 AIM: dfetter666 Skype: davidfetter Remember to vote! Consider donating to PostgreSQL: http://www.postgresql.org/about/donate