Re: [HACKERS] REFRESH MATERIALIZED VIEW command in PL block hitting Assert

2013-04-24 Thread Michael Paquier 
On Tue, Apr 23, 2013 at 11:05 PM, Andres Freund and...@2ndquadrant.comwrote:

 On 2013-04-23 19:33:24 +0530, Jeevan Chalke wrote:
  On Tue, Apr 23, 2013 at 7:01 PM, Merlin Moncure mmonc...@gmail.com
 wrote:
 
   On Tue, Apr 23, 2013 at 7:18 AM, Jeevan Chalke
   jeevan.cha...@enterprisedb.com wrote:
Hi Tom,
   
Since we are close to release, we should not have crashes like this.
  
   huh? we are not even in beta yet
  
 
  I mean, beta release.
 
  BTW, as per Bruce's mail we are planning to package 9.3
  beta1 on *April 29*, with a release on May 2, we are close enough.

 The 2nd May date is for the release of packaged beta, not overall
 release unless I missed something major like months of testing ;)

Last year 9.2.0 was released at the beginning of September, so there would
be at least 4 months to test and improve 9.3 during beta assuming next
release is made on the same timeline.
-- 
Michael

Re: [HACKERS] REFRESH MATERIALIZED VIEW command in PL block hitting Assert

2013-04-24 Thread Jeevan Chalke 
On Wed, Apr 24, 2013 at 3:04 AM, Kevin Grittner kgri...@ymail.com wrote:

 Jeevan Chalke jeevan.cha...@enterprisedb.com wrote:
  On Mon, Apr 22, 2013 at 6:41 PM, Andres Freund and...@2ndquadrant.com
 wrote:
  On 2013-04-22 18:35:04 +0530, Jeevan Chalke wrote:

  I have observed that following sequence is causing server crash.
 
  CREATE MATERIALIZED VIEW temp_class_mv AS
SELECT * FROM pg_class
WITH NO DATA;
 
  CREATE OR REPLACE FUNCTION test_refresh_mv()
  RETURNS int
  AS $$
  BEGIN
   REFRESH MATERIALIZED VIEW temp_class_mv;
   return 1;
  END; $$ LANGUAGE plpgsql;
 
  SELECT test_refresh_mv();
 
 
  I had a quick look over the crash and it is hitting following Assert in
  spi.c:
 
  else if (IsA(stmt, RefreshMatViewStmt))
  {
  Assert(strncmp(completionTag,
 REFRESH MATERIALIZED VIEW , 23)
 == 0);
  _SPI_current-processed = strtoul(completionTag +
 23,
NULL, 10);
  }
 
  It seems like we are missing expected value for completionTag in
  ExecRefreshMatView()

  Possibly independent from this issue, but where did that 23 come from?

 When the consensus developed to change the syntax from LOAD
 MATERIALIZED VIEW I failed to noticed the length here when making
 the changes for that.

  BTW, attached is the patch which works well for me, but need details
 review.

 I suggest that we just rip out this section of code.  Trying to
 provide a number here is probably all wrong, anyway.  As the
 features evolve, there may not be a readily accessible rowcount for
 this command in all cases.


OK.



 Any objections to the attached to fix this issue?


Nope. Fine with me.

Thanks


 --
 Kevin Grittner
 EnterpriseDB: http://www.enterprisedb.com
 The Enterprise PostgreSQL Company




-- 
Jeevan B Chalke
Senior Software Engineer, RD
EnterpriseDB Corporation
The Enterprise PostgreSQL Company

Phone: +91 20 30589500

Website: www.enterprisedb.com
EnterpriseDB Blog: http://blogs.enterprisedb.com/
Follow us on Twitter: http://www.twitter.com/enterprisedb

This e-mail message (and any attachment) is intended for the use of the
individual or entity to whom it is addressed. This message contains
information from EnterpriseDB Corporation that may be privileged,
confidential, or exempt from disclosure under applicable law. If you are
not the intended recipient or authorized to receive this for the intended
recipient, any use, dissemination, distribution, retention, archiving, or
copying of this communication is strictly prohibited. If you have received
this e-mail in error, please notify the sender immediately by reply e-mail
and delete this message.

