Skip to content

Conversation

@timwaizenegger
Copy link
Collaborator

No description provided.

Comment on lines +97 to +100
#define GET_DDL_PRETTY_FLAGS(pretty) \
((pretty) ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) \
: 0)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#define GET_DDL_PRETTY_FLAGS(pretty) \
((pretty) ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) \
: 0)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like adding the GET_DDL_PRETTY_FLAGS, but maybe it's just me. If you have a strong feeling, let it be as it is

Comment on lines +13921 to +13925
bool pretty = PG_GETARG_BOOL(1);
char *res;
int prettyFlags;

prettyFlags = GET_DDL_PRETTY_FLAGS(pretty);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
bool pretty = PG_GETARG_BOOL(1);
char *res;
int prettyFlags;
prettyFlags = GET_DDL_PRETTY_FLAGS(pretty);
bool pretty;
char *res;
int prettyFlags = true;
if (PG_ARGISNULL(1))
pretty = PG_GETARG_BOOL(1);
prettyFlags = pretty ? PRETTYFLAG_INDENT : 0;

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I propose this, instead of the GET_DDL_PRETTY_FLAGS, but it's your choice.

Comment on lines +660 to +664
CREATE OR REPLACE FUNCTION
pg_get_domain_ddl(domain_name regtype, pretty bool DEFAULT false)
RETURNS text
LANGUAGE internal
AS 'pg_get_domain_ddl_ext';
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this?
Isn't it better to have it in pg_proc.dat, also the one with a one-argument signature?

{ oid => '8024', descr => 'get CREATE statement for DOMAIN with pretty option',
proname => 'pg_get_domain_ddl', prorettype => 'text',
proargtypes => 'regtype', prosrc => 'pg_get_domain_ddl' },
proargtypes => 'regtype bool', prosrc => 'pg_get_domain_ddl_ext' },
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
proargtypes => 'regtype bool', prosrc => 'pg_get_domain_ddl_ext' },
proargtypes => 'regtype bool', prosrc => 'pg_get_domain_ddl_ext' },
{ oid => '8025', descr => 'get CREATE statement for DOMAIN',
proname => 'pg_get_domain_ddl', prorettype => 'text',
proargtypes => 'regtype', prosrc => 'pg_get_domain_ddl_ext' },

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this instead of adding the create function, better to have all teh changes in one place, maybe...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants