Re: [PATCHES] Allow an alias for the target table in UPDATE/DELETE

[Martijn van Oosterhout]

 Another possibility is to disallow SET here, but not in other places
 where ColId is used. That is, some hack like:

Quick point: If all you want to do if disallow SET here as ColID but
allow SET in other places with a ColId, you don't have to change
anything at all. Just make sure the rule for the want you prefer is
earlier in the file.

Shift/reduce and reduce/reduce errors still produce valid working
parsers, it's just that bison has to resolve an ambiguity by the
default (shift, otherwise earliest rule wins. maximum munch rule
really).

If you don't like relying on file order to resolve this, appropriate
use of %prec would have the same effect (just like for operator
precedence). The output file tell you which way bison went.

Have a nice day,
-- 
Martijn van Oosterhout   kleptog@svana.org   http://svana.org/kleptog/
 Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
 tool for doing 5% of the work and then sitting around waiting for someone
 else to do the other 95% so you can sue them.


signature.asc
Description: Digital signature

Re: [PATCHES] Allow an alias for the target table in UPDATE/DELETE

[Andrew Dunstan]

Martijn van Oosterhout wrote:


On Sun, Jan 22, 2006 at 09:04:14AM -0500, Andrew Dunstan wrote:
 


If you don't like relying on file order to resolve this, appropriate
use of %prec would have the same effect (just like for operator
precedence). The output file tell you which way bison went.
 



 

If we allow shift/reduce or reduce/reduce conflicts, debugging future 
development becomes more difficult. Right now we have the nice property 
that if you see one of those you know you've done something wrong (and 
using the expect directive isn't really a good answer, and only applies 
to shift/reduce conflicts anyway).
   



But that's the point of the %prec directive. To force bison to choose
one or the other, thus removing the warning... For an ambiguity that
only appears in one statement, it seems a better solution than upgrade
SET to a new class of identifier.

 



Quite so. We already use %prec extensively. All I was pointing out was 
that using file order isn't an acceptable option.


Presumably the effect in this case would be to prevent anyone from using 
SET as an alias unless there was a preceding AS.


cheers

andrew



---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
  choose an index scan if your joining column's datatypes do not
  match

Re: [PATCHES] Allow an alias for the target table in UPDATE/DELETE

[Tom Lane]

Andrew Dunstan [EMAIL PROTECTED] writes:
 Presumably the effect in this case would be to prevent anyone from using 
 SET as an alias unless there was a preceding AS.

I fooled around with this and found three feasible alternatives.

The simplest thing is:

relation_expr_opt_alias: relation_expr
| relation_expr IDENT
| relation_expr AS ColId

which at least gets us to the point of being able to use anything you
normally can use as an alias in other contexts, so long as you write AS.
Given the large and ever-increasing list of unreserved_keywords, though,
I don't think this is sufficient.

Then there's the precedence way.  This seems to work:

431a432
 %nonassoc SET
5883c5884
 relation_expr_opt_alias: relation_expr
---
 relation_expr_opt_alias: relation_expr%prec UMINUS
5887c5888,5895
   | relation_expr opt_as IDENT
---
   | relation_expr ColId
   {
   Alias *alias = makeNode(Alias);
   alias-aliasname = $2;
   $1-alias = alias;
   $$ = $1;
   }
   | relation_expr AS ColId

The effect of this, as Andrew says, is that in this particular context
you can't write SET as an alias unless you write AS first.  This is
probably not going to surprise anyone in the UPDATE case, since the
ambiguity inherent in writing
UPDATE foo set SET ...
is pretty obvious.  However it might surprise someone in the DELETE
context.  Another thing that worries me is that assigning a precedence
to the SET keyword might someday bite us by masking some other
grammatical ambiguity.  (As far as I can tell by comparing y.output
files, it's not changing the behavior of any other part of the grammar
today, but ...)

The third alternative is to stop allowing SET as an unreserved_keyword.
I found that moving it up to func_name_keyword was enough to get rid
of the conflict, without using precedence.  This is somewhat attractive
on the grounds of future-proofing (we may have to make SET more reserved
someday anyway for something else); and you can argue that the principle
of least astonishment demands that SET should be either always OK as an
alias or always not OK.  On the other hand this would raise some
backwards-compatibility issues.  Is it likely that anyone out there is
using SET as a table or column name?

I could live with either choice #2 or choice #3.  Comments?

regards, tom lane

---(end of broadcast)---
TIP 6: explain analyze is your friend

Re: [PATCHES] TupleDesc refcounting

[Neil Conway]