Re: [HACKERS] REFRESH MATERIALIZED VIEW command in PL block hitting Assert

2013-04-24 Thread Kevin Grittner 
Jeevan Chalke jeevan.cha...@enterprisedb.com wrote:
 On Wed, Apr 24, 2013 at 3:04 AM, Kevin Grittner kgri...@ymail.com wrote:

 Any objections to the attached to fix this issue?

 Nope. Fine with me.

Pushed.  Thanks for the report!

--
Kevin Grittner
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] REFRESH MATERIALIZED VIEW command in PL block hitting Assert

2013-04-23 Thread Jeevan Chalke 
Hi Tom,

Since we are close to release, we should not have crashes like this.

Please have a look. My patch may not be correct as I haven't looked closely.

Thanks


On Mon, Apr 22, 2013 at 7:28 PM, Jeevan Chalke 
jeevan.cha...@enterprisedb.com wrote:




 On Mon, Apr 22, 2013 at 6:41 PM, Andres Freund and...@2ndquadrant.comwrote:

 On 2013-04-22 18:35:04 +0530, Jeevan Chalke wrote:
  Hi,
 
  I have observed that following sequence is causing server crash.
 
  CREATE MATERIALIZED VIEW temp_class_mv AS
SELECT * FROM pg_class
WITH NO DATA;
 
  CREATE OR REPLACE FUNCTION test_refresh_mv()
  RETURNS int
  AS $$
  BEGIN
   REFRESH MATERIALIZED VIEW temp_class_mv;
   return 1;
  END; $$ LANGUAGE plpgsql;
 
  SELECT test_refresh_mv();
 
 
  I had a quick look over the crash and it is hitting following Assert in
  spi.c:
 
  else if (IsA(stmt, RefreshMatViewStmt))
  {
  Assert(strncmp(completionTag,
 REFRESH MATERIALIZED VIEW , 23) ==
 0);
  _SPI_current-processed = strtoul(completionTag +
 23,
NULL, 10);
  }
 
  It seems like we are missing expected value for completionTag in
  ExecRefreshMatView()

 Possibly independent from this issue, but where did that 23 come from?


 23 is also bogus here.
 It should be 26 i.e. length of REFRESH MATERIALIZED VIEW 

 BTW, attached is the patch which works well for me, but need details
 review.

 Thanks


 ISTM we're strtoul()ing EW somenumber here.

 Greetings,

 Andres Freund

 --
  Andres Freund http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training  Services




 --
 Jeevan B Chalke
 Senior Software Engineer, RD
 EnterpriseDB Corporation
 The Enterprise PostgreSQL Company

 Phone: +91 20 30589500

 Website: www.enterprisedb.com
 EnterpriseDB Blog: http://blogs.enterprisedb.com/
 Follow us on Twitter: http://www.twitter.com/enterprisedb

 This e-mail message (and any attachment) is intended for the use of the
 individual or entity to whom it is addressed. This message contains
 information from EnterpriseDB Corporation that may be privileged,
 confidential, or exempt from disclosure under applicable law. If you are
 not the intended recipient or authorized to receive this for the intended
 recipient, any use, dissemination, distribution, retention, archiving, or
 copying of this communication is strictly prohibited. If you have received
 this e-mail in error, please notify the sender immediately by reply e-mail
 and delete this message.




-- 
Jeevan B Chalke
Senior Software Engineer, RD
EnterpriseDB Corporation
The Enterprise PostgreSQL Company

Phone: +91 20 30589500

Website: www.enterprisedb.com
EnterpriseDB Blog: http://blogs.enterprisedb.com/
Follow us on Twitter: http://www.twitter.com/enterprisedb

This e-mail message (and any attachment) is intended for the use of the
individual or entity to whom it is addressed. This message contains
information from EnterpriseDB Corporation that may be privileged,
confidential, or exempt from disclosure under applicable law. If you are
not the intended recipient or authorized to receive this for the intended
recipient, any use, dissemination, distribution, retention, archiving, or
copying of this communication is strictly prohibited. If you have received
this e-mail in error, please notify the sender immediately by reply e-mail
and delete this message.

