Hello
Is ruleutils.c the best place for this function?
It's already huge, and it has a different scope: "Functions to convert
stored expressions/querytrees back to source text"
+ /* Fetch the value of COLLATION_VERSION */
+ dbvalue = SysCacheGetAttr(DATABASEOID, tuple_database,
+ Anum_pg_database_datcollversion, &attr_isnull);
+ if (!attr_isnull)
+ get_formatted_string(&buf, pretty_flags, 8, "COLLATION_VERSION = %s",
+ quote_literal_cstr(TextDatumGetCString(dbvalue)));
pg_dumpall only shows this for binary upgrade, otherwise skips it. Is
it okay for this command to print it by default, shouldn't it depend
on is_with_defaults or something similar?
+#ifndef DDL_DEFAULTS_H
+#define DDL_DEFAULTS_H
+
+static const struct
+{
+ struct
+ {
....
This file seems strange. A static const struct in a header with
uppercase names doesn't seem to follow postgres conventions?
DATCONNLIMIT_UNLIMITED alredy exists as a definition, and probably
should be used instead or referenced, or the existing uses should
refer to the new way of defining it.
+ /* Fetch the value of LC_COLLATE */
+ dbvalue = SysCacheGetAttr(DATABASEOID, tuple_database,
+ Anum_pg_database_datcollate, &attr_isnull);
+ if (!attr_isnull)
+ get_formatted_string(&buf, pretty_flags, 8, "LC_COLLATE = %s",
+ quote_literal_cstr(TextDatumGetCString(dbvalue)));
+ /* Fetch the value of LC_CTYPE */
+ dbvalue = SysCacheGetAttr(DATABASEOID, tuple_database,
+ Anum_pg_database_datctype, &attr_isnull);
Can these be ever nulls?
Also, pg_dump only emits LOCALE if ctype==collate, shouldn't this
follow the same pattern?
+ else if (is_with_defaults)
+ get_formatted_string(&buf, pretty_flags, 8, "LOCALE_PROVIDER = libc");
Doesn't pg_dump always emit this? Shouldn't this function follow the
same convention? Emitting it seems to be a safer default, in case
postgres ever changes this.
+ /* Build the CREATE DATABASE statement */
+ appendStringInfo(&buf, "CREATE DATABASE %s",
+ quote_identifier(dbform->datname.data));
+ get_formatted_string(&buf, pretty_flags, 4, "WITH");
Shouldn't we only emit "WITH" if it is actually followed by something,
not unconditionally?
+/*
+ * get_formatted_string
+ *
+ * Return a formatted version of the string.
But it's a void function.
+ else if (!attr_isnull)
+ get_formatted_string(&buf, pretty_flags, 8, "LOCALE = %s",
+ quote_literal_cstr(TextDatumGetCString(dbvalue)));
+
Can this ever happen, shouldn't it be an assertion instead?
