Adding SHOW CREATE TABLE

Greetings,

* Nathaniel Sabanski (sabanski.n@gmail.com) wrote:

HN had a thread regarding the challenges faced by new users during the
adoption of Postgres in 2023.

One particular issue that garnered significant votes was the lack of a
"SHOW CREATE TABLE" command, and seems like it would be an easy one to
implement: https://news.ycombinator.com/item?id=35908991

Considering the popularity of this request and its potential ease of
implementation, I wanted to bring it to your attention, as it would likely
enhance the user experience and alleviate some of the difficulties
encountered by newcomers.

This isn't as easy as it seems actually ...

Note that using pg_dump for this purpose works quite well and also works
to address cross-version issues. Consider that pg_dump v15 is able to
connect to v14, v13, v12, v11, and more, and produce a CREATE TABLE
command that will work with *v15*. If you connected to a v14 database
and did a SHOW CREATE TABLE, there's no guarantee that the CREATE TABLE
statement returned would work for PG v15 due to keyword changes and
other differences that can cause issues between major versions of PG.

Now, that said, we have started ending up with some similar code between
pg_dump and postgres_fdw in the form of IMPORT FOREIGN SCHEMA and maybe
we should consider if that code could be moved into the common library
and made available to pg_dump, postgres_fdw, and as a SHOW CREATE TABLE
command with the caveat that the produced CREATE TABLE command may not
work with newer versions of PG. There's an interesting question around
if we'd consider it a bug worthy of fixing if IMPORT FOREIGN SCHEMA in
v14 doesn't work when connecting to a v15 PG instance. Not sure if
anyone's contemplated that. There's certainly going to be cases that we
wouldn't accept fixing (we wouldn't add some new partitioning strategy
to v14 just because it's in v15, for example, to make IMPORT FOREIGN
SCHEMA work...).

Thanks,

Stephen

I believe most users would anticipate a CREATE TABLE statement that aligns
with the currently installed version- this is the practical solution for
the vast majority.

In situations where a CREATE TABLE statement compatible with an older
version of Postgres is required, users can opt for an additional step of
using tools like pg_dump or an older version of Postgres itself. This
allows them to ensure compatibility without compromising the practicality
of the process.

On Fri, 12 May 2023 at 06:47, Stephen Frost <sfrost@snowman.net> wrote:

Greetings,

Please don't top-post on these lists.

* Nathaniel Sabanski (sabanski.n@gmail.com) wrote:

I believe most users would anticipate a CREATE TABLE statement that aligns
with the currently installed version- this is the practical solution for
the vast majority.

Perhaps a bit more discussion about what exactly the use-case is would
be helpful- what would you use this feature for?

In situations where a CREATE TABLE statement compatible with an older
version of Postgres is required, users can opt for an additional step of
using tools like pg_dump or an older version of Postgres itself. This
allows them to ensure compatibility without compromising the practicality
of the process.

The issue is really both older and newer versions, not just older ones
and not just newer ones.

To the extent you're interested in this, I pointed out where you could
go look at the existing code as well as an idea for how to move this
forward.

Thanks,

Stephen

On Fri, 12 May 2023, Nathaniel Sabanski wrote:

I believe most users would anticipate a CREATE TABLE statement that aligns
with the currently installed version- this is the practical solution for

The currently installed version of what, the server or the client?

bye,
//mirabilos
--
15:41⎜<Lo-lan-do:#fusionforge> Somebody write a testsuite for helloworld :-)

Nathaniel Sabanski schrieb am 12.05.2023 um 13:29:

HN had a thread regarding the challenges faced by new users during
the adoption of Postgres in 2023.

One particular issue that garnered significant votes was the lack of
a "SHOW CREATE TABLE" command, and seems like it would be an easy one
to implement: https://news.ycombinator.com/item?id=35908991

Considering the popularity of this request and its potential ease of
implementation, I wanted to bring it to your attention, as it would
likely enhance the user experience and alleviate some of the
difficulties encountered by newcomers.

While it would be nice to have something like that, I don't think
it isn't really necessary. Pretty much every (GUI) SQL client provides
a way to see the DDL for objects in the database.

For psql fans \d typically is enough, and they would probably not mind
running pg_dump to get the full DDL.

I would think that especially newcomers start with a GUI client
that can do this.

If you check the source of any of the popular SQL clients that generates
the DDL for a table, then you will also quickly realize that this isn't
a trivial thing to do.

Thomas

On Fri, May 12, 2023, 08:35 Thorsten Glaser <tg@evolvis.org> wrote:

On Fri, 12 May 2023, Nathaniel Sabanski wrote:

I believe most users would anticipate a CREATE TABLE statement that aligns
with the currently installed version- this is the practical solution for

The currently installed version of what, the server or the client?

It's an SQL Command, no specific client can/should be presumed.

David J.

On Fri, 12 May 2023 at 09:12, David G. Johnston <david.g.johnston@gmail.com>
wrote:

Perhaps a bit more discussion about what exactly the use-case is would
be helpful- what would you use this feature for?

App writers: To facilitate table creation and simplify schema verification,
without relying on a GUI tool or ORM (or system calls out to pg_dump).

Tool writers: Would drastically cut down the implementation time and
complexity to support Postgres. I am one of the devs of Piccolo ORM (Python
lib supporting Postgres) and we have a lot of code dedicated to
re-generating the CREATE TABLE statements (creation, during migrations,
etc) that could be done better by Postgres itself.

Ecosystem cohesion: SHOW CREATE TABLE has already been implemented in
CockroachDB, a popular Postgres derivative.

Moving to Postgres: It would help ease migrations for developers wanting to
move from MySQL / Percona / MariaDB to Postgres. Also it's a nice developer
experience to see how Postgres generates X table without extra tooling.

The intention of SHOW CREATE TABLE is not to replace the existing suite of
\d in psql but rather to be a developer friendly complement within SQL
itself.

Greetings,

* Nathaniel Sabanski (sabanski.n@gmail.com) wrote:

Perhaps a bit more discussion about what exactly the use-case is would
be helpful- what would you use this feature for?

App writers: To facilitate table creation and simplify schema verification,
without relying on a GUI tool or ORM (or system calls out to pg_dump).

Not sure how it would simplify schema verification?

Tool writers: Would drastically cut down the implementation time and
complexity to support Postgres. I am one of the devs of Piccolo ORM (Python
lib supporting Postgres) and we have a lot of code dedicated to
re-generating the CREATE TABLE statements (creation, during migrations,
etc) that could be done better by Postgres itself.

I'm curious- have you compared what you're doing to pg_dump's output?
Are you confident that there aren't any distinctions between those that,
for whatever reason, need to exist?

Moving to Postgres: It would help ease migrations for developers wanting to
move from MySQL / Percona / MariaDB to Postgres. Also it's a nice developer
experience to see how Postgres generates X table without extra tooling.

Seems unlikely that this would actually be all that helpful there- tools
like ora2pg and similar know how to query other database systems and
write appropriate CREATE TABLE statements for PostgreSQL.

The intention of SHOW CREATE TABLE is not to replace the existing suite of
\d in psql but rather to be a developer friendly complement within SQL
itself.

Sure, I get that.

Again, would be great to see someone actually work on this. There's
already a good chunk of code in core in pg_dump and in the postgres_fdw
for doing exactly this and it'd be great to consolidate that and at the
same time expose it via SQL.

Another possible option would be to add this to libpq, which is used by
postgres_fdw, psql, pg_dump, and lots of other drivers and client
utilities. If it's all broadly the same, personally I'd prefer it to be
in the common library and available as a backend SQL command too, but
perhaps there's reasons that it would be easier to implement in libpq
instead.

Thanks,

Stephen

Stephen Frost <sfrost@snowman.net> writes:

Again, would be great to see someone actually work on this. There's
already a good chunk of code in core in pg_dump and in the postgres_fdw
for doing exactly this and it'd be great to consolidate that and at the
same time expose it via SQL.

Note that this is hardly new ground: we've heard more-or-less the same
proposal many times before. I think the reason it's gone nowhere is
that most of the existing infrastructure is either in pg_dump or designed
to support pg_dump, and pg_dump is *extremely* opinionated about what
it wants and how it wants the data sliced up, for very good reasons.
Reconciling those requirements with a typical user's "just give me a
reconstructed CREATE TABLE command" request seems fairly difficult.

Also, since pg_dump will still need to support old servers, it's hard
to believe we'd accept any proposal to move that functionality into
the server side, which in turn means that it's not going to be an easy
SQL command.

These issues probably could be surmounted with enough hard work, but
please understand that just coming along with a request is not going
to cause it to happen. People have already done that. (Searching
the mailing list archives might be edifying.)

regards, tom lane

Greetings,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

Stephen Frost <sfrost@snowman.net> writes:

Again, would be great to see someone actually work on this. There's
already a good chunk of code in core in pg_dump and in the postgres_fdw
for doing exactly this and it'd be great to consolidate that and at the
same time expose it via SQL.

Note that this is hardly new ground: we've heard more-or-less the same
proposal many times before. I think the reason it's gone nowhere is
that most of the existing infrastructure is either in pg_dump or designed
to support pg_dump, and pg_dump is *extremely* opinionated about what
it wants and how it wants the data sliced up, for very good reasons.
Reconciling those requirements with a typical user's "just give me a
reconstructed CREATE TABLE command" request seems fairly difficult.

Yet we're already duplicating much of this in postgres_fdw. If we don't
want to get involved in pg_dump's feelings on the subject, we could look
to postgres_fdw's independent implementation which might be more
in-line with what users are expecting. Having two separate copies of
code that does this and continuing to refuse to give users a way to ask
for it themselves seems at the least like an odd choice.

Also, since pg_dump will still need to support old servers, it's hard
to believe we'd accept any proposal to move that functionality into
the server side, which in turn means that it's not going to be an easy
SQL command.

No, it won't make sense to have yet another copy that's for the
currently-running-server-only, which is why I suggested it go into
either a common library or maybe into libpq. I don't feel it would
be bad for the common code to have the multi-version understanding even
if the currently running backend will only ever have the option to ask
for the code path that matches its version.

These issues probably could be surmounted with enough hard work, but
please understand that just coming along with a request is not going
to cause it to happen. People have already done that. (Searching
the mailing list archives might be edifying.)

Agreed- someone needs to have a fair bit of time and willingness to push
on this to make it happen.

Thanks,

Stephen

On Fri, May 12, 2023 at 4:37 PM Stephen Frost <sfrost@snowman.net> wrote:

Greetings,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

Stephen Frost <sfrost@snowman.net> writes:

Again, would be great to see someone actually work on this. There's
already a good chunk of code in core in pg_dump and in the postgres_fdw
for doing exactly this and it'd be great to consolidate that and at the
same time expose it via SQL.

...
No, it won't make sense to have yet another copy that's for the
currently-running-server-only, which is why I suggested it go into
either a common library or maybe into libpq. I don't feel it would
be bad for the common code to have the multi-version understanding even
if the currently running backend will only ever have the option to ask
for the code path that matches its version.

Hmmm... What's wrong with only being for the currently running server?

That's all I would expect. Also, if it was there, it limits the
expectations to DDL that
works for that server version.

Also, if it's on the backend (or an extension), then it's available to
everything.

Agreed- someone needs to have a fair bit of time and willingness to push
on this to make it happen.

If we can work through a CLEAR discussion of what it is, and is not. I
would be
happy to work on this. I like referencing the FDW. I also thought of
referencing
the CREATE TABLE xyz(LIKE abc INCLUDING ALL). While it's not doing DDL,
it certainly has to be checking options, etc. And pg_dump is the "gold
standard".

My approach would be to get a version working. Then figure out how to
generate "literally" all table options, and work the process. The good news
is that at a certain point the resulting DDL should be "comparable" against
a
ton of test tables.

Where do we draw the lines? Does Table DDL include all indexes?
It should include constraints, clearly. I would not think it should have
triggers.
Literally everything within the <<CREATE TABLE X(...);>>. (ie, no ALTER ..
OWNER TO...)

Next, I would want psql \st to simply call this?

FWIW, we parse our pg_dump output, and store the objects as individual DDL
files.
So, I have about 1,000 tables to play with, for which I already know the
DDL that pg_dump uses.

But it's a big commitment. I don't mind if it has a reasonable chance of
being accepted.
I accept that I will make a few mistakes (and learn) along the way.
If there are ANY deal killers that would prevent a reasonable solution from
being accepted,
please let me know.

Kirk...

Greetings,

* Kirk Wolak (wolakk@gmail.com) wrote:

On Fri, May 12, 2023 at 4:37 PM Stephen Frost <sfrost@snowman.net> wrote:

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

Stephen Frost <sfrost@snowman.net> writes:

Again, would be great to see someone actually work on this. There's
already a good chunk of code in core in pg_dump and in the postgres_fdw
for doing exactly this and it'd be great to consolidate that and at the
same time expose it via SQL.

...
No, it won't make sense to have yet another copy that's for the
currently-running-server-only, which is why I suggested it go into
either a common library or maybe into libpq. I don't feel it would
be bad for the common code to have the multi-version understanding even
if the currently running backend will only ever have the option to ask
for the code path that matches its version.

Hmmm... What's wrong with only being for the currently running server?
That's all I would expect. Also, if it was there, it limits the
expectations to DDL that
works for that server version.

I didn't say anything was wrong with that, merely pointing out that
having the same set of code for these various use-cases would be better
than having multiple copies of it. The existing code works just fine to
answer the question of "when on v15, what is the v15 query?", it just
happens to *also* answer "when on v15, what is the v14 query?" and we
need that already for postgres_fdw and for pg_dump.

Also, if it's on the backend (or an extension), then it's available to
everything.

I mean ... it's already in postgres_fdw, just not in a way that can be
returned to the user. I don't think I'd want this functionality to
depend on postgres_fdw or generally on an extension though, it should
be part of core in some fashion.

Agreed- someone needs to have a fair bit of time and willingness to push
on this to make it happen.

If we can work through a CLEAR discussion of what it is, and is not. I
would be
happy to work on this. I like referencing the FDW. I also thought of
referencing
the CREATE TABLE xyz(LIKE abc INCLUDING ALL). While it's not doing DDL,
it certainly has to be checking options, etc. And pg_dump is the "gold
standard".

I'd think the FDW code would be the best starting point, but, sure, look
at all the options.

My approach would be to get a version working. Then figure out how to
generate "literally" all table options, and work the process. The good news
is that at a certain point the resulting DDL should be "comparable" against
a ton of test tables.

Where do we draw the lines? Does Table DDL include all indexes?
It should include constraints, clearly. I would not think it should have
triggers.
Literally everything within the <<CREATE TABLE X(...);>>. (ie, no ALTER ..
OWNER TO...)

I'd look at the IMPORT FOREIGN SCHEMA stuff in postgres_fdw. We're
already largely answering these questions by what options that takes.
To some extent, the same is true of pg_dump, but at least postgres_fdw
is already backend code and probably a bit simpler than the pg_dump
code. Still, looking at both would be a good idea.

Next, I would want psql \st to simply call this?

Eh, that's an independent discussion and effort, especially because
people are possibly going to want that to generate the necessary ALTER
TABLE commands from the result and not just a DROP/CREATE TABLE.

FWIW, we parse our pg_dump output, and store the objects as individual DDL
files.
So, I have about 1,000 tables to play with, for which I already know the
DDL that pg_dump uses.

Sure.

But it's a big commitment. I don't mind if it has a reasonable chance of
being accepted.

Yes, it's a large effort, no doubt.

I accept that I will make a few mistakes (and learn) along the way.
If there are ANY deal killers that would prevent a reasonable solution from
being accepted, please let me know.

I don't think we can say one way or the other on this ...

Thanks,

Stephen

On 5/12/23 18:00, Kirk Wolak wrote:
[snip]

Where do we draw the lines?

At other tables.

Does Table DDL include all indexes?

Absolutely!

It should include constraints, clearly.  I would not think it should have
triggers.

Definitely triggers.  And foreign keys.

Literally everything within the <<CREATE TABLE X(...);>>.  (ie, no ALTER
.. OWNER TO...)

ALTER statements, too.  If CREATE TABLE ... LIKE ... { INCLUDING | EXCLUDING
} { COMMENTS | COMPRESSION | CONSTRAINTS | DEFAULTS | GENERATED | IDENTITY |
INDEXES | STATISTICS | STORAGE | ALL } can do it, then so should SHOW CREATE
TABLE.

--
Born in Arizona, moved to Babylonia.

On Sat, May 13, 2023 at 1:03 AM Ron <ronljohnsonjr@gmail.com> wrote:

On 5/12/23 18:00, Kirk Wolak wrote:

[snip]

Where do we draw the lines?

At other tables.

Does Table DDL include all indexes?

Absolutely!

It should include constraints, clearly. I would not think it should have
triggers.

Definitely triggers. And foreign keys.

Literally everything within the <<CREATE TABLE X(...);>>. (ie, no ALTER
.. OWNER TO...)

ALTER statements, too. If CREATE TABLE ... LIKE ... { INCLUDING |
EXCLUDING } { COMMENTS | COMPRESSION | CONSTRAINTS | DEFAULTS | GENERATED |
IDENTITY | INDEXES | STATISTICS | STORAGE | ALL } can do it, then so should
SHOW CREATE TABLE.

--
Born in Arizona, moved to Babylonia.

I can see the ALTER statements now. Which is why I asked.
I don't like the idea of including the trigger DDL, because that would
never execute in a clean environment.
(I've never used a tool that tried to do that when I've wanted the DDL)
I can go either way on index creation.

Does this imply SQL SYNTAX like:

SHOW CREATE TABLE <table_name>
[ INCLUDING { ALL | INDEXES | SEQUENCES | ??? }]
[EXCLUDING { PK | FK | COMMENTS | STORAGE | } ]
[FOR {V11 | V12 | V13 | V14 | V15 }] ??
?

The goal for me is to open the discussion, and then CONSTRAIN the focus.

Personally, the simple syntax:
SHOW CREATE TABLE table1;

Should give me a create table command with the table attributes and the
column attributes, FKs, PKs, Defaults. Etc.
But I would not expect it to generate index commands, etc.

On Fri, May 12, 2023 at 8:37 PM Stephen Frost <sfrost@snowman.net> wrote:

Greetings,
..
I mean ... it's already in postgres_fdw, just not in a way that can be
returned to the user. I don't think I'd want this functionality to
depend on postgres_fdw or generally on an extension though, it should
be part of core in some fashion.

I will start with postgres_fdw then, but gladly review the other source...
Just thinking about the essence of the syntax.
SHOW CREATE TABLE abc(LIKE real_table); -- Output CREATE TABLE abc();
using real_table?

I'd look at the IMPORT FOREIGN SCHEMA stuff in postgres_fdw. We're

already largely answering these questions by what options that takes.

Will do.

But it's a big commitment. I don't mind if it has a reasonable chance of
being accepted.

Yes, it's a large effort, no doubt.

At least there is a base of code to start with.
I see a strong need to come up with a shell script to that could:

FOR <every schema.table> DO
psql -c "SHOW... \g | cat > <schema.table.DDL> "
pg_dump -- <schema.table> only | remove_comments_normalize | cat
<schema.table.pg_dump>
DIFF <schema.table.DDL> <schema.table.pg_dump>

Of course, since our tests are usually all perl, a perl version.
But I would clearly want some heavy testing/validation.

On 5/13/23 02:25, Kirk Wolak wrote:

On Sat, May 13, 2023 at 1:03 AM Ron <ronljohnsonjr@gmail.com> wrote:

On 5/12/23 18:00, Kirk Wolak wrote:
[snip]

Where do we draw the lines?

At other tables.

Does Table DDL include all indexes?

Absolutely!

It should include constraints, clearly.  I would not think it should
have triggers.

Definitely triggers.  And foreign keys.

Literally everything within the <<CREATE TABLE X(...);>>.  (ie, no
ALTER .. OWNER TO...)

ALTER statements, too.  If CREATE TABLE ... LIKE ... { INCLUDING |
EXCLUDING } { COMMENTS | COMPRESSION | CONSTRAINTS | DEFAULTS |
GENERATED | IDENTITY | INDEXES | STATISTICS | STORAGE | ALL } can do
it, then so should SHOW CREATE TABLE.

--
Born in Arizona, moved to Babylonia.

I can see the ALTER statements now.  Which is why I asked.
I don't like the idea of including the trigger DDL, because that would
never execute in a clean environment.

I would not be grumpy if trigger statements weren't included.

(I've never used a tool that tried to do that when I've wanted the DDL)
I can go either way on index creation.

Does this imply SQL SYNTAX like:

SHOW CREATE TABLE <table_name>
  [ INCLUDING { ALL | INDEXES |  SEQUENCES | ??? }]
  [EXCLUDING { PK | FK | COMMENTS | STORAGE | } ]
  [FOR {V11 | V12 | V13 | V14 | V15 }] ??
?

"FOR {V...}" is a complication too far, IMO.  No one expects "pg_dump
--schema-only" to have a --version= option, so one should not expect SHOW
CREATE TABLE to have a "FOR {V...}" clause.

The goal for me  is to open the discussion, and then CONSTRAIN the focus.

Personally, the simple syntax:
SHOW CREATE TABLE table1;

Should give me a create table command with the table attributes and the
column attributes, FKs, PKs, Defaults. Etc.
But I would not expect it to generate index commands, etc.

--
Born in Arizona, moved to Babylonia.

On Sat, May 13, 2023, 3:25 AM Kirk Wolak <wolakk@gmail.com> wrote:

Does this imply SQL SYNTAX like:

SHOW CREATE TABLE <table_name>
[ INCLUDING { ALL | INDEXES | SEQUENCES | ??? }]
[EXCLUDING { PK | FK | COMMENTS | STORAGE | } ]
[FOR {V11 | V12 | V13 | V14 | V15 }] ??
?

Personally, I would expect a function, like pg_get_tabledef(oid), to match
the other pg_get_*def functions instead of overloading SHOW. To me, this
also argues that we shouldn't include indexes because we already have a
pg_get_indexdef function.

-Jeremy

On Sat, May 13, 2023 at 3:34 PM Jeremy Smith <jeremy@musicsmith.net> wrote:

On Sat, May 13, 2023, 3:25 AM Kirk Wolak <wolakk@gmail.com> wrote:

Does this imply SQL SYNTAX like:

SHOW CREATE TABLE <table_name>
[ INCLUDING { ALL | INDEXES | SEQUENCES | ??? }]
[EXCLUDING { PK | FK | COMMENTS | STORAGE | } ]
[FOR {V11 | V12 | V13 | V14 | V15 }] ??
?

Personally, I would expect a function, like pg_get_tabledef(oid), to match
the other pg_get_*def functions instead of overloading SHOW. To me, this
also argues that we shouldn't include indexes because we already have a
pg_get_indexdef function.

