date:20180222

Re: [HACKERS] Add support for tuple routing to foreign partitions

2018-02-22 Thread Amit Langote

Fujita-san,

On Thu, Feb 22, 2018 at 8:49 PM, Etsuro Fujita
 wrote:
> (2018/02/22 11:52), Amit Langote wrote:
>> I wonder why partition_index needs to be made part of this API?
>
> The reason for that is because I think the FDW might want to look at the
> partition info stored in mtstate->mt_partition_tuple_routing for some reason
> or other, such as parent_child_tupconv_maps and child_parent_tupconv_maps,
> which are accessed with the given partition index.

I see.  I guess that makes sense.

>> Perhaps an independent concern, but one thing I noticed is that it does
>> not seem to play well with the direct modification (update push-down)
>> feature.  Now because updates (at least local, let's say) support
>> re-routing, I thought we'd be able move rows across servers via the local
>> server, but with direct modification we'd never get the chance.  However,
>> since update tuple routing is triggered by partition constraint failure,
>> which we don't enforce for foreign tables/partitions anyway, I'm not sure
>> if we need to do anything about that, and even if we did, whether it
>> concerns this proposal.
>
> Good point!  Actually, update tuple routing we have in HEAD doesn't allow
> re-routing tuples from foreign partitions even without direct modification.
> Here is an example using postgres_fdw:
>
> postgres=# create table pt (a int, b text) partition by list (a);
> CREATE TABLE
> postgres=# create table loct (a int check (a in (1)), b text);
> CREATE TABLE
> postgres=# create foreign table remp partition of pt for values in (1)
> server loopback options (table_name 'loct');
> CREATE FOREIGN TABLE
> postgres=# create table locp partition of pt for values in (2);
> CREATE TABLE
> postgres=# insert into remp values (1, 'foo');
> INSERT 0 1
> postgres=# insert into locp values (2, 'bar');
> INSERT 0 1
> postgres=# select tableoid::regclass, * from pt;
>  tableoid | a |  b
> --+---+-
>  remp | 1 | foo
>  locp | 2 | bar
> (2 rows)
>
> postgres=# create function postgres_fdw_abs(int) returns int as $$begin
> return abs($1); end$$ language plpgsql immutable;
> CREATE FUNCTION
> postgres=# explain verbose update pt set a = postgres_fdw_abs(a) + 1 where b
> = 'foo';
>  QUERY PLAN
>
> 
> -
>  Update on public.pt  (cost=100.00..154.54 rows=12 width=42)
>Foreign Update on public.remp
>  Remote SQL: UPDATE public.loct SET a = $2 WHERE ctid = $1
>Update on public.locp
>->  Foreign Scan on public.remp  (cost=100.00..127.15 rows=6 width=42)
>  Output: (postgres_fdw_abs(remp.a) + 1), remp.b, remp.ctid
>  Remote SQL: SELECT a, b, ctid FROM public.loct WHERE ((b =
> 'foo'::text)) FOR UPDATE
>->  Seq Scan on public.locp  (cost=0.00..27.39 rows=6 width=42)
>  Output: (postgres_fdw_abs(locp.a) + 1), locp.b, locp.ctid
>  Filter: (locp.b = 'foo'::text)
> (10 rows)
>
> postgres=# update pt set a = postgres_fdw_abs(a) + 1 where b = 'foo';
> ERROR:  new row for relation "loct" violates check constraint "loct_a_check"
> DETAIL:  Failing row contains (2, foo).
> CONTEXT:  Remote SQL command: UPDATE public.loct SET a = $2 WHERE ctid = $1
>
> To be able to move tuples across foreign servers, I think we would first
> have to do something to allow re-routing tuples from foreign partitions.
> The patches I proposed hasn't done anything about that, so the patched
> version would behave the same way as HEAD with/without direct modification.

Yes.

As I said, since update re-routing is triggered by partition
constraint failure for the new row and we don't check (any)
constraints for foreign tables, that means re-routing won't occur for
foreign partitions.

>> That said, I saw in the changes to ExecSetupPartitionTupleRouting() that
>> BeginForeignRouting() is called for a foreign partition even if direct
>> modification might already have been set up.  If direct modification is
>> set up, then ExecForeignRouting() will never get called, because we'd
>> never call ExecUpdate() or ExecInsert().
>
> Yeah, but I am thinking to leave the support for re-routing tuples across
> foreign servers for another patch.
>
> One difference between HEAD and the patched version would be: we can
> re-route tuples from a plain partition to a foreign partition if the foreign
> partition supports this tuple-routing API.  Here is an example using the
> above data:
>
> postgres=# select tableoid::regclass, * from pt;
>  tableoid | a |  b
> --+---+-
>  remp | 1 | foo
>  locp | 2 | bar
> (2 rows)
>
> postgres=# update pt set a = 1 where b = 'bar';
> UPDATE 1
> postgres=# select tableoid::regclass, * from pt;
>  tableoid | a |  b
> --+---+-
>  remp | 1 | foo
>  remp | 1 | bar
> (2 rows)
>
> This would introduce an asymmetry (we can move tuples from plain partitions
> to foreign partitions, but the

RTLD_GLOBAL (& JIT inlining)

2018-02-22 Thread Andres Freund

Hi Peter, All,

First question:

Why do we currently use RTLD_GLOBAL loading extension libraries, but
take pains ([1]) to make things work without RTLD_GLOBAL. It seems like
it'd be both safer to RTLD_LOCAL on platforms supporting it (or
equivalent), as well as less error-prone because we'd be less likely to
depend on it like we did e.g. during transforms development.

It seems error prone because on systmes I'm aware of (IOW linux ;))
conflicting symbols will not cause errors. Neither when there's a
conflict between a .so and the main binary, nor between different
shlibs. My reading of glibc's ld is that the search order for
unresolved references in a .so is main binary, and then other
RTLD_GLOBAL shared libraries in order of time they're loaded.

Right now it appears that e.g. hstore_plperl has some issue, building
with RTLD_LOCAL makes its test fail. Which means it'll probably not work
on some platforms.

I think using RTLD_LOCAL on most machines would be a much better
idea. I've not found proper explanations why GLOBAL is used. We started
using it ages ago, with [2], but that commit contains no explanation,
and a quick search didn't show up anything either. Peter?

Secondly:

For JITing internal & C operators, and other functions referenced in
JITed code, can be inlined if the definition is available in a suitable
form. The details how that works don't matter much here.

For my jitting work I've so far treated symbols of binaries and all
referenced extensions as being part of a single namespace. Currently the
order in which symbols with multiple definitions were looked up was in
essentially in filesystem order.

While I'm not aware of any observable problems, reviewing that decision
I think that's a terrible idea. First and foremost it seems obviously
wrong to not load symbols from the main postgres binary first. But
leaving that aside, I'm uncomfortable doing inlining between shlibs for
JITing, because that can lead to symbols beeing reloaded differently
between non JITed code (where all external libraries loaded in the
current library are searched in load/usage order) and JITed code (where
the symbols would be resolved according to some static order).

I'm about to rewrite it so that functions are inlined just from the main
binary, or the basename of the shared library explicitly referenced in
the function definition. That's still different, but at least easy to
understand. But that still means there's inconsistent order between
different methods of execution, which isn't great.

Comments?

Greetings,

Andres Freund

[1] http://archives.postgresql.org/message-id/2652.1475512158%40sss.pgh.pa.us
[2]
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=c87bc779d4e9f109e92f8b8f1dfad5d6739f8e97

1 2 >

1 - 100 of 111 matches

Mail list logo