current is broken

Started by Tatsuo Ishiiover 25 years ago19 messages
#1Tatsuo Ishii
t-ishii@sra.co.jp

It seems current source is broken if MB is enabled.

gcc -c -I../../../src/interfaces/libpq -I../../../src/include -I../../../src/interfaces/libpq -O2 -Wall -Wmissing-prototypes -Wmissing-declarations pg_dump.c -o pg_dump.o
pg_dump.c: In function `isViewRule':
pg_dump.c:267: parse error before `int'
pg_dump.c:268: `len' undeclared (first use in this function)
pg_dump.c:268: (Each undeclared identifier is reported only once
pg_dump.c:268: for each function it appears in.)
pg_dump.c:268: warning: implicit declaration of function `pg_mbcliplen'
make[3]: *** [pg_dump.o] Error 1

The problem is:

#ifdef MULTIBYTE
int len;
len = pg_mbcliplen(rulename,strlen(rulename),NAMEDATALEN-1);
rulename[len] = '\0';
#else
:
:

Moreover, pg_mbcliplen cannot be used in frontend. It seems what we
need here is new backendside functiontion what does same as
pg_mbcliplen. Will look into this...
--
Tatsuo Ishii

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tatsuo Ishii (#1)
Re: current is broken

Tatsuo Ishii <t-ishii@sra.co.jp> writes:

It seems current source is broken if MB is enabled.
gcc -c -I../../../src/interfaces/libpq -I../../../src/include -I../../../src/interfaces/libpq -O2 -Wall -Wmissing-prototypes -Wmissing-declarations pg_dump.c -o pg_dump.o
pg_dump.c: In function `isViewRule':
pg_dump.c:267: parse error before `int'

I just fixed one of these in the backend --- looks like someone was
testing with a C++ compiler instead of an ANSI-C-compliant compiler.
Need to put the 'int len;' declaration at the top of the function.

pg_dump.c:268: warning: implicit declaration of function `pg_mbcliplen'

Moreover, pg_mbcliplen cannot be used in frontend.

Ooops. I guess libpq needs to supply a copy of this function?

regards, tom lane

#3Tatsuo Ishii
t-ishii@sra.co.jp
In reply to: Tom Lane (#2)
Re: current is broken

I just fixed one of these in the backend --- looks like someone was
testing with a C++ compiler instead of an ANSI-C-compliant compiler.
Need to put the 'int len;' declaration at the top of the function.

Ok.

pg_dump.c:268: warning: implicit declaration of function `pg_mbcliplen'

Moreover, pg_mbcliplen cannot be used in frontend.

Ooops. I guess libpq needs to supply a copy of this function?

Simply copying the function won't work since the way to know what
encoding is used for this session is different between backend and
frontend.

Even better idea would be creating a new function that returns the
actual rule name (after being shorten) from given view name. I don't
think it's a good idea to have codes to get an actual rule name in two
separate places.
--
Tatsuo Ishii

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tatsuo Ishii (#3)
Re: current is broken

Tatsuo Ishii <t-ishii@sra.co.jp> writes:

Ooops. I guess libpq needs to supply a copy of this function?

Simply copying the function won't work since the way to know what
encoding is used for this session is different between backend and
frontend.

Good point --- in fact, the encoding itself might be different between
the backend and frontend. That seems to imply that "truncate to
NAMEDATALEN bytes" could yield different results in the frontend than
what the backend would get.

Even better idea would be creating a new function that returns the
actual rule name (after being shorten) from given view name. I don't
think it's a good idea to have codes to get an actual rule name in two
separate places.

Given the above point about encoding differences, I think we *must*
do the truncation in the backend ...

regards, tom lane

#5Hiroshi Inoue
Inoue@tpf.co.jp
In reply to: Tom Lane (#4)
RE: current is broken

-----Original Message-----
From: Tom Lane

Tatsuo Ishii <t-ishii@sra.co.jp> writes:

Ooops. I guess libpq needs to supply a copy of this function?

Simply copying the function won't work since the way to know what
encoding is used for this session is different between backend and
frontend.

Good point --- in fact, the encoding itself might be different between
the backend and frontend. That seems to imply that "truncate to
NAMEDATALEN bytes" could yield different results in the frontend than
what the backend would get.

Even better idea would be creating a new function that returns the
actual rule name (after being shorten) from given view name. I don't
think it's a good idea to have codes to get an actual rule name in two
separate places.

Given the above point about encoding differences, I think we *must*
do the truncation in the backend ...

I agree with Tatsuo.
However we already have relkind for views.
Why must we rely on rulename to implement isViewRule()
in the first place ?

Regards.

Hiroshi Inoue

#6Philip Warner
pjw@rhyme.com.au
In reply to: Tatsuo Ishii (#1)
Re: current is broken

At 13:07 13/09/00 +0900, Tatsuo Ishii wrote:

It seems current source is broken if MB is enabled.

gcc -c -I../../../src/interfaces/libpq -I../../../src/include

-I../../../src/interfaces/libpq -O2 -Wall -Wmissing-prototypes
-Wmissing-declarations pg_dump.c -o pg_dump.o

pg_dump.c: In function `isViewRule':
pg_dump.c:267: parse error before `int'
pg_dump.c:268: `len' undeclared (first use in this function)
pg_dump.c:268: (Each undeclared identifier is reported only once
pg_dump.c:268: for each function it appears in.)
pg_dump.c:268: warning: implicit declaration of function `pg_mbcliplen'
make[3]: *** [pg_dump.o] Error 1

I haven't looked at the code yet, but isViewRule is going to change to use
the new reltype for views. Maybe this will side-step the problem?

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

#7Tatsuo Ishii
t-ishii@sra.co.jp
In reply to: Hiroshi Inoue (#5)
RE: current is broken

Good point --- in fact, the encoding itself might be different between
the backend and frontend. That seems to imply that "truncate to
NAMEDATALEN bytes" could yield different results in the frontend than
what the backend would get.

Even better idea would be creating a new function that returns the
actual rule name (after being shorten) from given view name. I don't
think it's a good idea to have codes to get an actual rule name in two
separate places.

Given the above point about encoding differences, I think we *must*
do the truncation in the backend ...

I agree with Tatsuo.
However we already have relkind for views.
Why must we rely on rulename to implement isViewRule()
in the first place ?

Oh, I forgot about that.

BTW, does anybody know about the status of pg_dump? It seems tons of
features have been added, but I wonder if all of them are going to
appear in 7.1. Especially now it seems to have an ability to dump
Blobs, but the flag (-b) to enable the feature has been disabled. Why?
--
Tatsuo Ishii

#8Tatsuo Ishii
t-ishii@sra.co.jp
In reply to: Tatsuo Ishii (#7)
RE: current is broken

BTW, does anybody know about the status of pg_dump? It seems tons of
features have been added, but I wonder if all of them are going to
appear in 7.1. Especially now it seems to have an ability to dump
Blobs, but the flag (-b) to enable the feature has been disabled. Why?

Is it? AFAIK, it should not have been disabled. The long params version is
--blob - does that work?

Ok, long params works. It seems just adding 'b' to the third argument
of getopt() solves "-b" option problem. Do you wnat to fix by yourself?

Personally, I'd like to see them in 7.1, unless the testing period
reveals a swag of major flaws... Once CVS is up again, I intend to
do a little more work on pg_dump, so now would be a good time to let
me know if there are problems.

New pg_dump looks great. Could you add docs for it so that we can test
it out?
--
Tatsuo Ishii

#9Philip Warner
pjw@rhyme.com.au
In reply to: Tatsuo Ishii (#7)
RE: current is broken

At 19:41 13/09/00 +0900, Tatsuo Ishii wrote:

BTW, does anybody know about the status of pg_dump? It seems tons of
features have been added, but I wonder if all of them are going to
appear in 7.1. Especially now it seems to have an ability to dump
Blobs, but the flag (-b) to enable the feature has been disabled. Why?

Is it? AFAIK, it should not have been disabled. The long params version is
--blob - does that work?

Personally, I'd like to see them in 7.1, unless the testing period reveals
a swag of major flaws...

Once CVS is up again, I intend to do a little more work on pg_dump, so now
would be a good time to let me know if there are problems.

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

#10Thomas Lockhart
lockhart@alumni.caltech.edu
In reply to: Philip Warner (#9)
Re: current is broken

New pg_dump looks great. Could you add docs for it so that we can test
it out?

It's on the list - I'm still waiting for an upgrade to Framemaker, since I
can't stomach raw SGML. If it doesn't arrive soon, I'll just have to do it
the hard way.

If necessary, you can type straight into the existing docs without doing
much about markup, and I'll fix up the sgml tags later.

afaik I've never seen sgml from FM; it will be interesting to see how it
turns out.

- Thomas

#11Philip Warner
pjw@rhyme.com.au
In reply to: Tatsuo Ishii (#8)
RE: current is broken

At 21:35 13/09/00 +0900, Tatsuo Ishii wrote:

BTW, does anybody know about the status of pg_dump? It seems tons of
features have been added, but I wonder if all of them are going to
appear in 7.1. Especially now it seems to have an ability to dump
Blobs, but the flag (-b) to enable the feature has been disabled. Why?

Is it? AFAIK, it should not have been disabled. The long params version is
--blob - does that work?

Ok, long params works. It seems just adding 'b' to the third argument
of getopt() solves "-b" option problem. Do you wnat to fix by yourself?

May as well, since I'll be doing some other stuff.

Personally, I'd like to see them in 7.1, unless the testing period
reveals a swag of major flaws... Once CVS is up again, I intend to
do a little more work on pg_dump, so now would be a good time to let
me know if there are problems.

New pg_dump looks great. Could you add docs for it so that we can test
it out?

It's on the list - I'm still waiting for an upgrade to Framemaker, since I
can't stomach raw SGML. If it doesn't arrive soon, I'll just have to do it
the hard way.

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

#12Ross J. Reedstrom
reedstrm@rice.edu
In reply to: Philip Warner (#6)
Re: current is broken

On Wed, Sep 13, 2000 at 03:44:25PM +1000, Philip Warner wrote:

At 13:07 13/09/00 +0900, Tatsuo Ishii wrote:

It seems current source is broken if MB is enabled.

Gah, I Was afraid of this. My patch, I'm afraid.

I haven't looked at the code yet, but isViewRule is going to change to use
the new reltype for views. Maybe this will side-step the problem?

Yes, it should. However, I've just done a quick test in a non-MB compile, and
it looks like char(n) = 'a string constant' returns true if the first n chars
match. If this is correct behavior, and holds in the multibyte case, then
you can strip out _all_ the rulename truncation from pg_dump.
Here's the example:

I've got a view named: " this_is_a_really_really_long_vi", with matching rule:
"_RETthis_is_a_really_really_lon"

Without truncation, pgdump generates (with addition of relname for
readability) this query (hand wrapped)

reedstrm=# select relname,rulename from pg_class, pg_rewrite
where pg_class.oid = ev_class and pg_rewrite.ev_type = '1'
and rulename = '_RETthis_is_a_really_really_long_vi';

relname | rulename
---------------------------------+---------------------------------
this_is_a_really_really_long_vi | _RETthis_is_a_really_really_lon
(1 row)

reedstrm=#

Ross
--
Ross J. Reedstrom, Ph.D., <reedstrm@rice.edu>
NSBRI Research Scientist/Programmer
Computer and Information Technology Institute
Rice University, 6100 S. Main St., Houston, TX 77005

#13Hiroshi Inoue
Inoue@tpf.co.jp
In reply to: Ross J. Reedstrom (#12)
RE: current is broken

-----Original Message-----
From: Ross J. Reedstrom

On Wed, Sep 13, 2000 at 03:44:25PM +1000, Philip Warner wrote:

At 13:07 13/09/00 +0900, Tatsuo Ishii wrote:

It seems current source is broken if MB is enabled.

Gah, I Was afraid of this. My patch, I'm afraid.

I haven't looked at the code yet, but isViewRule is going to

change to use

the new reltype for views. Maybe this will side-step the problem?

Yes, it should. However, I've just done a quick test in a non-MB
compile, and
it looks like char(n) = 'a string constant' returns true if the
first n chars
match. If this is correct behavior, and holds in the multibyte case, then
you can strip out _all_ the rulename truncation from pg_dump.

Isn't it a problem of backend side ?
It seems quite strange to me that clients should/could assume
such a complicated rule. I was surprized to see how many
applications have used complicated(but incomplete in some
cases) definiton(criterion ?)s of views to see if a table is a
view.
Now we have a relkind for views and in addtion haven't we
already had pg_views view to encapsulate the definition of
views.

Regards.

Hiroshi Inoue

#14Tom Lane
tgl@sss.pgh.pa.us
In reply to: Ross J. Reedstrom (#12)
Re: current is broken

Philip Warner <pjw@rhyme.com.au> writes:

The only thing that's missing is a 'rulekind' for rules - it would be very
nice if pg_dump could use a simple method (that didn't involve munging
names) to determin is a rule is a 'view rule'.

Oh, I finally see the problem: when you come to dump out the rules, you
need to avoid dumping the rules that correspond to views because you're
going to emit the CREATE VIEW commands separately.

You don't really need a rulekind though. If it's an ON SELECT rule for
a relation that you've determined to be a view, then the rule is a
view rule. Otherwise, you print the rule.

regards, tom lane

#15Hiroshi Inoue
Inoue@tpf.co.jp
In reply to: Tom Lane (#14)
RE: current is broken

-----Original Message-----
From: Tom Lane [mailto:tgl@sss.pgh.pa.us]

Philip Warner <pjw@rhyme.com.au> writes:

The only thing that's missing is a 'rulekind' for rules - it

would be very

nice if pg_dump could use a simple method (that didn't involve munging
names) to determin is a rule is a 'view rule'.

Oh, I finally see the problem: when you come to dump out the rules, you
need to avoid dumping the rules that correspond to views because you're
going to emit the CREATE VIEW commands separately.

You don't really need a rulekind though. If it's an ON SELECT rule for
a relation that you've determined to be a view, then the rule is a
view rule. Otherwise, you print the rule.

Is it guaranteed that ON SELECT rule is unique per view in the future ?

Regards.

Hiroshi Inoue

#16Philip Warner
pjw@rhyme.com.au
In reply to: Hiroshi Inoue (#13)
RE: current is broken

At 10:40 15/09/00 +0900, Hiroshi Inoue wrote:

Now we have a relkind for views and in addtion haven't we
already had pg_views view to encapsulate the definition of
views.

The only thing that's missing is a 'rulekind' for rules - it would be very
nice if pg_dump could use a simple method (that didn't involve munging
names) to determin is a rule is a 'view rule'.

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

#17Philip Warner
pjw@rhyme.com.au
In reply to: Tom Lane (#14)
Re: current is broken

At 22:23 14/09/00 -0400, Tom Lane wrote:

Oh, I finally see the problem: when you come to dump out the rules, you
need to avoid dumping the rules that correspond to views because you're
going to emit the CREATE VIEW commands separately.

You don't really need a rulekind though. If it's an ON SELECT rule for
a relation that you've determined to be a view, then the rule is a
view rule. Otherwise, you print the rule.

It looks like some nice person has defined the 'pg_rules' view which does
not return rules used in views. I'll try using this since it removes
another layer of backend knowledge from pg_dump.

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

#18Philip Warner
pjw@rhyme.com.au
In reply to: Philip Warner (#17)
New pg_dump committed...

I've now committed the latest pg_dump. The following is the list of changes:

- Support for relkind = RELKIND_VIEW.
- Use symbols for tests on relkind (ie. use RELKIND_VIEW, not 'v')
- Fix bug in support for -b option (== --blobs).
- Dump views as views (using 'create view').
- Remove 'isViewRule' since we check the relkind when getting tables.
- Now uses temp table 'pgdump_oid' rather than 'pg_dump_oid' (errors
otherwise).
- Added extra param for specifying handling of OID=0 and which typename to
output.
- Fixed bug in SQL scanner when SQL contained braces. (in rules)
- Use format_type function wherever possible

It works on all my DBs, and on the regression DB, so I am at least mildly
optimistic. The main issue that I think might arise are from the use of
format_type in function definitions. It's easy to change if we have to go
back to using typname.

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/

#19Philip Warner
pjw@rhyme.com.au
In reply to: Philip Warner (#17)
Re: current is broken

At 04:41 15/09/00 -0500, Jan Wieck wrote:

So if you find an ON SELECT rule on a relation, it is a VIEW.

Thanks for this, but I'm using 'pg_views' now since it means pg_dump does
not have to interpret the meanings of the various columns. With time, I
would like to remove as much internal knowledge as I can from pg_dump.

----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/