Re: [HACKERS] REFRESH MATERIALIZED VIEW command in PL block hitting Assert

2013-04-23 Thread Andres Freund 
Hi,

On 2013-04-23 17:48:49 +0530, Jeevan Chalke wrote:
 Hi Tom,
 
 Since we are close to release, we should not have crashes like this.
 
 Please have a look. My patch may not be correct as I haven't looked closely.

Isn't that Kevin's departement?

Andres

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] REFRESH MATERIALIZED VIEW command in PL block hitting Assert

2013-04-23 Thread Kevin Grittner 
Andres Freund and...@2ndquadrant.com wrote:
 On 2013-04-23 17:48:49 +0530, Jeevan Chalke wrote:
 Hi Tom,

 Since we are close to release, we should not have crashes like
 this.

 Please have a look. My patch may not be correct as I haven't
 looked closely.
 
 Isn't that Kevin's departement?

I'll gladly pick this up.  Tom had moved some code around and
dropped a couple comments that made it sound like he was still
working on it, so I was trying to stay out of his way; but if I've
misinterpreted that, I can jump in here.

-- 
Kevin Grittner
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] REFRESH MATERIALIZED VIEW command in PL block hitting Assert

2013-04-23 Thread Merlin Moncure 
On Tue, Apr 23, 2013 at 7:18 AM, Jeevan Chalke
jeevan.cha...@enterprisedb.com wrote:
 Hi Tom,

 Since we are close to release, we should not have crashes like this.

huh? we are not even in beta yet

merlin


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] REFRESH MATERIALIZED VIEW command in PL block hitting Assert

2013-04-23 Thread Jeevan Chalke 
On Tue, Apr 23, 2013 at 7:01 PM, Merlin Moncure mmonc...@gmail.com wrote:

 On Tue, Apr 23, 2013 at 7:18 AM, Jeevan Chalke
 jeevan.cha...@enterprisedb.com wrote:
  Hi Tom,
 
  Since we are close to release, we should not have crashes like this.

 huh? we are not even in beta yet


I mean, beta release.

BTW, as per Bruce's mail we are planning to package 9.3
beta1 on *April 29*, with a release on May 2, we are close enough.

Thanks


 merlin




-- 
Jeevan B Chalke
Senior Software Engineer, RD
EnterpriseDB Corporation
The Enterprise PostgreSQL Company

Phone: +91 20 30589500

Website: www.enterprisedb.com
EnterpriseDB Blog: http://blogs.enterprisedb.com/
Follow us on Twitter: http://www.twitter.com/enterprisedb

This e-mail message (and any attachment) is intended for the use of the
individual or entity to whom it is addressed. This message contains
information from EnterpriseDB Corporation that may be privileged,
confidential, or exempt from disclosure under applicable law. If you are
not the intended recipient or authorized to receive this for the intended
recipient, any use, dissemination, distribution, retention, archiving, or
copying of this communication is strictly prohibited. If you have received
this e-mail in error, please notify the sender immediately by reply e-mail
and delete this message.

Re: [HACKERS] REFRESH MATERIALIZED VIEW command in PL block hitting Assert

2013-04-23 Thread Andres Freund 
On 2013-04-23 19:33:24 +0530, Jeevan Chalke wrote:
 On Tue, Apr 23, 2013 at 7:01 PM, Merlin Moncure mmonc...@gmail.com wrote:
 
  On Tue, Apr 23, 2013 at 7:18 AM, Jeevan Chalke
  jeevan.cha...@enterprisedb.com wrote:
   Hi Tom,
  
   Since we are close to release, we should not have crashes like this.
 
  huh? we are not even in beta yet
 
 
 I mean, beta release.
 
 BTW, as per Bruce's mail we are planning to package 9.3
 beta1 on *April 29*, with a release on May 2, we are close enough.