On Sun, 2006-01-22 at 13:43 -0500, Tom Lane wrote:
 I object ... this is still trying to enforce tupdesc refcounting on much
 more of the system than I think useful or prudent.

Well, the patch adds management of TupleDesc refcounts solely for the
return value of lookup_rowtype_tupdesc(). If we're going to use
reference counting there at all, I don't see a way do make the
refcounting any less invasive.

-Neil



---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match

Re: [PATCHES] Allow an alias for the target table in UPDATE/DELETE

[Andrew Dunstan]

Tom Lane wrote:


The effect of this, as Andrew says, is that in this particular context
you can't write SET as an alias unless you write AS first.  This is
probably not going to surprise anyone in the UPDATE case, since the
ambiguity inherent in writing
UPDATE foo set SET ...
is pretty obvious.  However it might surprise someone in the DELETE
context.  
 



You probably avoid that if you have a separate rule for the DELETE case. 
That raises this question: how far do we want to go in unfactoring the 
grammar to handle such cases?


cheers

andrew

---(end of broadcast)---
TIP 6: explain analyze is your friend

Re: [PATCHES] Allow an alias for the target table in UPDATE/DELETE

[Neil Conway]

On Sun, 2006-01-22 at 13:26 -0500, Tom Lane wrote:
 The effect of this, as Andrew says, is that in this particular context
 you can't write SET as an alias unless you write AS first.

Did you actually test this?

neilc=# create table t1 (a int, b int);
CREATE TABLE
neilc=# update t1 set set a = 500 where set.a  1000;
UPDATE 0

(Using essentially the patch you posted.)

 This is
 probably not going to surprise anyone in the UPDATE case, since the
 ambiguity inherent in writing
   UPDATE foo set SET ...
 is pretty obvious.  However it might surprise someone in the DELETE
 context.

Well, if necessary we can just use an alternate production for the
DELETE case, as there is no SET ambiguity to worry about.

 The third alternative is to stop allowing SET as an unreserved_keyword.
 I found that moving it up to func_name_keyword was enough to get rid
 of the conflict, without using precedence.

I think we should avoid this if possible: it seems quite likely to me
that there are applications using SET as the name of a type, table or
column (making SET a func_name_keyword would disallow its use as a type
name, AFAICS). So I'm inclined to favor #2.

-Neil



---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match

Re: [PATCHES] Allow an alias for the target table in UPDATE/DELETE

[Tom Lane]

Neil Conway [EMAIL PROTECTED] writes:
 Did you actually test this?

No, I was just looking at the y.output file to see what would happen.

 neilc=# update t1 set set a = 500 where set.a  1000;
 UPDATE 0
 (Using essentially the patch you posted.)

[ scratches head... ]  That shouldn't have worked.  I'll have to look
again.

 Well, if necessary we can just use an alternate production for the
 DELETE case, as there is no SET ambiguity to worry about.

Yeah, I thought of that too and rejected it as being too much trouble
for too small a case.  I'm really considerably more worried about the
question of whether attaching a precedence to SET might cause trouble
later.  But it's only a hypothetical problem at this point.

 So I'm inclined to favor #2.

OK, motion carries.  I'll check what's happening in the case above
and commit if there's not something wrong.

regards, tom lane

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster

[PATCHES] TODO item: locale per database patch (new iteration)

[Alexey Slynko]

Hi,

it's a renewed locale per database patch. Unfortunately, i've not found 
clean way to rebuild database indexes automatically, if locale settings 
of two databases (created and template) are differs. Now it's only 
raises a NOTICE. So, if anyone has a right notion about it - let will 
express. Comment and suggestions are highly appreciated
Index: src/backend/access/transam/xlog.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/access/transam/xlog.c,v
retrieving revision 1.226
diff -u -r1.226 xlog.c
--- src/backend/access/transam/xlog.c   11 Jan 2006 08:43:12 -  1.226
+++ src/backend/access/transam/xlog.c   22 Jan 2006 16:41:02 -
@@ -3394,7 +3394,6 @@
 {
int fd;
charbuffer[BLCKSZ]; /* need not be aligned */
-   char   *localeptr;
 
/*
 * Initialize version and compatibility-check fields
@@ -3418,18 +3417,6 @@
ControlFile-enableIntTimes = FALSE;
 #endif
 
-   ControlFile-localeBuflen = LOCALE_NAME_BUFLEN;
-   localeptr = setlocale(LC_COLLATE, NULL);
-   if (!localeptr)
-   ereport(PANIC,
-   (errmsg(invalid LC_COLLATE setting)));
-   StrNCpy(ControlFile-lc_collate, localeptr, LOCALE_NAME_BUFLEN);
-   localeptr = setlocale(LC_CTYPE, NULL);
-   if (!localeptr)
-   ereport(PANIC,
-   (errmsg(invalid LC_CTYPE setting)));
-   StrNCpy(ControlFile-lc_ctype, localeptr, LOCALE_NAME_BUFLEN);
-
/* Contents are protected with a CRC */
INIT_CRC32(ControlFile-crc);
COMP_CRC32(ControlFile-crc,
@@ -3612,34 +3599,6 @@
but the server was compiled without 
HAVE_INT64_TIMESTAMP.),
 errhint(It looks like you need to recompile 
or initdb.)));
 #endif
