psql: add \si, \sm, \st and \sr functions to show CREATE commands for indexes, matviews, triggers and tables
Hi,
I've attached a patch that implements \si, \sm, \st and \sr functions
that show the CREATE command for indexes, matviews, triggers and tables.
The functions are implemented similarly to the existing sf/sv functions
with some modifications.
For triggers, I've decided to change input format to "table_name TRIGGER
trigger_name", as multiple tables are allowed to have a trigger of the
same name. Because we need to verify not only the name of the trigger,
but also the name of the table, I've implemented a separate function
lookup_trigger_oid that takes an additional argument.
Triggers and indexes use pg_catalog.pg_get_triggerdef() and
pg_indexes.indexdef, while tables and matviews have separate queries for
reconstruction. Get_create_object_cmd also runs three additional queries
for tables, to get information on constraints, parents and columns.
There is also the question, if this functionality should be realised on
the server instead of the client, but some people may think that changes
to the language are "not the postgres way". However, server realisation
may have some advantages, such as independence from the client and
server version.
Best regards,
Alexandra Pervushina.
Attachments:
si_st_sm_sr.patchtext/x-diff; name=si_st_sm_sr.patchDownload+1103-7
On 2020-07-28 20:46, a.pervushina@postgrespro.ru wrote:
I've attached a patch that implements \si, \sm, \st and \sr functions
that show the CREATE command for indexes, matviews, triggers and
tables. The functions are implemented similarly to the existing sf/sv
functions with some modifications.
To me these functions seem useful.
As for adding them to server side, I don't see a big need for it. It
feels more logical to follow the already eatablished pattern for the
\s[...] commands.
About the patch:
1) There is some code duplication for the exec_command_[sm|si|st|sr]
functions. Plus, it seems weird to separate sm (show matview) from sv
(show view). Perhaps it would be more convenient to combine some of the
code? Maybe by editing the already-existing exec_command_sf_sv()
function.
2) Seeing how \s and \e functions were added together, I'm wondering -
should there be \e functions too for any objects affected by this patch?
--
Anna Akenteva
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
Anna Akenteva wrote 2020-08-11 13:37:
About the patch:
1) There is some code duplication for the exec_command_[sm|si|st|sr]
functions. Plus, it seems weird to separate sm (show matview) from sv
(show view). Perhaps it would be more convenient to combine some of
the code? Maybe by editing the already-existing exec_command_sf_sv()
function.
I've combined most of the functions into one, as the code was mostly
duplicated. Had to change the argument from is_func to object type,
because the number of values has increased. I've attached a patch with
those changes.
Attachments:
si_st_sm_sr_v2.patchtext/x-diff; name=si_st_sm_sr_v2.patchDownload+894-30
a.pervushina@postgrespro.ru writes:
[ si_st_sm_sr_v2.patch ]
I hadn't particularly noticed this thread before, but I happened to
look through this patch, and I've got to say that this proposed feature
seems like an absolute disaster from a maintenance standpoint. There
will be no value in an \st command that is only 90% accurate; the produced
DDL has to be 100% correct. This means that, if we accept this feature,
psql will have to know everything pg_dump knows about how to construct the
DDL describing tables, indexes, views, etc. That is a lot of code, and
it's messy, and it changes nontrivially on a very regular basis. I can't
accept that we want another copy in psql --- especially one that looks
nothing like what pg_dump has.
There've been repeated discussions about somehow extracting pg_dump's
knowledge into a library that would also be available to other client
programs (see e.g. the concurrent thread at [1]/messages/by-id/9df8a3d3-13d2-116d-26ab-6a273c1ed38c@2ndquadrant.com). That's quite a tall
order, which is why it's not happened yet. But I think we really need
to have something like that before we can accept this feature for psql.
BTW, as an example of why this is far more difficult than it might
seem at first glance, this patch doesn't even begin to meet the
expectation stated at the top of describe.c:
* Support for the various \d ("describe") commands. Note that the current
* expectation is that all functions in this file will succeed when working
* with servers of versions 7.4 and up. It's okay to omit irrelevant
* information for an old server, but not to fail outright.
It might be okay for this to cut off at 8.0 or so, as I think pg_dump
does, but not to just fail on older servers.
Another angle, which I'm not even sure how we want to think about it, is
security. It will not do for "\et" to allow some attacker to replace
function calls appearing in the table's CHECK constraints, for instance.
So this means you've got to be very aware of CVE-2018-1058-style attacks.
Our answer to that for pg_dump has partially depended on restricting the
search_path used at both dump and restore time ... but I don't think \et
gets to override the search path that the psql user is using. I'm not
sure what that means in practice but it certainly requires some thought
before we add the feature, not after.
Anyway, I can see the attraction of having psql commands like these,
but "write a bunch of new code that we'll have to maintain" does not
seem like a desirable way to get them.
regards, tom lane
[1]: /messages/by-id/9df8a3d3-13d2-116d-26ab-6a273c1ed38c@2ndquadrant.com
On 18.08.2020 17:25, Tom Lane wrote:
a.pervushina@postgrespro.ru writes:
[ si_st_sm_sr_v2.patch ]
I hadn't particularly noticed this thread before, but I happened to
look through this patch, and I've got to say that this proposed feature
seems like an absolute disaster from a maintenance standpoint. There
will be no value in an \st command that is only 90% accurate; the produced
DDL has to be 100% correct. This means that, if we accept this feature,
psql will have to know everything pg_dump knows about how to construct the
DDL describing tables, indexes, views, etc. That is a lot of code, and
it's messy, and it changes nontrivially on a very regular basis. I can't
accept that we want another copy in psql --- especially one that looks
nothing like what pg_dump has.There've been repeated discussions about somehow extracting pg_dump's
knowledge into a library that would also be available to other client
programs (see e.g. the concurrent thread at [1]). That's quite a tall
order, which is why it's not happened yet. But I think we really need
to have something like that before we can accept this feature for psql.BTW, as an example of why this is far more difficult than it might
seem at first glance, this patch doesn't even begin to meet the
expectation stated at the top of describe.c:* Support for the various \d ("describe") commands. Note that the current
* expectation is that all functions in this file will succeed when working
* with servers of versions 7.4 and up. It's okay to omit irrelevant
* information for an old server, but not to fail outright.It might be okay for this to cut off at 8.0 or so, as I think pg_dump
does, but not to just fail on older servers.Another angle, which I'm not even sure how we want to think about it, is
security. It will not do for "\et" to allow some attacker to replace
function calls appearing in the table's CHECK constraints, for instance.
So this means you've got to be very aware of CVE-2018-1058-style attacks.
Our answer to that for pg_dump has partially depended on restricting the
search_path used at both dump and restore time ... but I don't think \et
gets to override the search path that the psql user is using. I'm not
sure what that means in practice but it certainly requires some thought
before we add the feature, not after.Anyway, I can see the attraction of having psql commands like these,
but "write a bunch of new code that we'll have to maintain" does not
seem like a desirable way to get them.regards, tom lane
[1] /messages/by-id/9df8a3d3-13d2-116d-26ab-6a273c1ed38c@2ndquadrant.com
Since there has been no activity on this thread since before the CF and
no response from the author I have marked this "returned with feedback".
Alexandra, feel free to resubmit it to the next commitfest, when you
have time to address the issues raised in the review.
--
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company