Justin Pryzby <pry...@telsasoft.com> writes:
> 1. Maybe pg_restore ExecuteSqlCommandBuf() should (always?) call
> ExecuteSimpleCommands() instead of ExecuteSqlCommand(). It doesn't seem to
> break anything (although that surprised me).
That certainly does break everything, which I imagine is the reason
why the cfbot shows that this patch is failing the pg_upgrade tests.
Note the comments for ExecuteSimpleCommands:
* We have to lex the data to the extent of identifying literals and quoted
* identifiers, so that we can recognize statement-terminating semicolons.
* We assume that INSERT data will not contain SQL comments, E'' literals,
* or dollar-quoted strings, so this is much simpler than a full SQL lexer.
IOW, where that says "Simple", it means *simple* --- in practice,
we only risk using it on commands that we know pg_dump itself built
earlier. There is no reasonable prospect of getting pg_restore to
split arbitrary SQL at command boundaries. We'd need something
comparable to psql's lexer, which is huge, and from a future-proofing
standpoint it would be just awful. (The worst that happens if psql
misparses your string is that it won't send the command when you
expected. If pg_restore misparses stuff, your best case is that the
restore fails cleanly; the worst case could easily result in
SQL-injection compromises.) So I think we cannot follow this
approach.
What we'd need to do if we want this to work with direct-to-DB restore
is to split off the ATTACH PARTITION command as a separate TOC entry.
That doesn't seem amazingly difficult, and it would even offer the
possibility that you could extract the partition standalone without
having to ignore errors. (You'd use -l/-L to select the CREATE TABLE,
the data, etc, but not the ATTACH object.)
That would possibly come out as a larger patch than you have here,
but maybe not by much. I don't think there's too much more involved
than setting up the proper command strings and calling ArchiveEntry().
You'd need to do some testing to verify that cases like --clean
work sanely.
Also, I read the 0002 patch briefly and couldn't make heads or tails
of it, except that it seemed to be abusing the PQExpBuffer abstraction
well beyond anything I'd consider acceptable. If you want separate
strings, make a PQExpBuffer for each one.
regards, tom lane
