Review: Typed Table

From: Hitoshi Harada <umi(dot)tanuki(at)gmail(dot)com>
To: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Review: Typed Table
Date: 2010-01-18 16:01:18
Message-ID: e08cc0401001180801h112437dfxbbe59816534626e0@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I reviewed this patch today.

Overview:
Almost everything is OK. Applied with few hunks (in psql/describle.c 2
lines offset), compiled without warnings, passed regression tests. The
results of advertised queries are as expected. Coding style is of
course satisfied. Since this is utility changes performance does not
go down.

* in namespace.c
I didn't see why this file has been changed.
case 0:
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("improper qualified name (zero-length name list)")));
break;
in DeconstructQualifiedName().

* Crash with wrong column name
regression=# create type persons_type as (name text, bdate date);
CREATE TYPE
regression=# create table persons of persons_type (myname with options
not null);
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Succeeded.

* Conflict between transactions
I'm not sure if this is related with the patch but I met this situation;

A: regression=# create type persons_type as (name text, bdate date);
A: CREATE TYPE

A: regression=# begin;
A: BEGIN

A: regression=# drop type persons_type;
A: DROP TYPE

B: regression=# create table persons of persons_type; (LOCK)
A: regression=# rollback;
A: ROLLBACK
B: CREATE TABLE

B: regression=# drop table persons;
B: DROP TABLE

A: regression=# begin;
A: BEGIN

A: regression=# drop type persons_type;
A: DROP TYPE

B: regression=# create table persons of persons_type; (NO LOCK)
B: CREATE TABLE

A: regression=# commit;
A: COMMIT

B: regression=# select 'persons_type'::regtype;
B: ERROR: type "persons_type" does not exist
B: LINE 1: select 'persons_type'::regtype;

I have at all no idea why the second create table doesn't lock.

* Comment needed in pg_dump.h
Please add comment on reloftype of struct _tableInfo

* Consistency between add/drop and rename
Typed table can rename its column but can NOT add/drop column. Is this
what the spec requires? IMHO, if it allows rename, do so for add/drop
and if do not allow add/drop, do not so rename.

I read SQL standard about typed table for few minutes but did not find
any problems. It mentions about "reference column" as the patch
document says and I, too, think our oid mechanism will do for that as
the doc says.

Regards,

--
Hitoshi Harada

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Markus Wanner 2010-01-18 16:19:08 Re: Testing with concurrent sessions
Previous Message Alvaro Herrera 2010-01-18 15:52:49 Re: mailing list archiver chewing patches