On Wed, Sep 18, 2019 at 12:19 AM Fabien COELHO <coe...@cri.ensmp.fr> wrote: > > > Attached v9: > > - remove the PART_UNKNOWN and use partitions = -1 to tell > that there is an error, and partitions >= 1 to print info > - use search_path to find at most one pgbench_accounts > It still uses left join because I still think that it is appropriate. > I added a lateral to avoid repeating the array_position call > to manage the search_path, and use explicit pg_catalog everywhere.
It would be good if you can add some more comments to explain the intent of query. Few more comments: * else + { + /* PQntupes(res) == 1: normal case, extract the partition status */ + char *ps = PQgetvalue(res, 0, 1); + + if (ps == NULL) + partition_method = PART_NONE; When can we expect ps as NULL? If this is not a valid case, then probably and Assert would be better. * + else if (PQntuples(res) == 0) + { + /* no pgbench_accounts found, builtin script should fail later */ + partition_method = PART_NONE; + partitions = -1; + } If we don't find pgbench_accounts, let's give error here itself rather than later unless you have a valid case in mind. * + + /* + * Partition information. Assume no partitioning on any failure, so as + * to avoid failing on an older version. + */ .. + if (PQresultStatus(res) != PGRES_TUPLES_OK) + { + /* probably an older version, coldly assume no partitioning */ + partition_method = PART_NONE; + partitions = 0; + } So, here we are silently absorbing the error when pgbench is executed against older server version which doesn't support partitioning. If that is the case, then I think if user gives --partitions for the old server version, it will also give an error? It is not clear in documentation whether we support or not using pgbench with older server versions. I guess it didn't matter, but with this feature, it can matter. Do we need to document this? -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com