-Jeremy

+1

In fact, making it a function will make my life easier for testing, that's
for certain. I don't need to involve the parser,etc. Others can help with
that after the function works.
Thanks for the suggestion!

On Fri, May 12, 2023 at 8:37 PM Stephen Frost <sfrost@snowman.net> wrote:

Greetings,
...
Yes, it's a large effort, no doubt.

Stephen, I started looking at the code.
And I have the queries from \set SHOW_HIDDEN
that psql uses. And also the pg_dump output.

My first table was an ID bigint NOT NULL PRIMARY KEY GENERATED ALWAYS AS
IDENTITY

pg_dump puts the decorations on the SEQUENCE
\dt puts that text as the "Default" value

But the STRANGE part for me is the query I Assembled from the FDW returns
nothing for extra attributes.
And only seems to care about the "GENERATED AS (%s) STORED" syntax.

For me, generating the INLINE syntax will produce the SEQUENCE
automatically, so this is my preference.
Let me know if I am missing anything... Please.

Finally, I cannot GRASP this additional syntax:
appendStringInfo(&buf, "\n) SERVER %s\nOPTIONS (",

This is at the end of the create table syntax:
CREATE TABLE %s ( ... ) SERVER %s\n OPTIONS (" ...");

Is this special "FDW" Decorations because I don't see those on the create
table documentation?
It's easy enough to ignore, but I don't want to miss something.

Kirk...

On Sun, May 14, 2023 at 2:20 AM Kirk Wolak <wolakk@gmail.com> wrote:

On Sat, May 13, 2023 at 3:34 PM Jeremy Smith <jeremy@musicsmith.net>
wrote:

On Sat, May 13, 2023, 3:25 AM Kirk Wolak <wolakk@gmail.com> wrote:

Does this imply SQL SYNTAX like:

SHOW CREATE TABLE <table_name>
[ INCLUDING { ALL | INDEXES | SEQUENCES | ??? }]
[EXCLUDING { PK | FK | COMMENTS | STORAGE | } ]
[FOR {V11 | V12 | V13 | V14 | V15 }] ??
?

Personally, I would expect a function, like pg_get_tabledef(oid), to
match the other pg_get_*def functions instead of overloading SHOW. To me,
this also argues that we shouldn't include indexes because we already have
a pg_get_indexdef function.

-Jeremy

+1

In fact, making it a function will make my life easier for testing, that's
for certain. I don't need to involve the parser,etc. Others can help with
that after the function works.
Thanks for the suggestion!

I am moving this over to the Hackers Group.
My approach for now is to develop this as the \st command.
After reviewing the code/output from the 3 sources (psql, fdw, and
pg_dump). This trivializes the approach,
and requires the smallest set of changes (psql is already close with
existing queries, etc).

And frankly, I would rather have an \st feature that handles 99% of the use
cases then go 15yrs waiting for a perfect solution.
Once this works well for the group. Then, IMO, that would be the time to
discuss moving it.

Support Or Objections Appreciated.
Kirk...

Greetings,

* Kirk Wolak (wolakk@gmail.com) wrote:

My approach for now is to develop this as the \st command.
After reviewing the code/output from the 3 sources (psql, fdw, and
pg_dump). This trivializes the approach,
and requires the smallest set of changes (psql is already close with
existing queries, etc).

And frankly, I would rather have an \st feature that handles 99% of the use
cases then go 15yrs waiting for a perfect solution.
Once this works well for the group. Then, IMO, that would be the time to
discuss moving it.

Having this only available via psql seems like the least desirable
option as then it wouldn't be available to any other callers..

Having it in libpq, on the other hand, would make it available to psql
as well as any other utility, library, or language / driver which uses
libpq, including pg_dump..

Using libpq would also make sense from the perspective that libpq can be
used to connect to a number of different major versions of PG and this
could work work for them all in much the way that pg_dump does.

The downside with this apporach is that drivers which don't use libpq
(eg: JDBC) would have to re-implement this if they wanted to keep
feature parity with libpq..

Thanks,

Stephen

On 2023-05-18 Th 19:53, Stephen Frost wrote:

Greetings,

* Kirk Wolak (wolakk@gmail.com) wrote:

My approach for now is to develop this as the \st command.
After reviewing the code/output from the 3 sources (psql, fdw, and
pg_dump). This trivializes the approach,
and requires the smallest set of changes (psql is already close with
existing queries, etc).

And frankly, I would rather have an \st feature that handles 99% of the use
cases then go 15yrs waiting for a perfect solution.
Once this works well for the group. Then, IMO, that would be the time to
discuss moving it.

Having this only available via psql seems like the least desirable
option as then it wouldn't be available to any other callers..

Having it in libpq, on the other hand, would make it available to psql
as well as any other utility, library, or language / driver which uses
libpq, including pg_dump..

Using libpq would also make sense from the perspective that libpq can be
used to connect to a number of different major versions of PG and this
could work work for them all in much the way that pg_dump does.

The downside with this apporach is that drivers which don't use libpq
(eg: JDBC) would have to re-implement this if they wanted to keep
feature parity with libpq..

I think the ONLY place we should have this is in server side functions.
More than ten years ago I did some work in this area (see below), but
it's one of those things that have been on my ever growing personal TODO
list

See <https://bitbucket.org/adunstan/retailddl/src/master/&gt; and
<https://www.youtube.com/watch?v=fBarFKOL3SI&gt;

cheers

andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com

I think the ONLY place we should have this is in server side functions.
More than ten years ago I did some work in this area (see below), but it's
one of those things that have been on my ever growing personal TODO list

See <https://bitbucket.org/adunstan/retailddl/src/master/&gt;
<https://bitbucket.org/adunstan/retailddl/src/master/&gt; and
<https://www.youtube.com/watch?v=fBarFKOL3SI&gt;
<https://www.youtube.com/watch?v=fBarFKOL3SI&gt;

Some additional backstory, as this has come up before...

A direction discussion of a previous SHOW CREATE effort:
/messages/by-id/20190705163203.GD24679@fetter.org

Other databases' implementations of SHOW CREATE came up in this discussion
as well:
/messages/by-id/CADkLM=fxfsrHASKk_bY_A4uomJ1Te5MfGgD_rwwQfV8wP68ewg@mail.gmail.com

Clearly, there is customer demand for something like this, and has been for
a long time.

On Fri, 2023-05-19 at 13:08 -0400, Andrew Dunstan wrote:

On 2023-05-18 Th 19:53, Stephen Frost wrote:

* Kirk Wolak (wolakk@gmail.com) wrote:

My approach for now is to develop this as the \st command.
After reviewing the code/output from the 3 sources (psql, fdw, and
pg_dump).

Having this only available via psql seems like the least desirable
option as then it wouldn't be available to any other callers..

Having it in libpq, on the other hand, would make it available to psql
as well as any other utility, library, or language / driver which uses
libpq, including pg_dump..

I think the ONLY place we should have this is in server side functions.

+1

A function "pg_get_tabledef" would blend nicely into the following list:

\df pg_get_*def
List of functions
Schema │ Name │ Result data type │ Argument data types │ Type
════════════╪════════════════════════════════╪══════════════════╪═══════════════════════╪══════
pg_catalog │ pg_get_constraintdef │ text │ oid │ func
pg_catalog │ pg_get_constraintdef │ text │ oid, boolean │ func
pg_catalog │ pg_get_functiondef │ text │ oid │ func
pg_catalog │ pg_get_indexdef │ text │ oid │ func
pg_catalog │ pg_get_indexdef │ text │ oid, integer, boolean │ func
pg_catalog │ pg_get_partition_constraintdef │ text │ oid │ func
pg_catalog │ pg_get_partkeydef │ text │ oid │ func
pg_catalog │ pg_get_ruledef │ text │ oid │ func
pg_catalog │ pg_get_ruledef │ text │ oid, boolean │ func
pg_catalog │ pg_get_statisticsobjdef │ text │ oid │ func
pg_catalog │ pg_get_triggerdef │ text │ oid │ func
pg_catalog │ pg_get_triggerdef │ text │ oid, boolean │ func
pg_catalog │ pg_get_viewdef │ text │ oid │ func
pg_catalog │ pg_get_viewdef │ text │ oid, boolean │ func
pg_catalog │ pg_get_viewdef │ text │ oid, integer │ func
pg_catalog │ pg_get_viewdef │ text │ text │ func
pg_catalog │ pg_get_viewdef │ text │ text, boolean │ func
(17 rows)

A server function can be conveniently called from any client code.

Yours,
Laurenz Albe

On 19/05/2023 19:08, Andrew Dunstan wrote:

I think the ONLY place we should have this is in server side
functions. More than ten years ago I did some work in this area (see
below), but it's one of those things that have been on my ever growing
personal TODO list

See <https://bitbucket.org/adunstan/retailddl/src/master/&gt; and
<https://www.youtube.com/watch?v=fBarFKOL3SI&gt;

I have my own implementation of SHOW CREATE as a ddlx extension
available at https://github.com/lacanoid/pgddl

Far from perfect but getting better and it seems good enough to be used
by some people...

Ž.

Greetings,

* Laurenz Albe (laurenz.albe@cybertec.at) wrote:

On Fri, 2023-05-19 at 13:08 -0400, Andrew Dunstan wrote:

On 2023-05-18 Th 19:53, Stephen Frost wrote:

* Kirk Wolak (wolakk@gmail.com) wrote:

My approach for now is to develop this as the \st command.
After reviewing the code/output from the 3 sources (psql, fdw, and
pg_dump).

Having this only available via psql seems like the least desirable
option as then it wouldn't be available to any other callers..

Having it in libpq, on the other hand, would make it available to psql
as well as any other utility, library, or language / driver which uses
libpq, including pg_dump..

I think the ONLY place we should have this is in server side functions.

+1

... but it already exists in pg_dump, so I'm unclear why folks seem to
be pushing to have a duplicate of it in core? We certainly can't remove
it from pg_dump even if we add it to core because pg_dump has to
understand the changes between major versions.

A function "pg_get_tabledef" would blend nicely into the following list:

\df pg_get_*def
List of functions
Schema │ Name │ Result data type │ Argument data types │ Type
════════════╪════════════════════════════════╪══════════════════╪═══════════════════════╪══════
pg_catalog │ pg_get_constraintdef │ text │ oid │ func
pg_catalog │ pg_get_constraintdef │ text │ oid, boolean │ func
pg_catalog │ pg_get_functiondef │ text │ oid │ func
pg_catalog │ pg_get_indexdef │ text │ oid │ func
pg_catalog │ pg_get_indexdef │ text │ oid, integer, boolean │ func
pg_catalog │ pg_get_partition_constraintdef │ text │ oid │ func
pg_catalog │ pg_get_partkeydef │ text │ oid │ func
pg_catalog │ pg_get_ruledef │ text │ oid │ func
pg_catalog │ pg_get_ruledef │ text │ oid, boolean │ func
pg_catalog │ pg_get_statisticsobjdef │ text │ oid │ func
pg_catalog │ pg_get_triggerdef │ text │ oid │ func
pg_catalog │ pg_get_triggerdef │ text │ oid, boolean │ func
pg_catalog │ pg_get_viewdef │ text │ oid │ func
pg_catalog │ pg_get_viewdef │ text │ oid, boolean │ func
pg_catalog │ pg_get_viewdef │ text │ oid, integer │ func
pg_catalog │ pg_get_viewdef │ text │ text │ func
pg_catalog │ pg_get_viewdef │ text │ text, boolean │ func
(17 rows)

A server function can be conveniently called from any client code.

Clearly any client using libpq can conveniently call code which is in
libpq.

Thanks,

Stephen

On Sat, May 20, 2023 at 10:26 AM Stephen Frost <sfrost@snowman.net> wrote:

A server function can be conveniently called from any client code.

Clearly any client using libpq can conveniently call code which is in
libpq.

Clearly there are clients that don't use libpq. JDBC comes to mind.

David J.

"David G. Johnston" <david.g.johnston@gmail.com> writes:

On Sat, May 20, 2023 at 10:26 AM Stephen Frost <sfrost@snowman.net> wrote:

Clearly any client using libpq can conveniently call code which is in
libpq.

Clearly there are clients that don't use libpq. JDBC comes to mind.

Yeah. I'm also rather concerned about the bloat factor; it's
hardly unlikely that this line of development would double the
size of libpq, to add functionality that only a small minority
of applications would use. A client-side implementation also has
no choice but to cope with multiple server versions, and to
figure out what it's going to do about a too-new server.
Up to now we broke compatibility between libpq and server maybe
once every couple decades, but it'd be once a year for this.

regards, tom lane

Greetings,

On Sat, May 20, 2023 at 13:32 David G. Johnston <david.g.johnston@gmail.com>
wrote:

On Sat, May 20, 2023 at 10:26 AM Stephen Frost <sfrost@snowman.net> wrote:

A server function can be conveniently called from any client code.

Clearly any client using libpq can conveniently call code which is in
libpq.

Clearly there are clients that don't use libpq. JDBC comes to mind.

Indeed … as I mentioned up-thread already.

Are we saying that we want this to be available server side, and largely
duplicated, specifically to cater to non-libpq users? I’ll put out there,
again, the idea that perhaps we put it into the common library then and
make it available via both libpq and as a server side function ..?

We also have similar code in postgres_fdw.. ideally, imv anyway, we’d not
end up with three copies of it.

Thanks,

Stephen

On Sat, May 20, 2023 at 2:33 PM Stephen Frost <sfrost@snowman.net> wrote:

Greetings,

On Sat, May 20, 2023 at 13:32 David G. Johnston <
david.g.johnston@gmail.com> wrote:

On Sat, May 20, 2023 at 10:26 AM Stephen Frost <sfrost@snowman.net>
wrote:

A server function can be conveniently called from any client code.

Clearly any client using libpq can conveniently call code which is in
libpq.

Clearly there are clients that don't use libpq. JDBC comes to mind.

Indeed … as I mentioned up-thread already.

Are we saying that we want this to be available server side, and largely
duplicated, specifically to cater to non-libpq users? I’ll put out there,
again, the idea that perhaps we put it into the common library then and
make it available via both libpq and as a server side function ..?

We also have similar code in postgres_fdw.. ideally, imv anyway, we’d not
end up with three copies of it.

Thanks,

Stephen

First, as the person chasing this down, and a JDBC user, I really would
prefer pg_get_tabledef() as Laurenz mentioned.

Next, I have reviewed all 3 implementations (pg_dump [most appropriate],
psql \d (very similar), and the FDW which is "way off",
since it actually focuses on "CREATE FOREIGN TABLE" exclusively, and
already fails to handle many pieces not required in
creating a "real" table, as it creates a "reflection" of table.

I am using pg_dump as my source of truth. But I noticed it does not create
"TEMPORARY" tables with that syntax.
[Leading to a question on mutating the pg_temp_# schema name back to
pg_temp. or just stripping it, in favor of the TEMPORARY]

I was surprised to see ~ 2,000 lines of code in the FDW and in psql...
Whereas pg_dump is shorter because it gets more detailed
table information in a structure passed in.

I would love to leverage existing code, in the end. But I want to take my
time on this, and become intimate with the details.
Each of the above 3 approaches have different goals. And I would prefer
the lowest risk:reward possible, and the least expensive
maintenance. Having it run server side hides a ton of details, and as Tom
pointed out, obviates DDL versioning control for other server versions.

Thanks for the references to the old discussions. I have queued them up to
review.

Kirk...

On Fri, May 19, 2023 at 1:08 PM Andrew Dunstan <andrew@dunslane.net> wrote:

I think the ONLY place we should have this is in server side functions.
More than ten years ago I did some work in this area (see below), but it's
one of those things that have been on my ever growing personal TODO list

See <https://bitbucket.org/adunstan/retailddl/src/master/&gt;
<https://bitbucket.org/adunstan/retailddl/src/master/&gt; and
<https://www.youtube.com/watch?v=fBarFKOL3SI&gt;
<https://www.youtube.com/watch?v=fBarFKOL3SI&gt;

Andrew,
Thanks for sharing that. I reviewed your code. 10yrs, clearly it's not
working (as-is, but close), something interesting about the
structure you ended up in. You check the type of the object and redirect
accordingly at the top level. Hmmm...
What I liked was that each type gets handled (I was focused on "table"),
but I realized similarities.

I don't know what the group would think, but I like the thought of
calling this, and having it "Correct" to call the appropriate function.
But not sure it will stand. It does make obvious that some of these should
be spun out as "pg_get_typedef"..
pg_get_typedef
pg_get_domaindef
pg_get_sequencedef

Finally, since you started this a while back, part of me is "leaning"
towards a function:
pg_get_columndef

Which returns a properly formatted column for a table, type, or domain?
(one of the reasons for this, is that this is
the function with the highest probability to change, and potentially the
easiest to share reusability).

Finally, I am curious about your opinion. I noticed you used the
internal pg_ tables, versus the information_schema...
I am *thinking* that the information_schema will be more stable over
time... Thoughts?

Thank you for sharing your thoughts...
Kirk...

po 22. 5. 2023 v 7:19 odesílatel Kirk Wolak <wolakk@gmail.com> napsal:

On Fri, May 19, 2023 at 1:08 PM Andrew Dunstan <andrew@dunslane.net>
wrote:

I think the ONLY place we should have this is in server side functions.
More than ten years ago I did some work in this area (see below), but it's
one of those things that have been on my ever growing personal TODO list

See <https://bitbucket.org/adunstan/retailddl/src/master/&gt;
<https://bitbucket.org/adunstan/retailddl/src/master/&gt; and
<https://www.youtube.com/watch?v=fBarFKOL3SI&gt;
<https://www.youtube.com/watch?v=fBarFKOL3SI&gt;

Andrew,
Thanks for sharing that. I reviewed your code. 10yrs, clearly it's not
working (as-is, but close), something interesting about the
structure you ended up in. You check the type of the object and redirect
accordingly at the top level. Hmmm...
What I liked was that each type gets handled (I was focused on "table"),
but I realized similarities.

I don't know what the group would think, but I like the thought of
calling this, and having it "Correct" to call the appropriate function.
But not sure it will stand. It does make obvious that some of these
should be spun out as "pg_get_typedef"..
pg_get_typedef
pg_get_domaindef
pg_get_sequencedef

Finally, since you started this a while back, part of me is "leaning"
towards a function:
pg_get_columndef

Which returns a properly formatted column for a table, type, or domain?
(one of the reasons for this, is that this is
the function with the highest probability to change, and potentially the
easiest to share reusability).

Finally, I am curious about your opinion. I noticed you used the
internal pg_ tables, versus the information_schema...
I am *thinking* that the information_schema will be more stable over
time... Thoughts?

I think inside the core, the information schema is never used. And there
was a performance issue (fixed in PostgreSQL 12), that blocked index usage.

Regards

Pavel

On 2023-05-22 Mo 05:24, Pavel Stehule wrote:

po 22. 5. 2023 v 7:19 odesílatel Kirk Wolak <wolakk@gmail.com> napsal:

On Fri, May 19, 2023 at 1:08 PM Andrew Dunstan
<andrew@dunslane.net> wrote:

I think the ONLY place we should have this is in server side
functions. More than ten years ago I did some work in this
area (see below), but it's one of those things that have been
on my ever growing personal TODO list

See <https://bitbucket.org/adunstan/retailddl/src/master/&gt;
<https://bitbucket.org/adunstan/retailddl/src/master/&gt; and
<https://www.youtube.com/watch?v=fBarFKOL3SI&gt;
<https://www.youtube.com/watch?v=fBarFKOL3SI&gt;

Andrew,
  Thanks for sharing that.  I reviewed your code. 10yrs, clearly
it's not working (as-is, but close), something interesting about the
structure you ended up in.  You check the type of the object and
redirect accordingly at the top level. Hmmm...
What I liked was that each type gets handled (I was focused on
"table"), but I realized similarities.

  I don't know what the group would think, but I like the thought
of calling this, and having it "Correct" to call the appropriate
function.
But not sure it will stand.  It does make obvious that some of
these should be spun out as "pg_get_typedef"..
pg_get_typedef
pg_get_domaindef
pg_get_sequencedef

  Finally, since you started this a while back, part of me is
"leaning" towards a function:
pg_get_columndef

  Which returns a properly formatted column for a table, type, or
domain? (one of the reasons for this, is that this is
the function with the highest probability to change, and
potentially the easiest to share reusability).

  Finally, I am curious about your opinion.  I noticed you used
the internal pg_ tables, versus the information_schema...
I am *thinking* that the information_schema will be more stable
over time... Thoughts?

I think inside the core, the information schema is never used.  And
there was a performance issue (fixed in PostgreSQL 12), that blocked
index usage.

A performant server side set of functions would be written in C and
follow the patterns in ruleutils.c.

cheers

andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com

On Mon, 22 May 2023 at 13:52, Andrew Dunstan <andrew@dunslane.net> wrote:

A performant server side set of functions would be written in C and follow the patterns in ruleutils.c.

We have lots of DDL ruleutils in our Citus codebase:
https://github.com/citusdata/citus/blob/main/src/backend/distributed/deparser/citus_ruleutils.c

I'm pretty sure we'd be happy to upstream those if that meant, we
wouldn't have to update them for every postgres release.

We also have the master_get_table_ddl_events UDF, which does what SHOW
CREATE TABLE would do.

On 2023-05-25 Th 09:23, Jelte Fennema wrote:

On Mon, 22 May 2023 at 13:52, Andrew Dunstan<andrew@dunslane.net> wrote:

A performant server side set of functions would be written in C and follow the patterns in ruleutils.c.

We have lots of DDL ruleutils in our Citus codebase:
https://github.com/citusdata/citus/blob/main/src/backend/distributed/deparser/citus_ruleutils.c

I'm pretty sure we'd be happy to upstream those if that meant, we
wouldn't have to update them for every postgres release.

We also have the master_get_table_ddl_events UDF, which does what SHOW
CREATE TABLE would do.

Sounds promising. I'd love to see a patch.

cheers

andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com

On Thu, May 25, 2023 at 9:23 AM Jelte Fennema <postgres@jeltef.nl> wrote:

On Mon, 22 May 2023 at 13:52, Andrew Dunstan <andrew@dunslane.net> wrote:

A performant server side set of functions would be written in C and

follow the patterns in ruleutils.c.

We have lots of DDL ruleutils in our Citus codebase:

https://github.com/citusdata/citus/blob/main/src/backend/distributed/deparser/citus_ruleutils.c

I'm pretty sure we'd be happy to upstream those if that meant, we
wouldn't have to update them for every postgres release.

We also have the master_get_table_ddl_events UDF, which does what SHOW
CREATE TABLE would do.

Jelte, this looks promising, although it is a radically different approach
(Querying from it to get the details).

I was just getting ready to write up a bit of an RFC... On the following
approach...

I have been trying to determine how to "focus" this effort to move it
forward. Here is where I am at:
1) It should be 100% server side (and psql \st would only work by calling
the server side code, if it was there)
In reviewing... This simplifies the implementation to the current
version of PG DDL being generated.
Also, as others have mentioned, it should be C based code, and use
only the internal tables.
2) Since pg_get_{ triggerdef | indexdef | constraintdef } already exists, I
was strongly recommending to not include those.
-- Although including the inlined constraints would be fine by me
(potentially a boolean to turn it off?)
3) Then focusing the reloptions WITH (%s)

It appears CITUS code handles ALL of this on a cursory review!

The ONLY thing I did not see was "CREATE TEMPORARY " syntax? If you did
this on a TEMP table,
does it generate normal table syntax or TEMPORARY TABLE syntax???

So, from my take... This is a great example of solving the problem with
existing "Production Quality" Code...
I like it...

Can this get turned into a Patch? Were you offering this code up for
others (me?) to pull, and work into a patch?
[If I do the patch, I am not sure it gives you the value of reducing what
CITUS has to maintain. But it dawns on
me that you might be pushing a much bigger patch... But I would take that,
as I think there is other value in there]

Others???

Thanks,

Kirk...
PS: It dawned on me that if pg_dump had used server side code to generate
its DDL, its complexity would drop.

On 2023-06-01 Th 12:57, Kirk Wolak wrote:

PS: It dawned on me that if pg_dump had used server side code to
generate its DDL, its complexity would drop.

Maybe, that remains to be seen. pg_dump needs to produce SQL that is
suitable for the target database, not the source database.

cheers

andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com

On Thu, Jun 1, 2023 at 3:13 PM Andrew Dunstan <andrew@dunslane.net> wrote:

On 2023-06-01 Th 12:57, Kirk Wolak wrote:

PS: It dawned on me that if pg_dump had used server side code to generate
its DDL, its complexity would drop.

Maybe, that remains to be seen. pg_dump needs to produce SQL that is
suitable for the target database, not the source database.

First, pg_dump has some special needs in addressing how it creates tables,
to be able to load the data BEFORE indexing, and constraining (lest you
have to struggle with dependencies of FKs, etc etc)...

But version checking is interesting... Because I found no way to tell
pg_dump what DB to target. The code I did see was checking what server
options were available. (I clearly could have missed something)... But
exactly how would that work? All it knows is who it is (pg_dump V14, for
example. So the best case is that it ignores new things that would be in
V15, V16 because it doesn't even know what to check for or what to do).
Which, to me, makes it more of a side-effect than a feature, no?

And that if it used a server side solution, it gets an interesting "super
power". In that even pg_dump V20 could dump a V21 db with V21 syntax
enhancements. (I cannot say if that is desirable or not as I lack 20yrs of
history here). Finally, the thoughts of implementing this change at this
point, when it would impact anyone using the NEW version to dump an OLD
version without the server side support... So that migration may never
happen. Or only after the only supported DBs have the required server side
functions...

The feedback is welcome...

On 2023-06-01 Th 16:39, Kirk Wolak wrote:

On Thu, Jun 1, 2023 at 3:13 PM Andrew Dunstan <andrew@dunslane.net> wrote:

On 2023-06-01 Th 12:57, Kirk Wolak wrote:

PS: It dawned on me that if pg_dump had used server side code to
generate its DDL, its complexity would drop.

Maybe, that remains to be seen. pg_dump needs to produce SQL that
is suitable for the target database, not the source database.

First, pg_dump has some special needs in addressing how it creates
tables, to be able to load the data BEFORE indexing, and constraining
(lest you have to struggle with dependencies of FKs, etc etc)...

But version checking is interesting... Because I found no way to tell
pg_dump what DB to target.

The target version is implicitly the version it's built from.

cheers

andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com

On Thu, Jun 1, 2023 at 4:46 PM Andrew Dunstan <andrew@dunslane.net> wrote:

On 2023-06-01 Th 16:39, Kirk Wolak wrote:

On Thu, Jun 1, 2023 at 3:13 PM Andrew Dunstan <andrew@dunslane.net> wrote:

On 2023-06-01 Th 12:57, Kirk Wolak wrote:

PS: It dawned on me that if pg_dump had used server side code to generate
its DDL, its complexity would drop.

Maybe, that remains to be seen. pg_dump needs to produce SQL that is
suitable for the target database, not the source database.

First, pg_dump has some special needs in addressing how it creates tables,
to be able to load the data BEFORE indexing, and constraining (lest you
have to struggle with dependencies of FKs, etc etc)...

But version checking is interesting... Because I found no way to tell
pg_dump what DB to target.

The target version is implicitly the version it's built from.

Andrew,
Thank you. Someone else confirmed for me that the code is designed to
create accurate DDL for the pg_dump version.
So, for example, WITH OIDs are not included with later versions of pg_dump,
even though they are hitting a server with them.
Great to know.

I like that CITUS offered up something. I think that might be the
current best approach. It's a win-win. They get something,
we get something.

Kirk...

On Thu, 1 Jun 2023 at 18:57, Kirk Wolak <wolakk@gmail.com> wrote:

Can this get turned into a Patch? Were you offering this code up for others (me?) to pull, and work into a patch?
[If I do the patch, I am not sure it gives you the value of reducing what CITUS has to maintain. But it dawns on
me that you might be pushing a much bigger patch... But I would take that, as I think there is other value in there]

Attached is a patch which adds the relevant functions from Citus to
Postgres (it compiles without warnings on my machine). I checked with
a few others on the Citus team at Microsoft and everyone thought that
upstreaming this was a good idea, because it's quite a bit of work to
update with every postgres release.

To set expectations though, I don't really have time to work on this
patch. So if you can take it over from here that would be great.

The patch only contains the C functions which generate SQL based on
some oids. The wrappers such as the master_get_table_ddl_events
function were too hard for me to pull out of Citus code, because they
integrated a lot with other pieces. But the bulk of the logic is in
the functions in this patch. Essentially all that
master_get_table_ddl_events does is call the functions in this patch
in the right order.

The ONLY thing I did not see was "CREATE TEMPORARY " syntax? If you did this on a TEMP table,
does it generate normal table syntax or TEMPORARY TABLE syntax???

Yeah, the Citus code only handles things that Citus supports in
distributed tables. Which is quite a lot, but indeed not everything
yet. Temporary and inherited tables are not supported in this code
afaik. Possibly more. See the commented out
EnsureRelationKindSupported for what should be supported (normal
tables and partitioned tables afaik).

On Mon, Jun 5, 2023 at 7:43 AM Jelte Fennema <postgres@jeltef.nl> wrote:

On Thu, 1 Jun 2023 at 18:57, Kirk Wolak <wolakk@gmail.com> wrote:

Can this get turned into a Patch? Were you offering this code up for

others (me?) to pull, and work into a patch?

[If I do the patch, I am not sure it gives you the value of reducing

what CITUS has to maintain. But it dawns on

me that you might be pushing a much bigger patch... But I would take

that, as I think there is other value in there]

Attached is a patch which adds the relevant functions from Citus to
Postgres (it compiles without warnings on my machine). I checked with
a few others on the Citus team at Microsoft and everyone thought that
upstreaming this was a good idea, because it's quite a bit of work to
update with every postgres release.

To set expectations though, I don't really have time to work on this
patch. So if you can take it over from here that would be great.

The patch only contains the C functions which generate SQL based on
some oids. The wrappers such as the master_get_table_ddl_events
function were too hard for me to pull out of Citus code, because they
integrated a lot with other pieces. But the bulk of the logic is in
the functions in this patch. Essentially all that
master_get_table_ddl_events does is call the functions in this patch
in the right order.

The ONLY thing I did not see was "CREATE TEMPORARY " syntax? If you did

this on a TEMP table,

does it generate normal table syntax or TEMPORARY TABLE syntax???

Yeah, the Citus code only handles things that Citus supports in
distributed tables. Which is quite a lot, but indeed not everything
yet. Temporary and inherited tables are not supported in this code
afaik. Possibly more. See the commented out
EnsureRelationKindSupported for what should be supported (normal
tables and partitioned tables afaik).

Jelte,
Thank you for this.
Let me see what I can do with this, it seems like a solid starting point.
At this point, based on previous feedback, finding a way to make
get_tabledef() etc. to work as functions is my goal.
I will see how inherited tables and temporary tables will be dealt with.

Hopefully, this transfer works to please anyone concerned with
integrating this code into our project from the Citus code.

Kirk...

On Wed, Jun 21, 2023 at 8:52 PM Kirk Wolak <wolakk@gmail.com> wrote:

On Mon, Jun 5, 2023 at 7:43 AM Jelte Fennema <postgres@jeltef.nl> wrote:

On Thu, 1 Jun 2023 at 18:57, Kirk Wolak <wolakk@gmail.com> wrote:

Can this get turned into a Patch? Were you offering this code up for

others (me?) to pull, and work into a patch?

[If I do the patch, I am not sure it gives you the value of reducing

what CITUS has to maintain. But it dawns on

me that you might be pushing a much bigger patch... But I would take

that, as I think there is other value in there]

Yeah, the Citus code only handles things that Citus supports in

distributed tables. Which is quite a lot, but indeed not everything
yet. Temporary and inherited tables are not supported in this code
afaik. Possibly more. See the commented out
EnsureRelationKindSupported for what should be supported (normal
tables and partitioned tables afaik).

Okay, apologies for the long delay on this. I have the code Jelte
submitted working. And I have (almost) figured out how to add the function
so it shows up in the pg_catalog... (I edited files I should not have, I
need to know the proper process... Anyone...)

Not sure if it is customary to attach the code when asking about stuff.
For the most part, it was what Jelte Gave us with a pg_get_tabledef()
wrapper to call...

Here is the output it produces for *select
pg_get_tabledef('pg_class'::regclass); * (Feedback Welcome)

CREATE TABLE pg_class (oid oid NOT NULL, relname name NOT NULL COLLATE "C",
relnamespace oid NOT NULL, reltype oid NOT NULL, reloftype oid NOT NULL,
relowner oid NOT NULL, relam oid NOT NULL, relfilenode oid NOT NULL,
reltablespace oid NOT NULL, relpages integer NOT NULL, reltuples real NOT
NULL, relallvisible integer NOT NULL, reltoastrelid oid NOT NULL,
relhasindex boolean NOT NULL, relisshared boolean NOT NULL, relpersistence
"char" NOT NULL, relkind "char" NOT NULL, relnatts smallint NOT NULL,
relchecks smallint NOT NULL, relhasrules boolean NOT NULL, relhastriggers
boolean NOT NULL, relhassubclass boolean NOT NULL, relrowsecurity boolean
NOT NULL, relforcerowsecurity boolean NOT NULL, relispopulated boolean NOT
NULL, relreplident "char" NOT NULL, relispartition boolean NOT NULL,
relrewrite oid NOT NULL, relfrozenxid xid NOT NULL, relminmxid xid NOT
NULL, relacl aclitem[], reloptions text[] COLLATE "C", relpartbound
pg_node_tree COLLATE "C") USING heap

==
My Comments/Questions:
1) I would prefer Legible output, like below
2) I would prefer to leave off COLLATE "C" IFF that is the DB Default
3) The USING heap... I want to pull UNLESS the value is NOT the default
(That's a theme in my comments)
4) I *THINK* including the schema would be nice?
5) This version will work with a TEMP table, but NOT EMIT "TEMPORARY"...
Thoughts? Is emitting [pg_temp.] good enough?
6) This version enumerates sequence values (Drop always, or Drop if they
are the default values?)
7) Should I enable the pg_get_seqdef() code
8) It does NOT handle Inheritance (Yet... Is this important? Is it okay to
just give the table structure for this table?)
9) I have not tested against Partitions, etc... I SIMPLY want initial
feedback on Formatting

-- Legible:
CREATE TABLE pg_class (oid oid NOT NULL,
relname name NOT NULL COLLATE "C",
relnamespace oid NOT NULL,
reltype oid NOT NULL,
...
reloptions text[] COLLATE "C",
relpartbound pg_node_tree COLLATE "C"
)

-- Too verbose with "*DEFAULT*" Sequence Values:
CREATE TABLE t1 (id bigint GENERATED BY DEFAULT AS IDENTITY *(INCREMENT BY
1 MINVALUE 1 MAXVALUE 9223372036854775807 START WITH 1 CACHE 1 NO CYCLE)*
NOT NULL,
f1 text
) WITH (autovacuum_vacuum_cost_delay='0', fillfactor='80',
autovacuum_vacuum_insert_threshold='-1',
autovacuum_analyze_threshold='500000000',
autovacuum_vacuum_threshold='500000000',
autovacuum_vacuum_scale_factor='1.5')

Thanks,

Kirk...
PS: After I get feedback on Formatting the output, etc. I will gladly
generate a new .patch file and send it along. Otherwise Jelte gets 100% of
the credit, and I don't want to look like I am changing that.

On Fri, Jun 30, 2023 at 1:56 PM Kirk Wolak <wolakk@gmail.com> wrote:

On Wed, Jun 21, 2023 at 8:52 PM Kirk Wolak <wolakk@gmail.com> wrote:

On Mon, Jun 5, 2023 at 7:43 AM Jelte Fennema <postgres@jeltef.nl> wrote:

On Thu, 1 Jun 2023 at 18:57, Kirk Wolak <wolakk@gmail.com> wrote:

Definitely have the questions from the previous email, but I CERTAINLY
appreciate this output.
(Don't like the +, but pg_get_viewdef() creates the view the same way)...
Will psql doing \st pg_class be able to just call/output this so that the
output is nice and clean?

At this point... I will keep pressing forward, cleaning things up. And
then send a patch for others to play with....
(Probably bad timing with wrapping up V16)

*select pg_get_tabledef('pg_class'::regclass);*
pg_get_tabledef
----------------------------------------
CREATE TABLE pg_class (oid oid NOT NULL,+
relname name NOT NULL COLLATE "C", +
relnamespace oid NOT NULL, +
reltype oid NOT NULL, +
reloftype oid NOT NULL, +
relowner oid NOT NULL, +
relam oid NOT NULL, +
relfilenode oid NOT NULL, +
reltablespace oid NOT NULL, +
relpages integer NOT NULL, +
reltuples real NOT NULL, +
relallvisible integer NOT NULL, +
reltoastrelid oid NOT NULL, +
relhasindex boolean NOT NULL, +
relisshared boolean NOT NULL, +
relpersistence "char" NOT NULL, +
relkind "char" NOT NULL, +
relnatts smallint NOT NULL, +
relchecks smallint NOT NULL, +
relhasrules boolean NOT NULL, +
relhastriggers boolean NOT NULL, +
relhassubclass boolean NOT NULL, +
relrowsecurity boolean NOT NULL, +
relforcerowsecurity boolean NOT NULL, +
relispopulated boolean NOT NULL, +
relreplident "char" NOT NULL, +
relispartition boolean NOT NULL, +
relrewrite oid NOT NULL, +
relfrozenxid xid NOT NULL, +
relminmxid xid NOT NULL, +
relacl aclitem[], +
reloptions text[] COLLATE "C", +
relpartbound pg_node_tree COLLATE "C" +
) USING heap