REFRESH MATERIALIZED VIEW command in PL block hitting Assert

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

On Mon, Apr 22, 2013 at 6:41 PM, Andres Freund <andres@2ndquadrant.com>wrote:

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, R&D
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.

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.chalke@enterprisedb.com> wrote:

On Mon, Apr 22, 2013 at 6:41 PM, Andres Freund <andres@2ndquadrant.com>wrote:

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, R&D
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, R&D
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.

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

Andres Freund <andres@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

On Tue, Apr 23, 2013 at 7:18 AM, Jeevan Chalke
<jeevan.chalke@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

On Tue, Apr 23, 2013 at 7:01 PM, Merlin Moncure <mmoncure@gmail.com> wrote:

On Tue, Apr 23, 2013 at 7:18 AM, Jeevan Chalke
<jeevan.chalke@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, R&D
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.

On 2013-04-23 19:33:24 +0530, Jeevan Chalke wrote:

On Tue, Apr 23, 2013 at 7:01 PM, Merlin Moncure <mmoncure@gmail.com> wrote:

On Tue, Apr 23, 2013 at 7:18 AM, Jeevan Chalke
<jeevan.chalke@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

Jeevan Chalke <jeevan.chalke@enterprisedb.com> wrote:

On Mon, Apr 22, 2013 at 6:41 PM, Andres Freund <andres@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 Company

On Tue, Apr 23, 2013 at 11:05 PM, Andres Freund <andres@2ndquadrant.com>wrote:

On 2013-04-23 19:33:24 +0530, Jeevan Chalke wrote:

On Tue, Apr 23, 2013 at 7:01 PM, Merlin Moncure <mmoncure@gmail.com>

wrote:

On Tue, Apr 23, 2013 at 7:18 AM, Jeevan Chalke
<jeevan.chalke@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

On Wed, Apr 24, 2013 at 3:04 AM, Kevin Grittner <kgrittn@ymail.com> wrote:

Jeevan Chalke <jeevan.chalke@enterprisedb.com> wrote:

On Mon, Apr 22, 2013 at 6:41 PM, Andres Freund <andres@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, R&D
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 Chalke <jeevan.chalke@enterprisedb.com> wrote:

On Wed, Apr 24, 2013 at 3:04 AM, Kevin Grittner <kgrittn@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