The 2nd May date is for the release of packaged beta, not overall
release unless I missed something major like months of testing ;)

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] REFRESH MATERIALIZED VIEW command in PL block hitting Assert

2013-04-23 Thread Kevin Grittner 
Jeevan Chalke jeevan.cha...@enterprisedb.com wrote:
 On Mon, Apr 22, 2013 at 6:41 PM, Andres Freund and...@2ndquadrant.com wrote:
 On 2013-04-22 18:35:04 +0530, Jeevan Chalke wrote:

 I have observed that following sequence is causing server crash.
 
 CREATE MATERIALIZED VIEW temp_class_mv AS
   SELECT * FROM pg_class
   WITH NO DATA;
 
 CREATE OR REPLACE FUNCTION test_refresh_mv()
 RETURNS int
 AS $$
 BEGIN
  REFRESH MATERIALIZED VIEW temp_class_mv;
  return 1;
 END; $$ LANGUAGE plpgsql;
 
 SELECT test_refresh_mv();
 
 
 I had a quick look over the crash and it is hitting following Assert in
 spi.c:
 
 else if (IsA(stmt, RefreshMatViewStmt))
 {
 Assert(strncmp(completionTag,
    REFRESH MATERIALIZED VIEW , 23) == 0);
 _SPI_current-processed = strtoul(completionTag + 23,
   NULL, 10);
 }
 
 It seems like we are missing expected value for completionTag in
 ExecRefreshMatView()

 Possibly independent from this issue, but where did that 23 come from?

When the consensus developed to change the syntax from LOAD
MATERIALIZED VIEW I failed to noticed the length here when making
the changes for that.

 BTW, attached is the patch which works well for me, but need details review.

I suggest that we just rip out this section of code.  Trying to
provide a number here is probably all wrong, anyway.  As the
features evolve, there may not be a readily accessible rowcount for
this command in all cases.

Any objections to the attached to fix this issue?

--
Kevin Grittner
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Companydiff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index cc7764d..de8d59a 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -2122,13 +2122,6 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI,
 	if (((CreateTableAsStmt *) stmt)-is_select_into)
 		res = SPI_OK_SELINTO;
 }
-else if (IsA(stmt, RefreshMatViewStmt))
-{
-	Assert(strncmp(completionTag,
-   REFRESH MATERIALIZED VIEW , 23) == 0);
-	_SPI_current-processed = strtoul(completionTag + 23,
-	  NULL, 10);
-}
 else if (IsA(stmt, CopyStmt))
 {
 	Assert(strncmp(completionTag, COPY , 5) == 0);

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] REFRESH MATERIALIZED VIEW command in PL block hitting Assert

2013-04-22 Thread Jeevan Chalke 
Hi,

I have observed that following sequence is causing server crash.

CREATE MATERIALIZED VIEW temp_class_mv AS
  SELECT * FROM pg_class
  WITH NO DATA;

CREATE OR REPLACE FUNCTION test_refresh_mv()
RETURNS int
AS $$
BEGIN
 REFRESH MATERIALIZED VIEW temp_class_mv;
 return 1;
END; $$ LANGUAGE plpgsql;

SELECT test_refresh_mv();


I had a quick look over the crash and it is hitting following Assert in
spi.c:

else if (IsA(stmt, RefreshMatViewStmt))
{
Assert(strncmp(completionTag,
   REFRESH MATERIALIZED VIEW , 23) == 0);
_SPI_current-processed = strtoul(completionTag + 23,
  NULL, 10);
}

It seems like we are missing expected value for completionTag in
ExecRefreshMatView()

Thanks

-- 
Jeevan B Chalke
Senior Software Engineer, RD
EnterpriseDB Corporation
The Enterprise PostgreSQL Company

Phone: +91 20 30589500

Website: www.enterprisedb.com
EnterpriseDB Blog: http://blogs.enterprisedb.com/
Follow us on Twitter: http://www.twitter.com/enterprisedb

