Re: CINE in CREATE TABLE AS ... and CREATE MATERIALIZED VIEW ...

Hi all,

We already have IF NOT EXISTS for CREATE TABLE. There are some reason to
don't have to CREATE TABLE AS and CREATE MATERIALIZED VIEW??

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello

On Wed, Oct 1, 2014 at 9:17 AM, Fabrízio de Royes Mello <
fabriziomello(at)gmail(dot)com> wrote:
>
> Hi all,
>
> We already have IF NOT EXISTS for CREATE TABLE. There are some reason to
don't have to CREATE TABLE AS and CREATE MATERIALIZED VIEW??
>

Patch attached to add CINE support to:

- CREATE TABLE AS
- CREATE MATERIALIZED VIEW

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello

Hi All,

- Patch got applied cleanly.
- Regression make check run fine.
- Patch covered the documentation changes

Here are few comments:

1) What the need of following change:

diff --git a/src/backend/storage/lmgr/lwlock.c
b/src/backend/storage/lmgr/lwlock.c
index bcec173..9fe6855 100644
--- a/src/backend/storage/lmgr/lwlock.c
+++ b/src/backend/storage/lmgr/lwlock.c
@@ -1005,12 +1005,6 @@ LWLockWaitForVar(LWLock *lock, uint64 *valptr,
uint64 oldval, uint64 *newval)
lock->tail = proc;
lock->head = proc;

- /*
- * Set releaseOK, to make sure we get woken up as soon as the lock
is
- * released.
- */
- lock->releaseOK = true;
-
/* Can release the mutex now */
SpinLockRelease(&lock->mutex);

It doesn't look like related to this patch.

2)

diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index ae5fe88..4d11952 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -247,7 +247,7 @@ slashUsage(unsigned short int pager)
fprintf(output, _(" \\f [STRING] show or set field
separator for unaligned query output\n"));
fprintf(output, _(" \\H toggle HTML output mode
(currently %s)\n"),
ON(pset.popt.topt.format == PRINT_HTML));
- fprintf(output, _(" \\pset [NAME [VALUE]] set table output option\n"
+ fprintf(output, _(" \\pset [NAME [VALUE]] set table output
option\n"
" (NAME :=
{format|border|expanded|fieldsep|fieldsep_zero|footer|null|\n"
"
numericlocale|recordsep|recordsep_zero|tuples_only|title|tableattr|pager|\n"
"
unicode_border_linestyle|unicode_column_linestyle|unicode_header_linestyle})\n"));

Why above changes reqired ?

3) This patch adding IF NOT EXIST_S for CREATE TABLE AS and CREATE
MATERIALIZED
TABLE, but testcase coverage for CREATE MATERIALIZED TABLE is missing.

Apart from this changes looks good to me.

On Tue, Oct 14, 2014 at 10:28 PM, Fabrízio de Royes Mello <
fabriziomello(at)gmail(dot)com> wrote:

> On Wed, Oct 1, 2014 at 9:17 AM, Fabrízio de Royes Mello <
> fabriziomello(at)gmail(dot)com> wrote:
> >
> > Hi all,
> >
> > We already have IF NOT EXISTS for CREATE TABLE. There are some reason to
> don't have to CREATE TABLE AS and CREATE MATERIALIZED VIEW??
> >
>
> Patch attached to add CINE support to:
>
> - CREATE TABLE AS
> - CREATE MATERIALIZED VIEW
>
> Regards,
>
> --
> Fabrízio de Royes Mello
> Consultoria/Coaching PostgreSQL
> >> Timbira: http://www.timbira.com.br
> >> Blog: http://fabriziomello.github.io
> >> Linkedin: http://br.linkedin.com/in/fabriziomello
> >> Twitter: http://twitter.com/fabriziomello
> >> Github: http://github.com/fabriziomello
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>

--
Rushabh Lathia

On Mon, Oct 27, 2014 at 4:15 AM, Rushabh Lathia <rushabh(dot)lathia(at)gmail(dot)com>
wrote:
>
> Hi All,
>
> - Patch got applied cleanly.
> - Regression make check run fine.
> - Patch covered the documentation changes
>
> Here are few comments:
>
> 1) What the need of following change:
>
> diff --git a/src/backend/storage/lmgr/lwlock.c
b/src/backend/storage/lmgr/lwlock.c
> index bcec173..9fe6855 100644
> --- a/src/backend/storage/lmgr/lwlock.c
> +++ b/src/backend/storage/lmgr/lwlock.c
> @@ -1005,12 +1005,6 @@ LWLockWaitForVar(LWLock *lock, uint64 *valptr,
uint64 oldval, uint64 *newval)
> lock->tail = proc;
> lock->head = proc;
>
> - /*
> - * Set releaseOK, to make sure we get woken up as soon as the
lock is
> - * released.
> - */
> - lock->releaseOK = true;
> -
> /* Can release the mutex now */
> SpinLockRelease(&lock->mutex);
>
>
> It doesn't look like related to this patch.
>

Sorry... my mistake when diff to master (more updated than my branch).

Fixed.

> 2)
>
> diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
> index ae5fe88..4d11952 100644
> --- a/src/bin/psql/help.c
> +++ b/src/bin/psql/help.c
> @@ -247,7 +247,7 @@ slashUsage(unsigned short int pager)
> fprintf(output, _(" \\f [STRING] show or set field
separator for unaligned query output\n"));
> fprintf(output, _(" \\H toggle HTML output mode
(currently %s)\n"),
> ON(pset.popt.topt.format == PRINT_HTML));
> - fprintf(output, _(" \\pset [NAME [VALUE]] set table output
option\n"
> + fprintf(output, _(" \\pset [NAME [VALUE]] set table output
option\n"
> " (NAME :=
{format|border|expanded|fieldsep|fieldsep_zero|footer|null|\n"
> "
numericlocale|recordsep|recordsep_zero|tuples_only|title|tableattr|pager|\n"
> "
unicode_border_linestyle|unicode_column_linestyle|unicode_header_linestyle})\n"));
>
>
> Why above changes reqired ?
>

Same previous mistake.

Fixed.

> 3) This patch adding IF NOT EXIST_S for CREATE TABLE AS and CREATE
MATERIALIZED
> TABLE, but testcase coverage for CREATE MATERIALIZED TABLE is missing.
>
> Apart from this changes looks good to me.
>

Fixed.

Thanks for your review.

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello

Patch looks good, assigning to committer.

On Fri, Oct 31, 2014 at 8:36 AM, Fabrízio de Royes Mello <
fabriziomello(at)gmail(dot)com> wrote:

>
>
> On Mon, Oct 27, 2014 at 4:15 AM, Rushabh Lathia <rushabh(dot)lathia(at)gmail(dot)com>
> wrote:
> >
> > Hi All,
> >
> > - Patch got applied cleanly.
> > - Regression make check run fine.
> > - Patch covered the documentation changes
> >
> > Here are few comments:
> >
> > 1) What the need of following change:
> >
> > diff --git a/src/backend/storage/lmgr/lwlock.c
> b/src/backend/storage/lmgr/lwlock.c
> > index bcec173..9fe6855 100644
> > --- a/src/backend/storage/lmgr/lwlock.c
> > +++ b/src/backend/storage/lmgr/lwlock.c
> > @@ -1005,12 +1005,6 @@ LWLockWaitForVar(LWLock *lock, uint64 *valptr,
> uint64 oldval, uint64 *newval)
> > lock->tail = proc;
> > lock->head = proc;
> >
> > - /*
> > - * Set releaseOK, to make sure we get woken up as soon as the
> lock is
> > - * released.
> > - */
> > - lock->releaseOK = true;
> > -
> > /* Can release the mutex now */
> > SpinLockRelease(&lock->mutex);
> >
> >
> > It doesn't look like related to this patch.
> >
>
> Sorry... my mistake when diff to master (more updated than my branch).
>
> Fixed.
>
>
> > 2)
> >
> > diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
> > index ae5fe88..4d11952 100644
> > --- a/src/bin/psql/help.c
> > +++ b/src/bin/psql/help.c
> > @@ -247,7 +247,7 @@ slashUsage(unsigned short int pager)
> > fprintf(output, _(" \\f [STRING] show or set field
> separator for unaligned query output\n"));
> > fprintf(output, _(" \\H toggle HTML output
> mode (currently %s)\n"),
> > ON(pset.popt.topt.format == PRINT_HTML));
> > - fprintf(output, _(" \\pset [NAME [VALUE]] set table output
> option\n"
> > + fprintf(output, _(" \\pset [NAME [VALUE]] set table output
> option\n"
> > " (NAME :=
> {format|border|expanded|fieldsep|fieldsep_zero|footer|null|\n"
> > "
> numericlocale|recordsep|recordsep_zero|tuples_only|title|tableattr|pager|\n"
> > "
> unicode_border_linestyle|unicode_column_linestyle|unicode_header_linestyle})\n"));
> >
> >
> > Why above changes reqired ?
> >
>
> Same previous mistake.
>
> Fixed.
>
>
> > 3) This patch adding IF NOT EXIST_S for CREATE TABLE AS and CREATE
> MATERIALIZED
> > TABLE, but testcase coverage for CREATE MATERIALIZED TABLE is missing.
> >
> > Apart from this changes looks good to me.
> >
>
> Fixed.
>
> Thanks for your review.
>
> Regards,
>
> --
> Fabrízio de Royes Mello
> Consultoria/Coaching PostgreSQL
> >> Timbira: http://www.timbira.com.br
> >> Blog: http://fabriziomello.github.io
> >> Linkedin: http://br.linkedin.com/in/fabriziomello
> >> Twitter: http://twitter.com/fabriziomello
> >> Github: http://github.com/fabriziomello
>

--
Rushabh Lathia

On Mon, Nov 3, 2014 at 3:12 AM, Rushabh Lathia <rushabh(dot)lathia(at)gmail(dot)com>
wrote:
>
> Patch looks good, assigning to committer.
>

Thanks for your review!

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello

On 11/03/2014 07:35 AM, Fabrízio de Royes Mello wrote:
> On Mon, Nov 3, 2014 at 3:12 AM, Rushabh Lathia
> <rushabh(dot)lathia(at)gmail(dot)com <mailto:rushabh(dot)lathia(at)gmail(dot)com>> wrote:
> >
> > Patch looks good, assigning to committer.
> >
>
> Thanks for your review!
>

Committed.

cheers

andrew

Em sábado, 13 de dezembro de 2014, Andrew Dunstan <andrew(at)dunslane(dot)net>
escreveu:

>
> On 11/03/2014 07:35 AM, Fabrízio de Royes Mello wrote:
>
>> On Mon, Nov 3, 2014 at 3:12 AM, Rushabh Lathia <rushabh(dot)lathia(at)gmail(dot)com
>> <mailto:rushabh(dot)lathia(at)gmail(dot)com>> wrote:
>> >
>> > Patch looks good, assigning to committer.
>> >
>>
>> Thanks for your review!
>>
>>
>
> Committed.
>
>
Thanks.

Fabrízio

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello