2

I have a table address_all and it is inherited by several address tables. address_history inherits from parent table history_all and keeps current address information. I am creating new table which inherits address_all table and copies information from address_history to new table.

My stored procedure is like this below. I am having some error when I call it. To better explain error I am using line number.

1  CREATE OR REPLACE FUNCTION somefunc()
2  RETURNS void AS
3  $BODY$
4  DECLARE
5   year_id INTEGER;
6   month_id INTEGER;
7   week_id INTEGER;
8   addresstablename text; 
9   backupdays text;
10 BEGIN
11  week_id := EXTRACT(DAY FROM TIMESTAMP 'now()');
12  month_id := EXTRACT(MONTH FROM TIMESTAMP 'now()');
13  year_id := EXTRACT(YEAR FROM TIMESTAMP 'now()');
14  addresstablename := 'address_history_' || week_id || '_' || month_id || '_' || year_id;
15  backupdays:= date_trunc('hour',CURRENT_TIMESTAMP - interval '7 days');
16  EXECUTE 'create table ' || addresstablename || '() INHERITS (address_all)';
17  EXECUTE 'insert into ' || addresstablename || ' select * from address_history where address_timestamp >= ' || backupdays || ''; --AS timestamp without time zone);  
18 END;
19 $BODY$
20 LANGUAGE 'plpgsql' VOLATILE;

When I run:

select somefunc()

I get this error:

ERROR:  syntax error at or near "12"
LINE 1: ...story where address_timestamp >= 2012-07-31 12:00:00-0...
                                                         ^
QUERY:  insert into address_history_7_8_2012 select * from address_history where address_timestamp >= 2012-07-31 12:00:00-04
CONTEXT:  PL/pgSQL function "somefunc" line 14 at EXECUTE statement

 ********** Error **********

ERROR: syntax error at or near "12"
SQL state: 42601
Context: PL/pgSQL function "somefunc" line 14 at EXECUTE statement
Erwin Brandstetter
  • 605,456
  • 145
  • 1,078
  • 1,228
prakashpoudel
  • 565
  • 2
  • 9
  • 25

3 Answers3

6

Try this largely simplified form:

CREATE OR REPLACE FUNCTION somefunc()
  RETURNS void AS
$func$
DECLARE
 addresstablename text := 'address_history_' || to_char(now(), 'FMDD_MM_YYYY');

BEGIN
 EXECUTE 
 'CREATE TABLE ' || addresstablename || '() INHERITS (address_all)';

 EXECUTE
 'INSERT INTO ' || addresstablename || '
  SELECT *
  FROM   address_history
  WHERE  address_timestamp >= $1'
 USING date_trunc('hour', now() - interval '7 days');

END
$func$ LANGUAGE plpgsql;

Major points:

  • You can assign variables in plpgsql at declaration time. Simplifies code.

  • Use to_char() to format your date. Much simpler.

  • now() and CURRENT_TIMESTAMP do the same.

  • Don't quote 'now()', use now() (without quotes) if you want the current timestamp.

  • Use the USING clause with EXECUTE, so you don't have to convert the timestamp to text and back - possibly running into quoting issues like you did. Faster, simpler, safer.

  • In LANGUAGE plpgsql, plpgsql is a keyword and should not be quoted.

  • You may want to check if the table already exists with CREATE TABLE IF NOT EXISTS, available since PostgreSQL 9.1.

Erwin Brandstetter
  • 605,456
  • 145
  • 1,078
  • 1,228
2

Apparently you need to quote backupdays, or it is not seen as a string from where to parse a timestamp.

LSerni
  • 55,617
  • 10
  • 65
  • 107
1

You're building SQL using string manipulation so you have to properly quote everything just like in any other language. There are a few functions that you'll want to know about:

  • quote_ident: quote an identifier such as a table name.
  • quote_literal: quote a string to use as a string literal.
  • quote_nullable: as quote_literal but properly handles NULLs as well.

Something like this will server you better:

EXECUTE 'create table ' || quote_ident(addresstablename) || ...
EXECUTE 'insert into '  || quote_ident(addresstablename) || ... || quote_literal(backupdays) ...

The quote_ident calls aren't necessary in your case but they're a good habit.

mu is too short
  • 426,620
  • 70
  • 833
  • 800
  • `quote_literal()` and `quote_ident()` are good advice, generally. In this case you don't need either, I would claim. The tablename `addresstablename` is generated inside the function and does not take user input, so it can be used as is. The variable `backupdays` can be applied with the `USING` clause, which saves casting and quoting altogether. – Erwin Brandstetter Aug 07 '12 at 17:18
  • Thanks for sharing about quote_ident, quote_literal and quote_nullable. I will surely use these in my queries. Thanks alot. – prakashpoudel Aug 07 '12 at 19:30
  • @ErwinBrandstetter: Hence the "The `quote_ident` calls aren't necessary in your case but they're a good habit." I would claim that `quote_ident` should be the default thing that your fingers do, then remove it if you're certain you don't need it. – mu is too short Aug 07 '12 at 20:13
  • @muistooshort: I can't disagree with that. Don't get me wrong, I upvoted your answer - it's the deluxe version of the lserni's simple comment. – Erwin Brandstetter Aug 07 '12 at 20:28
  • @ErwinBrandstetter: No worries, I'm just putting on my "database guy" hat and trying to make sure everything is well defined and free of ambiguity :) – mu is too short Aug 07 '12 at 21:19