[Home]

Summary:ASTERISK-17050: [patch] 'usegmtime=yes' is ignored and CDR(end) is not inserted
Reporter:Josep Casals C (joscas)Labels:
Date Opened:2010-12-01 06:44:56.000-0600Date Closed:2011-06-07 14:10:20
Priority:TrivialRegression?No
Status:Closed/CompleteComponents:CDR/cdr_pgsql
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) bug_18407.patch
( 1) cdr_pgsql.conf.sample.issue18407.patch
( 2) cdr.conf.sample.issue18407.patch
Description:CDRs get inserted on the server local time and it doesn't insert the CDR(end).

****** ADDITIONAL INFORMATION ******

Using:
pgsql 8.3
_______
cdr.conf
[general]
....
[pgsql]
usegmtime=yes
_______
cdr_pgsql.conf
[general]
....
[columns]
alias start => calldate  ;'start' is a SQL reserved word in PostgreSQL
alias end => callend     ;'end' is a SQL reserved word in PostgreSQL
Comments:By: Tilghman Lesher (tilghman) 2010-12-07 14:48:13.000-0600

Uh, who told you that the postgres CDR driver pays any attention whatsoever to cdr.conf?
By: Tilghman Lesher (tilghman) 2010-12-07 14:56:39.000-0600

Additionally, please dump the SQL structure of your CDR table and upload the file here.
By: Josep Casals C (joscas) 2010-12-07 16:36:38.000-0600

The sample for cdr.conf mentions a [pgsql] backend so I assumed that the options included there would apply to all backends. I see this was a wrong assumption.
By: Josep Casals C (joscas) 2010-12-07 16:39:49.000-0600

--
-- PostgreSQL database dump
--

-- Dumped from database version 8.3.12
-- Dumped by pg_dump version 9.0.1
-- Started on 2010-11-26 14:21:47 CET

SET statement_timeout = 0;
SET client_encoding = 'UTF8';
SET standard_conforming_strings = off;
SET check_function_bodies = false;
SET client_min_messages = warning;
SET escape_string_warning = off;

SET search_path = public, pg_catalog;

SET default_tablespace = '';

SET default_with_oids = false;

--
-- TOC entry 1462 (class 1259 OID 16387)
-- Dependencies: 3
-- Name: cdr; Type: TABLE; Schema: public; Owner: asterisk; Tablespace:
--

CREATE TABLE cdr (
   id bigserial PRIMARY KEY,
   clidnum character varying(80),
   clidname character varying(80),
   dialednum character varying(80),
   calldate timestamp with time zone,
   answer timestamp with time zone,
   callend timestamp with time zone,
   duration numeric(14,6),
   billsec numeric(14,6),
   disposition character varying(45),
   uniqueid character varying(32),
   sequence integer
);


ALTER TABLE public.cdr OWNER TO asterisk;

--
-- TOC entry 1733 (class 0 OID 0)
-- Dependencies: 3
-- Name: public; Type: ACL; Schema: -; Owner: postgres
--

REVOKE ALL ON SCHEMA public FROM PUBLIC;
REVOKE ALL ON SCHEMA public FROM postgres;
GRANT ALL ON SCHEMA public TO postgres;
GRANT ALL ON SCHEMA public TO PUBLIC;


-- Completed on 2010-11-26 14:21:49 CET

--
-- PostgreSQL database dump complete
--
By: Josep Casals C (joscas) 2010-12-20 05:40:12.000-0600

Some additional information:
cdr_pgsql.c outputs the SQL statement being inserted as CDR when verbosity is set to 11 or above. That shows how CDR(end) is excluded from the SQL statement:

From the CLI:
> [INSERT INTO cdr ("clidnum","clidname","dialednum","calldate","answer","duration","billsec","disposition","uniqueid","sequence") VALUES (....)] ->Values being removed not to show personal info.

Perhaps, should I open a specific issue related to the non insertion of CDR(end)?
By: snuffy (snuffy) 2010-12-20 21:19:31.000-0600

Try the uploaded patch, as 'end' i think is a reserved word in psql?
There was a check for 'calldate' vs 'start', but none for 'callend' vs 'end'
By: Tilghman Lesher (tilghman) 2010-12-21 03:29:05.000-0600

snuffy:  callend is not a special word to us; it's merely what the reporter has chosen to name his column.  Hardcoding names is precisely what the design of this driver was intended to get away from.
By: Tilghman Lesher (tilghman) 2010-12-21 03:35:14.000-0600

Now I see what the problem is.  This section that you named [columns] with alias definitions?  It's not supported in the Postgres driver, only in the ODBC and MySQL drivers.  You HAVE to look at the sample configurations; only those shown in the samples are supported.
By: Josep Casals C (joscas) 2010-12-21 04:21:56.000-0600

As I see it, the situation is as follows:

- The pgsql driver doesn't pay attention neither to a [pgsql] section of cdr.conf nor to a [columns] section of cdr_pgsql.conf

- 'start' is a reserved sql word and the pgsql driver is inserting the CDR(start) field as "calldate" and this is hardcoded in the driver since no alias for this word can be indicated.

- 'end' is a reserved sql word and the pgsql driver is not inserting the CDR(end) field since no hardcoding for an alternative word is available and no attention is paid to any alias indicated by the user.

In my opinion, the patch provided by snuffy is good because it at least allows for the insertion of an end of call to the CDR in the same way it is done currently with the start of the call. Otherwise, no end of call indication can be inserted.

A better alternative would be to raise the overall quality of this driver so that it matches the set of features provided by alternative drivers allowing a [columns] section and some control from cdr.conf but perhaps this should be a feature request.
By: Josep Casals C (joscas) 2010-12-21 05:07:57.000-0600

Also, I've tested the patch and it is not working. I think this is because also the following section from cdr_pgsql.c should be replicated for 'end' and 'callend':


               AST_RWLIST_TRAVERSE(&psql_columns, cur, list) {
                       /* For fields not set, simply skip them */
                       ast_cdr_getvar(cdr, cur->name, &value, buf, sizeof(buf), 0, 0);
                       if (strcmp(cur->name, "calldate") == 0 && !value) {
                               ast_cdr_getvar(cdr, "start", &value, buf, sizeof(buf), 0, 0);
                       }
By: Tilghman Lesher (tilghman) 2010-12-21 14:44:07.000-0600

Yes, but... both "start" and "end" may be used as column names, simply by quoting them, something that this CDR driver already does (see http://www.postgresql.org/docs/8.1/interactive/sql-keywords-appendix.html ).  The only reason calldate is set as a hardcoded alias for "start" is due to legacy issues.  I agree that adding alias support is a feature request.


By: Josep Casals C (joscas) 2010-12-22 07:46:40.000-0600

I hadn't thought of using "end" with quotes as a column name. It does work, thanks. Then I agree that no patch is needed for cdr_pgsql.c but I'm afraid that other people might do the same wrong assumptions I did. I think there's a need for improving documentation rather than adding new features.

I upload a couple of patches to cdr.conf.sample and cdr_pgsql.conf.sample with my proposal for improving the information available to users.