Re: Propsed pgagent patch: pgAgent reports failure upon success - For Review

Hi

Can you resend the patch as an attachment please? It's getting
corrupted inline in email.

Thanks.

On Mon, Jul 16, 2012 at 2:46 PM, Martin French
<Martin(dot)French(at)romaxtech(dot)com> wrote:
> Hi,
>
> I have attempted a patch for an issue where pgAgent incorrectly reports
> failure on a plain SQL job. Please see below.
>
> Feedback/comments welcome.
>
> Thanks
>
> Martin French.
>
>
> diff --git a/connection.cpp b/connection.cpp
> index 5e10078..6103c00 100644
> --- a/connection.cpp
> +++ b/connection.cpp
> @@ -336,6 +336,7 @@ DBresult::DBresult(DBconn *conn, const wxString &query)
> if (result)
> {
> int rc = PQresultStatus(result);
> + conn->SetLastResult(rc);
> if (rc == PGRES_TUPLES_OK)
> maxRows = PQntuples(result);
> else if (rc != PGRES_COMMAND_OK)
> diff --git a/include/connection.h b/include/connection.h
> index 04087db..3cd1c0d 100644
> --- a/include/connection.h
> +++ b/include/connection.h
> @@ -51,7 +51,33 @@ public:
> {
> return conn != 0;
> }
> -
> + bool LastCommandOk()
> + {
> + return IsCommandOk((ExecStatusType)lastResult);
> + }
> + bool IsCommandOk(ExecStatusType ret)
> + {
> + switch (ret)
> + {
> + case PGRES_COMMAND_OK:
> + case PGRES_TUPLES_OK:
> + case PGRES_COPY_OUT:
> + case PGRES_COPY_IN:
> + case PGRES_COPY_BOTH:
> + return true;
> + default:
> + return false;
> + };
> + }
> + void SetLastResult(int res)
> + {
> + lastResult = res;
> + }
> + int GetLastResult()
> + {
> + return lastResult;
> + }
> +
> DBresult *Execute(const wxString &query);
> wxString ExecuteScalar(const wxString &query);
> int ExecuteVoid(const wxString &query);
> @@ -70,7 +96,7 @@ protected:
> PGconn *conn;
> DBconn *next, *prev;
> bool inUse;
> -
> + int lastResult;
> friend class DBresult;
>
> };
> diff --git a/job.cpp b/job.cpp
> index 9a5bb08..ba270be 100644
> --- a/job.cpp
> +++ b/job.cpp
> @@ -80,6 +80,7 @@ Job::~Job()
> int Job::Execute()
> {
> int rc = 0;
> + bool succeeded = false;
> DBresult *steps = threadConn->Execute(
> wxT("SELECT * ")
> wxT(" FROM pgagent.pga_jobstep ")
> @@ -138,13 +139,14 @@ int Job::Execute()
> {
> LogMessage(wxString::Format(_("Executing SQL step %s (part of job %s)"),
> stepid.c_str(), jobid.c_str()), LOG_DEBUG);
> rc = stepConn->ExecuteVoid(steps->GetString(wxT("jstcode")));
> + succeeded = stepConn->LastCommandOk();
> output = stepConn->GetLastError();
> stepConn->Return();
> }
> else
> {
> output = _("Couldn't get a connection to the database!");
> - rc = -1;
> + succeeded = false;
> }
>
>
> @@ -292,6 +294,9 @@ int Job::Execute()
> rc = WEXITSTATUS(rc);
> else
> rc = -1;
> + // set success status for batch runs, be pessimistic bt default
> + if (rc == 0)
> + succeeded = true;
> #endif
>
> // Delete the file/directory. If we fail, don't overwrite the script output
> in the log, just throw warnings.
> @@ -319,7 +324,7 @@ int Job::Execute()
> }
>
> wxString stepstatus;
> - if (rc == 0)
> + if (succeeded)
> stepstatus = wxT("s");
> else
> stepstatus = steps->GetString(wxT("jstonerror"));
> ============================================= Romax Technology Limited
> Rutherford House Nottingham Science & Technology Park Nottingham, NG7 2PZ
> England Telephone numbers: +44 (0)115 951 88 00 (main) For other office
> locations see: http://www.romaxtech.com/Contact
> ================================= =============== E-mail: info(at)romaxtech(dot)com
> Website: www.romaxtech.com =================================
> ================ Confidentiality Statement This transmission is for the
> addressee only and contains information that is confidential and privileged.
> Unless you are the named addressee, or authorised to receive it on behalf of
> the addressee you may not copy or use it, or disclose it to anyone else. If
> you have received this transmission in error please delete from your system
> and contact the sender. Thank you for your cooperation.
> =================================================

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company