pg_restore recognizing $-quotes

This is quite a large patch, but we do need a solution to this problem.
Should it be applied?

---------------------------------------------------------------------------

Philip Warner wrote:

Not sure if this is the desired approach, but since it works, I thought I'd
send it.

This patch allows pg_restore to recognize $-quotes in SQL queries. It will
treat any unquoted string that starts with a $ and has no preceding
identifier chars as a potential $-quote tag, it then makes sure that the
tag chars are valid. If so, it processes the $-quote.

Tested against local DBs and regression DB.

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

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?

http://archives.postgresql.org

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Bruce Momjian said:

This is quite a large patch, but we do need a solution to this problem.
Should it be applied?

I looked at it briefly, but felt sufficiently daunted that I gave up until I
have time to review it (my time is extremely limited right now).

ISTM that a very simple alternative would be to force pg_dump to inhibit
dollar quoting for non-text dumps. Then we could revisit the issue later on,
and maybe bite the bullet and use flex in pg_restore.

cheers

andrew

---------------------------------------------------------------------------

At 01:44 PM 18/08/2004, Bruce Momjian wrote:

This is quite a large patch, but we do need a solution to this problem.
Should it be applied?

It's not as large as you might think; I had to indent a large chunk of
code, and that shows up in the diff. Try applying it, and looking at a
'diff -b'.

There are two issues that I can see:

- I think the logic used to check for $-quotes is valid, but someone
else should look at it.

- Adding a full parser would be too big a change for now; and should
be avoided if possible.

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

"Andrew Dunstan" <andrew@dunslane.net> writes:

ISTM that a very simple alternative would be to force pg_dump to inhibit
dollar quoting for non-text dumps.

I don't want to do that, but I did think that a simpler alternative
would be to inhibit pg_restore from attempting to parse FUNCTION
entries. I can't see any strong need for it to do so.

(I haven't had time to review Philip's patch either... but I think Bruce
was asking whether to put it on the "awaiting review" list... definitely
yes.)

regards, tom lane

At 12:47 AM 19/08/2004, Tom Lane wrote:

I don't want to do that, but I did think that a simpler alternative
would be to inhibit pg_restore from attempting to parse FUNCTION
entries. I can't see any strong need for it to do so.

I don't like hard-coding stuff based on the TOC tags; but we *might* be
able to get away with a more general rule: do not parse if it's an object
definition (as opposed to data).

In the longer term I think we will need to continue to parse TOC entries.
In playing around with pg_dump(all), I put user definitions in one TOC
entry. For those, we will need to add as many users as possible and ignore
individual failures (something we can't to if a single multi-statement
string is sent to the backend). Other TOC entries may need to be atomic.
Not sure.

If the patch is not kosher, then I'd vote for adding a "do not parse" flag
on the TOC entries when dumping them. Or a statement count.

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

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

If the patch is not kosher, then I'd vote for adding a "do not parse" flag
on the TOC entries when dumping them. Or a statement count.

Unless you plan to abandon compatibility with existing dump files,
this doesn't seem like much of a solution ... pg_restore will still
have to include code to figure it out for itself ...

regards, tom lane

Per Tom ...

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.

---------------------------------------------------------------------------

Philip Warner wrote:

Not sure if this is the desired approach, but since it works, I thought I'd
send it.

This patch allows pg_restore to recognize $-quotes in SQL queries. It will
treat any unquoted string that starts with a $ and has no preceding
identifier chars as a potential $-quote tag, it then makes sure that the
tag chars are valid. If so, it processes the $-quote.

Tested against local DBs and regression DB.

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

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?

http://archives.postgresql.org

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Tom Lane said:

"Andrew Dunstan" <andrew@dunslane.net> writes:

ISTM that a very simple alternative would be to force pg_dump to
inhibit dollar quoting for non-text dumps.

I don't want to do that, but I did think that a simpler alternative
would be to inhibit pg_restore from attempting to parse FUNCTION
entries. I can't see any strong need for it to do so.

(I haven't had time to review Philip's patch either... but I think
Bruce was asking whether to put it on the "awaiting review" list...
definitely yes.)

OK. Not sure I understand why, though - the whole point of putting dollar
quoting into pg_dump was to make text dumps nicer to read, I thought (and I
outght to know ;-) ). Once the function is created you have no idea at all
if the body was originally dollar quoted or not.

cheers

andrew

"Andrew Dunstan" <andrew@dunslane.net> writes:

ISTM that a very simple alternative would be to force pg_dump to
inhibit dollar quoting for non-text dumps.
...
OK. Not sure I understand why, though - the whole point of putting dollar
quoting into pg_dump was to make text dumps nicer to read, I thought (and I
outght to know ;-) ).

But you can get a text dump out of pg_restore.

Also, for testing purposes I think it's important that
"pg_dump -Fc | pg_restore" generate exactly the same script as pg_dump.
This is true currently (unless someone's broken it again recently)
and I don't want to give up the property.

regards, tom lane

Tom Lane wrote:

Also, for testing purposes I think it's important that
"pg_dump -Fc | pg_restore" generate exactly the same script as pg_dump.
This is true currently (unless someone's broken it again recently)
and I don't want to give up the property.

Ok, that part I certainly buy.

cheers

andrew

At 01:33 AM 19/08/2004, Tom Lane wrote:

"pg_dump -Fc | pg_restore" generate exactly the same script as pg_dump.

Hard to believe this was not always true. Not sure I like the requirement
that pg_restore to a database behave just like 'pg_restore | psql', though 8-}.

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

At 01:22 AM 19/08/2004, Tom Lane wrote:

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

If the patch is not kosher, then I'd vote for adding a "do not parse" flag
on the TOC entries when dumping them. Or a statement count.

Unless you plan to abandon compatibility with existing dump files,
this doesn't seem like much of a solution

Not really; pg_restore is able to read all old formats; in the case of old
dump files it would behave as though statement count > 1. In the case of
new dump files, it would use the statement count. It's only new dump files
that exhibit the problem, so this will work.

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

Patch applied. Thanks.

---------------------------------------------------------------------------

Philip Warner wrote:

Not sure if this is the desired approach, but since it works, I thought I'd
send it.

This patch allows pg_restore to recognize $-quotes in SQL queries. It will
treat any unquoted string that starts with a $ and has no preceding
identifier chars as a potential $-quote tag, it then makes sure that the
tag chars are valid. If so, it processes the $-quote.

Tested against local DBs and regression DB.

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

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?

http://archives.postgresql.org

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073