-
-   if (ControlFile-localeBuflen != LOCALE_NAME_BUFLEN)
-   ereport(FATAL,
-   (errmsg(database files are incompatible with 
server),
-errdetail(The database cluster was 
initialized with LOCALE_NAME_BUFLEN %d,
-  but the server was compiled with 
LOCALE_NAME_BUFLEN %d.,
-  ControlFile-localeBuflen, 
LOCALE_NAME_BUFLEN),
-errhint(It looks like you need to recompile 
or initdb.)));
-   if (pg_perm_setlocale(LC_COLLATE, ControlFile-lc_collate) == NULL)
-   ereport(FATAL,
-   (errmsg(database files are incompatible with operating 
system),
-errdetail(The database cluster was initialized with 
LC_COLLATE \%s\,
-   which is not recognized by 
setlocale().,
-  ControlFile-lc_collate),
-errhint(It looks like you need to initdb or install 
locale support.)));
-   if (pg_perm_setlocale(LC_CTYPE, ControlFile-lc_ctype) == NULL)
-   ereport(FATAL,
-   (errmsg(database files are incompatible with operating 
system),
-   errdetail(The database cluster was initialized with LC_CTYPE 
\%s\,
-  which is not recognized by setlocale().,
- ControlFile-lc_ctype),
-errhint(It looks like you need to initdb or install 
locale support.)));
-
-   /* Make the fixed locale settings visible as GUC variables, too */
-   SetConfigOption(lc_collate, ControlFile-lc_collate,
-   PGC_INTERNAL, PGC_S_OVERRIDE);
-   SetConfigOption(lc_ctype, ControlFile-lc_ctype,
-   PGC_INTERNAL, PGC_S_OVERRIDE);
 }
 
 void
Index: src/backend/commands/dbcommands.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/commands/dbcommands.c,v
retrieving revision 1.175
diff -u -r1.175 dbcommands.c
--- src/backend/commands/dbcommands.c   22 Nov 2005 18:17:08 -  1.175
+++ src/backend/commands/dbcommands.c   22 Jan 2006 16:41:03 -
@@ -25,6 +25,10 @@
 #include unistd.h
 #include sys/stat.h
 
+#ifdef HAVE_LANGINFO_H
+#include langinfo.h
+#endif
+
 #include access/genam.h
 #include access/heapam.h
 #include catalog/catalog.h
@@ -49,6 +53,7 @@
 #include utils/fmgroids.h
 #include utils/guc.h
 #include utils/lsyscache.h
+#include utils/pg_locale.h
 #include utils/syscache.h
 
 
@@ -57,9 +62,11 @@
int *encodingP, bool *dbIsTemplateP, bool *dbAllowConnP,
Oid *dbLastSysOidP,
TransactionId *dbVacuumXidP, TransactionId 
*dbFrozenXidP,
-   Oid *dbTablespace);
+   Oid *dbTablespace, char **dbCollate, char **dbCtype);
 static bool have_createdb_privilege(void);
 

Re: [PATCHES] Allow an alias for the target table in UPDATE/DELETE

[Tom Lane]

Neil Conway [EMAIL PROTECTED] writes:
 Did you actually test this?

 neilc=# create table t1 (a int, b int);
 CREATE TABLE
 neilc=# update t1 set set a = 500 where set.a  1000;
 UPDATE 0

I get 

regression=# update t1 set set a = 500 where set.a  1000;
ERROR:  syntax error at or near a at character 19
LINE 1: update t1 set set a = 500 where set.a  1000;
  ^

I think possibly you put the %nonassoc entry in the wrong place --- it
has to go someplace before the UMINUS entry to get the desired effect.
My fault for putting up a non-context diff.

regards, tom lane

---(end of broadcast)---
TIP 6: explain analyze is your friend