Unary % operator is broken in current sources

Started by Tom Laneabout 27 years ago7 messageshackers
Jump to latest
#1Tom Lane
tgl@sss.pgh.pa.us

These used to work:

regression=> select %f.f1 FROM FLOAT8_TBL f;
ERROR: parser: parse error at or near "%"
regression=> select f.f1 % FROM FLOAT8_TBL f;
ERROR: parser: parse error at or near "from"

This is causing the float8 regress test to fail.

I suspect this has to do with Bruce's recent hacking on operator
associativity.

regards, tom lane

#2Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#1)
Re: [HACKERS] Unary % operator is broken in current sources

These used to work:

regression=> select %f.f1 FROM FLOAT8_TBL f;
ERROR: parser: parse error at or near "%"
regression=> select f.f1 % FROM FLOAT8_TBL f;
ERROR: parser: parse error at or near "from"

This is causing the float8 regress test to fail.

I suspect this has to do with Bruce's recent hacking on operator
associativity.

I see. I see the same problem with / and +:

test=> select %f.f1 FROM FLOAT8_TBL f;
ERROR: parser: parse error at or near "%"
test=> select /f.f1 FROM FLOAT8_TBL f;
ERROR: parser: parse error at or near "/"
test=> select +f.f1 FROM FLOAT8_TBL f;
ERROR: parser: parse error at or near "+"

\do % shows:

test=> \do %
op|left_arg|right_arg|result |description
--+--------+---------+-------+-------------------
% | |float8 |float8 |truncate to integer
% |float8 | |float8 |truncate to integer
% |int2 |int2 |int2 |modulus
% |int2 |int4 |int4 |modulus
% |int4 |int2 |int4 |modulus
% |int4 |int4 |int4 |modulus
% |numeric |numeric |numeric|modulus
(7 rows)

OK, I made the change. It works now with special entries for %4 and 4%
in the grammer, similar to our handling of -4:

regression=> select %f.f1 FROM FLOAT8_TBL f;
?column?
---------------------
0
-34
-1004
-1.2345678901234e+200
0
(5 rows)

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#2)
Re: [HACKERS] Unary % operator is broken in current sources

Bruce Momjian <maillist@candle.pha.pa.us> writes:

OK, I made the change. It works now with special entries for %4 and 4%
in the grammer, similar to our handling of -4:

Hmm, is that an adequate solution? Are you sure there are no other
operators like % ?

regards, tom lane

#4Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#3)
Re: [HACKERS] Unary % operator is broken in current sources

Bruce Momjian <maillist@candle.pha.pa.us> writes:

OK, I made the change. It works now with special entries for %4 and 4%
in the grammer, similar to our handling of -4:

Hmm, is that an adequate solution? Are you sure there are no other
operators like % ?

Not sure. I know I only changed % to have precedence like /. No one is
complaining, and I think the problems are restricted to +,-,*,/, and %.
Should I fix any of these other ones?

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#4)
Re: [HACKERS] Unary % operator is broken in current sources

Bruce Momjian <maillist@candle.pha.pa.us> writes:

Not sure. I know I only changed % to have precedence like /. No one is
complaining, and I think the problems are restricted to +,-,*,/, and %.
Should I fix any of these other ones?

Right now I think % is the only problem, since it's the only operator
that has all three syntaxes (infix, prefix, postfix):

regression=> select distinct p1.oprname, p1.oprkind, p2.oprkind from
regression-> pg_operator as p1, pg_operator as p2
regression-> where p1.oprname = p2.oprname and p1.oprkind < p2.oprkind;
oprname|oprkind|oprkind
-------+-------+-------
# |b |l
% |b |l
% |b |r
% |l |r
- |b |l
?- |b |l
?| |b |l
@ |b |l
(8 rows)

Having both infix and prefix syntaxes doesn't seem to confuse the
parser --- at least, we have regress tests of both prefix @ and
infix @ (likewise #) and they're not complaining. Probably you need
a postfix syntax plus one or both of the other syntaxes to yield an
ambiguity that will confuse the parser. I haven't tried to track it
down in the grammar, however.

My concern with hacking in a special case for '%' in the grammar
is that we'll need to do it again anytime someone adds an operator
with the right set of syntaxes. It'd be better to understand *why*
the parser is having a hard time with this all of a sudden, and fix it
without reference to any particular operator. Postgres is supposed to
be extensible after all...

regards, tom lane

#6Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#5)
Re: [HACKERS] Unary % operator is broken in current sources

Bruce Momjian <maillist@candle.pha.pa.us> writes:

Not sure. I know I only changed % to have precedence like /. No one is
complaining, and I think the problems are restricted to +,-,*,/, and %.
Should I fix any of these other ones?

Right now I think % is the only problem, since it's the only operator
that has all three syntaxes (infix, prefix, postfix):

regression=> select distinct p1.oprname, p1.oprkind, p2.oprkind from
regression-> pg_operator as p1, pg_operator as p2
regression-> where p1.oprname = p2.oprname and p1.oprkind < p2.oprkind;
oprname|oprkind|oprkind
-------+-------+-------
# |b |l
% |b |l
% |b |r
% |l |r
- |b |l
?- |b |l
?| |b |l
@ |b |l
(8 rows)

Having both infix and prefix syntaxes doesn't seem to confuse the
parser --- at least, we have regress tests of both prefix @ and
infix @ (likewise #) and they're not complaining. Probably you need
a postfix syntax plus one or both of the other syntaxes to yield an
ambiguity that will confuse the parser. I haven't tried to track it
down in the grammar, however.

My concern with hacking in a special case for '%' in the grammar
is that we'll need to do it again anytime someone adds an operator
with the right set of syntaxes. It'd be better to understand *why*
the parser is having a hard time with this all of a sudden, and fix it
without reference to any particular operator. Postgres is supposed to
be extensible after all...

I can tell you what I think. +,-,*,/,% have special precedence so */ is
done before +-. This is causing infix/prefix to break. When % did not
behave with precidence like /, it worked fine.

So, I would only have to add cases for +,-,/,*. We already have "-"
prefix done for negative numbers.

Comments on how to proceed?

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#6)
Re: [HACKERS] Unary % operator is broken in current sources

Bruce Momjian <maillist@candle.pha.pa.us> writes:

So, I would only have to add cases for +,-,/,*. We already have "-"
prefix done for negative numbers.

Comments on how to proceed?

Tom Lockhart probably knows this stuff better than anyone else.
I vote we put the issue on "hold" until he's caught up with his
email ;-)

regards, tom lane