Re: [POC] FETCH limited by bytes.

On 2015-02-27 13:50:22 -0500, Corey Huinker wrote:
> +static DefElem*
> +get_option(List *options, char *optname)
> +{
> +    ListCell *lc;
> +
> +    foreach(lc, options)
> +    {
> +        DefElem    *def = (DefElem *) lfirst(lc);
> +
> +        if (strcmp(def->defname, optname) == 0)
> +            return def;
> +    }
> +    return NULL;
> +}


>      /*
>       * Do nothing in EXPLAIN (no ANALYZE) case.  node->fdw_state stays NULL.
> @@ -915,6 +933,23 @@ postgresBeginForeignScan(ForeignScanState *node, int eflags)
>      server = GetForeignServer(table->serverid);
>      user = GetUserMapping(userid, server->serverid);
>  
> +    /* Reading table options */
> +    fsstate->fetch_size = -1;
> +
> +    def = get_option(table->options, "fetch_size");
> +    if (!def)
> +        def = get_option(server->options, "fetch_size");
> +
> +    if (def)
> +    {
> +        fsstate->fetch_size = strtod(defGetString(def), NULL);
> +        if (fsstate->fetch_size < 0)
> +            elog(ERROR, "invalid fetch size for foreign table \"%s\"",
> +                 get_rel_name(table->relid));
> +    }
> +    else
> +        fsstate->fetch_size = 100;

I don't think it's a good idea to make such checks at runtime - and
either way it's somethign that should be reported back using an
ereport(), not an elog.

Also, it seems somewhat wrong to determine this at execution
time. Shouldn't this rather be done when creating the foreign scan node?
And be a part of the scan state?

Have you thought about how this option should cooperate with join
pushdown once implemented?

Greetings,

Andres Freund