Problem with parser

Hi,

who's the parser guru? I need help!

I have a table t1(a int4, b int4)

When I

update t1 set b = 2 where a = 1;

I get a targetlist with 1 entry and resno=2.

But when I

update t1 set b = t2.b where a = t2.a;

I get the same 1 entry targetlist with resno=1 for attribute
"b". That causes deep deep trouble in the rewrite system,
when fixing the expressions for *new* variable references.
*new*.attr defaults to *old*.attr except given in the query
to be updated. When fixing *new* references, the rewrite
system looks up the original parsetree to find a TLE with the
same resno as the attribute number in the *new*.attr. So it
depends on having the resno same to the attno of the result
relation.

I'm absolutely unfamiliar with the parser in this area and I
don't want to hack around and break things so close before
6.4. Who knows how to fix this?

BTW: up to now the rewrite system looks much better. It works
for insert, update and delete when using constant values.
insert ... select ... works too.

Let me take a look at this. I am in the middle of a major patch, so it
may take a day or two.

-- 
Bruce Momjian                          |  830 Blythe Avenue
maillist@candle.pha.pa.us              |  Drexel Hill, Pennsylvania 19026
  +  If your life is a hard drive,     |  (610) 353-9879(w)
  +  Christ can be your backup.        |  (610) 853-3000(h)

Hi,

who's the parser guru? I need help!

I have a table t1(a int4, b int4)

When I

update t1 set b = 2 where a = 1;

I get a targetlist with 1 entry and resno=2.

But when I

update t1 set b = t2.b where a = t2.a;

Is the parser code correct here? You are actually doing:

update t1 set b = t2.b from t2 where a = t2.a;
^^^^^^^

I don't have a running system right now because of my other patch. Can
you send me a tree pointing to the problem? In the above example, what
is the first resno? EXPLAIN VERBOSE should give you the plans, and dump
detailed plans into the postmaster log file.

I think I see the problem. In MakeTargetlistExpr(), we have this code:

/* Processes target columns that will be receiving results */
if (pstate->p_is_insert || pstate->p_is_update)
{
/*
* insert or update query -- insert, update work only on one
* relation, so multiple occurence of same resdomno is bogus
*/
rd = pstate->p_target_relation;
Assert(rd != NULL);
---> resdomno = attnameAttNum(rd, colname);
attrisset = attnameIsSet(rd, colname);
attrtype = attnumTypeId(rd, resdomno);
if ((arrayRef != NIL) && (lfirst(arrayRef) == NIL))
attrtype = GetArrayElementType(attrtype);
attrtypmod = rd->rd_att->attrs[resdomno - 1]->atttypmod;

This looks bad to me, especially because you have a join going on in the
update. In fact, the comment clearly shows a false assertion, that ther
is only one relation in UPDATE.

Is the update rewrite code assuming that the resdomno of an updated
column must match the attribute number? And the join is messing this
up?

I get the same 1 entry targetlist with resno=1 for attribute
"b". That causes deep deep trouble in the rewrite system,
when fixing the expressions for *new* variable references.
*new*.attr defaults to *old*.attr except given in the query
to be updated. When fixing *new* references, the rewrite
system looks up the original parsetree to find a TLE with the
same resno as the attribute number in the *new*.attr. So it
depends on having the resno same to the attno of the result
relation.

I'm absolutely unfamiliar with the parser in this area and I
don't want to hack around and break things so close before
6.4. Who knows how to fix this?

BTW: up to now the rewrite system looks much better. It works
for insert, update and delete when using constant values.
insert ... select ... works too.

Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#======================================== jwieck@debis.com (Jan Wieck) #

-- 
Bruce Momjian                          |  830 Blythe Avenue
maillist@candle.pha.pa.us              |  Drexel Hill, Pennsylvania 19026
  +  If your life is a hard drive,     |  (610) 353-9879(w)
  +  Christ can be your backup.        |  (610) 853-3000(h)

This looks bad to me, especially because you have a join going on in the
update. In fact, the comment clearly shows a false assertion, that ther
is only one relation in UPDATE.

Is the update rewrite code assuming that the resdomno of an updated
column must match the attribute number? And the join is messing this
up?

--
Bruce Momjian | 830 Blythe Avenue

Right! The rewrite code assumes that the resdomno of the
updated columns match the attribute number in the target
relation. I don't know if the join is messing it up - but
looks like. Thanks for the help - I think I have to look for
usage of p_last_resno to find all the places where this can
happen.

Little joke:

At the place in analyze.c, where a TLE is created, there is a
comment that not creating a proper target list with correct
resdomno's might break rules :-)

I love those comments. And especially all that have XXXXX
somewhere.

Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#======================================== jwieck@debis.com (Jan Wieck) #

This looks bad to me, especially because you have a join going on in the
update. In fact, the comment clearly shows a false assertion, that ther
is only one relation in UPDATE.

Is the update rewrite code assuming that the resdomno of an updated
column must match the attribute number? And the join is messing this
up?

--
Bruce Momjian | 830 Blythe Avenue

Right! The rewrite code assumes that the resdomno of the
updated columns match the attribute number in the target
relation. I don't know if the join is messing it up - but
looks like. Thanks for the help - I think I have to look for
usage of p_last_resno to find all the places where this can
happen.

OK, do you need me to supply a patch? I would be glad to review
anything you have.

Little joke:

At the place in analyze.c, where a TLE is created, there is a
comment that not creating a proper target list with correct
resdomno's might break rules :-)

I love those comments. And especially all that have XXXXX
somewhere.

Yes, I have a few favorites.

-- 
Bruce Momjian                          |  830 Blythe Avenue
maillist@candle.pha.pa.us              |  Drexel Hill, Pennsylvania 19026
  +  If your life is a hard drive,     |  (610) 353-9879(w)
  +  Christ can be your backup.        |  (610) 853-3000(h)

OK, do you need me to supply a patch? I would be glad to review
anything you have.

I decided to reassign the redomno's when entering
RewriteQuery() and there's a resultRelation. It's the only
way I think not to miss a single place where a bad resdomno
is created.

Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#======================================== jwieck@debis.com (Jan Wieck) #

OK, do you need me to supply a patch? I would be glad to review
anything you have.

I decided to reassign the redomno's when entering
RewriteQuery() and there's a resultRelation. It's the only
way I think not to miss a single place where a bad resdomno
is created.

I am going to fix the parser, so you don't have to do this in the
rewrite system. I will let you know when it is ready.

-- 
Bruce Momjian                          |  830 Blythe Avenue
maillist@candle.pha.pa.us              |  Drexel Hill, Pennsylvania 19026
  +  If your life is a hard drive,     |  (610) 353-9879(w)
  +  Christ can be your backup.        |  (610) 853-3000(h)

This looks bad to me, especially because you have a join going on in the
update. In fact, the comment clearly shows a false assertion, that ther
is only one relation in UPDATE.

Is the update rewrite code assuming that the resdomno of an updated
column must match the attribute number? And the join is messing this
up?

--
Bruce Momjian | 830 Blythe Avenue

Right! The rewrite code assumes that the resdomno of the
updated columns match the attribute number in the target
relation. I don't know if the join is messing it up - but
looks like. Thanks for the help - I think I have to look for
usage of p_last_resno to find all the places where this can
happen.

Jan, I will be fixing this.

Jan, I am attaching the current TODO list. Can you tell me which items
are fixed in 6.4?

-- 
Bruce Momjian                          |  830 Blythe Avenue
maillist@candle.pha.pa.us              |  Drexel Hill, Pennsylvania 19026
  +  If your life is a hard drive,     |  (610) 353-9879(w)
  +  Christ can be your backup.        |  (610) 853-3000(h)

Jan, I am attaching the current TODO list. Can you tell me which items
are fixed in 6.4?

--
Bruce Momjian | 830 Blythe Avenue

* CREATE VIEW requires super-user priviledge

Fixed - regular users can create views and rules

* views on subselects fail

Must check that - hope it is already fixed

* computations in views fail:
create view test as select usesysid * usesysid from pg_shadow;

Will check this too

* Add full ANSI SQL capabilities
* -Implement HAVING clause(Stephan)
* add OUTER joins, left and right (Thomas)
* make VIEWs updateable where possible

Rules can be used to make views updateable

* Fix the rules system(Jan,Soo-Ho)
* robust
* making INSTEAD rules work
* add CONSTRAINT

The restricted part of rules are robust and instead rules
work. Constraints could be implemented as rules but would
require a new RAISE statement. I'll work on constraints
for 6.5.

* Allow INSERT INTO ... SELECT ... FROM view to work

Should work - will check it.

* Allow views on a UNION
* Allow DISTINCT on view
* Allow views of aggregate columns

Check - not yet - check

* Allow views to specify column names outside SELECT statement

??? what is meant by that?

Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#======================================== jwieck@debis.com (Jan Wieck) #

Jan, I am attaching the current TODO list. Can you tell me which items
are fixed in 6.4?

--
Bruce Momjian | 830 Blythe Avenue

* CREATE VIEW requires super-user priviledge

Fixed - regular users can create views and rules

* views on subselects fail

Must check that - hope it is already fixed

* computations in views fail:
create view test as select usesysid * usesysid from pg_shadow;

Will check this too

* Add full ANSI SQL capabilities
* -Implement HAVING clause(Stephan)
* add OUTER joins, left and right (Thomas)
* make VIEWs updateable where possible

Rules can be used to make views updateable

* Fix the rules system(Jan,Soo-Ho)
* robust
* making INSTEAD rules work
* add CONSTRAINT

The restricted part of rules are robust and instead rules
work. Constraints could be implemented as rules but would
require a new RAISE statement. I'll work on constraints
for 6.5.

* Allow INSERT INTO ... SELECT ... FROM view to work

Should work - will check it.

* Allow views on a UNION
* Allow DISTINCT on view
* Allow views of aggregate columns

Check - not yet - check

* Allow views to specify column names outside SELECT statement

??? what is meant by that?

I have no idea. I am removing it. Does anyone else know what it means?

I have updated the TODO list. If you find more items you have fixed,
let me know. The rewrite system had a large chunk of the TODO list.
That is why I was so excited when you said you could fix it.

-- 
Bruce Momjian                          |  830 Blythe Avenue
maillist@candle.pha.pa.us              |  Drexel Hill, Pennsylvania 19026
  +  If your life is a hard drive,     |  (610) 353-9879(w)
  +  Christ can be your backup.        |  (610) 853-3000(h)

On Wed, 26 Aug 1998, Bruce Momjian wrote:

* Allow views to specify column names outside SELECT statement

??? what is meant by that?

I have no idea. I am removing it. Does anyone else know what it means?

Might it have had something to do with the inability to do a WHERE
clause on a field not in the target list?

* Allow views to specify column names outside SELECT statement

??? what is meant by that?

I have no idea. I am removing it. Does anyone else know what it
means?

I think it was the example from ...
Well, the syntax was
CREATE VIEW my_view(column1, column2, column3) AS
SELECT t1 * m2, c2, j FROM my_table1
UNION ALL
SELECT m2, c3, n FROM my_table2;
instead of:
CREATE VIEW my_view AS
SELECT t1 * m2 AS column1, c2 AS column2, j AS column3 FROM my_table1
UNION ALL
SELECT m2, c3, n FROM my_table2;

Hope this helps,
-DEJ

* Allow views to specify column names outside SELECT statement

??? what is meant by that?

I have no idea. I am removing it. Does anyone else know what it means?

It probably means:
create view myview (name, salary, hours) as select lname, currsal, whours from wages;

Andreas

* Allow views to specify column names outside SELECT statement

??? what is meant by that?

I have no idea. I am removing it. Does anyone else know what it means?

It probably means:
create view myview (name, salary, hours) as select lname, currsal, whours from wages;

Added to TODO:

* CREATE VIEW myview (name) AS SELECT lname FROM wages fails

-- 
Bruce Momjian                          |  830 Blythe Avenue
maillist@candle.pha.pa.us              |  Drexel Hill, Pennsylvania 19026
  +  If your life is a hard drive,     |  (610) 353-9879(w)
  +  Christ can be your backup.        |  (610) 853-3000(h)

* Allow views to specify column names outside SELECT statement

??? what is meant by that?

I have no idea. I am removing it. Does anyone else know what it means?

It probably means:
create view myview (name, salary, hours) as select lname, currsal, whours from wages;

Andreas

But that doesn't have to do anything with the rewrite system.
It has to do with the view creation step on setting up the
instead rule for view selection. There the AS clauses have to
be moved into the target list. The rewrite system doesn't
know if this rule is something that builds up a view.

Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#======================================== jwieck@debis.com (Jan Wieck) #

* Allow views to specify column names outside SELECT statement

??? what is meant by that?

I have no idea. I am removing it. Does anyone else know what it means?

It probably means:
create view myview (name, salary, hours) as select lname, currsal, whours from wages;

Andreas

But that doesn't have to do anything with the rewrite system.
It has to do with the view creation step on setting up the
instead rule for view selection. There the AS clauses have to
be moved into the target list. The rewrite system doesn't
know if this rule is something that builds up a view.

Yes, I think this is a parser issue.
It can be handled in the same way as the following syntax (only the above is the standard):
create view myview as select lname as name, currsal as salary, whours as hours from wages;

Andreas