Subject: Re: [PATCH] Opclass parameters
To: Tomas Vondra <tomas.vondra@2ndquadrant.com>,
 "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>,
 Robert Haas <robertmhaas@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>,
 Nikolay Shaplov <dhyan@nataraj.su>, Oleg Bartunov <obartunov@gmail.com>
References: <20190903220254.GA22389@alvherre.pgsql>
 <dcb740af-2a93-8785-eb26-a4273029f2fb@postgrespro.ru>
 <20190910220358.6tj37k4v6vyaqac7@development>
From: Nikita Glukhov <n.gluhov@postgrespro.ru>
Organization: Postgres Professional
Message-ID: <0807f67b-c1bd-3760-7fc5-89900ae65408@postgrespro.ru>
Date: Wed, 11 Sep 2019 01:44:28 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.8.0
MIME-Version: 1.0
In-Reply-To: <20190910220358.6tj37k4v6vyaqac7@development>
Content-Type: multipart/alternative;
 boundary="------------056D4DABD5848D7AFBB1913C"
Content-Language: en-US
Precedence: bulk

This is a multi-part message in MIME format.
--------------056D4DABD5848D7AFBB1913C
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit

On 11.09.2019 1:03, Tomas Vondra wrote:

> On Tue, Sep 10, 2019 at 04:30:41AM +0300, Nikita Glukhov wrote:
>
>> 2. New AM method amattoptions().
>>
>>   amattoptions() is used to specify per-column AM-specific options.
>>   The example is signature length for bloom indexes (patch #3).
>>
>
> I'm somewhat confused how am I supposed to use this, considering the 
> patch
> set only defines this for the contrib/bloom index AM. So let's say I want
> to create a custom BRIN opclass with per-attribute options (like the two
> BRIN opclasses I work on in the other thread). Clearly, I can't tweak the
> IndexAmRoutine from the extension. ISTM the patch series should modify 
> all
> existing index AMs to have a valid amattoptions() implementation, calling
> the new amproc if defined.
>
> Or what is the correct way to define custom opclass for existing index AM
> (e.g. BRIN) with attribute options?
>
Per-attribute opclass options are implemented independently from per-attribute
AM options.  amattoptions() is optional and needs to be defined only if AM has
per-attribute options.  amproc #0 is called regardless of whether amattoptions
is defined or not.  That was the main reason why uniform procnum 0 was picked.

You should simply define function like that and use it as amproc #0:
  

Datum
brin_bloom_options(PG_FUNCTION_ARGS)
{
     local_relopts *relopts = (local_relopts *) PG_GETARG_POINTER(0);
     BloomOptions *blopts = NULL;

     extend_local_reloptions(relopts, blopts, sizeof(*blopts));

     add_local_real_reloption(relopts, "n_distinct_per_range", "desc",
                              -0.1, -1.0, INT_MAX, &blopts->nDistinctPerRange);

     add_local_real_reloption(relopts, "false_positive_rate", "desc",
                              0.01, 0.001, 1.0, &blopts->falsePositiveRate);

     PG_RETURN_VOID();
}

-- 
Nikita Glukhov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

--------------056D4DABD5848D7AFBB1913C
Content-Type: text/html; charset=utf-8
Content-Transfer-Encoding: 8bit

<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <p>On 11.09.2019 1:03, Tomas Vondra wrote:<br>
    </p>
    <blockquote type="cite"
      cite="mid:20190910220358.6tj37k4v6vyaqac7@development">On Tue, Sep
      10, 2019 at 04:30:41AM +0300, Nikita Glukhov wrote:
      <br>
      <br>
      <blockquote type="cite">2. New AM method amattoptions().
        <br>
        <br>
          amattoptions() is used to specify per-column AM-specific
        options.
        <br>
          The example is signature length for bloom indexes (patch #3).
        <br>
        <br>
      </blockquote>
      <br>
      I'm somewhat confused how am I supposed to use this, considering
      the patch
      <br>
      set only defines this for the contrib/bloom index AM. So let's say
      I want
      <br>
      to create a custom BRIN opclass with per-attribute options (like
      the two
      <br>
      BRIN opclasses I work on in the other thread). Clearly, I can't
      tweak the
      <br>
      IndexAmRoutine from the extension. ISTM the patch series should
      modify all
      <br>
      existing index AMs to have a valid amattoptions() implementation,
      calling
      <br>
      the new amproc if defined.
      <br>
      <br>
    </blockquote>
    <blockquote type="cite"
      cite="mid:20190910220358.6tj37k4v6vyaqac7@development">Or what is
      the correct way to define custom opclass for existing index AM
      <br>
      (e.g. BRIN) with attribute options?
      <br>
      <br>
    </blockquote>
    <pre>Per-attribute opclass options are implemented independently from per-attribute 
AM options.  amattoptions() is optional and needs to be defined only if AM has 
per-attribute options.  amproc #0 is called regardless of whether amattoptions
is defined or not.  That was the main reason why uniform procnum 0 was picked.
</pre>
    <pre>
You should simply define function like that and use it as amproc #0:
 </pre>
    <pre>Datum
brin_bloom_options(PG_FUNCTION_ARGS)
{
    local_relopts *relopts = (local_relopts *) PG_GETARG_POINTER(0);
    BloomOptions *blopts = NULL;

    extend_local_reloptions(relopts, blopts, sizeof(*blopts));

    add_local_real_reloption(relopts, "n_distinct_per_range", "desc",
                             -0.1, -1.0, INT_MAX, &amp;blopts-&gt;nDistinctPerRange);

    add_local_real_reloption(relopts, "false_positive_rate", "desc",
                             0.01, 0.001, 1.0, &amp;blopts-&gt;falsePositiveRate);

    PG_RETURN_VOID();
}
</pre>
    <div class="moz-signature">-- <br>
      Nikita Glukhov<br>
      Postgres Professional: <a href="http://www.postgrespro.com">http://www.postgrespro.com</a><br>
      The Russian Postgres Company<br>
    </div>
  </body>
</html>

--------------056D4DABD5848D7AFBB1913C--