Use C99 designated initializers for some structs
Here is a patch to change some struct initializations to use C99-style
designated initializers. These are just a few particularly egregious
cases that were hard to read and write, and error prone because of many
similar adjacent types.
(The PL/Python changes currently don't compile with Python 3 because of
the situation described in the parallel thread "PL/Python: Remove use of
simple slicing API".)
Thoughts?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 29/08/2018 12:13, Peter Eisentraut wrote:
Here is a patch to change some struct initializations to use C99-style
designated initializers. These are just a few particularly egregious
cases that were hard to read and write, and error prone because of many
similar adjacent types.(The PL/Python changes currently don't compile with Python 3 because of
the situation described in the parallel thread "PL/Python: Remove use of
simple slicing API".)Thoughts?
and with the actual patch
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
0001-Use-C99-designated-initializers-for-some-structs.patchtext/plain; charset=UTF-8; name=0001-Use-C99-designated-initializers-for-some-structs.patch; x-mac-creator=0; x-mac-type=0Download
From 8af20d62778b44fc5bfe91f2f8fe7991ffc09bb9 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Wed, 29 Aug 2018 08:36:30 +0200
Subject: [PATCH 1/2] Use C99 designated initializers for some structs
---
src/backend/access/transam/xact.c | 23 +-
src/backend/catalog/heap.c | 98 ++++--
src/bin/psql/tab-complete.c | 434 ++++++++-------------------
src/pl/plpython/plpy_cursorobject.c | 39 +--
src/pl/plpython/plpy_planobject.c | 37 +--
src/pl/plpython/plpy_plpymodule.c | 22 +-
src/pl/plpython/plpy_resultobject.c | 57 +---
src/pl/plpython/plpy_subxactobject.c | 37 +--
8 files changed, 242 insertions(+), 505 deletions(-)
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index cd8270d5fb..875be180fe 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -199,27 +199,8 @@ typedef TransactionStateData *TransactionState;
* transaction at all, or when in a top-level transaction.
*/
static TransactionStateData TopTransactionStateData = {
- 0, /* transaction id */
- 0, /* subtransaction id */
- NULL, /* savepoint name */
- 0, /* savepoint level */
- TRANS_DEFAULT, /* transaction state */
- TBLOCK_DEFAULT, /* transaction block state from the client
- * perspective */
- 0, /* transaction nesting depth */
- 0, /* GUC context nesting depth */
- NULL, /* cur transaction context */
- NULL, /* cur transaction resource owner */
- NULL, /* subcommitted child Xids */
- 0, /* # of subcommitted child Xids */
- 0, /* allocated size of childXids[] */
- InvalidOid, /* previous CurrentUserId setting */
- 0, /* previous SecurityRestrictionContext */
- false, /* entry-time xact r/o state */
- false, /* startedInRecovery */
- false, /* didLogXid */
- 0, /* parallelModeLevel */
- NULL /* link to parent state block */
+ .state = TRANS_DEFAULT,
+ .blockState = TBLOCK_DEFAULT,
};
/*
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index ac5a677c5f..ecbe88569f 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -145,39 +145,87 @@ static List *insert_ordered_unique_oid(List *list, Oid datum);
*/
static FormData_pg_attribute a1 = {
- 0, {"ctid"}, TIDOID, 0, sizeof(ItemPointerData),
- SelfItemPointerAttributeNumber, 0, -1, -1,
- false, 'p', 's', true, false, false, '\0', false, true, 0
+ .attname = {"ctid"},
+ .atttypid = TIDOID,
+ .attlen = sizeof(ItemPointerData),
+ .attnum = SelfItemPointerAttributeNumber,
+ .attcacheoff = -1,
+ .atttypmod = -1,
+ .attbyval = false,
+ .attstorage = 'p',
+ .attalign = 's',
+ .attnotnull = true,
+ .attislocal = true,
};
static FormData_pg_attribute a2 = {
- 0, {"oid"}, OIDOID, 0, sizeof(Oid),
- ObjectIdAttributeNumber, 0, -1, -1,
- true, 'p', 'i', true, false, false, '\0', false, true, 0
+ .attname = {"oid"},
+ .atttypid = OIDOID,
+ .attlen = sizeof(Oid),
+ .attnum = ObjectIdAttributeNumber,
+ .attcacheoff = -1,
+ .atttypmod = -1,
+ .attbyval = true,
+ .attstorage = 'p',
+ .attalign = 'i',
+ .attnotnull = true,
+ .attislocal = true,
};
static FormData_pg_attribute a3 = {
- 0, {"xmin"}, XIDOID, 0, sizeof(TransactionId),
- MinTransactionIdAttributeNumber, 0, -1, -1,
- true, 'p', 'i', true, false, false, '\0', false, true, 0
+ .attname = {"xmin"},
+ .atttypid = XIDOID,
+ .attlen = sizeof(TransactionId),
+ .attnum = MinTransactionIdAttributeNumber,
+ .attcacheoff = -1,
+ .atttypmod = -1,
+ .attbyval = true,
+ .attstorage = 'p',
+ .attalign = 'i',
+ .attnotnull = true,
+ .attislocal = true,
};
static FormData_pg_attribute a4 = {
- 0, {"cmin"}, CIDOID, 0, sizeof(CommandId),
- MinCommandIdAttributeNumber, 0, -1, -1,
- true, 'p', 'i', true, false, false, '\0', false, true, 0
+ .attname = {"cmin"},
+ .atttypid = CIDOID,
+ .attlen = sizeof(CommandId),
+ .attnum = MinCommandIdAttributeNumber,
+ .attcacheoff = -1,
+ .atttypmod = -1,
+ .attbyval = true,
+ .attstorage = 'p',
+ .attalign = 'i',
+ .attnotnull = true,
+ .attislocal = true,
};
static FormData_pg_attribute a5 = {
- 0, {"xmax"}, XIDOID, 0, sizeof(TransactionId),
- MaxTransactionIdAttributeNumber, 0, -1, -1,
- true, 'p', 'i', true, false, false, '\0', false, true, 0
+ .attname = {"xmax"},
+ .atttypid = XIDOID,
+ .attlen = sizeof(TransactionId),
+ .attnum = MaxTransactionIdAttributeNumber,
+ .attcacheoff = -1,
+ .atttypmod = -1,
+ .attbyval = true,
+ .attstorage = 'p',
+ .attalign = 'i',
+ .attnotnull = true,
+ .attislocal = true,
};
static FormData_pg_attribute a6 = {
- 0, {"cmax"}, CIDOID, 0, sizeof(CommandId),
- MaxCommandIdAttributeNumber, 0, -1, -1,
- true, 'p', 'i', true, false, false, '\0', false, true, 0
+ .attname = {"cmax"},
+ .atttypid = CIDOID,
+ .attlen = sizeof(CommandId),
+ .attnum = MaxCommandIdAttributeNumber,
+ .attcacheoff = -1,
+ .atttypmod = -1,
+ .attbyval = true,
+ .attstorage = 'p',
+ .attalign = 'i',
+ .attnotnull = true,
+ .attislocal = true,
};
/*
@@ -187,9 +235,17 @@ static FormData_pg_attribute a6 = {
* used in SQL.
*/
static FormData_pg_attribute a7 = {
- 0, {"tableoid"}, OIDOID, 0, sizeof(Oid),
- TableOidAttributeNumber, 0, -1, -1,
- true, 'p', 'i', true, false, false, '\0', false, true, 0
+ .attname = {"tableoid"},
+ .atttypid = OIDOID,
+ .attlen = sizeof(Oid),
+ .attnum = TableOidAttributeNumber,
+ .attcacheoff = -1,
+ .atttypmod = -1,
+ .attbyval = true,
+ .attstorage = 'p',
+ .attalign = 'i',
+ .attnotnull = true,
+ .attislocal = true,
};
static const Form_pg_attribute SysAtt[] = {&a1, &a2, &a3, &a4, &a5, &a6, &a7};
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index bb696f8ee9..97dfd01b5e 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -391,418 +391,222 @@ do { \
static const SchemaQuery Query_for_list_of_aggregates[] = {
{
- /* min_server_version */
- 110000,
- /* catname */
- "pg_catalog.pg_proc p",
- /* selcondition */
- "p.prokind = 'a'",
- /* viscondition */
- "pg_catalog.pg_function_is_visible(p.oid)",
- /* namespace */
- "p.pronamespace",
- /* result */
- "pg_catalog.quote_ident(p.proname)",
- /* qualresult */
- NULL
+ .min_server_version = 110000,
+ .catname = "pg_catalog.pg_proc p",
+ .selcondition = "p.prokind = 'a'",
+ .viscondition = "pg_catalog.pg_function_is_visible(p.oid)",
+ .namespace = "p.pronamespace",
+ .result = "pg_catalog.quote_ident(p.proname)",
},
{
- /* min_server_version */
- 0,
- /* catname */
- "pg_catalog.pg_proc p",
- /* selcondition */
- "p.proisagg",
- /* viscondition */
- "pg_catalog.pg_function_is_visible(p.oid)",
- /* namespace */
- "p.pronamespace",
- /* result */
- "pg_catalog.quote_ident(p.proname)",
- /* qualresult */
- NULL
+ .catname = "pg_catalog.pg_proc p",
+ .selcondition = "p.proisagg",
+ .viscondition = "pg_catalog.pg_function_is_visible(p.oid)",
+ .namespace = "p.pronamespace",
+ .result = "pg_catalog.quote_ident(p.proname)",
}
};
static const SchemaQuery Query_for_list_of_datatypes = {
- /* min_server_version */
- 0,
- /* catname */
- "pg_catalog.pg_type t",
+ .catname = "pg_catalog.pg_type t",
/* selcondition --- ignore table rowtypes and array types */
- "(t.typrelid = 0 "
+ .selcondition = "(t.typrelid = 0 "
" OR (SELECT c.relkind = " CppAsString2(RELKIND_COMPOSITE_TYPE)
" FROM pg_catalog.pg_class c WHERE c.oid = t.typrelid)) "
"AND t.typname !~ '^_'",
- /* viscondition */
- "pg_catalog.pg_type_is_visible(t.oid)",
- /* namespace */
- "t.typnamespace",
- /* result */
- "pg_catalog.format_type(t.oid, NULL)",
- /* qualresult */
- "pg_catalog.quote_ident(t.typname)"
+ .viscondition = "pg_catalog.pg_type_is_visible(t.oid)",
+ .namespace = "t.typnamespace",
+ .result = "pg_catalog.format_type(t.oid, NULL)",
+ .qualresult = "pg_catalog.quote_ident(t.typname)",
};
static const SchemaQuery Query_for_list_of_domains = {
- /* min_server_version */
- 0,
- /* catname */
- "pg_catalog.pg_type t",
- /* selcondition */
- "t.typtype = 'd'",
- /* viscondition */
- "pg_catalog.pg_type_is_visible(t.oid)",
- /* namespace */
- "t.typnamespace",
- /* result */
- "pg_catalog.quote_ident(t.typname)",
- /* qualresult */
- NULL
+ .catname = "pg_catalog.pg_type t",
+ .selcondition = "t.typtype = 'd'",
+ .viscondition = "pg_catalog.pg_type_is_visible(t.oid)",
+ .namespace = "t.typnamespace",
+ .result = "pg_catalog.quote_ident(t.typname)",
};
/* Note: this intentionally accepts aggregates as well as plain functions */
static const SchemaQuery Query_for_list_of_functions[] = {
{
- /* min_server_version */
- 110000,
- /* catname */
- "pg_catalog.pg_proc p",
- /* selcondition */
- "p.prokind != 'p'",
- /* viscondition */
- "pg_catalog.pg_function_is_visible(p.oid)",
- /* namespace */
- "p.pronamespace",
- /* result */
- "pg_catalog.quote_ident(p.proname)",
- /* qualresult */
- NULL
+ .min_server_version = 110000,
+ .catname = "pg_catalog.pg_proc p",
+ .selcondition = "p.prokind != 'p'",
+ .viscondition = "pg_catalog.pg_function_is_visible(p.oid)",
+ .namespace = "p.pronamespace",
+ .result = "pg_catalog.quote_ident(p.proname)",
},
{
- /* min_server_version */
- 0,
- /* catname */
- "pg_catalog.pg_proc p",
- /* selcondition */
- NULL,
- /* viscondition */
- "pg_catalog.pg_function_is_visible(p.oid)",
- /* namespace */
- "p.pronamespace",
- /* result */
- "pg_catalog.quote_ident(p.proname)",
- /* qualresult */
- NULL
+ .catname = "pg_catalog.pg_proc p",
+ .viscondition = "pg_catalog.pg_function_is_visible(p.oid)",
+ .namespace = "p.pronamespace",
+ .result = "pg_catalog.quote_ident(p.proname)",
}
};
static const SchemaQuery Query_for_list_of_indexes = {
- /* min_server_version */
- 0,
- /* catname */
- "pg_catalog.pg_class c",
- /* selcondition */
+ .catname = "pg_catalog.pg_class c",
+ .selcondition =
"c.relkind IN (" CppAsString2(RELKIND_INDEX) ", "
CppAsString2(RELKIND_PARTITIONED_INDEX) ")",
- /* viscondition */
- "pg_catalog.pg_table_is_visible(c.oid)",
- /* namespace */
- "c.relnamespace",
- /* result */
- "pg_catalog.quote_ident(c.relname)",
- /* qualresult */
- NULL
+ .viscondition = "pg_catalog.pg_table_is_visible(c.oid)",
+ .namespace = "c.relnamespace",
+ .result = "pg_catalog.quote_ident(c.relname)",
};
static const SchemaQuery Query_for_list_of_procedures[] = {
{
- /* min_server_version */
- 110000,
- /* catname */
- "pg_catalog.pg_proc p",
- /* selcondition */
- "p.prokind = 'p'",
- /* viscondition */
- "pg_catalog.pg_function_is_visible(p.oid)",
- /* namespace */
- "p.pronamespace",
- /* result */
- "pg_catalog.quote_ident(p.proname)",
- /* qualresult */
- NULL
+ .min_server_version = 110000,
+ .catname = "pg_catalog.pg_proc p",
+ .selcondition = "p.prokind = 'p'",
+ .viscondition = "pg_catalog.pg_function_is_visible(p.oid)",
+ .namespace = "p.pronamespace",
+ .result = "pg_catalog.quote_ident(p.proname)",
},
- {0, NULL}
+ {
+ .min_server_version = 0,
+ .catname = NULL,
+ }
};
static const SchemaQuery Query_for_list_of_routines = {
- /* min_server_version */
- 0,
- /* catname */
- "pg_catalog.pg_proc p",
- /* selcondition */
- NULL,
- /* viscondition */
- "pg_catalog.pg_function_is_visible(p.oid)",
- /* namespace */
- "p.pronamespace",
- /* result */
- "pg_catalog.quote_ident(p.proname)",
- /* qualresult */
- NULL
+ .catname = "pg_catalog.pg_proc p",
+ .viscondition = "pg_catalog.pg_function_is_visible(p.oid)",
+ .namespace = "p.pronamespace",
+ .result = "pg_catalog.quote_ident(p.proname)",
};
static const SchemaQuery Query_for_list_of_sequences = {
- /* min_server_version */
- 0,
- /* catname */
- "pg_catalog.pg_class c",
- /* selcondition */
- "c.relkind IN (" CppAsString2(RELKIND_SEQUENCE) ")",
- /* viscondition */
- "pg_catalog.pg_table_is_visible(c.oid)",
- /* namespace */
- "c.relnamespace",
- /* result */
- "pg_catalog.quote_ident(c.relname)",
- /* qualresult */
- NULL
+ .catname = "pg_catalog.pg_class c",
+ .selcondition = "c.relkind IN (" CppAsString2(RELKIND_SEQUENCE) ")",
+ .viscondition = "pg_catalog.pg_table_is_visible(c.oid)",
+ .namespace = "c.relnamespace",
+ .result = "pg_catalog.quote_ident(c.relname)",
};
static const SchemaQuery Query_for_list_of_foreign_tables = {
- /* min_server_version */
- 0,
- /* catname */
- "pg_catalog.pg_class c",
- /* selcondition */
- "c.relkind IN (" CppAsString2(RELKIND_FOREIGN_TABLE) ")",
- /* viscondition */
- "pg_catalog.pg_table_is_visible(c.oid)",
- /* namespace */
- "c.relnamespace",
- /* result */
- "pg_catalog.quote_ident(c.relname)",
- /* qualresult */
- NULL
+ .catname = "pg_catalog.pg_class c",
+ .selcondition = "c.relkind IN (" CppAsString2(RELKIND_FOREIGN_TABLE) ")",
+ .viscondition = "pg_catalog.pg_table_is_visible(c.oid)",
+ .namespace = "c.relnamespace",
+ .result = "pg_catalog.quote_ident(c.relname)",
};
static const SchemaQuery Query_for_list_of_tables = {
- /* min_server_version */
- 0,
- /* catname */
- "pg_catalog.pg_class c",
- /* selcondition */
+ .catname = "pg_catalog.pg_class c",
+ .selcondition =
"c.relkind IN (" CppAsString2(RELKIND_RELATION) ", "
CppAsString2(RELKIND_PARTITIONED_TABLE) ")",
- /* viscondition */
- "pg_catalog.pg_table_is_visible(c.oid)",
- /* namespace */
- "c.relnamespace",
- /* result */
- "pg_catalog.quote_ident(c.relname)",
- /* qualresult */
- NULL
+ .viscondition = "pg_catalog.pg_table_is_visible(c.oid)",
+ .namespace = "c.relnamespace",
+ .result = "pg_catalog.quote_ident(c.relname)",
};
static const SchemaQuery Query_for_list_of_partitioned_tables = {
- /* min_server_version */
- 0,
- /* catname */
- "pg_catalog.pg_class c",
- /* selcondition */
- "c.relkind IN (" CppAsString2(RELKIND_PARTITIONED_TABLE) ")",
- /* viscondition */
- "pg_catalog.pg_table_is_visible(c.oid)",
- /* namespace */
- "c.relnamespace",
- /* result */
- "pg_catalog.quote_ident(c.relname)",
- /* qualresult */
- NULL
+ .catname = "pg_catalog.pg_class c",
+ .selcondition = "c.relkind IN (" CppAsString2(RELKIND_PARTITIONED_TABLE) ")",
+ .viscondition = "pg_catalog.pg_table_is_visible(c.oid)",
+ .namespace = "c.relnamespace",
+ .result = "pg_catalog.quote_ident(c.relname)",
};
static const SchemaQuery Query_for_list_of_constraints_with_schema = {
- /* min_server_version */
- 0,
- /* catname */
- "pg_catalog.pg_constraint c",
- /* selcondition */
- "c.conrelid <> 0",
- /* viscondition */
- "true", /* there is no pg_constraint_is_visible */
- /* namespace */
- "c.connamespace",
- /* result */
- "pg_catalog.quote_ident(c.conname)",
- /* qualresult */
- NULL
+ .catname = "pg_catalog.pg_constraint c",
+ .selcondition = "c.conrelid <> 0",
+ .viscondition = "true", /* there is no pg_constraint_is_visible */
+ .namespace = "c.connamespace",
+ .result = "pg_catalog.quote_ident(c.conname)",
};
/* Relations supporting INSERT, UPDATE or DELETE */
static const SchemaQuery Query_for_list_of_updatables = {
- /* min_server_version */
- 0,
- /* catname */
- "pg_catalog.pg_class c",
- /* selcondition */
+ .catname = "pg_catalog.pg_class c",
+ .selcondition =
"c.relkind IN (" CppAsString2(RELKIND_RELATION) ", "
CppAsString2(RELKIND_FOREIGN_TABLE) ", "
CppAsString2(RELKIND_VIEW) ", "
CppAsString2(RELKIND_PARTITIONED_TABLE) ")",
- /* viscondition */
- "pg_catalog.pg_table_is_visible(c.oid)",
- /* namespace */
- "c.relnamespace",
- /* result */
- "pg_catalog.quote_ident(c.relname)",
- /* qualresult */
- NULL
+ .viscondition = "pg_catalog.pg_table_is_visible(c.oid)",
+ .namespace = "c.relnamespace",
+ .result = "pg_catalog.quote_ident(c.relname)",
};
static const SchemaQuery Query_for_list_of_relations = {
- /* min_server_version */
- 0,
- /* catname */
- "pg_catalog.pg_class c",
- /* selcondition */
- NULL,
- /* viscondition */
- "pg_catalog.pg_table_is_visible(c.oid)",
- /* namespace */
- "c.relnamespace",
- /* result */
- "pg_catalog.quote_ident(c.relname)",
- /* qualresult */
- NULL
+ .catname = "pg_catalog.pg_class c",
+ .viscondition = "pg_catalog.pg_table_is_visible(c.oid)",
+ .namespace = "c.relnamespace",
+ .result = "pg_catalog.quote_ident(c.relname)",
};
static const SchemaQuery Query_for_list_of_tsvmf = {
- /* min_server_version */
- 0,
- /* catname */
- "pg_catalog.pg_class c",
- /* selcondition */
+ .catname = "pg_catalog.pg_class c",
+ .selcondition =
"c.relkind IN (" CppAsString2(RELKIND_RELATION) ", "
CppAsString2(RELKIND_SEQUENCE) ", "
CppAsString2(RELKIND_VIEW) ", "
CppAsString2(RELKIND_MATVIEW) ", "
CppAsString2(RELKIND_FOREIGN_TABLE) ", "
CppAsString2(RELKIND_PARTITIONED_TABLE) ")",
- /* viscondition */
- "pg_catalog.pg_table_is_visible(c.oid)",
- /* namespace */
- "c.relnamespace",
- /* result */
- "pg_catalog.quote_ident(c.relname)",
- /* qualresult */
- NULL
+ .viscondition = "pg_catalog.pg_table_is_visible(c.oid)",
+ .namespace = "c.relnamespace",
+ .result = "pg_catalog.quote_ident(c.relname)",
};
static const SchemaQuery Query_for_list_of_tmf = {
- /* min_server_version */
- 0,
- /* catname */
- "pg_catalog.pg_class c",
- /* selcondition */
+ .catname = "pg_catalog.pg_class c",
+ .selcondition =
"c.relkind IN (" CppAsString2(RELKIND_RELATION) ", "
CppAsString2(RELKIND_MATVIEW) ", "
CppAsString2(RELKIND_FOREIGN_TABLE) ")",
- /* viscondition */
- "pg_catalog.pg_table_is_visible(c.oid)",
- /* namespace */
- "c.relnamespace",
- /* result */
- "pg_catalog.quote_ident(c.relname)",
- /* qualresult */
- NULL
+ .viscondition = "pg_catalog.pg_table_is_visible(c.oid)",
+ .namespace = "c.relnamespace",
+ .result = "pg_catalog.quote_ident(c.relname)",
};
static const SchemaQuery Query_for_list_of_tpm = {
- /* min_server_version */
- 0,
- /* catname */
- "pg_catalog.pg_class c",
- /* selcondition */
+ .catname = "pg_catalog.pg_class c",
+ .selcondition =
"c.relkind IN (" CppAsString2(RELKIND_RELATION) ", "
CppAsString2(RELKIND_PARTITIONED_TABLE) ", "
CppAsString2(RELKIND_MATVIEW) ")",
- /* viscondition */
- "pg_catalog.pg_table_is_visible(c.oid)",
- /* namespace */
- "c.relnamespace",
- /* result */
- "pg_catalog.quote_ident(c.relname)",
- /* qualresult */
- NULL
+ .viscondition = "pg_catalog.pg_table_is_visible(c.oid)",
+ .namespace = "c.relnamespace",
+ .result = "pg_catalog.quote_ident(c.relname)",
};
static const SchemaQuery Query_for_list_of_tm = {
- /* min_server_version */
- 0,
- /* catname */
- "pg_catalog.pg_class c",
- /* selcondition */
+ .catname = "pg_catalog.pg_class c",
+ .selcondition =
"c.relkind IN (" CppAsString2(RELKIND_RELATION) ", "
CppAsString2(RELKIND_MATVIEW) ")",
- /* viscondition */
- "pg_catalog.pg_table_is_visible(c.oid)",
- /* namespace */
- "c.relnamespace",
- /* result */
- "pg_catalog.quote_ident(c.relname)",
- /* qualresult */
- NULL
+ .viscondition = "pg_catalog.pg_table_is_visible(c.oid)",
+ .namespace = "c.relnamespace",
+ .result = "pg_catalog.quote_ident(c.relname)",
};
static const SchemaQuery Query_for_list_of_views = {
- /* min_server_version */
- 0,
- /* catname */
- "pg_catalog.pg_class c",
- /* selcondition */
- "c.relkind IN (" CppAsString2(RELKIND_VIEW) ")",
- /* viscondition */
- "pg_catalog.pg_table_is_visible(c.oid)",
- /* namespace */
- "c.relnamespace",
- /* result */
- "pg_catalog.quote_ident(c.relname)",
- /* qualresult */
- NULL
+ .catname = "pg_catalog.pg_class c",
+ .selcondition = "c.relkind IN (" CppAsString2(RELKIND_VIEW) ")",
+ .viscondition = "pg_catalog.pg_table_is_visible(c.oid)",
+ .namespace = "c.relnamespace",
+ .result = "pg_catalog.quote_ident(c.relname)",
};
static const SchemaQuery Query_for_list_of_matviews = {
- /* min_server_version */
- 0,
- /* catname */
- "pg_catalog.pg_class c",
- /* selcondition */
- "c.relkind IN (" CppAsString2(RELKIND_MATVIEW) ")",
- /* viscondition */
- "pg_catalog.pg_table_is_visible(c.oid)",
- /* namespace */
- "c.relnamespace",
- /* result */
- "pg_catalog.quote_ident(c.relname)",
- /* qualresult */
- NULL
+ .catname = "pg_catalog.pg_class c",
+ .selcondition = "c.relkind IN (" CppAsString2(RELKIND_MATVIEW) ")",
+ .viscondition = "pg_catalog.pg_table_is_visible(c.oid)",
+ .namespace = "c.relnamespace",
+ .result = "pg_catalog.quote_ident(c.relname)",
};
static const SchemaQuery Query_for_list_of_statistics = {
- /* min_server_version */
- 0,
- /* catname */
- "pg_catalog.pg_statistic_ext s",
- /* selcondition */
- NULL,
- /* viscondition */
- "pg_catalog.pg_statistics_obj_is_visible(s.oid)",
- /* namespace */
- "s.stxnamespace",
- /* result */
- "pg_catalog.quote_ident(s.stxname)",
- /* qualresult */
- NULL
+ .catname = "pg_catalog.pg_statistic_ext s",
+ .viscondition = "pg_catalog.pg_statistics_obj_is_visible(s.oid)",
+ .namespace = "s.stxnamespace",
+ .result = "pg_catalog.quote_ident(s.stxname)",
};
diff --git a/src/pl/plpython/plpy_cursorobject.c b/src/pl/plpython/plpy_cursorobject.c
index e32bc568bc..45ac25b2ae 100644
--- a/src/pl/plpython/plpy_cursorobject.c
+++ b/src/pl/plpython/plpy_cursorobject.c
@@ -43,37 +43,14 @@ static PyMethodDef PLy_cursor_methods[] = {
static PyTypeObject PLy_CursorType = {
PyVarObject_HEAD_INIT(NULL, 0)
- "PLyCursor", /* tp_name */
- sizeof(PLyCursorObject), /* tp_size */
- 0, /* tp_itemsize */
-
- /*
- * methods
- */
- PLy_cursor_dealloc, /* tp_dealloc */
- 0, /* tp_print */
- 0, /* tp_getattr */
- 0, /* tp_setattr */
- 0, /* tp_compare */
- 0, /* tp_repr */
- 0, /* tp_as_number */
- 0, /* tp_as_sequence */
- 0, /* tp_as_mapping */
- 0, /* tp_hash */
- 0, /* tp_call */
- 0, /* tp_str */
- 0, /* tp_getattro */
- 0, /* tp_setattro */
- 0, /* tp_as_buffer */
- Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_ITER, /* tp_flags */
- PLy_cursor_doc, /* tp_doc */
- 0, /* tp_traverse */
- 0, /* tp_clear */
- 0, /* tp_richcompare */
- 0, /* tp_weaklistoffset */
- PyObject_SelfIter, /* tp_iter */
- PLy_cursor_iternext, /* tp_iternext */
- PLy_cursor_methods, /* tp_tpmethods */
+ .tp_name = "PLyCursor",
+ .tp_basicsize = sizeof(PLyCursorObject),
+ .tp_dealloc = PLy_cursor_dealloc,
+ .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_ITER,
+ .tp_doc = PLy_cursor_doc,
+ .tp_iter = PyObject_SelfIter,
+ .tp_iternext = PLy_cursor_iternext,
+ .tp_methods = PLy_cursor_methods,
};
void
diff --git a/src/pl/plpython/plpy_planobject.c b/src/pl/plpython/plpy_planobject.c
index 390b4e90d4..96ea24cbcf 100644
--- a/src/pl/plpython/plpy_planobject.c
+++ b/src/pl/plpython/plpy_planobject.c
@@ -34,37 +34,12 @@ static PyMethodDef PLy_plan_methods[] = {
static PyTypeObject PLy_PlanType = {
PyVarObject_HEAD_INIT(NULL, 0)
- "PLyPlan", /* tp_name */
- sizeof(PLyPlanObject), /* tp_size */
- 0, /* tp_itemsize */
-
- /*
- * methods
- */
- PLy_plan_dealloc, /* tp_dealloc */
- 0, /* tp_print */
- 0, /* tp_getattr */
- 0, /* tp_setattr */
- 0, /* tp_compare */
- 0, /* tp_repr */
- 0, /* tp_as_number */
- 0, /* tp_as_sequence */
- 0, /* tp_as_mapping */
- 0, /* tp_hash */
- 0, /* tp_call */
- 0, /* tp_str */
- 0, /* tp_getattro */
- 0, /* tp_setattro */
- 0, /* tp_as_buffer */
- Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */
- PLy_plan_doc, /* tp_doc */
- 0, /* tp_traverse */
- 0, /* tp_clear */
- 0, /* tp_richcompare */
- 0, /* tp_weaklistoffset */
- 0, /* tp_iter */
- 0, /* tp_iternext */
- PLy_plan_methods, /* tp_tpmethods */
+ .tp_name = "PLyPlan",
+ .tp_basicsize = sizeof(PLyPlanObject),
+ .tp_dealloc = PLy_plan_dealloc,
+ .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,
+ .tp_doc = PLy_plan_doc,
+ .tp_methods = PLy_plan_methods,
};
void
diff --git a/src/pl/plpython/plpy_plpymodule.c b/src/pl/plpython/plpy_plpymodule.c
index c55cd959c2..23e49e4b75 100644
--- a/src/pl/plpython/plpy_plpymodule.c
+++ b/src/pl/plpython/plpy_plpymodule.c
@@ -115,23 +115,17 @@ static PyMethodDef PLy_exc_methods[] = {
#if PY_MAJOR_VERSION >= 3
static PyModuleDef PLy_module = {
- PyModuleDef_HEAD_INIT, /* m_base */
- "plpy", /* m_name */
- NULL, /* m_doc */
- -1, /* m_size */
- PLy_methods, /* m_methods */
+ PyModuleDef_HEAD_INIT,
+ .m_name = "plpy",
+ .m_size = -1,
+ .m_methods = PLy_methods,
};
static PyModuleDef PLy_exc_module = {
- PyModuleDef_HEAD_INIT, /* m_base */
- "spiexceptions", /* m_name */
- NULL, /* m_doc */
- -1, /* m_size */
- PLy_exc_methods, /* m_methods */
- NULL, /* m_reload */
- NULL, /* m_traverse */
- NULL, /* m_clear */
- NULL /* m_free */
+ PyModuleDef_HEAD_INIT,
+ .m_name = "spiexceptions",
+ .m_size = -1,
+ .m_methods = PLy_exc_methods,
};
/*
diff --git a/src/pl/plpython/plpy_resultobject.c b/src/pl/plpython/plpy_resultobject.c
index ca70e25689..92670865a6 100644
--- a/src/pl/plpython/plpy_resultobject.c
+++ b/src/pl/plpython/plpy_resultobject.c
@@ -31,19 +31,16 @@ static char PLy_result_doc[] = {
};
static PySequenceMethods PLy_result_as_sequence = {
- PLy_result_length, /* sq_length */
- NULL, /* sq_concat */
- NULL, /* sq_repeat */
- PLy_result_item, /* sq_item */
- PLy_result_slice, /* sq_slice */
- NULL, /* sq_ass_item */
- PLy_result_ass_slice, /* sq_ass_slice */
+ .sq_length = PLy_result_length,
+ .sq_item = PLy_result_item,
+ .sq_slice = PLy_result_slice,
+ .sq_ass_slice = PLy_result_ass_slice,
};
static PyMappingMethods PLy_result_as_mapping = {
- PLy_result_length, /* mp_length */
- PLy_result_subscript, /* mp_subscript */
- PLy_result_ass_subscript, /* mp_ass_subscript */
+ .mp_length = PLy_result_length,
+ .mp_subscript = PLy_result_subscript,
+ .mp_ass_subscript = PLy_result_ass_subscript,
};
static PyMethodDef PLy_result_methods[] = {
@@ -57,37 +54,15 @@ static PyMethodDef PLy_result_methods[] = {
static PyTypeObject PLy_ResultType = {
PyVarObject_HEAD_INIT(NULL, 0)
- "PLyResult", /* tp_name */
- sizeof(PLyResultObject), /* tp_size */
- 0, /* tp_itemsize */
-
- /*
- * methods
- */
- PLy_result_dealloc, /* tp_dealloc */
- 0, /* tp_print */
- 0, /* tp_getattr */
- 0, /* tp_setattr */
- 0, /* tp_compare */
- 0, /* tp_repr */
- 0, /* tp_as_number */
- &PLy_result_as_sequence, /* tp_as_sequence */
- &PLy_result_as_mapping, /* tp_as_mapping */
- 0, /* tp_hash */
- 0, /* tp_call */
- &PLy_result_str, /* tp_str */
- 0, /* tp_getattro */
- 0, /* tp_setattro */
- 0, /* tp_as_buffer */
- Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */
- PLy_result_doc, /* tp_doc */
- 0, /* tp_traverse */
- 0, /* tp_clear */
- 0, /* tp_richcompare */
- 0, /* tp_weaklistoffset */
- 0, /* tp_iter */
- 0, /* tp_iternext */
- PLy_result_methods, /* tp_tpmethods */
+ .tp_name = "PLyResult",
+ .tp_basicsize = sizeof(PLyResultObject),
+ .tp_dealloc = PLy_result_dealloc,
+ .tp_as_sequence = &PLy_result_as_sequence,
+ .tp_as_mapping = &PLy_result_as_mapping,
+ .tp_str = &PLy_result_str,
+ .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,
+ .tp_doc = PLy_result_doc,
+ .tp_methods = PLy_result_methods,
};
void
diff --git a/src/pl/plpython/plpy_subxactobject.c b/src/pl/plpython/plpy_subxactobject.c
index 331d2b859c..53fd36edba 100644
--- a/src/pl/plpython/plpy_subxactobject.c
+++ b/src/pl/plpython/plpy_subxactobject.c
@@ -38,37 +38,12 @@ static PyMethodDef PLy_subtransaction_methods[] = {
static PyTypeObject PLy_SubtransactionType = {
PyVarObject_HEAD_INIT(NULL, 0)
- "PLySubtransaction", /* tp_name */
- sizeof(PLySubtransactionObject), /* tp_size */
- 0, /* tp_itemsize */
-
- /*
- * methods
- */
- PLy_subtransaction_dealloc, /* tp_dealloc */
- 0, /* tp_print */
- 0, /* tp_getattr */
- 0, /* tp_setattr */
- 0, /* tp_compare */
- 0, /* tp_repr */
- 0, /* tp_as_number */
- 0, /* tp_as_sequence */
- 0, /* tp_as_mapping */
- 0, /* tp_hash */
- 0, /* tp_call */
- 0, /* tp_str */
- 0, /* tp_getattro */
- 0, /* tp_setattro */
- 0, /* tp_as_buffer */
- Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */
- PLy_subtransaction_doc, /* tp_doc */
- 0, /* tp_traverse */
- 0, /* tp_clear */
- 0, /* tp_richcompare */
- 0, /* tp_weaklistoffset */
- 0, /* tp_iter */
- 0, /* tp_iternext */
- PLy_subtransaction_methods, /* tp_tpmethods */
+ .tp_name = "PLySubtransaction",
+ .tp_basicsize = sizeof(PLySubtransactionObject),
+ .tp_dealloc = PLy_subtransaction_dealloc,
+ .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,
+ .tp_doc = PLy_subtransaction_doc,
+ .tp_methods = PLy_subtransaction_methods,
};
--
2.18.0
On 8/29/18 5:14 AM, Peter Eisentraut wrote:
On 29/08/2018 12:13, Peter Eisentraut wrote:
Here is a patch to change some struct initializations to use C99-style
designated initializers. These are just a few particularly egregious
cases that were hard to read and write, and error prone because of many
similar adjacent types.(The PL/Python changes currently don't compile with Python 3 because of
the situation described in the parallel thread "PL/Python: Remove use of
simple slicing API".)Thoughts?
+1. This is an incredible win for readability/maintainability.
One thing: I'm not sure that excluding the InvalidOid assignment in the
TopTransactionStateData initializer is a good idea. That is, it's not
clear that InvalidOid is 0.
NULL, false, and 0 seem like no-brainers, but maybe it would be better
to explicitly include constants that we define that are not obviously 0,
or maybe just all of them.
Regards,
--
-David
david@pgmasters.net
Hi,
On 2018-08-29 15:51:07 -0500, David Steele wrote:
One thing: I'm not sure that excluding the InvalidOid assignment in the
TopTransactionStateData initializer is a good idea. That is, it's not clear
that InvalidOid is 0.NULL, false, and 0 seem like no-brainers, but maybe it would be better to
explicitly include constants that we define that are not obviously 0, or
maybe just all of them.
We rely on that in many other places imo. So I don't think it's worth
starting to care about it here.
Greetings,
Andres Freund
On Thu, Aug 30, 2018 at 8:51 AM David Steele <david@pgmasters.net> wrote:
On 8/29/18 5:14 AM, Peter Eisentraut wrote:
On 29/08/2018 12:13, Peter Eisentraut wrote:
Here is a patch to change some struct initializations to use C99-style
designated initializers. These are just a few particularly egregious
cases that were hard to read and write, and error prone because of many
similar adjacent types.(The PL/Python changes currently don't compile with Python 3 because of
the situation described in the parallel thread "PL/Python: Remove use of
simple slicing API".)Thoughts?
+1. This is an incredible win for readability/maintainability.
+1. Much nicer.
I know several people have the goal of being able to compile
PostgreSQL as C and C++, and this syntax is not in the C++ standard.
Happily, popular compilers seem OK with, and it's already been voted
into the draft for C++20. So no problem on that front.
--
Thomas Munro
http://www.enterprisedb.com
Andres Freund <andres@anarazel.de> writes:
On 2018-08-29 15:51:07 -0500, David Steele wrote:
One thing: I'm not sure that excluding the InvalidOid assignment in the
TopTransactionStateData initializer is a good idea. That is, it's not clear
that InvalidOid is 0.
NULL, false, and 0 seem like no-brainers, but maybe it would be better to
explicitly include constants that we define that are not obviously 0, or
maybe just all of them.
We rely on that in many other places imo. So I don't think it's worth
starting to care about it here.
I agree that assuming that they're physically zeroes is OK from a
portability standpoint, because we'd have a whole lot of other issues
if they weren't. But I have a different point to make, which is that
it's fairly standard practice for us to initialize all fields of a struct
explicitly, even when setting them to zero/false/NULL. I don't think we
should walk away from that practice just because C99 offers a shiny new
syntax for doing so.
The main practical advantage I see to writing such "redundant" explicit
field initializations is that it aids greppability: when you're adding a
new field Y beside field X, grepping for X is a good way of finding the
places where you need to initialize/copy/write/read/generically-frob Y
too. Omitting mention of X just because you're implicitly initializing
it puts a big hole in the reliability of that technique.
As against that, of course, explicitly zeroing fields that you know very
well are already zero eats some cycles. I've occasionally wondered if
it's worth doing things like
mynode = makeNode(MyNodeType);
mynode->w = 42;
/* mynode->x = 0; */
/* mynode->y = NULL; */
mynode->z = ptr;
but that seems mighty ugly.
An argument I *don't* buy is that leaving out redundant field assignments
is a good savings of programmer time. It's not a useful savings to the
original developer, and it's a net negative for anyone who has to review
or modify such code later. I think it was Brooks who said something to
the effect that any programmer would happily type out every line of code
thrice over, if only that would guarantee no bugs. It doesn't, of course,
but there are virtues in verbosity nonetheless.
regards, tom lane
On 08/29/18 18:51, Tom Lane wrote:
As against that, of course, explicitly zeroing fields that you know very
well are already zero eats some cycles. I've occasionally wondered if
I haven't checked what a smart C99 compiler actually emits for a
designated initializer giving a field a compile-time known constant zero.
Is it sure to eat any more cycles than the same initializer with the field
unmentioned?
-Chap
Hi,
On 2018-08-29 20:35:57 -0400, Chapman Flack wrote:
On 08/29/18 18:51, Tom Lane wrote:
As against that, of course, explicitly zeroing fields that you know very
well are already zero eats some cycles. I've occasionally wondered ifI haven't checked what a smart C99 compiler actually emits for a
designated initializer giving a field a compile-time known constant zero.
Is it sure to eat any more cycles than the same initializer with the field
unmentioned?
It's unlikely that any compiler worth its salt will emit redundant zero
initializations after a struct initialization (it's dead trivial to
recognize than in any SSA like form, which I think most compilers use
these days, certainly gcc and clang). What it can't optimize away
however is the x = makeNode(SomeType); x->foo = EquivalentToZero;
case. Currently the compiler has no way to know that the memory is zero
initialized (except for the type member, which the compiler can see
being set).
Greetings,
Andres Freund
Hi,
On 2018-08-29 18:51:24 -0400, Tom Lane wrote:
I agree that assuming that they're physically zeroes is OK from a
portability standpoint, because we'd have a whole lot of other issues
if they weren't. But I have a different point to make, which is that
it's fairly standard practice for us to initialize all fields of a struct
explicitly, even when setting them to zero/false/NULL. I don't think we
should walk away from that practice just because C99 offers a shiny new
syntax for doing so.The main practical advantage I see to writing such "redundant" explicit
field initializations is that it aids greppability: when you're adding a
new field Y beside field X, grepping for X is a good way of finding the
places where you need to initialize/copy/write/read/generically-frob Y
too. Omitting mention of X just because you're implicitly initializing
it puts a big hole in the reliability of that technique.
FWIW, I think this has for bigger costs than gains. You can't rely on
it being done everywhere anyway - there's *heaps* of places were we
don't set all members - and it makes changing fieldnames etc. way more
verbose than it has to be.
Greetings,
Andres Freund
On Aug 29, 2018, at 1:51 PM, David Steele <david@pgmasters.net> wrote:
On 8/29/18 5:14 AM, Peter Eisentraut wrote:
On 29/08/2018 12:13, Peter Eisentraut wrote:
Here is a patch to change some struct initializations to use C99-style
designated initializers. These are just a few particularly egregious
cases that were hard to read and write, and error prone because of many
similar adjacent types.(The PL/Python changes currently don't compile with Python 3 because of
the situation described in the parallel thread "PL/Python: Remove use of
simple slicing API".)Thoughts?
+1. This is an incredible win for readability/maintainability.
I tried doing this perhaps a year ago, and there are a few files with arrays
of structs whose representations get much larger when you change the format
in this way. For instance, in guc.c:
static struct config_bool ConfigureNamesBool[] =
{
{
{"enable_seqscan", PGC_USERSET, QUERY_TUNING_METHOD,
gettext_noop("Enables the planner's use of sequential-scan plans."),
NULL
},
&enable_seqscan,
true,
NULL, NULL, NULL
},
becomes:
static struct config_bool ConfigureNamesBool[] =
{
{
.gen = {
.name = "enable_seqscan",
.context = PGC_USERSET,
.group = QUERY_TUNING_METHOD,
.short_desc = gettext_noop("Enables the planner's use of sequential-scan plans."),
.long_desc = NULL
},
.variable = &enable_seqscan,
.boot_val = true,
.check_hook = NULL,
.assign_hook = NULL,
.show_hook = NULL
},
and then gets much longer if you do as per Tom's general suggestion and make
each field explicit (though Tom might not apply that rule to this case):
static struct config_bool ConfigureNamesBool[] =
{
{
.gen = {
.name = "enable_seqscan",
.context = PGC_USERSET,
.group = QUERY_TUNING_METHOD,
.short_desc = gettext_noop("Enables the planner's use of sequential-scan plans."),
.long_desc = NULL,
.flags = 0,
.vartype = 0,
.status = 0,
.source = 0,
.reset_source = 0,
.scontext = 0,
.reset_scontext = 0,
.stack = NULL,
.extra = NULL,
.sourcefile = NULL,
.sourceline = 0
},
.variable = &enable_seqscan,
.boot_val = true,
.check_hook = NULL,
.assign_hook = NULL,
.show_hook = NULL,
.reset_val = false,
.reset_extra = NULL
},
This sort of thing happens an awful lot in guc.c, but it comes up in syscache.c
also, and perhaps other places, though I don't recall any longer which other files
were like this.
What should the general rule be for initializing arrays of structs such as these?
mark
On 2018-Aug-30, Mark Dilger wrote:
static struct config_bool ConfigureNamesBool[] =
{
{
{"enable_seqscan", PGC_USERSET, QUERY_TUNING_METHOD,
gettext_noop("Enables the planner's use of sequential-scan plans."),
NULL
},
&enable_seqscan,
true,
NULL, NULL, NULL
},
Personally, I dislike this form -- it's very opaque and I have to refer
to the struct definition each time I want to add a new member, to make
sure I'm assigning the right thing. I welcome designated initializers
in this case even though it becomes more verbose. I don't think
explicitly initializing to NULLs is sensible in this case; let's just
omit those fields.
What should the general rule be for initializing arrays of structs such as these?
I don't know what a general rule would be. Maybe we can try hand-
inspecting a few cases, and come up with a general rule once we acquire
sufficient experience.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Wed, Aug 29, 2018 at 6:51 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I agree that assuming that they're physically zeroes is OK from a
portability standpoint, because we'd have a whole lot of other issues
if they weren't. But I have a different point to make, which is that
it's fairly standard practice for us to initialize all fields of a struct
explicitly, even when setting them to zero/false/NULL. I don't think we
should walk away from that practice just because C99 offers a shiny new
syntax for doing so.
I hate that style with a fiery passion that cannot be quenched. I
think you're basically the only one who has routinely done it like
this, and so it results in uselessly wasting a lot of mental energy
trying to decide whether a new member ought to be handled like the
ones Tom added or the ones somebody else added. It makes initializers
that could be quite short and compact long and hard to read. In
InitProcess(), it's entirely unclear which of those initializations
are merely insurance and which ones are actually doing something
useful, and there and in other places it's quite hard to tell whether
we might be wasting cycles (or instruction cache space) in sufficient
quantities to care about. If it were up to me, which it isn't, we'd
remove every useless initialize-to-zero statement in the entire code
base and then have a party to celebrate their demise. The party would
include burning the removed code in effigy. :-)
All that being said, this is MOSTLY a coding style issue and it comes
down to a matter of preference, so in my opinion, it doesn't really
matter that much in the end what we decide to do. Still, my
preference would definitely be for nuking it.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Hi,
On 2018-08-30 13:54:41 -0300, Alvaro Herrera wrote:
On 2018-Aug-30, Mark Dilger wrote:
static struct config_bool ConfigureNamesBool[] =
{
{
{"enable_seqscan", PGC_USERSET, QUERY_TUNING_METHOD,
gettext_noop("Enables the planner's use of sequential-scan plans."),
NULL
},
&enable_seqscan,
true,
NULL, NULL, NULL
},Personally, I dislike this form -- it's very opaque and I have to refer
to the struct definition each time I want to add a new member, to make
sure I'm assigning the right thing.
Dito. Especially because it looks different for the different types of
GUCs.
I welcome designated initializers in this case even though it becomes
more verbose. I don't think explicitly initializing to NULLs is
sensible in this case; let's just omit those fields.
Yea - I mean a large portion of them previously weren't initialized
either, so there's really not a good reason to change that now.
What should the general rule be for initializing arrays of structs such as these?
I don't know what a general rule would be. Maybe we can try hand-
inspecting a few cases, and come up with a general rule once we acquire
sufficient experience.
I think we should have as rules:
1) Members should be defined in the same order as in the struct, that's
the requirement C++ standard is going to impose. Think it's also
reasonable stylistically.
2) It's OK to omit setting members if zero-initialization obviously is
correct.
We probably should also check how well pgindent copes, and whether that
dictates some formatting choices.
Greetings,
Andres Freund
Mark Dilger <hornschnorter@gmail.com> writes:
I tried doing this perhaps a year ago, and there are a few files with arrays
of structs whose representations get much larger when you change the format
in this way. For instance, in guc.c:
...
What should the general rule be for initializing arrays of structs such as these?
I'd argue that there is no reason to convert initializers except where
it's a clear notational win. If it isn't, leaving things alone will
be less of a maintenance problem.
regards, tom lane
On 30/08/2018 22:14, Andres Freund wrote:
I think we should have as rules:
1) Members should be defined in the same order as in the struct, that's
the requirement C++ standard is going to impose. Think it's also
reasonable stylistically.
2) It's OK to omit setting members if zero-initialization obviously is
correct.
It seems like most people were OK with that, so I committed the patch.
This is something that we'll likely gain more experience with over time.
We probably should also check how well pgindent copes, and whether that
dictates some formatting choices.
The patch I submitted was run through pgindent. I did not experience
any problem, and it didn't reformat anything about what I had originally
typed in (except one comment I think).
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services