This e-mail message (and any attachment) is intended for the use of the
individual or entity to whom it is addressed. This message contains
information from EnterpriseDB Corporation that may be privileged,
confidential, or exempt from disclosure under applicable law. If you are
not the intended recipient or authorized to receive this for the intended
recipient, any use, dissemination, distribution, retention, archiving, or
copying of this communication is strictly prohibited. If you have received
this e-mail in error, please notify the sender immediately by reply e-mail
and delete this message.

Re: [HACKERS] REFRESH MATERIALIZED VIEW command in PL block hitting Assert

2013-04-22 Thread Andres Freund 
On 2013-04-22 18:35:04 +0530, Jeevan Chalke wrote:
 Hi,
 
 I have observed that following sequence is causing server crash.
 
 CREATE MATERIALIZED VIEW temp_class_mv AS
   SELECT * FROM pg_class
   WITH NO DATA;
 
 CREATE OR REPLACE FUNCTION test_refresh_mv()
 RETURNS int
 AS $$
 BEGIN
  REFRESH MATERIALIZED VIEW temp_class_mv;
  return 1;
 END; $$ LANGUAGE plpgsql;
 
 SELECT test_refresh_mv();
 
 
 I had a quick look over the crash and it is hitting following Assert in
 spi.c:
 
 else if (IsA(stmt, RefreshMatViewStmt))
 {
 Assert(strncmp(completionTag,
REFRESH MATERIALIZED VIEW , 23) == 0);
 _SPI_current-processed = strtoul(completionTag + 23,
   NULL, 10);
 }
 
 It seems like we are missing expected value for completionTag in
 ExecRefreshMatView()

Possibly independent from this issue, but where did that 23 come from?
ISTM we're strtoul()ing EW somenumber here.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] REFRESH MATERIALIZED VIEW command in PL block hitting Assert

2013-04-22 Thread Jeevan Chalke 
On Mon, Apr 22, 2013 at 6:41 PM, Andres Freund and...@2ndquadrant.comwrote:

 On 2013-04-22 18:35:04 +0530, Jeevan Chalke wrote:
  Hi,
 
  I have observed that following sequence is causing server crash.
 
  CREATE MATERIALIZED VIEW temp_class_mv AS
SELECT * FROM pg_class
WITH NO DATA;
 
  CREATE OR REPLACE FUNCTION test_refresh_mv()
  RETURNS int
  AS $$
  BEGIN
   REFRESH MATERIALIZED VIEW temp_class_mv;
   return 1;
  END; $$ LANGUAGE plpgsql;
 
  SELECT test_refresh_mv();
 
 
  I had a quick look over the crash and it is hitting following Assert in
  spi.c:
 
  else if (IsA(stmt, RefreshMatViewStmt))
  {
  Assert(strncmp(completionTag,
 REFRESH MATERIALIZED VIEW , 23) ==
 0);
  _SPI_current-processed = strtoul(completionTag + 23,
NULL, 10);
  }
 
  It seems like we are missing expected value for completionTag in
  ExecRefreshMatView()

 Possibly independent from this issue, but where did that 23 come from?


23 is also bogus here.
It should be 26 i.e. length of REFRESH MATERIALIZED VIEW 

BTW, attached is the patch which works well for me, but need details review.

Thanks


 ISTM we're strtoul()ing EW somenumber here.

 Greetings,

 Andres Freund

 --
  Andres Freund http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training  Services




-- 
Jeevan B Chalke
Senior Software Engineer, RD
EnterpriseDB Corporation
The Enterprise PostgreSQL Company

Phone: +91 20 30589500

Website: www.enterprisedb.com
EnterpriseDB Blog: http://blogs.enterprisedb.com/
Follow us on Twitter: http://www.twitter.com/enterprisedb

This e-mail message (and any attachment) is intended for the use of the
individual or entity to whom it is addressed. This message contains
information from EnterpriseDB Corporation that may be privileged,
confidential, or exempt from disclosure under applicable law. If you are
not the intended recipient or authorized to receive this for the intended
recipient, any use, dissemination, distribution, retention, archiving, or
copying of this communication is strictly prohibited. If you have received
this e-mail in error, please notify the sender immediately by reply e-mail
and delete this message.


mv_crash.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers