On Wed, Sep 18, 2019 at 12:19 AM Fabien COELHO <coelho@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