Horton

Thread

Commits

  1. Fix ancient get_object_address_opf_member bug

  2. Remove objname/objargs split for referring to objects

  3. Support opfamily members in get_object_address

  1. pgsql: Remove objname/objargs split for referring to objects

    Peter Eisentraut <peter_e@gmx.net> — 2017-03-06T18:34:49Z

    Remove objname/objargs split for referring to objects

In simpler times, it might have worked to refer to all kinds of objects
by a list of name components and an optional argument list.  But this
doesn't work for all objects, which has resulted in a collection of
hacks to place various other nodes types into these fields, which have
to be unpacked at the other end.  This makes it also weird to represent
lists of such things in the grammar, because they would have to be lists
of singleton lists, to make the unpacking work consistently.  The other
problem is that keeping separate name and args fields makes it awkward
to deal with lists of functions.

Change that by dropping the objargs field and have objname, renamed to
object, be a generic Node, which can then be flexibly assigned and
managed using the normal Node mechanisms.  In many cases it will still
be a List of names, in some cases it will be a string Value, for types
it will be the existing Typename, for functions it will now use the
existing ObjectWithArgs node type.  Some of the more obscure object
types still use somewhat arbitrary nested lists.

Reviewed-by: Jim Nasby <Jim.Nasby@BlueTreble.com>
Reviewed-by: Michael Paquier <michael.paquier@gmail.com>

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/8b6d6cf853aab12f0dc2adba7c99c3e458662734

Modified Files
--------------
src/backend/catalog/objectaddress.c          | 447 +++++++++++++++------------
src/backend/commands/alter.c                 |  33 +-
src/backend/commands/comment.c               |  11 +-
src/backend/commands/dropcmds.c              | 175 +++++------
src/backend/commands/extension.c             |  13 +-
src/backend/commands/seclabel.c              |   4 +-
src/backend/commands/tablecmds.c             |  10 +-
src/backend/commands/typecmds.c              |   2 +-
src/backend/nodes/copyfuncs.c                |  16 +-
src/backend/nodes/equalfuncs.c               |  16 +-
src/backend/parser/gram.y                    | 440 ++++++++++++--------------
src/backend/parser/parse_utilcmd.c           |  14 +-
src/include/catalog/objectaddress.h          |   8 +-
src/include/commands/extension.h             |   2 +-
src/include/nodes/parsenodes.h               |  24 +-
src/test/regress/expected/event_trigger.out  |   3 -
src/test/regress/expected/object_address.out |  24 +-
src/test/regress/sql/event_trigger.sql       |   3 -
18 files changed, 610 insertions(+), 635 deletions(-)

  2. Re: pgsql: Remove objname/objargs split for referring to objects

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2017-03-15T15:18:29Z

    Peter Eisentraut wrote:
> Remove objname/objargs split for referring to objects

I don't know if this is the guilty commit, but I'm now getting these
compiler warnings:


/pgsql/source/master/src/backend/catalog/objectaddress.c: In function 'get_object_address':
/pgsql/source/master/src/backend/catalog/objectaddress.c:1624:33: warning: 'typenames[1]' may be used uninitialized in this function [-Wmaybe-uninitialized]
       ereport(ERROR,
                                 ^
/pgsql/source/master/src/backend/catalog/objectaddress.c:1578:8: note: 'typenames[1]' was declared here
  char    *typenames[2];
        ^
/pgsql/source/master/src/backend/catalog/objectaddress.c:1624:33: warning: 'typenames[0]' may be used uninitialized in this function [-Wmaybe-uninitialized]
       ereport(ERROR,
                                 ^
/pgsql/source/master/src/backend/catalog/objectaddress.c:1578:8: note: 'typenames[0]' was declared here
  char    *typenames[2];
        ^

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  3. Re: [COMMITTERS] pgsql: Remove objname/objargs split for referring to objects

    Michael Paquier <michael.paquier@gmail.com> — 2017-03-16T02:36:45Z

    On Thu, Mar 16, 2017 at 12:18 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> Peter Eisentraut wrote:
>> Remove objname/objargs split for referring to objects
>
> I don't know if this is the guilty commit, but I'm now getting these
> compiler warnings:
>
>
> /pgsql/source/master/src/backend/catalog/objectaddress.c: In function 'get_object_address':
> /pgsql/source/master/src/backend/catalog/objectaddress.c:1624:33: warning: 'typenames[1]' may be used uninitialized in this function [-Wmaybe-uninitialized]
>        ereport(ERROR,
>                                  ^
> /pgsql/source/master/src/backend/catalog/objectaddress.c:1578:8: note: 'typenames[1]' was declared here
>   char    *typenames[2];
>         ^
> /pgsql/source/master/src/backend/catalog/objectaddress.c:1624:33: warning: 'typenames[0]' may be used uninitialized in this function [-Wmaybe-uninitialized]
>        ereport(ERROR,
>                                  ^
> /pgsql/source/master/src/backend/catalog/objectaddress.c:1578:8: note: 'typenames[0]' was declared here
>   char    *typenames[2];
>         ^

What are you using as CFLAGS? As both typenames should be normally
set, what about initializing those fields with NULL and add an
assertion like the attached?
-- 
Michael

  4. Re: [COMMITTERS] pgsql: Remove objname/objargs split for referring to objects

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2017-03-16T15:56:24Z

    Michael Paquier wrote:

> What are you using as CFLAGS? As both typenames should be normally
> set, what about initializing those fields with NULL and add an
> assertion like the attached?

Actually, my compiler was right -- this was an ancient bug I introduced
in 9.5 (commit a61fd533), and this new warning was my compiler being a
bit smarter now for some reason.  The problem is we were trying to
extract String value from a TypeName node, which obviously doesn't work
very well.

I pushed a real fix, not just a compiler-silencer, along with a few
lines in object_address.sql to make sure it works properly.  Maybe we
need a few more tests cases for other parts of pg_get_object_address.

Pushed fix, backpatched to 9.5.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  5. Re: [COMMITTERS] pgsql: Remove objname/objargs split for referring to objects

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2017-03-17T03:13:06Z

    On 3/16/17 11:56, Alvaro Herrera wrote:
> Michael Paquier wrote:
> 
>> What are you using as CFLAGS? As both typenames should be normally
>> set, what about initializing those fields with NULL and add an
>> assertion like the attached?
> 
> Actually, my compiler was right -- this was an ancient bug I introduced
> in 9.5 (commit a61fd533), and this new warning was my compiler being a
> bit smarter now for some reason.  The problem is we were trying to
> extract String value from a TypeName node, which obviously doesn't work
> very well.
> 
> I pushed a real fix, not just a compiler-silencer, along with a few
> lines in object_address.sql to make sure it works properly.  Maybe we
> need a few more tests cases for other parts of pg_get_object_address.
> 
> Pushed fix, backpatched to 9.5.

I am now seeing the warnings that Michael was reporting *after* your
commit in 9.5 and 9.6.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  6. Re: [COMMITTERS] pgsql: Remove objname/objargs split for referring to objects

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2017-03-17T15:06:07Z

    Peter Eisentraut wrote:
> On 3/16/17 11:56, Alvaro Herrera wrote:
> > Michael Paquier wrote:
> > 
> >> What are you using as CFLAGS? As both typenames should be normally
> >> set, what about initializing those fields with NULL and add an
> >> assertion like the attached?
> > 
> > Actually, my compiler was right -- this was an ancient bug I introduced
> > in 9.5 (commit a61fd533), and this new warning was my compiler being a
> > bit smarter now for some reason.  The problem is we were trying to
> > extract String value from a TypeName node, which obviously doesn't work
> > very well.
> > 
> > I pushed a real fix, not just a compiler-silencer, along with a few
> > lines in object_address.sql to make sure it works properly.  Maybe we
> > need a few more tests cases for other parts of pg_get_object_address.
> > 
> > Pushed fix, backpatched to 9.5.
> 
> I am now seeing the warnings that Michael was reporting *after* your
> commit in 9.5 and 9.6.

Do you mean the ones I reported?

I don't get anything with my compiler (gcc (Debian 4.9.2-10) 4.9.2) and
I don't see anything in the few logs I looked at (clang, 9.6) from
buildfarm either.  What are you seeing specifically?

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  7. Re: [COMMITTERS] pgsql: Remove objname/objargs split for referring to objects

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2017-03-17T16:28:49Z

    On 3/17/17 11:06, Alvaro Herrera wrote:
> I don't get anything with my compiler (gcc (Debian 4.9.2-10) 4.9.2) and
> I don't see anything in the few logs I looked at (clang, 9.6) from
> buildfarm either.  What are you seeing specifically?

cc -Wall -Wmissing-prototypes -Wpointer-arith
-Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute
-Wformat-security -fno-strict-aliasing -fwrapv
-fexcess-precision=standard -g -O2 -I../../../src/include
-D_FORTIFY_SOURCE=2 -DLINUX_OOM_ADJ -D_GNU_SOURCE -I/usr/include/libxml2
-c -o objectaddress.o objectaddress.c
objectaddress.c: In function ‘get_object_address’:
objectaddress.c:1650:24: warning: ‘typeoids[1]’ may be used
uninitialized in this function [-Wmaybe-uninitialized]
objectaddress.c:1582:6: note: ‘typeoids[1]’ was declared here
objectaddress.c:1627:43: warning: ‘typenames[1]’ may be used
uninitialized in this function [-Wmaybe-uninitialized]
objectaddress.c:1581:12: note: ‘typenames[1]’ was declared here
objectaddress.c:1650:24: warning: ‘typeoids[0]’ may be used
uninitialized in this function [-Wmaybe-uninitialized]
objectaddress.c:1582:6: note: ‘typeoids[0]’ was declared here
objectaddress.c:1627:43: warning: ‘typenames[0]’ may be used
uninitialized in this function [-Wmaybe-uninitialized]
objectaddress.c:1581:12: note: ‘typenames[0]’ was declared here

cc (Debian 4.7.2-5) 4.7.2

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  8. Re: [COMMITTERS] pgsql: Remove objname/objargs split for referring to objects

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2017-03-17T17:39:26Z

    Peter Eisentraut wrote:

> cc -Wall -Wmissing-prototypes -Wpointer-arith
> -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute
> -Wformat-security -fno-strict-aliasing -fwrapv
> -fexcess-precision=standard -g -O2 -I../../../src/include
> -D_FORTIFY_SOURCE=2 -DLINUX_OOM_ADJ -D_GNU_SOURCE -I/usr/include/libxml2
> -c -o objectaddress.o objectaddress.c
> objectaddress.c: In function ‘get_object_address’:
> objectaddress.c:1650:24: warning: ‘typeoids[1]’ may be used
> uninitialized in this function [-Wmaybe-uninitialized]
> objectaddress.c:1582:6: note: ‘typeoids[1]’ was declared here
> objectaddress.c:1627:43: warning: ‘typenames[1]’ may be used
> uninitialized in this function [-Wmaybe-uninitialized]
> objectaddress.c:1581:12: note: ‘typenames[1]’ was declared here
> objectaddress.c:1650:24: warning: ‘typeoids[0]’ may be used
> uninitialized in this function [-Wmaybe-uninitialized]
> objectaddress.c:1582:6: note: ‘typeoids[0]’ was declared here
> objectaddress.c:1627:43: warning: ‘typenames[0]’ may be used
> uninitialized in this function [-Wmaybe-uninitialized]
> objectaddress.c:1581:12: note: ‘typenames[0]’ was declared here
> 
> cc (Debian 4.7.2-5) 4.7.2

Oh yeah, buildfarm member sitella shows that.  Does the attached patch
fix it? (pretty much the same thing Michael attached, but we also need
to handle typeoids)

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  9. Re: [COMMITTERS] pgsql: Remove objname/objargs split for referring to objects

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2017-03-17T18:45:47Z

    On 3/17/17 13:39, Alvaro Herrera wrote:
> Oh yeah, buildfarm member sitella shows that.  Does the attached patch
> fix it? (pretty much the same thing Michael attached, but we also need
> to handle typeoids)

Yes, that makes the warning go away.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services